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.

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

Remove CreditManager from Http2Stream #32624

Merged
merged 1 commit into from
Feb 21, 2020

Conversation

stephentoub
Copy link
Member

The CreditManager implementation supports multiple awaiters, but Http2Stream's CreditManager never has more than one waiter at a time. We can instead just encode similar logic into Http2Stream, and make its waiter a reusable singleton, such that if we have to allocate it, we can just keep reusing it for all subsequent waits. This means we avoid the CreditManager allocation per Http2Stream as well as the Waiter allocation per wait (other than the first).

cc: @scalablecory

The CreditManager implementation supports multiple awaiters, but Http2Stream's CreditManager never has more than one waiter at a time.  We can instead just encode similar logic into Http2Stream, and make its waiter a reusable singleton, such that if we have to allocate it, we can just keep reusing it for all subsequent waits.  This means we avoid the CreditManager allocation per Http2Stream as well as the Waiter allocation per wait (other than the first).
@stephentoub stephentoub added the tenet-performance Performance related issue label Feb 20, 2020
@stephentoub stephentoub added this to the 5.0 milestone Feb 20, 2020
@stephentoub stephentoub merged commit 857529d into dotnet:master Feb 21, 2020
@stephentoub stephentoub deleted the http2credit branch February 21, 2020 18:08
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants