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

perf(consensus): Run broadcast routines out of process (backport #318… #135

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

ValarDragon
Copy link
Member

…0) (cometbft#3477)

Run broadcast routines out of process. Right now each broadcast routine blocks the consensus mutex for roughly num_peers * process_creation_time, which is genuinely notable! This PR reduces the consensus blocking overhead to just be process_creation_time.

On the latest osmosis branch with improvements, thats 20s of blocking time out of 140s (over the course of 1 hour. This 140s includes block execution!)

image

Note that WAL write time should go significantly down with open PR's. For HasVote, this is a meaningful increase to consensus mutex lock time, so its worth reducing.


PR checklist



PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

mergify bot and others added 2 commits August 19, 2024 14:08
…tbft#3180) (cometbft#3477)

Run broadcast routines out of process. Right now each broadcast routine
blocks the consensus mutex for roughly `num_peers *
process_creation_time`, which is genuinely notable! This PR reduces the
consensus blocking overhead to just be `process_creation_time`.

On the latest osmosis branch with improvements, thats 20s of blocking
time out of 140s (over the course of 1 hour. This 140s includes block
execution!)

![image](https://github.com/cometbft/cometbft/assets/6440154/4c202988-a0d1-460e-89bc-7c1be11fd36f)

Note that WAL write time should go significantly down with open PR's.
For `HasVote`, this is a meaningful increase to consensus mutex lock
time, so its worth reducing.

---

#### PR checklist

- [x] Tests written/updated - I can't think of any test to add
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments - I don't know of any related docs here
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#3180 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>
@ValarDragon ValarDragon merged commit 462ba0a into osmo/v0.38.11 Aug 19, 2024
15 of 16 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.

1 participant