KEMBAR78
Avoid a few allocations in StringContent by MihaZupan · Pull Request #103008 · dotnet/runtime · GitHub
Skip to content

Avoid a few allocations in StringContent #103008

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
Merged
merged 1 commit into from
Jun 6, 2024
Merged

Avoid a few allocations in StringContent #103008

merged 1 commit into from
Jun 6, 2024

Conversation

@MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Jun 3, 2024 •
edited
Loading

Same idea as #102859, but for StringContent. Saves the 5 smaller allocations each time.

AFAICT UTF-8 is practically the only encoding used here, with text/plain (default) and application/json being by far the most common media types. This PR special-cases these two combinations.

[Benchmark]
public void Test()
{
    var content = new StringContent("Foo", Encoding.UTF8, "application/json");
    foreach (var header in content.Headers.NonValidated)
    {
        _ = header.Value.ToString();
    }
}
Method Toolchain Mean Error Ratio Allocated Alloc Ratio
Test main 296.5 ns 2.95 ns 1.00 520 B 1.00
Test pr 147.2 ns 0.55 ns 0.50 304 B 0.58

@MihaZupan MihaZupan added this to the 9.0.0 milestone Jun 3, 2024
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@build-analysis build-analysis bot mentioned this pull request Jun 4, 2024
Closed
ManickaP
ManickaP reviewed Jun 4, 2024
View reviewed changes
src/libraries/System.Net.Http/src/System/Net/Http/StringContent.cs

if (knownValue is not null)
{
Headers.TryAddWithoutValidation(KnownHeaders.ContentType.Descriptor, knownValue);
Copy link
Member

Choose a reason for hiding this comment

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

Will Header.ContentType be populated in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, behavior-wise as far as the user can see, the two should be the same.

The ContentType property uses the underlying dictionary as the source of truth / storage, and will lazily parse the value when first accessed.

@MihaZupan MihaZupan force-pushed the stringcontent-alloc branch from b7fb7ab to 5a02e23 Compare June 5, 2024 22:11
This was referenced Jun 6, 2024
Closed
Closed
Closed
Closed
@MihaZupan MihaZupan merged commit b2601ed into dotnet:main Jun 6, 2024
@MihaZupan MihaZupan mentioned this pull request Jun 10, 2024
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ManickaP ManickaP ManickaP left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees

@MihaZupan MihaZupan

Labels

area-System.Net.Http

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@MihaZupan @stephentoub @ManickaP