Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Faster output header handling #528

Closed
benaadams wants to merge 1 commit into from
Closed

Faster output header handling #528

benaadams wants to merge 1 commit into from

Conversation

@benaadams
Copy link
Contributor

@benaadams benaadams commented Dec 23, 2015

From #519

@davidfowl
Copy link
Member

davidfowl commented Dec 23, 2015

How much does this actually save?

@benaadams
Copy link
Contributor Author

benaadams commented Dec 23, 2015

It could save more, but are some oddities with nulls (being valid stored values but not output; is even a test for it and related things)

Otherwise a fair bit and moves CreateResponseHeader out of the heavy functions; as without it, it repeatedly does:

new Enumerator -> Movenext -> Current -> Movenext for every header; as well as keep-alive check

@davidfowl
Copy link
Member

davidfowl commented Dec 23, 2015

@benaadams I'd like to see numbers. This looks like an uber micro-optimization.

@benaadams
Copy link
Contributor Author

benaadams commented Dec 23, 2015

Perf is in #519 (comment)

Numbers are hard to get matching; but look at the ratios for FrameResponseHeaders.CopyToFast - CopyFrom is the same in both and doesn't include enumerator .ctor

Pre change
pre-change
Without
post-change

@benaadams
Copy link
Contributor Author

benaadams commented Dec 23, 2015

Can run longer perf tests if you want more equivalent numbers?

@benaadams
Copy link
Contributor Author

benaadams commented Dec 26, 2015

StringValues enumerator is currently fairly inefficient; this might become less a factor after PR dotnet/extensions#74

@benaadams benaadams mentioned this pull request Dec 28, 2015
Closed
@benaadams benaadams force-pushed the stringvalue-enumerator branch from b2611a2 to 36789e4 Compare December 29, 2015 09:28
@benaadams benaadams changed the title Skip StringValue enumerator when < 2 values Faster output header handling Dec 29, 2015
@benaadams benaadams force-pushed the stringvalue-enumerator branch from 36789e4 to da2d666 Compare December 29, 2015 09:32
benaadams added a commit to benaadams/KestrelHttpServer that referenced this pull request Dec 29, 2015
@benaadams
Known headers have known offsets
3a501ec 
Also has aspnet#528
Faster output header handling aspnet#528 in it :-/
benaadams added a commit to benaadams/KestrelHttpServer that referenced this pull request Dec 29, 2015
@benaadams
Known headers have known offsets
0578baf 
Also has aspnet#528
Faster output header handling aspnet#528 in it :-/
@benaadams benaadams mentioned this pull request Dec 29, 2015
Closed
@benaadams
Copy link
Contributor Author

benaadams commented Dec 29, 2015

Merged with #524 due to Generated file clash

@benaadams benaadams closed this Dec 29, 2015
benaadams added a commit to benaadams/KestrelHttpServer that referenced this pull request Dec 30, 2015
@benaadams
Known headers have known offsets
429c04f 
Also has "Faster output header handling" aspnet#528 in it :-/
benaadams added a commit to benaadams/KestrelHttpServer that referenced this pull request Jan 2, 2016
@benaadams
Known headers have known offsets
aeff8fc 
Also has "Faster output header handling" aspnet#528 in it :-/
benaadams added a commit to benaadams/KestrelHttpServer that referenced this pull request Jan 5, 2016
@benaadams
Known headers have known offsets
650e6e7 
Also has "Faster output header handling" aspnet#528 in it :-/
benaadams added a commit to benaadams/KestrelHttpServer that referenced this pull request Jan 5, 2016
@benaadams
Known headers have known offsets
f096964 
Also has "Faster output header handling" aspnet#528 in it :-/
@benaadams benaadams deleted the stringvalue-enumerator branch May 10, 2016 10:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@benaadams @davidfowl @dnfclas