[Code Origin] Optimize per-assembly PDB cache#8631
Conversation
BenchmarksBenchmark execution time: 2026-05-14 12:36:16 Comparing candidate commit 3c1086a in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in baseline:
Found 3 performance improvements and 7 performance regressions! Performance is the same for 44 metrics, 18 unstable metrics, 88 known flaky benchmarks, 38 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8631) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (74ms) : 70, 79
master - mean (74ms) : 70, 78
section Bailout
This PR (8631) - mean (78ms) : 75, 81
master - mean (77ms) : 75, 79
section CallTarget+Inlining+NGEN
This PR (8631) - mean (1,114ms) : 1051, 1178
master - mean (1,103ms) : 1043, 1164
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (115ms) : 109, 121
master - mean (114ms) : 109, 120
section Bailout
This PR (8631) - mean (115ms) : 112, 119
master - mean (114ms) : 110, 117
section CallTarget+Inlining+NGEN
This PR (8631) - mean (791ms) : 761, 821
master - mean (793ms) : 762, 823
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (105ms) : 99, 111
master - mean (102ms) : 97, 108
section Bailout
This PR (8631) - mean (104ms) : 100, 107
master - mean (102ms) : 100, 104
section CallTarget+Inlining+NGEN
This PR (8631) - mean (956ms) : 918, 994
master - mean (947ms) : 902, 991
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (101ms) : 95, 107
master - mean (101ms) : 96, 107
section Bailout
This PR (8631) - mean (104ms) : 97, 110
master - mean (103ms) : 98, 108
section CallTarget+Inlining+NGEN
This PR (8631) - mean (822ms) : 782, 863
master - mean (821ms) : 778, 864
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (200ms) : 194, 206
master - mean (199ms) : 193, 205
section Bailout
This PR (8631) - mean (204ms) : 198, 209
master - mean (202ms) : 199, 206
section CallTarget+Inlining+NGEN
This PR (8631) - mean (1,205ms) : 1143, 1266
master - mean (1,197ms) : 1153, 1240
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (289ms) : 282, 297
master - mean (287ms) : 278, 296
section Bailout
This PR (8631) - mean (290ms) : 281, 298
master - mean (287ms) : 281, 294
section CallTarget+Inlining+NGEN
This PR (8631) - mean (965ms) : 942, 989
master - mean (961ms) : 945, 978
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (281ms) : 274, 287
master - mean (276ms) : 268, 285
section Bailout
This PR (8631) - mean (280ms) : 274, 286
master - mean (278ms) : 272, 284
section CallTarget+Inlining+NGEN
This PR (8631) - mean (1,156ms) : 1102, 1210
master - mean (1,157ms) : 1125, 1190
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8631) - mean (280ms) : 274, 286
master - mean (279ms) : 274, 285
section Bailout
This PR (8631) - mean (280ms) : 275, 285
master - mean (279ms) : 275, 284
section CallTarget+Inlining+NGEN
This PR (8631) - mean (1,041ms) : 996, 1086
master - mean (1,037ms) : 995, 1078
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR optimizes code-origin PDB analysis by switching SpanCodeOrigin’s per-assembly caching to a weak-keyed ConditionalWeakTable<Assembly, Lazy<...>>, reducing cross-assembly contention and enabling a metadata-only PDB reader path that avoids full PE prefetch when IL bodies aren’t needed.
Changes:
- Replaced the old per-assembly caches in
SpanCodeOriginwith a weakly-keyed, per-assemblyLazy<AssemblyAnalysis>to deduplicate same-assembly first-touch work while allowing parallel analysis across assemblies. - Added
metadataOnlysupport toDatadogMetadataReader.CreatePdbReader(...), including more explicit resource-ownership/disposal to prevent file-handle leaks (notably for sidecar portable PDB providers). - Added a regression test to ensure
metadataOnly: truestill discovers associated PDBs equivalently to the full-prefetch path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tracer/test/Datadog.Trace.Tests/Pdb/DatadogMetadataReaderTests.cs | Adds a regression test asserting PDB discovery equivalence for metadataOnly: true vs full-prefetch. |
| tracer/src/Datadog.Trace/PDBs/DatadogMetadataReader.cs | Adds metadataOnly mode and improves explicit disposal/ownership for PE/PDB resources. |
| tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOrigin.cs | Reworks per-assembly analysis caching using ConditionalWeakTable<Assembly, Lazy<AssemblyAnalysis>> and uses metadataOnly: true PDB reads for endpoint sequence points. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5bd7948 to
2cd4aab
Compare
…se metadata reader resources
Co-authored-by: Cursor <[email protected]>
Co-authored-by: Cursor <[email protected]>
Co-authored-by: Cursor <[email protected]>
2cd4aab to
3c1086a
Compare
Summary of changes
ConcurrentAdaptiveCache/ separate skip-cache path with a weakly-keyedConditionalWeakTable<Assembly, Lazy<AssemblyAnalysis>>.Reason for change
Implementation details
SpanCodeOriginnow stores aLazy<AssemblyAnalysis>perAssembly, keyed byConditionalWeakTable, so:AssemblyAnalysisstores both the skip decision and the endpoint token-to-sequence-point dictionary.DatadogMetadataReader.CreatePdbReader()now supportsmetadataOnly: true.PEStreamOptions.Defaultinstead ofPrefetchMetadata | PrefetchEntireImage.PEReader,MetadataReaderProvider, dnlib module, and dnlib symbol reader are disposed if construction exits early.Test coverage
Added unit coverage for
CreatePdbReader(metadataOnly: true)to verify it still discovers the same PDB availability as the full-prefetch path.Ran local BenchmarkDotNet comparison on Windows, .NET 10.0.7, BenchmarkDotNet 0.15.8:
Other details