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

HttpRequestStream & DuplexPipeStream don't amortize their ValueTask ReadAsync calls #11940

Open
benaadams opened this issue Jul 7, 2019 · 0 comments
Open

HttpRequestStream & DuplexPipeStream don't amortize their ValueTask ReadAsync calls #11940

benaadams opened this issue Jul 7, 2019 · 0 comments
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 Perf severity-nice-to-have This label is used by an internal tool
Milestone
Backlog

Comments

@benaadams
Copy link
Member

benaadams commented Jul 7, 2019 •
edited
Loading

In usage a HttpRequestStream & DuplexPipeStream normally has very many ReadAsync calls made on them. (Use case WebSockets/SignalR over TLS)

Using the ValueTask overloads for these Streams read allocates a AsyncStateMachineBox per read (when data is not immediately available)

image

However it could use TryRead for the sync-path and allocating a IValueTaskSource object the first time that fails to back the async read, and then reuse it each time the read needs to go async again.

Related https://github.com/dotnet/corefx/issues/39258
@benaadams benaadams mentioned this issue Jul 7, 2019
Closed
@analogrelay analogrelay added this to the Backlog milestone Jul 9, 2019
@benaadams benaadams mentioned this issue Aug 23, 2019
Merged
@benaadams benaadams mentioned this issue Feb 1, 2020
Closed
@jkotalik jkotalik added affected-very-few This issue impacts very few customers 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 labels Nov 13, 2020 — with ASP.NET Core Issue Ranking
@benaadams benaadams mentioned this issue Apr 8, 2021
Closed
@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 join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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 Perf severity-nice-to-have This label is used by an internal tool
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Amortize HttpRequestStream & DuplexPipeStream ReadAsync calls benaadams/AspNetCore
5 participants
@analogrelay @benaadams @jkotalik @amcasey @mkArtakMSFT