Skip to content

Avoid a few allocations in StringContent#103008

Merged
MihaZupan merged 1 commit intodotnet:mainfrom
MihaZupan:stringcontent-alloc
Jun 6, 2024
Merged

Avoid a few allocations in StringContent#103008
MihaZupan merged 1 commit intodotnet:mainfrom
MihaZupan:stringcontent-alloc

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

dotnet-policy-service bot commented Jun 3, 2024

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

@ManickaP ManickaP Jun 4, 2024

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

@MihaZupan MihaZupan Jun 4, 2024

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