Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Consume SequenceReader from CoreFX #4693

Closed
JeremyKuhne opened this issue Nov 13, 2018 · 6 comments
Closed

Consume SequenceReader from CoreFX #4693

JeremyKuhne opened this issue Nov 13, 2018 · 6 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors feature-kestrel
Milestone
Backlog

Comments

@JeremyKuhne
Copy link
Member

JeremyKuhne commented Nov 13, 2018 •
edited by analogrelay
Loading

SequenceReader<T> is the CoreFX implementation of BufferReader<T>. Any drop of CoreFX past dotnet/corefx@9ae14ba will have this type.

aspnet/KestrelHttpServer#3068 implements changes that are based on an API compatible BufferReader<T> that SequenceReader<T> should just drop in and replace.
@aspnet-hello aspnet-hello transferred this issue from aspnet/KestrelHttpServer Dec 13, 2018
@aspnet-hello aspnet-hello assigned halter73 and unassigned halter73 Dec 13, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0 milestone Dec 13, 2018
@davidfowl davidfowl modified the milestones: 3.0.0, 3.0.0-preview3 Jan 24, 2019
@davidfowl
Copy link
Member

@JeremyKuhne we got all of the performance infrastructure up and running now if you have cycles to port the PR over.

@JeremyKuhne JeremyKuhne mentioned this issue Jan 25, 2019
Merged
@JeremyKuhne
Copy link
Member Author

@davidfowl I just put up the PR to consume the CoreFX type without changing the parser. I'll port the change on top of that.

@muratg muratg modified the milestones: 3.0.0-preview3, 3.0.0-preview4 Feb 21, 2019
@analogrelay analogrelay mentioned this issue Mar 26, 2019
Closed
1 task
@analogrelay
Copy link
Contributor

@JeremyKuhne is there something left to do here after your earlier PR?

@JeremyKuhne
Copy link
Member Author

I didn't port my header parsing rearchitecture as it collided with other changes. It had significant improvements in microbenchmarks but it would need to be redone and reevaluated.

aspnet/KestrelHttpServer#3068

It would be worth examining again, but I don't think there is any need to do it for a given milestone.

@analogrelay
Copy link
Contributor

Gotcha, thanks for helping to catch me up!

@analogrelay analogrelay added the clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors label May 7, 2019
@analogrelay analogrelay modified the milestones: 3.0.0-preview8, Backlog Jul 9, 2019
@jkotalik
Copy link
Contributor

I believe this is mostly done and the rest is covered by #4720.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 13, 2020
@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.
Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions clean-up This issue is internal clean-up and has no effect on public APIs or expected behaviors feature-kestrel
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
9 participants
@analogrelay @halter73 @davidfowl @pranavkm @muratg @JeremyKuhne @jkotalik @amcasey @aspnet-hello