-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Handle NativeOverlapped* coming from both the Windows or Portable thread pool in NativeRuntimeEventSource #97365
Handle NativeOverlapped* coming from both the Windows or Portable thread pool in NativeRuntimeEventSource #97365
Conversation
Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti Issue Detailsnull
|
.../System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.Threading.cs
Show resolved
Hide resolved
...ate.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.Threading.NativeSinks.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -254,7 +259,7 @@ public void ThreadPoolIOEnqueue(RegisteredWaitHandle registeredWaitHandle) | |||
[Event(64, Level = EventLevel.Verbose, Message = Messages.IO, Task = Tasks.ThreadPool, Opcode = Opcodes.IODequeue, Version = 0, Keywords = Keywords.ThreadingKeyword | Keywords.ThreadTransferKeyword)] | |||
private unsafe void ThreadPoolIODequeue( | |||
IntPtr NativeOverlapped, | |||
IntPtr Overlapped, | |||
IntPtr Overlapped, // 0 if the Windows thread pool is used, the relevant info could be obtained from the NativeOverlapped* if necessary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do diagnostic tools use this value? Is 0 the best option to make them work seamlessly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know it's not used for anything specific, just additional info. We used to pass in the actual Overlapped object pointer and later switched to a hash code since that's not a stable value. I'm not sure how useful it is actually, since the NativeOverlapped* is stable, maybe a hash code is not very useful. For the Windows thread pool implementation there is an OverlappedData object that could be used, though it could also be obtained from the NativeOverlapped* if debugging side-by-side with a heap dump.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me
…s thread pool" This reverts commit a1a45927223a6ff1e154bab299b36f77394cdedc.
e0dc456
to
a3c111c
Compare
Removing the test as it could be improved and it should be configured to run with the Windows thread pool. I'll add it separately in another PR and merge this one with the fix only |
/backport to release/8.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/8257670127 |
Fix ThreadPoolIOEnqueue, ThreadPoolIODequeue, ThreadPoolIOPack in Native Runtime Event Source which are based on the assumption that the NativeOverlapped* comes from the Portable thread pool implementation but it can come from the Windows thread pool implementation as well.