Skip to content

Don't allocate the FormFeature eagerly per request #2699

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
aspnet-hello opened this issue Jan 2, 2018 · 3 comments
Closed

Don't allocate the FormFeature eagerly per request #2699

aspnet-hello opened this issue Jan 2, 2018 · 3 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-http-abstractions Needs: Design This issue requires design work before implementating. Perf

Comments

@aspnet-hello
Copy link

From @davidfowl on Thursday, June 29, 2017 8:01:27 AM

Today, in order to pass the form options to the FormFeature, we eagerly allocate it here https://github.com/aspnet/HttpAbstractions/blob/163836fe1f31dd1d2b288659c2dcaf1d827da3c4/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs#L49. We should find a creative way to avoid doing this so eagerly in the common cases. Here are some options:

  • Invent a new way to pass things to features lazily (hard, we tried this and ended up with crazy things)
  • Only new up the FormOptions if they were overridden. We'd need a way to detect if the FormOptions were actually set. This is doable but could get ugly as there's no way to detect if options were configured. /cc @HaoK
  • Only set the FormFeature if the request is a POST and HasFormContentType. Gross, but would be more pay for play than what we have today.

/cc @vancem @Tratcher

Copied from original issue: aspnet/HttpAbstractions#880

@aspnet-hello aspnet-hello added this to the Backlog milestone Jan 2, 2018
@aspnet-hello aspnet-hello added Needs: Design This issue requires design work before implementating. Perf feature-http-abstractions labels Jan 2, 2018
@aspnet-hello
Copy link
Author

From @HaoK on Thursday, June 29, 2017 9:19:53 AM

We could add a new bool property to IOptionsFactory<TOptions> like IsConfigured, so you could then avoid calling IOptions.Value in this case.

@aspnet-hello
Copy link
Author

From @muratg on Monday, October 30, 2017 10:26:33 AM

@davidfowl Backlogging this until we find a good design to prototype.

@davidfowl
Copy link
Member

This is done

@davidfowl davidfowl modified the milestones: Backlog, 3.0.0-preview3 Apr 12, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-http-abstractions Needs: Design This issue requires design work before implementating. Perf
Projects
None yet
Development

No branches or pull requests

4 participants