Fix excessive allocation in vendored SharedArrayPool#8442
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8442) 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 (8442) - mean (72ms) : 68, 75
master - mean (72ms) : 69, 74
section Bailout
This PR (8442) - mean (76ms) : 74, 77
master - mean (76ms) : 75, 78
section CallTarget+Inlining+NGEN
This PR (8442) - mean (1,067ms) : 1016, 1118
master - mean (1,066ms) : 1023, 1110
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 (8442) - mean (111ms) : 107, 115
master - mean (111ms) : 108, 115
section Bailout
This PR (8442) - mean (112ms) : 110, 115
master - mean (113ms) : 110, 116
section CallTarget+Inlining+NGEN
This PR (8442) - mean (784ms) : 759, 809
master - mean (783ms) : 763, 803
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8442) - mean (99ms) : 95, 103
master - mean (99ms) : 95, 103
section Bailout
This PR (8442) - mean (100ms) : 96, 105
master - mean (99ms) : 97, 102
section CallTarget+Inlining+NGEN
This PR (8442) - mean (933ms) : 891, 974
master - mean (931ms) : 903, 960
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8442) - mean (98ms) : 94, 101
master - mean (98ms) : 95, 101
section Bailout
This PR (8442) - mean (98ms) : 96, 100
master - mean (98ms) : 96, 100
section CallTarget+Inlining+NGEN
This PR (8442) - mean (814ms) : 776, 852
master - mean (811ms) : 778, 844
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 (8442) - mean (191ms) : 188, 194
master - mean (191ms) : 188, 195
section Bailout
This PR (8442) - mean (195ms) : 193, 197
master - mean (195ms) : 193, 196
section CallTarget+Inlining+NGEN
This PR (8442) - mean (1,145ms) : 1102, 1187
master - mean (1,141ms) : 1097, 1185
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 (8442) - mean (275ms) : 270, 279
master - mean (273ms) : 269, 277
section Bailout
This PR (8442) - mean (275ms) : 272, 278
master - mean (274ms) : 271, 277
section CallTarget+Inlining+NGEN
This PR (8442) - mean (930ms) : 902, 958
master - mean (925ms) : 905, 944
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8442) - mean (268ms) : 264, 272
master - mean (268ms) : 265, 272
section Bailout
This PR (8442) - mean (268ms) : 265, 271
master - mean (268ms) : 265, 270
section CallTarget+Inlining+NGEN
This PR (8442) - mean (1,138ms) : 1099, 1178
master - mean (1,136ms) : 1093, 1179
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8442) - mean (267ms) : 263, 270
master - mean (267ms) : 262, 271
section Bailout
This PR (8442) - mean (267ms) : 263, 271
master - mean (266ms) : 263, 270
section CallTarget+Inlining+NGEN
This PR (8442) - mean (1,020ms) : 987, 1052
master - mean (1,018ms) : 979, 1058
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmark execution time: 2026-04-13 09:00:01 Comparing candidate commit c6ac8de in PR branch Found 1 performance improvements and 1 performance regressions! Performance is the same for 25 metrics, 0 unstable metrics, 87 known flaky benchmarks.
|
|
The benchmarks in the PR description don't have before vs after 😅 |
The point was comparing .NET FX (i.e. vendored array pool) to built-in array pool 😉
|
Summary of changes
Fixes a critical allocation regression in vendored
ArrayPoolimplementationReason for change
When vendored, the
SharedArrayPoolintroduced a large allocation, by callingProcess.GetCurrentProcess()every time an array is rented or returned. This causes significant allocation (potentially even more than not using the pool would) on applicable TFMs (i.e. .NET Framework and <.NET Core 3.1).This was obviously introduced during vendoring due to the lack of the
Thread.GetCurrentProcessorId()method in .NET Framework, but unfortunately the chosen replacement is likely worse for performance than just using a constant 😢Implementation details
The immediate fix is instead of calling
Process.GetCurrentProcess(), useEnvironment.CurrentManagedThreadIdto try to get some kind of "round robin" behaviour.The longer-term fix is likely to update the vendor implementation to use the .NET Framework-appropriate versions instead of trying to backport .NET 7 to .NET Framework
Test coverage
I discovered this when benchmarking something else, where it showed up as a stark difference in .NET Framework, which was the only TFM using the vendored array
Other details
Should be fixed at source as part of https://datadoghq.atlassian.net/browse/APMLP-1207