Use the built-in System.Reflection.Metadata types for .NET Core#8454
Use the built-in System.Reflection.Metadata types for .NET Core#8454andrewlock wants to merge 10 commits intoandrew/update-vendorsfrom
Conversation
BenchmarksBenchmark execution time: 2026-04-16 14:26:59 Comparing candidate commit fa2694a in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 87 known flaky benchmarks.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8454) 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 (8454) - mean (75ms) : 71, 79
master - mean (75ms) : 71, 78
section Bailout
This PR (8454) - mean (78ms) : 75, 82
master - mean (78ms) : 74, 81
section CallTarget+Inlining+NGEN
This PR (8454) - mean (1,072ms) : 1033, 1111
master - mean (1,073ms) : 1030, 1117
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 (8454) - mean (118ms) : 112, 124
master - mean (117ms) : 111, 123
section Bailout
This PR (8454) - mean (116ms) : 111, 120
master - mean (115ms) : 112, 119
section CallTarget+Inlining+NGEN
This PR (8454) - mean (775ms) : 749, 801
master - mean (793ms) : 766, 821
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8454) - mean (103ms) : 98, 109
master - mean (103ms) : 98, 108
section Bailout
This PR (8454) - mean (102ms) : 100, 104
master - mean (103ms) : 99, 108
section CallTarget+Inlining+NGEN
This PR (8454) - mean (939ms) : 898, 980
master - mean (940ms) : 905, 974
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8454) - mean (101ms) : 96, 105
master - mean (99ms) : 95, 103
section Bailout
This PR (8454) - mean (104ms) : 99, 109
master - mean (103ms) : 98, 107
section CallTarget+Inlining+NGEN
This PR (8454) - mean (819ms) : 784, 854
master - mean (826ms) : 786, 865
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 (8454) - mean (211ms) : 205, 217
master - mean (205ms) : 200, 210
section Bailout
This PR (8454) - mean (215ms) : 210, 220
master - mean (208ms) : 204, 212
section CallTarget+Inlining+NGEN
This PR (8454) - mean (1,237ms) : 1186, 1287
master - mean (1,197ms) : 1155, 1238
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 (8454) - mean (307ms) : 300, 313
master - mean (293ms) : 286, 300
section Bailout
This PR (8454) - mean (309ms) : 302, 316
master - mean (294ms) : 288, 299
section CallTarget+Inlining+NGEN
This PR (8454) - mean (999ms) : 973, 1024
master - mean (978ms) : 939, 1017
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8454) - mean (302ms) : 293, 312
master - mean (288ms) : 281, 296
section Bailout
This PR (8454) - mean (296ms) : 287, 305
master - mean (286ms) : 282, 291
section CallTarget+Inlining+NGEN
This PR (8454) - mean (1,177ms) : 1131, 1223
master - mean (1,163ms) : 1129, 1198
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8454) - mean (295ms) : 279, 311
master - mean (285ms) : 278, 293
section Bailout
This PR (8454) - mean (291ms) : 285, 297
master - mean (287ms) : 281, 293
section CallTarget+Inlining+NGEN
This PR (8454) - mean (1,071ms) : 985, 1156
master - mean (1,055ms) : 1003, 1107
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
9f49119 to
7ea4e03
Compare
9f055d8 to
fa2694a
Compare
Summary of changes
Use the built-in System.Reflection.Metadata types for .NET Core 3.1+
Reason for change
In #6726, we switched from always using the vendored System.Memory and other BCL types, to using the built-in versions for .NET Core 3.1 where they're available. This was done with the goal of reducing artifact size, while also hopefully improving consistency, and potentially improving performance.
Separately, we're working on updating our vendored code to make updating it more repeatable (see #8391). This flagged a variety of modifications we'd made to the vendored System.Reflection.Metadata code to be able to use internal members.
Rather than include those modifications in the updated vendored code, this PR updates our existing usages to not use those internal members in any scenario. This also means we can stop using the vendored code for .NET Core 3.1+.
Implementation details
Hashtype used by Debugger code, which was internal to System.Reflection.MetadataFNV1aHash, it isn't usingFNV1a, so I chose to call it simple hash (as that's what it's doing, a simple "hash code" calculation).GlobalUsings.cs, because doing so causes a variety of namespace collisions across our vendored code (Newtonsoft.Json, dnlib)usingstatements needed removing.MethodDefinitionFromRowId.MethodDefinition.Handlewas a little more painful, as it required more refactoring, but fundamentally we were always deriving aMethodDefinitionfrom aMethodDefinitionHandle, so the changes are mostly a minor inconvenience, but they now only use public APIs.Test coverage
Should be covered by existing tests; all these changes are just "refactorings", nothing additional.
Assembly size comparison
netcoreapp3.1net6.0Estimated size per namespace
Before (
net6.0)After (
net6.0) (all gone)Other details
https://datadoghq.atlassian.net/browse/APMLP-1207
Part of a stack updating our vendored system code