Skip to content

HTTP2: Fix HttpRequestHeaders no longer being pooled #19329

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
Feb 25, 2020

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Feb 25, 2020

HttpRequestHeaders is showing up in profile:

image

I believe there was a regression caused when Latin1 headers #18255 was merged with Justin's stream pooling changes.

@jkotalik
Copy link
Contributor

I opened #19312 already, but this looks a bit more complete.

@@ -6105,7 +6105,7 @@ public unsafe void Append(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
}

// We didn't have a previous matching header value, or have already added a header, so get the string for this value.
var valueStr = value.GetRequestHeaderStringNonNullCharacters(_useLatin1);
var valueStr = value.GetRequestHeaderStringNonNullCharacters(UseLatin1);

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@JamesNK JamesNK merged commit f5eb3dc into master Feb 25, 2020
@JamesNK JamesNK deleted the jamesnk/httprequestmessage-alloc branch February 25, 2020 20:48
@mjsabby
Copy link
Contributor

mjsabby commented Feb 26, 2020

This change (in addition to the one already committed) will make it in 3.1.3 correct?

@JamesNK
Copy link
Member Author

JamesNK commented Feb 26, 2020

No. This was a regression to new functionality on master when the 3.1.3 branch was merged back.

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants