performance(ocl): skip WaitUtils yield on clSetEventCallback probe - #965
Open
latentPrion wants to merge 1 commit into
Open
performance(ocl): skip WaitUtils yield on clSetEventCallback probe#965latentPrion wants to merge 1 commit into
latentPrion wants to merge 1 commit into
Conversation
Not-ready tag polls in setCb were calling std::this_thread::yield(), which under CPU load can stall for multiple ms even though the callback is then registered asynchronously. SkipYieldGuard around tryFlush+addCallback returns immediately on miss; blocking waits keep default yield behavior. Signed-off-by: Latent Prion <latentprion@gmail.com>
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
1 participant
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.
Prelude:
I am open to feedback and very willing to rework this patch as needed to get it merged. Please tell me what you'd like changed and I'll update the patch.
What is happening?
Not-ready tag polls in setCb were calling std::this_thread::yield(). These yield() calls are unnecessary when the user is using the asynchronous flow (i.e: using clSetEventCallback()). In the asynchronous flow, NEO should check for GPU kernel completion once, and then if the kernel hasn't already finished executing, then NEO should proceed to enqueue the callback on the async queue -- and it should do this without calling yield().
Why is this a problem?
The problem is that this superfluous yield() causes periodic spikes of 5ms or more, which the user is forced to wait for. This significantly degrades the latency of realtime applications which are trying to use the GPU. An extra 5-10ms is a big deal if you have a 33ms budget to work with.
Proposed solution:
I've implemented the minimum necessary change here: just use a thread-local var to tell NEO when it should not call yield().