Preheat host memory for OpenCL read_host_from_device#21134
Merged
jenshannoschwalm merged 1 commit intoMay 31, 2026
Conversation
|
Seems good on my GB10. This also seems faster than my PR #21069 although they aren't rebased on the same parent commit so the benchmark runs aren't directly comparable. Using the 61MP benchmark from https://darktable.info/performance/benchmarks-beispiele/benchmark/: NVIDIA driver version 580.159.03 Builds:
Run controls:
Measured pixelpipe times:
OpenCL image readback profiling:
OpenCL command queue totals:
notebook tilingBy the way, in #21069 (comment) you also mentioned using the "notebook" resource level. But it's a LOT slower. Run controls:
Measured pixelpipe times:
OpenCL image readback profiling:
OpenCL command queue totals:
Anyway, seems good. Ship it!!!!!!! |
Collaborator
Author
|
Ok, so
|
jenshannoschwalm
added
bug: upstream
As some some systems are very slow in `clEnqueueReadImage()` if the host memoty is still cold we use linux specific `madvise()` for the host mem before. 1. Logs telling about errors in madvise() or if in -d verbose -d opencl debugging mode. 2. The preheating feature is currently enabled via the hidden `opencl_preheated` conf
jenshannoschwalm
force-pushed
the
host_preheating
branch
from
a589317 to
9e36df0
Compare
|
Even with this branch, I find that using darktable is extremely sluggish on NVIDIA GB10. Even basic operations like changing the exposure takes like 15 seconds of "working...". I have the following settings. 
Any ideas what else I need? |
Collaborator
Author
|
In fact - no. If you provide a "-d opencl -d pipe" log when you observe this - maybe i spot something "picky". |
Collaborator
Author
That would be pinned/mapped mode and lots of mem transfers. |
|
Never mind, I was dumb, I didn't turn on the Before: After: About 10 seconds --> 0.8 second. This PR does in fact fix the problem and I'll be using it on my DGX Spark from now on. |
Collaborator
Author
|
Fine. You might still want to check if buffer reading should use the same preheating. That would be Would be good to know so we can possibly backreport to nvidia folks. |
Collaborator
Author
|
@TurboGit would you revert this right now please! Nothing for master in this form... I didnt notice i pushed on any merge button and wasnt yet aware of my rights to do so. |
Collaborator
Author
|
@victoryforce TIA |
Collaborator
|
@jenshannoschwalm Just above this, in the same line where the information that you "merged commit 475a9f7 into darktable-org:master" is, there is a "Revert" button. You definitely have the rights to do this. Maybe it would be better if you did the revert yourself, so that we don't doubt whether we understood you correctly? |
Collaborator
Author
|
Did so. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As some some systems are very slow in
clEnqueueReadImage()if the host memory is still cold we use linux specificmadvise()for the host mem before.opencl_preheatedconf@dllu as you did the initial work on #21069 would you be able to test this?
@karolherbst would you mind to review? I am a bit worried about the
#define __USE_MISCi had to add here, otherwise onlyposix_advise()was available (habing less precise hints.)I tested here on strix halo/fedora44 44 so lots of unified mem. No problems/instability, could not see performance changes both for rusticl and rocm.