Skip to content

Caching per-request objects on reused HttpContexts #6895

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

Closed
JamesNK opened this issue Jan 20, 2019 · 7 comments · Fixed by #34360
Closed

Caching per-request objects on reused HttpContexts #6895

JamesNK opened this issue Jan 20, 2019 · 7 comments · Fixed by #34360
Labels
affected-very-few This issue impacts very few customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-nice-to-have This label is used by an internal tool
Milestone

Comments

@JamesNK
Copy link
Member

JamesNK commented Jan 20, 2019

ASP.NET Core 3 adds HttpContext reuse - Reuse HttpContext object per HTTP/1 connection and HTTP/2 stream

App frameworks and middleware often have their own objects that are allocated every request. Instead of app frameworks/middleware caching with a shared object pool (creating thread contention), what opportunities are there for to cache lightweight per-request objects on HttpContext?

Because cached objects would eventually be on every HttpContext this functionality should only be used for lightweight objects, and not something large like buffers.

e.g. generated gRPC methods have objects passed to them like a stream reader, stream writer and context - Task SayHellos(HelloRequest request, IServerStreamWriter<HelloReply> responseStream, ServerCallContext context). These objects are mostly wrappers over the underlying HttpContext.

  • Before: IServerStreamWriter and ServerCallContext are allocated every request
  • After: IServerStreamWriter and ServerCallContext are fetched from the HttpContext. App framework will create and cache them if they don't exist, or reset the existing instances for the current HttpContext.
@JamesNK
Copy link
Member Author

JamesNK commented Jan 20, 2019

@davidfowl

@benaadams
Copy link
Member

Because cached objects would eventually be on every HttpContext this functionality should only be used for lightweight objects, and not something large like buffers.

Could have a cache that drops any object from the pool if it wasn't requested during that request; so it would only live on for 1 request (or a parameterised number e.g. if not used for x Requests then drop)

@davidfowl
Copy link
Member

I think resettable features is the feature here. A feature can opt into controlling creation and reset.

@JamesNK
Copy link
Member Author

JamesNK commented Jan 21, 2019

Could have a cache that drops any object from the pool if it wasn't requested during that request; so it would only live on for 1 request (or a parameterised number e.g. if not used for x Requests then drop)

That's a clever idea. Rather than putting that logic in Kestrel it could be something a feature could track internally

e.g. each time the feature is reset it could increment an "unused" counter by 1. When it is used then "unused" counter goes back to zero. If counter exceeds a certain number then as part of the reset the heavy objects are culled (or could a feature remove itself from its HttpContext?).

I think resettable features is the feature here. A feature can opt into controlling creation and reset.

What happens to features when the HttpContext is reused today?

I don't know if this is a thing to worry about but imagine a web app that uses lots of middleware and app frameworks. Eventually there would be a lot of resettable features in the mix on the HttpContext. Could the time spent each request resetting those features have an impact?

@JamesNK
Copy link
Member Author

JamesNK commented Mar 17, 2019

Profiling gRPC and the largest allocation is the gRPC context. Caching this would almost halve allocated memory. Savings would be larger in streaming methods where the stream abstractions could also be cached.

image

@jkotalik
Copy link
Contributor

@JamesNK is there anything left here that we'd consider caching or does most of the perf work we did in 3.1/5.0 cover this?

@jkotalik jkotalik added affected-very-few This issue impacts very few customers severity-nice-to-have This label is used by an internal tool labels Nov 13, 2020 — with ASP.NET Core Issue Ranking
@JamesNK
Copy link
Member Author

JamesNK commented Nov 13, 2020

This is still useful IMO

@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-very-few This issue impacts very few customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants