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

Connectd speed up #1: spend less time polling. #7365

Merged

Conversation

rustyrussell
Copy link
Contributor

A fairly simple change: ccan/io will now call the underlying I/O
routines repeatedly until they indicate they are unfinished, or fail
with EAGAIN. This should make a significant difference to large
nodes, which currently spend far too much time calling poll() to
discover a single fd is still writable (mainly, for streaming gossip).

Changelog-Changed: connectd: now should use far less CPU on large nodes.
Fixes: #7243

@rustyrussell rustyrussell added this to the v24.08 milestone Jun 4, 2024
@rustyrussell rustyrussell mentioned this pull request Jun 4, 2024
A fairly simple change: ccan/io will now call the underlying I/O
routines repeatedly until they indicate they are unfinished, *or* fail
with EAGAIN.  This should make a significant difference to large
nodes, which currently spend far too much time calling poll() to
discover a single fd is still writable (mainly, for streaming gossip).

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Changed: connectd: now should use far less CPU on large nodes.
Interestingly, this patch and the last take my total test time on my
build machine down by about 10%.  From 403.93s (0:06:43) to 366.64s (0:06:06)
though there were some unrelated errors.

Changelog-Changed: connectd: I/O optimizations to significantly speed up larger nodes.
Signed-off-by: Rusty Russell <[email protected]>
Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

ACK 5abc184

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

Concept ACK 5abc184

LGTM but I want to rerun the CI for regression if any

@ShahanaFarooqui ShahanaFarooqui merged commit eb1c04e into ElementsProject:master Jul 2, 2024
30 of 39 checks passed
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.

Connections become unstable after large amount of connections
4 participants