Skip to content

Commit 102da2d

Browse files
authored
Fix excessive allocation in vendored SharedArrayPool (#8442)
## Summary of changes Fixes a critical allocation regression in vendored `ArrayPool` implementation ## Reason for change When vendored, the `SharedArrayPool` introduced a large allocation, by calling `Process.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 😢 ```csharp int index = (int)((uint)Process.GetCurrentProcess().Id % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1 // todo: fix int index = (int)((uint)Thread.GetCurrentProcessorId() % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1 ``` ## Implementation details The immediate fix is instead of calling `Process.GetCurrentProcess()`, use `Environment.CurrentManagedThreadId` to 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 | Method | Runtime | Mean | Allocated | | ------------------------------- | ------------------ | ----------: | --------: | | BuildKey_ClientSpanWithPeerTags | .NET 10.0 | 852.78 ns | 688 B | | BuildKey_ClientSpanWithPeerTags | .NET 6.0 | 1,144.97 ns | 688 B | | BuildKey_ClientSpanWithPeerTags | .NET Core 3.1 | 1,666.94 ns | 688 B | | BuildKey_ClientSpanWithPeerTags | .NET Framework 4.8 | 4,002.83 ns | 2391 B | ## Other details Should be fixed at source as part of https://datadoghq.atlassian.net/browse/APMLP-1207
1 parent 58f22e8 commit 102da2d

1 file changed

Lines changed: 2 additions & 2 deletions

File tree

tracer/src/Datadog.Trace/Vendors/System.Memory/Buffers/SharedArrayPool.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ public bool TryPush(T[] array)
298298
// Try to push on to the associated partition first. If that fails,
299299
// round-robin through the other partitions.
300300
Partition[] partitions = _partitions;
301-
int index = (int)((uint)Process.GetCurrentProcess().Id % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1
301+
int index = (int)((uint)Environment.CurrentManagedThreadId % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1
302302
// int index = (int)((uint)Thread.GetCurrentProcessorId() % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1
303303
for (int i = 0; i < partitions.Length; i++)
304304
{
@@ -319,7 +319,7 @@ public bool TryPush(T[] array)
319319
// Try to pop from the associated partition first. If that fails, round-robin through the other partitions.
320320
T[]? arr;
321321
Partition[] partitions = _partitions;
322-
int index = (int)((uint)Process.GetCurrentProcess().Id % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1
322+
int index = (int)((uint)Environment.CurrentManagedThreadId % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1
323323
// todo: fix int index = (int)((uint)Thread.GetCurrentProcessorId() % (uint)SharedArrayPoolStatics.s_partitionCount); // mod by constant in tier 1
324324
for (int i = 0; i < partitions.Length; i++)
325325
{

0 commit comments

Comments
 (0)