Fix profiler to report time spent on GPU kernels again instead of on 'wait for parallel tasks'. #8453
Conversation
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
mcourteaux
force-pushed
the
fix-profiler-gpu
branch
from
87a2549 to
2fcf06b
Compare
Contributor
|
We should extract the LLVM fix and land it separately |
Contributor
Author
|
I'll make separate PR. It's here: #8454. @steven-johnson |
…on 'wait for parallel tasks'.
mcourteaux
force-pushed
the
fix-profiler-gpu
branch
from
2fcf06b to
7165cda
Compare
abadams
approved these changes
Nov 4, 2024
Member
abadams
left a comment
abadams
left a comment
There was a problem hiding this comment.
Looks good to me. Feel free to merge once the bots are all green.
1 task
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.
So instead of
suspend_thread()while waiting for a GPU kernel to complete, which decrements the active thread count and sets the current func to "wait for parallel tasks", this PR now only decrements the active number of threads, such that the current function is still the Func that the kernel is actually producing. As such, you can see the time spent on that kernel, but still see that it was done by 0 threads, indicating it was the GPU that took care of it.Also don't report the "wait for parallel tasks" if it doesn't contain any time, like for overhead.
Additional drive-by typo fix.