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

Reduce redundant flush wakes #933

Closed
wants to merge 1 commit into from

Conversation

caspervonb
Copy link
Collaborator

@caspervonb caspervonb commented Apr 25, 2023

Closes #923
See #905

@caspervonb caspervonb requested a review from Jarema April 25, 2023 04:45
@caspervonb
Copy link
Collaborator Author

Brings idle usage down from a 8-6% to 0% when idle (e.g just waiting for a socket read from subscription).

Before

image

After

image

Use a sleep that is only active if we see a command rather than an
interval that is always active and waking.
@@ -338,6 +337,12 @@ impl ConnectionHandler {
mut receiver: mpsc::Receiver<Command>,
) -> Result<(), io::Error> {
loop {
let flush_sleep = if self.flush_wanted {
tokio::time::sleep(self.flush_period).boxed()
Copy link
Contributor

Choose a reason for hiding this comment

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

This will create a lot of allocator noise. What if we made the Interval very high (a few days long) when we don't want any flushes, and lowered it back to the configured value when we want it back?

Copy link
Contributor

@paolobarbolini paolobarbolini Apr 25, 2023

Choose a reason for hiding this comment

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

Forget that. There's no way to change an Interval's interval. I wander if we ignored the first tick, if it would stop waking the runtime up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or we could make our own Interval wrapper around Sleep

This function can be called both before and after the future has completed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will create a lot of allocator noise.
Aye, but we can take it out again with a little bit of work.

Or we could make our own Interval wrapper around Sleep

Yeah short rundown is that we'd want to return pending when flush not wanted, store the waker, wake up when flush wanted is.

@caspervonb
Copy link
Collaborator Author

Closing in favor of #1070

@caspervonb caspervonb closed this Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constant CPU usage with axum/tower & ConnectOptions
2 participants