Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@tomaka
Copy link
Contributor

@tomaka tomaka commented Mar 22, 2021

Better version of #8405 (and #8410)
Now that the legacy substream is gone, this cleans up the hack introduced in #7594

This PR does two things:

  • Ensure that all messages are properly sent before we potentially generate Poll::Ready, and not the other way around.
  • Avoids a lot of spurious wake ups when sending by removing the hack-y cx.waker().wake_by_ref() and by only calling Sink::poll_ready() on the substream only if there is actually something to send in the channel, and not all the time.

@tomaka tomaka added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 22, 2021
@tomaka tomaka requested review from bkchr, eskimor and romanb March 22, 2021 16:04
@tomaka tomaka changed the title Refactor handler Refactor NotifsHandler::poll Mar 22, 2021
Copy link
Member

@eskimor eskimor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like :-)

Copy link
Contributor

@romanb romanb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tomaka
Copy link
Contributor Author

tomaka commented Mar 23, 2021

Reduced polling rate seems to work:

image

@tomaka
Copy link
Contributor Author

tomaka commented Mar 23, 2021

bot merge

@ghost
Copy link

ghost commented Mar 23, 2021

Trying merge.

@ghost ghost merged commit 067f185 into paritytech:master Mar 23, 2021
@tomaka tomaka deleted the refactor-handler branch March 23, 2021 10:02
hirschenberger pushed a commit to hirschenberger/substrate that referenced this pull request Apr 14, 2021
* Refactor a bit NotifsHandler::poll

* Avoid some spurious wake-ups
jordy25519 pushed a commit to cennznet/substrate that referenced this pull request Sep 17, 2021
* Refactor a bit NotifsHandler::poll

* Avoid some spurious wake-ups
jordy25519 pushed a commit to cennznet/substrate that referenced this pull request Sep 17, 2021
* Refactor a bit NotifsHandler::poll

* Avoid some spurious wake-ups
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants