Skip to content

Avoid O(N) string[] allocations for N header values in GetCommaSeparatedValues #54808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void GlobalSetup()
public void SplitSingleHeader()
{
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "singleValue");
if (values.Count != 1)
if (values.Length != 1)
{
throw new Exception();
}
Expand All @@ -37,7 +37,7 @@ public void SplitSingleHeader()
public void SplitSingleQuotedHeader()
{
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "singleValueQuoted");
if (values.Count != 1)
if (values.Length != 1)
{
throw new Exception();
}
Expand All @@ -47,7 +47,7 @@ public void SplitSingleQuotedHeader()
public void SplitDoubleHeader()
{
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "doubleValue");
if (values.Count != 2)
if (values.Length != 2)
{
throw new Exception();
}
Expand All @@ -57,7 +57,7 @@ public void SplitDoubleHeader()
public void SplitManyHeaders()
{
var values = ParsingHelpers.GetHeaderSplit(_dictionary, "manyValue");
if (values.Count != 6)
if (values.Length != 6)
{
throw new Exception();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static string[] GetCommaSeparatedValues(this IHeaderDictionary headers, s
{
// GetHeaderSplit will return only non-null elements of the given IHeaderDictionary.
#pragma warning disable CS8619 // Nullability of reference types in value doesn't match target type.
return ParsingHelpers.GetHeaderSplit(headers, key).ToArray();
return ParsingHelpers.GetHeaderSplit(headers, key);
#pragma warning restore CS8619 // Nullability of reference types in value doesn't match target type.
}

Expand Down
9 changes: 6 additions & 3 deletions src/Http/Http.Abstractions/src/Internal/ParsingHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ public static StringValues GetHeader(IHeaderDictionary headers, string key)
return headers.TryGetValue(key, out value) ? value : StringValues.Empty;
}

public static StringValues GetHeaderSplit(IHeaderDictionary headers, string key)
public static string[] GetHeaderSplit(IHeaderDictionary headers, string key)
{
var values = GetHeaderUnmodified(headers, key);

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

foreach (var segment in new HeaderSegmentCollection(values))
{
Expand All @@ -27,11 +27,14 @@ public static StringValues GetHeaderSplit(IHeaderDictionary headers, string key)
var value = DeQuote(segment.Data.Value);
if (!string.IsNullOrEmpty(value))
{
result = StringValues.Concat(in result, value);
strings.Add(value);
}
}
}

string[] result = strings.AsSpan().ToArray();
strings.Dispose();

return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Microsoft.AspNetCore.Http.HttpResponse</Description>
<Compile Include="$(SharedSourceRoot)Debugger\DebuggerHelpers.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)Debugger\DictionaryItemDebugView.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)Debugger\DictionaryDebugView.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)ValueStringBuilder\ValueListBuilder.cs" LinkBase="Shared" />
</ItemGroup>

<ItemGroup>
Expand Down
101 changes: 101 additions & 0 deletions src/Shared/ValueStringBuilder/ValueListBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.Diagnostics;
using System.Runtime.CompilerServices;

// Copied from https://github.com/dotnet/runtime/blob/a9ed4168626c14b4d74db0d8c205c69e56fc45ed/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs
// with unused members removed.

namespace System.Collections.Generic;

internal ref partial struct ValueListBuilder<T>
{
private Span<T> _span;
private T[]? _arrayFromPool;
private int _pos;

public ValueListBuilder(Span<T> initialSpan)
{
_span = initialSpan;
_arrayFromPool = null;
_pos = 0;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Add(T item)
{
int pos = _pos;

// Workaround for https://github.com/dotnet/runtime/issues/72004
Span<T> span = _span;
if ((uint)pos < (uint)span.Length)
{
span[pos] = item;
_pos = pos + 1;
}
else
{
AddWithResize(item);
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void AddWithResize(T item)
{
Debug.Assert(_pos == _span.Length);
int pos = _pos;
Grow(1);
_span[pos] = item;
_pos = pos + 1;
}

public ReadOnlySpan<T> AsSpan() => _span.Slice(0, _pos);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Dispose()
{
T[]? toReturn = _arrayFromPool;
if (toReturn != null)
{
_arrayFromPool = null;
ArrayPool<T>.Shared.Return(toReturn);
}
}

// Note that consuming implementations depend on the list only growing if it's absolutely
// required. If the list is already large enough to hold the additional items be added,
// it must not grow. The list is used in a number of places where the reference is checked
// and it's expected to match the initial reference provided to the constructor if that
// span was sufficiently large.
private void Grow(int additionalCapacityRequired = 1)
{
const int ArrayMaxLength = 0x7FFFFFC7; // same as Array.MaxLength

// Double the size of the span. If it's currently empty, default to size 4,
// although it'll be increased in Rent to the pool's minimum bucket size.
int nextCapacity = Math.Max(_span.Length != 0 ? _span.Length * 2 : 4, _span.Length + additionalCapacityRequired);

// If the computed doubled capacity exceeds the possible length of an array, then we
// want to downgrade to either the maximum array length if that's large enough to hold
// an additional item, or the current length + 1 if it's larger than the max length, in
// which case it'll result in an OOM when calling Rent below. In the exceedingly rare
// case where _span.Length is already int.MaxValue (in which case it couldn't be a managed
// array), just use that same value again and let it OOM in Rent as well.
if ((uint)nextCapacity > ArrayMaxLength)
{
nextCapacity = Math.Max(Math.Max(_span.Length + 1, ArrayMaxLength), _span.Length);
}

T[] array = ArrayPool<T>.Shared.Rent(nextCapacity);
_span.CopyTo(array);

T[]? toReturn = _arrayFromPool;
_span = _arrayFromPool = array;
if (toReturn != null)
{
ArrayPool<T>.Shared.Return(toReturn);
}
}
}