Skip to content

Commit effb34d

Browse files
authored
Avoid O(N) string[] allocations for N header values in GetCommaSeparatedValues (#54808)
* Avoid O(N) string[] allocations for N header values in GetCommaSeparatedValues * Address PR feedback and fix internalsvisibleto perf tests
1 parent 3c7cad3 commit effb34d

File tree

5 files changed

+113
-8
lines changed

5 files changed

+113
-8
lines changed

src/Http/Http.Abstractions/perf/Microbenchmarks/GetHeaderSplitBenchmark.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public void GlobalSetup()
2727
public void SplitSingleHeader()
2828
{
2929
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "singleValue");
30-
if (values.Count != 1)
30+
if (values.Length != 1)
3131
{
3232
throw new Exception();
3333
}
@@ -37,7 +37,7 @@ public void SplitSingleHeader()
3737
public void SplitSingleQuotedHeader()
3838
{
3939
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "singleValueQuoted");
40-
if (values.Count != 1)
40+
if (values.Length != 1)
4141
{
4242
throw new Exception();
4343
}
@@ -47,7 +47,7 @@ public void SplitSingleQuotedHeader()
4747
public void SplitDoubleHeader()
4848
{
4949
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "doubleValue");
50-
if (values.Count != 2)
50+
if (values.Length != 2)
5151
{
5252
throw new Exception();
5353
}
@@ -57,7 +57,7 @@ public void SplitDoubleHeader()
5757
public void SplitManyHeaders()
5858
{
5959
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "manyValue");
60-
if (values.Count != 6)
60+
if (values.Length != 6)
6161
{
6262
throw new Exception();
6363
}

src/Http/Http.Abstractions/src/Extensions/HeaderDictionaryExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public static string[] GetCommaSeparatedValues(this IHeaderDictionary headers, s
4343
{
4444
// GetHeaderSplit will return only non-null elements of the given IHeaderDictionary.
4545
#pragma warning disable CS8619 // Nullability of reference types in value doesn't match target type.
46-
return ParsingHelpers.GetHeaderSplit(headers, key).ToArray();
46+
return ParsingHelpers.GetHeaderSplit(headers, key);
4747
#pragma warning restore CS8619 // Nullability of reference types in value doesn't match target type.
4848
}
4949

src/Http/Http.Abstractions/src/Internal/ParsingHelpers.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ public static StringValues GetHeader(IHeaderDictionary headers, string key)
1414
return headers.TryGetValue(key, out value) ? value : StringValues.Empty;
1515
}
1616

17-
public static StringValues GetHeaderSplit(IHeaderDictionary headers, string key)
17+
public static string[] GetHeaderSplit(IHeaderDictionary headers, string key)
1818
{
1919
var values = GetHeaderUnmodified(headers, key);
2020

21-
StringValues result = default;
21+
ValueListBuilder<string> strings = new([null!, null!, null!, null!]);
2222

2323
foreach (var segment in new HeaderSegmentCollection(values))
2424
{
@@ -27,11 +27,14 @@ public static StringValues GetHeaderSplit(IHeaderDictionary headers, string key)
2727
var value = DeQuote(segment.Data.Value);
2828
if (!string.IsNullOrEmpty(value))
2929
{
30-
result = StringValues.Concat(in result, value);
30+
strings.Add(value);
3131
}
3232
}
3333
}
3434

35+
string[] result = strings.AsSpan().ToArray();
36+
strings.Dispose();
37+
3538
return result;
3639
}
3740

src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Microsoft.AspNetCore.Http.HttpResponse</Description>
3131
<Compile Include="$(SharedSourceRoot)Debugger\DebuggerHelpers.cs" LinkBase="Shared" />
3232
<Compile Include="$(SharedSourceRoot)Debugger\DictionaryItemDebugView.cs" LinkBase="Shared" />
3333
<Compile Include="$(SharedSourceRoot)Debugger\DictionaryDebugView.cs" LinkBase="Shared" />
34+
<Compile Include="$(SharedSourceRoot)ValueStringBuilder\ValueListBuilder.cs" LinkBase="Shared" />
3435
</ItemGroup>
3536

