Skip to content
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.

Sign up for GitHub

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

TryAddWithoutValidation for multiple values could be more efficient #102845

Merged
merged 5 commits into from
Jun 10, 2024
Merged

TryAddWithoutValidation for multiple values could be more efficient #102845

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eee05d4
TryAddWithoutValidation for multiple values could be more efficient
pedrobsaila May 29, 2024
eb631d2
Merge remote-tracking branch 'upstream/main' into 64049
pedrobsaila May 31, 2024
7fde1c1
fix remarks
pedrobsaila May 31, 2024
dc32c5e
keep only ilist special implementation
pedrobsaila May 31, 2024
949cd7c
fix remarks 2
pedrobsaila Jun 3, 2024
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
106 changes: 93 additions & 13 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,28 +168,108 @@ public bool TryAddWithoutValidation(string name, IEnumerable<string?> values) =>
internal bool TryAddWithoutValidation(HeaderDescriptor descriptor, IEnumerable<string?> values)
{
ArgumentNullException.ThrowIfNull(values);
string?[]? valuesArray = values as string?[];
IList<string?>? valuesList = values as IList<string?>;
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved

using IEnumerator<string?> enumerator = values.GetEnumerator();
if (enumerator.MoveNext())
// the count is null when values is not a array nor IList
int? count = valuesArray != null ? valuesArray.Length : valuesList?.Count;

if (count == 0)
{
TryAddWithoutValidation(descriptor, enumerator.Current);
if (enumerator.MoveNext())
return true;
}

// read the store of header values
// The header store contain a single raw string value when header values are single.
// The header store contain HeaderStoreItemInfo that wraps around a List<string> with header values when they are more than once

ref object? storeValueRef = ref GetValueRefOrAddDefault(descriptor);
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved
object? storeValue = storeValueRef;

// check whether the header store contains already a value
// or header values count is more than one
// => allocate a HeaderStoreItemInfo on store that wraps around a List<string> of header values
if (storeValue is not null || (count.HasValue && count.Value > 1))
{
if (storeValue is not HeaderStoreItemInfo info)
{
storeValueRef = info = new HeaderStoreItemInfo { RawValue = storeValue };
}

object? rawValue = info.RawValue;
if (rawValue is not List<string> rawValues)
{
ref object? storeValueRef = ref GetValueRefOrAddDefault(descriptor);
Debug.Assert(storeValueRef is not null);
info.RawValue = rawValues = new List<string>();

object value = storeValueRef;
if (value is not HeaderStoreItemInfo info)
if (rawValue != null)
{
Debug.Assert(value is string);
storeValueRef = info = new HeaderStoreItemInfo { RawValue = value };
if (count.HasValue)
{
rawValues.EnsureCapacity(count.Value + 1);
}

rawValues.Add((string)rawValue);
}
}

if (count.HasValue)
{
rawValues.EnsureCapacity(count.Value);
}

do
if (valuesArray != null)
{
for (int i = 0; i < valuesArray.Length; i++)
{
AddRawValue(info, enumerator.Current ?? string.Empty);
rawValues.Add(valuesArray[i] ?? string.Empty);
}
}
else if (valuesList != null)
{
for (int i = 0; i < valuesList!.Count; i++)
{
rawValues.Add(valuesList[i] ?? string.Empty);
}
}
else
{
foreach (string? value in values)
{
rawValues.Add(value ?? string.Empty);
}
}
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved
}
else if (count == 1)
{
// when header values are single the store contains just the header value
storeValueRef = valuesArray != null ? valuesArray[0] : (valuesList != null ? valuesList![0] : null);
}
else
{
// handles the case when header values count is unknown because the values are abstracted by IEnumerable<string>
foreach (string? value in values)
{
if (storeValueRef is null)
{
storeValueRef = value;
}
else
{
if (storeValueRef is not HeaderStoreItemInfo info)
{
storeValueRef = info = new HeaderStoreItemInfo { RawValue = storeValueRef };
}

object? rawValue = info.RawValue;

if (rawValue is not List<string> rawValues)
{
info.RawValue = rawValues = new List<string>(2);
rawValues.Add(rawValue == null ? string.Empty : (string)rawValue);
}

rawValues.Add(value ?? string.Empty);
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved
}
while (enumerator.MoveNext());
}
}

Expand Down
90 changes: 90 additions & 0 deletions src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2544,6 +2544,96 @@ public void TryGetValues_InvalidValuesContainingNewLines_ShouldNotRemoveInvalidV
Assert.Equal(value, values.Single());
}

[Fact]
public void TryAddWithoutValidation_OneValidValueHeader_UseSpecialListImplementation()
{
const string Name = "customHeader1";
const string Value = "Value1";

var response = new HttpResponseMessage();
Assert.True(response.Headers.TryAddWithoutValidation(Name, new List<string> { Value }));

Assert.True(response.Headers.Contains(Name));

Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
Assert.Equal(Value, values.Single());
}

[Fact]
public void TryAddWithoutValidation_ThreeValidValueHeader_UseSpecialListImplementation()
{
const string Name = "customHeader1";
List<string> expectedValues = [ "Value1", "Value2", "Value3" ];

var response = new HttpResponseMessage();
Assert.True(response.Headers.TryAddWithoutValidation(Name, expectedValues));

Assert.True(response.Headers.Contains(Name));

Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
Assert.True(expectedValues.SequenceEqual(values));
}

[Fact]
public void TryAddWithoutValidation_OneValidValueHeader_UseSpecialArrayImplementation()
{
const string Name = "customHeader1";
const string Value = "Value1";

var response = new HttpResponseMessage();
Assert.True(response.Headers.TryAddWithoutValidation(Name, new [] { Value }));

Assert.True(response.Headers.Contains(Name));

Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
Assert.Equal(Value, values.Single());
}

[Fact]
public void TryAddWithoutValidation_ThreeValidValueHeader_UseSpecialArrayImplementation()
{
const string Name = "customHeader1";
string[] expectedValues = ["Value1", "Value2", "Value3"];

var response = new HttpResponseMessage();
Assert.True(response.Headers.TryAddWithoutValidation(Name, expectedValues));

Assert.True(response.Headers.Contains(Name));

Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
Assert.True(expectedValues.SequenceEqual(values));
}

[Fact]
public void TryAddWithoutValidation_OneValidValueHeader_UseGenericImplementation()
{
const string Name = "customHeader1";
const string Value = "Value1";

var response = new HttpResponseMessage();
Assert.True(response.Headers.TryAddWithoutValidation(Name, new HashSet<string> { Value }));

Assert.True(response.Headers.Contains(Name));

Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
Assert.Equal(Value, values.Single());
}

[Fact]
public void TryAddWithoutValidation_ThreeValidValueHeader_UseGenericImplementation()
{
const string Name = "customHeader1";
List<string> expectedValues = ["Value1", "Value2", "Value3"];

var response = new HttpResponseMessage();
Assert.True(response.Headers.TryAddWithoutValidation(Name, new HashSet<string>(expectedValues)));

Assert.True(response.Headers.Contains(Name));

Assert.True(response.Headers.TryGetValues(Name, out IEnumerable<string> values));
Assert.True(expectedValues.SequenceEqual(values));
}

public static IEnumerable<object[]> NumberOfHeadersUpToArrayThreshold_AddNonValidated_EnumerateNonValidated()
{
for (int i = 0; i <= HttpHeaders.ArrayThreshold; i++)
Expand Down