Repository navigation
[cDAC] Avoid allocating stack contexts for output - #136934
Conversation
Return stack context snapshots as read-only spans so output callers can copy directly without first allocating a temporary byte array. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: de3eab4e-b090-430a-8e30-6b204fb1519a
|
Azure Pipelines: Successfully started running 5 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
|
This introduced build break that many legs are failing on now. Reverting in #137101 /__w/1/s/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs(6789,13): error CS1929: 'ReadOnlySpan' does not contain a definition for 'AsSpan' and the best extension method overload 'MemoryExtensions.AsSpan(string?, int, int)' requires a receiver of type 'string?' [/__w/1/s/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Microsoft.Diagnostics.DataContractReader.Legacy.csproj] |
## Summary Reapply #136934 after it was reverted by #137101. Return retained stack-context snapshots as `ReadOnlySpan<byte>` instead of allocating a new `byte[]` for every output request. This changes only the context-output APIs: - `IStackWalk.GetRawContext` returns a read-only view of the frame's retained context snapshot. - `IPlatformAgnosticContext.GetBytes` returns a read-only view over the serialized platform context. - COM and debugger consumers copy directly from the span where possible. - APIs and tests that require owned, writable, or pinned context storage materialize arrays explicitly. The span remains backed by the context snapshot owned by the stack frame handle. Advancing the walk does not mutate that retained snapshot. The original PR passed its CI, but #130367 merged after that validation and added six new `GetBytes()` callers. The combined changes compiled neither in the three production callers that invoked `.AsSpan()` on the new `ReadOnlySpan<byte>` nor in three tests that assigned the span directly to `byte[]`. This replay accounts for all six callers. ## Performance The measurements from #136934 used frozen NativeAOT readers with provider identities reversed between two passes. Each pass used 3 launches with 30 observations per case, with no outlier removal. The primary run retained 2,520 observations across 14 timing cases. | Scenario | Before | After | Change | Reversed-order changes | |---|---:|---:|---:|---:| | Enum CodeHeavy Heap | 4.665 ms | 4.554 ms | -2.4% | -0.7% / -4.0% | | Enum DeepStacks Heap | 12.585 ms | 12.182 ms | -3.2% | -5.4% / -0.8% | | Enum SmallSimple Heap | 2.787 ms | 2.738 ms | -1.8% | -4.8% / +0.9% | | Stack DeepStacks Heap Cold | 9.040 ms | 8.668 ms | -4.1% | -2.6% / -5.7% | | Stack DeepStacks Heap Warm | 8.645 ms | 8.504 ms | -1.6% | -1.5% / -1.8% | | Stack DeepStacks Mini Cold | 11.450 ms | 11.640 ms | +1.7% | +2.3% / +1.0% | | Stack DeepStacks Mini Warm | 11.013 ms | 10.913 ms | -0.9% | +3.5% / -5.4% | A separate cold-walk confirmation used two more reversed passes and 720 additional observations: | Scenario | Before | After | Change | Reversed-order changes | |---|---:|---:|---:|---:| | Stack DeepStacks Heap Cold | 8.692 ms | 8.199 ms | -5.7% | -4.2% / -7.2% | | Stack DeepStacks Mini Cold | 11.185 ms | 10.637 ms | -4.9% | -4.2% / -5.6% | The change removes one temporary 1,232-byte array for each AMD64 context output. Dump-enumeration timing does not directly exercise the migrated output APIs, and shared-host timing differences are not guaranteed gains. ## Testing - `dotnet build src\native\managed\cdac\Microsoft.Diagnostics.DataContractReader.Legacy\Microsoft.Diagnostics.DataContractReader.Legacy.csproj -c Release --no-restore -m:1 -p:BuildInParallel=false -p:RuntimeConfiguration=Release -p:LibrariesConfiguration=Release` -- passed with 0 warnings and 0 errors. - `$dotnetHost = "$PWD\.dotnet\dotnet.exe"; dotnet test src\native\managed\cdac\tests\UnitTests\Microsoft.Diagnostics.DataContractReader.Tests.csproj -c Release --no-restore -m:1 -p:NuGetConsoleProcessFileName="$dotnetHost" -p:BuildInParallel=false -p:RuntimeConfiguration=Release -p:LibrariesConfiguration=Release` -- passed, 4,044 tests. - The filtered stack-walk dump-test project compiled successfully, but no dump tests executed because all available local and .NET 10 configurations were skipped in this checkout. > [!NOTE] > This pull request description was generated with GitHub Copilot. Co-authored-by: Max Charlamb <maxcharlamb@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6297d10c-2f94-48bb-b897-bc0b3e1a855f
Summary
Return retained stack-context snapshots as
ReadOnlySpan<byte>instead of allocating a newbyte[]for every output request.This changes only the context-output APIs:
IStackWalk.GetRawContextreturns a read-only view of the frame's retained context snapshot.IPlatformAgnosticContext.GetBytesreturns a read-only view over the serialized platform context.The span remains backed by the context snapshot owned by the stack frame handle. Advancing the walk does not mutate that retained snapshot.
Performance
Measured frozen NativeAOT readers with provider identities reversed between two passes. Each pass used 3 launches with 30 observations per case, with no outlier removal. The primary run retained 2,520 observations across 14 timing cases.
A separate cold-walk confirmation used two more reversed passes and 720 additional observations:
The change removes one temporary 1,232-byte array for each AMD64 context output. Dump-enumeration timing does not directly exercise the migrated output APIs, and shared-host timing differences are not guaranteed gains.
Correctness checks confirmed identical serialized bytes for all seven platform context layouts, exact stack parity across 18 native/baseline/current cases, identical Mini/Heap/Triage dump ranges and updates, and matching context bytes for six real-dump cases in cold and warm states.
Testing
dotnet test src\native\managed\cdac\tests\UnitTests\Microsoft.Diagnostics.DataContractReader.Tests.csproj -c Release --no-restore -m:1 -p:BuildInParallel=false -p:RuntimeConfiguration=Release -p:LibrariesConfiguration=Release-- passed, 3,914 tests.dotnet test src\native\managed\cdac\tests\DumpTests\Microsoft.Diagnostics.DataContractReader.DumpTests.csproj -c Release --no-restore -m:1 -p:BuildInParallel=false -p:RuntimeConfiguration=Release -p:LibrariesConfiguration=Release -p:SkipDumpVersions=net10.0 --filter 'FullyQualifiedName~StackWalkDumpTests|FullyQualifiedName~IXCLRDataFrameDumpTests|FullyQualifiedName~DacDbiStackWalkDumpTests'-- passed, 63 tests with 19 expected skips.Note
This pull request description was generated with GitHub Copilot.