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

Jump to bottom

TryAddWithoutValidation for multiple values could be more efficient #4240

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

TryAddWithoutValidation for multiple values could be more efficient #4240

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
057850e
TryAddWithoutValidation for multiple values could be more efficient
pedrobsaila May 29, 2024
a71b984
fix remarks
pedrobsaila May 31, 2024
1ce8bdf
adjust comment of Array case
pedrobsaila May 31, 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
44 changes: 44 additions & 0 deletions src/benchmarks/micro/libraries/System.Net.Http/TryAddWithoutValidation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Engines;
using MicroBenchmarks;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;

namespace System.Net.Http.Tests
{
[BenchmarkCategory(Categories.Libraries, Categories.LINQ)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[BenchmarkCategory(Categories.Libraries, Categories.LINQ)]
[BenchmarkCategory(Categories.Libraries)]

public class TryAddWithoutValidationTests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make a new class for HttpHeaders instead, with this being one of the tested methods there

{
[Benchmark]
[ArgumentsSource(nameof(IEnumerableArgument))]
public bool AddHeaders(AddWithoutValidationTestData data) => new HttpResponseMessage().Headers.TryAddWithoutValidation("headerName", data.Values);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make the test more focused on just the adding part, e.g.

private readonly HttpResponseHeaders _responseHeaders = new HttpResponseMessage().Headers;

public bool SomeTest()
{
    _responseHeaders.Add(...);
    _responseHeaders.Clear();
}


public IEnumerable<AddWithoutValidationTestData> IEnumerableArgument()
{
yield return new AddWithoutValidationTestData("List", Enumerable.Range(0, 5).Select(i => "value" + i).ToList());
yield return new AddWithoutValidationTestData("Array", Enumerable.Range(0, 5).Select(i => "value" + i).ToArray());
yield return new AddWithoutValidationTestData("Hashset", new HashSet<string>(Enumerable.Range(0, 5).Select(i => "value" + i)));
}
}

public class AddWithoutValidationTestData
{
public IEnumerable<string> Values { get; }

public string InstanceName { get; }

public AddWithoutValidationTestData(string instanceName, IEnumerable<string> values)
{
InstanceName = instanceName;
Values = values;
}

public override string ToString() => InstanceName;
}
}
Loading