This repository was archived by the owner on Nov 20, 2018. It is now read-only.
Propagate Form and ResponseCookie init options #675
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 22, 2016
|
Putting in 1.1 for investigation |
| public DefaultHttpContext() | ||
| : this(new FeatureCollection()) | ||
| { | ||
| Features.Set<IHttpRequestFeature>(new HttpRequestFeature()); | ||
| Features.Set<IHttpResponseFeature>(new HttpResponseFeature()); | ||
| } | ||
|
|
||
| public DefaultHttpContext(IFeatureCollection features) | ||
| public DefaultHttpContext(IFeatureCollection features, ObjectPool<StringBuilder> builderPool = null, FormOptions formOptions = null) |
There was a problem hiding this comment.
Split this into two constructors
benaadams
force-pushed
the
propagate-feature-init
branch
3 times, most recently
from
e3c48bd to
102ee28
Compare
|
@Tratcher @pakrym @davidfowl updated with currying, which makes creation is less messy and adds 16 more bytes; so only 72 bytes saved (72MB per 1M req) 😞 Still breaks the feature chaining @Tratcher referenced, to make it work this var feature = context.Features.Get<IResponseCookiesFeature>() ??
new ResponseCookiesFeature(context.Features);Would need to change to var feature = context.Response.Cookies != null ?
context.Features.Get<IResponseCookiesFeature>() :
new ResponseCookiesFeature(context.Features);Which isn't great... but if you are in an auth'd flow security ResponseCookies aren't being set on every request if you have a good RequestCookie? |
Also has side effect of allowing lazier load of the two features
benaadams
force-pushed
the
propagate-feature-init
branch
from
102ee28 to
7243252
Compare
|
Closing, even considering allocation gains we cant break feature chaining pattern. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also has nice side effect of allowing lazier load of the two features
Before
After
DefaultHttpContext increases by 16 bytes (120bytes->136bytes)
But it drops ResponseCookiesFeature at 56 bytes and FormFeature at 48 bytes
So overall 88 bytes and 2 allocations down per request or 83MB per 1M requests
Resolves #676
Resolves #677
/cc @Tratcher