Skip to content

Reduce HttpRequestHeader enumerator allocations #32236

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

Conversation

benaadams
Copy link
Member

Reduce allocations where the request headers are always enumerated for every request like in reverse-proxy/ReverseProxy/Service/Proxy/HttpTransformer.cs

public virtual ValueTask TransformRequestAsync(HttpContext httpContext, HttpRequestMessage proxyRequest, string destinationPrefix)
{
    foreach (var header in httpContext.Request.Headers)
    {
        var headerName = header.Key;
        var headerValue = header.Value;

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Apr 28, 2021
@BrennanConroy BrennanConroy assigned Tratcher and halter73 and unassigned Tratcher Apr 28, 2021
@BrennanConroy BrennanConroy removed the request for review from jkotalik June 16, 2021 20:21
# Conflicts:
#	src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs
@halter73 halter73 force-pushed the HttpHeader-enumerator-allocations branch from b194abb to 9285e59 Compare July 26, 2021 20:59
@benaadams benaadams requested a review from BrennanConroy as a code owner July 26, 2021 20:59
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

It'll be interesting to see if the reduced boxing makes an observable improvement to YARP benchmarks.

@halter73
Copy link
Member

halter73 commented Aug 4, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@adityamandaleeka
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@adityamandaleeka adityamandaleeka enabled auto-merge (squash) August 30, 2021 20:29
@adityamandaleeka adityamandaleeka merged commit ec01a2a into dotnet:main Aug 30, 2021
@ghost ghost added this to the 7.0-preview1 milestone Aug 30, 2021
@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 community-contribution Indicates that the PR has been added by a community member Perf
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants