Skip to content

Pixelpipe safety improvements and maintenance #17718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

jenshannoschwalm
Copy link
Collaborator

Inspired by #17684 (provided logs explaining the issue) and various reports like #17016 #15939 #13464

As we don't keep track of allocated memory we can at least make sure the background thumbnail pipes don't take too much memory. For sure this is not a "complete" fix but it will certainly help to restrict memory consumption for those.


Docs from commit messages

Pixelpipe memory log maintenance

  1. Avoid double reports for pipe starting
  2. refactor dt_print_mem_usage(), it now gets a message string passed avoiding
    am extra dt_print() call
  3. log memory consumption also after pixelpipe has ended
  4. log pixelpipe cache checks also with DT_DEBUG_MEMORY
  5. dt_tiling_piece_fits_host_memory() gets the piece as an additional parameter.
    This allows it to check for pipe type and more.

Introduce dt_get_available_pipe_mem()

size_t dt_get_available_pipe_mem(const dt_dev_pixelpipe_t *pipe);

returns dt_get_available_mem() reduced in case the pixelpipe is DT_DEV_PIXELPIPE_THUMBNAIL
to allow safer background thumbnail generation.
In case of low-memory systems (8GB or so) this will prevent crashes due to OOM killer or
too high memory stress.

All checks within tiling code or tests for CPU tiling-required now use this variant instead of
full dt_get_available_mem()

Fix OpenCL parameters when writing PFM diff files

Resulting file when writing CPU vs GPU difference could be wrong due to wrong kernel parameters
for dt_opencl_read_host_from_device().

1. Avoid double reports for pipe starting
2. refactor dt_print_mem_usage(), it now gets a message string passed avoiding
   am extra dt_print() call
3. log memory consumption also after pixelpipe has ended
4. log pixelpipe cache checks also with DT_DEBUG_MEMORY
5. dt_tiling_piece_fits_host_memory() gets the piece as an additional parameter.
   This allows it to check for pipe type and more.
size_t dt_get_available_pipe_mem(const dt_dev_pixelpipe_t *pipe);

returns dt_get_available_mem() reduced in case the pixelpipe is DT_DEV_PIXELPIPE_THUMBNAIL
to allow safer background thumbnail generation.
In case of low-memory systems (8GB or so) this will prevent crashes due to OOM killer or
too high memory stress.

All checks within tiling code or tests for CPU tiling-required now use this variant instead of
full dt_get_available_mem()

ASC
Resulting file when writing CPU vs GPU difference could be wrong due to wrong kernel parameters
for dt_opencl_read_host_from_device().
@jenshannoschwalm jenshannoschwalm added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes scope: codebase making darktable source code easier to manage OpenCL Related to darktable OpenCL code scope: debugging labels Oct 25, 2024
@jenshannoschwalm jenshannoschwalm added this to the 5.0 milestone Oct 25, 2024
Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TurboGit TurboGit merged commit b7288e1 into darktable-org:master Oct 25, 2024
6 checks passed
@jenshannoschwalm jenshannoschwalm deleted the pixelpipe_memory branch October 25, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix pull request fixing a bug OpenCL Related to darktable OpenCL code priority: high core features are broken and not usable at all, software crashes scope: codebase making darktable source code easier to manage scope: debugging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants