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

Try to mark child channel writable again once the parent channel beco… #9254

Merged
merged 1 commit into from Jun 18, 2019

Conversation

Projects
None yet
2 participants
@normanmaurer
Copy link
Member

commented Jun 18, 2019

…mes writable

Motivation:

f945a07 decoupled the writability state from the flow controller but could lead to the situation of a lot of writability updates events were propagated to the child channels. This change ensure we only take into account if the parent channel becomes writable again before we try to set the child channels to writable.

Modifications:

Only listen for channel writability changes for if the parent channel becomes writable again.

Result:

Less writability updates.

@normanmaurer normanmaurer requested a review from ejona86 Jun 18, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

@ejona86 this is a follow-up of f945a07.

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

alternative we could also store the channels that become unwritable in a ArrayDeque and drain it to not loop over all active channels. Not sure this is what you had in mind... let me know

@ejona86
Copy link
Member

left a comment

Storing the channels waiting for the parent in an ArrayDeque or a linked list using pointers in each channel would be fine. But that can be done later as an optimization. I was mainly just wanting the behavior we expose to allow for an efficient implementation, not as much have that implementation now.

Try to mark child channel writable again once the parent channel beco…
…mes writable

Motivation:

f945a07 decoupled the writability state from the flow controller but could lead to the situation of a lot of writability updates events were propagated to the child channels. This change ensure we only take into account if the parent channel becomes writable again before we try to set the child channels to writable.

Modifications:

Only listen for channel writability changes for if the parent channel becomes writable again.

Result:

Less writability updates.

@normanmaurer normanmaurer force-pushed the http2_writability_followup branch from c4e4d56 to dc827a1 Jun 18, 2019

@normanmaurer normanmaurer added this to the 4.1.37.Final milestone Jun 18, 2019

@normanmaurer normanmaurer merged commit 01cfd78 into 4.1 Jun 18, 2019

3 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details

@normanmaurer normanmaurer deleted the http2_writability_followup branch Jun 18, 2019

normanmaurer added a commit that referenced this pull request Jun 18, 2019

Try to mark child channel writable again once the parent channel beco…
…mes writable (#9254)

Motivation:

f945a07 decoupled the writability state from the flow controller but could lead to the situation of a lot of writability updates events were propagated to the child channels. This change ensure we only take into account if the parent channel becomes writable again before we try to set the child channels to writable.

Modifications:

Only listen for channel writability changes for if the parent channel becomes writable again.

Result:

Less writability updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.