3637
<ItemGroup>
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Buffers;
5+
using System.Diagnostics;
6+
using System.Runtime.CompilerServices;
7+
8+
// Copied from https://github.com/dotnet/runtime/blob/a9ed4168626c14b4d74db0d8c205c69e56fc45ed/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs
9+
// with unused members removed.
10+
11+
namespace System.Collections.Generic;
12+
13+
internal ref partial struct ValueListBuilder<T>
14+
{
15+
private Span<T> _span;
16+
private T[]? _arrayFromPool;
17+
private int _pos;
18+
19+
public ValueListBuilder(Span<T> initialSpan)
20+
{
21+
_span = initialSpan;
22+
_arrayFromPool = null;
23+
_pos = 0;
24+
}
25+
26+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
27+
public void Add(T item)
28+
{
29+
int pos = _pos;
30+
31+
// Workaround for https://github.com/dotnet/runtime/issues/72004
32+
Span<T> span = _span;
33+
if ((uint)pos < (uint)span.Length)
34+
{
35+
span[pos] = item;
36+
_pos = pos + 1;
37+
}
38+
else
39+
{
40+
AddWithResize(item);
41+
}
42+
}
43+
44+
[MethodImpl(MethodImplOptions.NoInlining)]
45+
private void AddWithResize(T item)
46+
{
47+
Debug.Assert(_pos == _span.Length);
48+
int pos = _pos;
49+
Grow(1);
50+
_span[pos] = item;
51+
_pos = pos + 1;
52+
}
53+
54+
public ReadOnlySpan<T> AsSpan() => _span.Slice(0, _pos);
55+
56+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
57+
public void Dispose()
58+
{
59+
T[]? toReturn = _arrayFromPool;
60+
if (toReturn != null)
61+
{
62+
_arrayFromPool = null;
63+
ArrayPool<T>.Shared.Return(toReturn);
64+
}
65+
}
66+
67+
// Note that consuming implementations depend on the list only growing if it's absolutely
68+
// required. If the list is already large enough to hold the additional items be added,
69+
// it must not grow. The list is used in a number of places where the reference is checked
70+
// and it's expected to match the initial reference provided to the constructor if that
71+
// span was sufficiently large.
72+
private void Grow(int additionalCapacityRequired = 1)
73+
{
74+
const int ArrayMaxLength = 0x7FFFFFC7; // same as Array.MaxLength
75+
76+
// Double the size of the span. If it's currently empty, default to size 4,
77+
// although it'll be increased in Rent to the pool's minimum bucket size.
78+
int nextCapacity = Math.Max(_span.Length != 0 ? _span.Length * 2 : 4, _span.Length + additionalCapacityRequired);
79+
80+
// If the computed doubled capacity exceeds the possible length of an array, then we
81+
// want to downgrade to either the maximum array length if that's large enough to hold
82+
// an additional item, or the current length + 1 if it's larger than the max length, in
83+
// which case it'll result in an OOM when calling Rent below. In the exceedingly rare
84+
// case where _span.Length is already int.MaxValue (in which case it couldn't be a managed
85+
// array), just use that same value again and let it OOM in Rent as well.
86+
if ((uint)nextCapacity > ArrayMaxLength)
87+
{
88+
nextCapacity = Math.Max(Math.Max(_span.Length + 1, ArrayMaxLength), _span.Length);
89+
}
90+
91+
T[] array = ArrayPool<T>.Shared.Rent(nextCapacity);
92+
_span.CopyTo(array);
93+
94+
T[]? toReturn = _arrayFromPool;
95+
_span = _arrayFromPool = array;
96+
if (toReturn != null)
97+
{
98+
ArrayPool<T>.Shared.Return(toReturn);
99+
}
100+
}
101+
}

0 commit comments

Comments
 (0)