Add NoInlining to ArrayPool Rent/Return#129319
Conversation
These methods result in inlining based on a lot of questionable heuristics. For example, the benchmark in dotnet#124285 went from 200 bytes in .NET 9 to 1567 bytes in .NET 10 because of inlining these two methods. Additionally `Return` has a tendency to appear in `finally` clauses, and we do not penalize inlining into these (even though we probably should), so this can also disable finally cloning. As a surgical fix late in the cycle this just marks these methods `NoInlining`.
|
Tagging subscribers to this area: @dotnet/area-system-buffers |
There was a problem hiding this comment.
Pull request overview
This PR applies [MethodImpl(MethodImplOptions.NoInlining)] to the Rent and Return overrides on the primary ArrayPool<T> implementations in System.Private.CoreLib, to prevent the JIT from inlining these relatively large methods into callers.
Changes:
- Mark
SharedArrayPool<T>.Rent/ReturnasNoInlining. - Mark
ConfigurableArrayPool<T>.Rent/ReturnasNoInlining(and add the requiredSystem.Runtime.CompilerServicesusing).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs | Adds NoInlining to Rent / Return on the shared pool implementation. |
| src/libraries/System.Private.CoreLib/src/System/Buffers/ConfigurableArrayPool.cs | Adds NoInlining to Rent / Return on the configurable pool implementation and adds the needed using. |
tannergooding
left a comment
There was a problem hiding this comment.
Overall LGTM.
It'd be good to get some SPMI and/or perf numbers included here and showcasing the improvement.
The general downside of NoInlining is of course that the JIT will not attempt to observe the IL and infer any heuristics about the method in question, but that should be fine here.
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Jobs;
using System;
using System.Buffers;
using System.Runtime.InteropServices;
namespace RentSpanUnmanagedPerfTests;
public class ArrayPoolForeachRepro
{
private const int Size = 1000;
[Benchmark(Description = "ArrayPool + Span - foreach (SLOW on .NET 10 and .NET 9)")]
public int ArrayPool_Foreach()
{
var pool = ArrayPool<int>.Shared;
var array = pool.Rent(Size);
try
{
var span = new Span<int>(array, 0, Size);
// Write
for (var i = 0; i < Size; i++)
{
span[i] = i * 2;
}
// Read with foreach
var sum = 0;
foreach (ref readonly var value in span)
{
sum += value;
}
return sum;
}
finally
{
pool.Return(array);
}
}
[Benchmark(Description = "ArrayPool + Span - for loop (OK on .NET 10 and .NET 9)")]
public int ArrayPool_ForLoop()
{
var pool = ArrayPool<int>.Shared;
var array = pool.Rent(Size);
try
{
var span = new Span<int>(array, 0, Size);
// Write
for (var i = 0; i < Size; i++)
{
span[i] = i * 2;
}
// Read with for loop
var sum = 0;
for (var i = 0; i < span.Length; i++)
{
sum += span[i];
}
return sum;
}
finally
{
pool.Return(array);
}
}
[Benchmark(Description = "RefStruct + Span field - foreach (SLOW on .NET 10, FAST on .NET 9)")]
public int RefStruct_Foreach()
{
using var wrapper = new SpanWrapper(Size);
// Write
for (var i = 0; i < Size; i++)
{
wrapper.Span[i] = i * 2;
}
// Read with foreach
var sum = 0;
var span = wrapper.Span;
foreach (ref readonly var value in span)
{
sum += value;
}
return sum;
}
[Benchmark(Description = "RefStruct + Span field - for loop (OK on .NET 10 and .NET 9)")]
public int RefStruct_ForLoop()
{
using var wrapper = new SpanWrapper(Size);
// Write
for (var i = 0; i < Size; i++)
{
wrapper.Span[i] = i * 2;
}
// Read with for loop
var sum = 0;
for (var i = 0; i < wrapper.Span.Length; i++)
{
sum += wrapper.Span[i];
}
return sum;
}
private readonly ref struct SpanWrapper
{
private readonly int[] m_Array;
public readonly Span<int> Span;
public SpanWrapper(int length)
{
m_Array = ArrayPool<int>.Shared.Rent(length);
Span = new Span<int>(m_Array, 0, length);
}
public void Dispose()
{
ArrayPool<int>.Shared.Return(m_Array);
}
}
} |
|
@EgorBot -intel using BenchmarkDotNet.Attributes;
using System;
using System.Buffers;
public class ArrayPoolForeachRepro
{
private const int Size = 1000;
[Benchmark]
public int RefStruct_Foreach()
{
using var wrapper = new SpanWrapper(Size);
for (var i = 0; i < Size; i++)
wrapper.Span[i] = i * 2;
var sum = 0;
var span = wrapper.Span;
foreach (ref readonly var value in span)
sum += value;
return sum;
}
private readonly ref struct SpanWrapper
{
private readonly int[] m_Array;
public readonly Span<int> Span;
public SpanWrapper(int length)
{
m_Array = ArrayPool<int>.Shared.Rent(length);
Span = new Span<int>(m_Array, 0, length);
}
public void Dispose() => ArrayPool<int>.Shared.Return(m_Array);
}
} |
|
On my machine the results look like:
Oddly Egorbot does not see an improvement, and when I look at its disassembly it has a well-optimized inner loop of 2026-06-17 15:24:04.417 G_M000_IG08: ;; offset=0x00E0
2026-06-17 15:24:04.417 add ecx, dword ptr [r14+rax]
2026-06-17 15:24:04.417 add rax, 4
2026-06-17 15:24:04.417 dec edx
2026-06-17 15:24:04.417 jne SHORT G_M000_IG08whereas my inner loop without this fix looks like G_M000_IG08: ;; offset=0x00E0
mov eax, dword ptr [rbp-0x3C]
add ecx, dword ptr [r14+4*rax]
mov eax, dword ptr [rbp-0x3C]
inc eax
mov dword ptr [rbp-0x3C], eax
cmp dword ptr [rbp-0x3C], 0x3E8
jl SHORT G_M000_IG08Need to investigate a bit further what's different in the bot's environment. |
|
@EgorBot --envvars DOTNET_JitDisasm:RefStruct_Foreach using BenchmarkDotNet.Attributes;
using System;
using System.Buffers;
public class ArrayPoolForeachRepro
{
private const int Size = 1000;
[Benchmark]
public int RefStruct_Foreach()
{
using var wrapper = new SpanWrapper(Size);
for (var i = 0; i < Size; i++)
wrapper.Span[i] = i * 2;
var sum = 0;
var span = wrapper.Span;
foreach (ref readonly var value in span)
sum += value;
return sum;
}
private readonly ref struct SpanWrapper
{
private readonly int[] m_Array;
public readonly Span<int> Span;
public SpanWrapper(int length)
{
m_Array = ArrayPool<int>.Shared.Rent(length);
Span = new Span<int>(m_Array, 0, length);
}
public void Dispose() => ArrayPool<int>.Shared.Return(m_Array);
}
} |
|
Still not seeing the perf improvement on egorbot, but I have validated this locally. I do see the large inlining happening even in egorbot's version, though the inner loop is efficient even with it. I am not totally sure what the difference is, but since the customer also has the perf problem I think we can roll with it. |
|
Azure Pipelines: Successfully started running 3 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
@jakobbotsch given you see a large improvement and EgorBot at best sees a small 5ns regression (generally within noise given the overall runtime), I'd say we take this and look at official perf lab results |
|
/ba-g Timeouts |
These methods result in inlining based on a lot of questionable heuristics. For example, the benchmark in #124285 went from 200 bytes in .NET 9 to 1567 bytes in .NET 10 because of inlining these two methods. Additionally
Returnhas a tendency to appear infinallyclauses, and we do not penalize inlining into these (even though we probably should), so this can also disable finally cloning.As a surgical fix late in the cycle this just marks these methods
NoInlining.Fix #124285