Add debug information for runtime async methods #120303
Conversation
- Add new JIT-EE API to report back debug information about the generated state machine and continuations - Refactor debug info storage on VM side to be more easily extensible. The new format has either a thin or fat header. The fat header is used when we have either uninstrumented bounds, patchpoint info, rich debug info or async debug info, and stores the blob sizes of all of those components in addition to the bounds and vars. - Add new async debug information to the storage on the VM side - Set get target method desc for async resumption stubs, to be used for mapping from continuations back to the async IL function that it will resume.
github-actions
bot
added
the
area-CodeGen-coreclr
jakobbotsch
changed the title
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Show resolved
Hide resolved
|
@AndyAyersMS Any other comments on the JIT part? |
|
I ran through the latest with EventPipe and was able to capture the stack trace in dotnet-stack as well as sample profile in dotnet-trace and load it in PerfView: 
Level9Async and below are coming in from the continuation chain, Level10Async is the current dispatched async frame. I also loaded the source code location, and it looks like it correctly matches the diagnostic IP through native <-> IL -> Source: 
I also looked at the location of diagnostic IP in generated assembler: 
So, it ends up at 00007FFD259C9B7F that is before the call, expected? |
Yes, the |
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
JIT changes LGTM, one small formatting issue.
|
This seems like its probably good. One question I have is what is the impact on the file size of System.Private.CoreLib. The cost of adding a new SourceType bit is probably not that bad, but I would like it measured before we merge. We recently went through a fair amount of effort to make it more efficient to read the compressed format (which is why it doesn't use compressed nibbles for most of the data anymore), but part of the sacrifice there was to reduce the number of bits available for SourceType. It turns out that reading this sort of debug data is on the critical path for applications which are experiencing high failure rates, so we have to be quite careful to avoid too much usage of decompressing nibbles, as that is an incredibly inefficient codepath. |
There was a problem hiding this comment.
DiagnosticIP on ResumeInfo and usage of it LGTM! Ability to go from continuation -> native IP looks good and can be used in regular EventPipe stack-walks + resolve to user code through the IL<->native mappings emitted in rundown. Captured samples are correctly translated to matching await calls in user code into PerfView. With this PR we are able to use our internal EventPipe sample profiler and profile AsyncV2 continuation chains with low overhead.
Measured this, SPC grows from 16244 KB to 16272 KB with the extra bit to encode the new source type. |
|
/ba-g Android timeout |
|
Thanks for the reviews everyone! |
In debug codegen we expect basic blocks to have valid start IL offsets, potentially to an invalid IL end offset. When we split a block at a node, we find the end offset by looking backwards from that node for an `IL_OFFSET` node. If we find such a node, we use it as the split point IL offset. Otherwise we use `BAD_IL_OFFSET` and start the next block at the end offset (making it have a 0 byte range). This latter behavior is problematic if the bottom portion of the split has its own `IL_OFFSET` nodes. In that case we can end up with `IL_OFFSET` nodes pointing outside the region of the basic block. If we later split again, then we can end up creating illegal IL offset ranges for unoptimized code. In an example case we first ended up with the bottom block having an IL offset range of `[0aa..0aa)` but still containing an `IL_OFFSET` with an offset of `092`. When we later split the bottom block again we ended up with `[0aa..0aa), [092..0aa)` and hit an assert. To address the problem change the behavior in the following way: * If we find no `IL_OFFSET` node in the upper block, then look for one in the lower block, and use its value as the split point * If we find no `IL_OFFSET` in both blocks then consider the lower block empty and the upper block to have everything (same behavior as before). * One exception to above: if the end was already `BAD_IL_OFFSET` then we have to consider the upper block to be empty as we cannot represent the other case This fixes #119616. However for that specific case, which is inside async code, we will have the exact IL offset of the async call we are splitting at after #120303, so we will be able to do better. I'll make that change as part of that PR.
11 participants
Continuation.Resumein favor of aContinuation.ResumeInfopointer that gives both the resumption stub and aDiagnosticIP. TheDiagnosticIPcan be used for async stackwalking and for keying into the async debug infoContinuation.ResumeInfoat this table's entries on suspensionICorDebugInfo::ASYNCsource type; emit mappings with this source type for generated suspension and resumption code