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(p2p): Remove broadcast return channel (backport #3182) (#3480) #134

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

ValarDragon
Copy link
Member

Remove the return channel and waitgroup from switch.Broadcast as its unused. This was costing us some CPU overhead (2% of broadcast routine as it stands right now, but broadcast routine time should be dropping) and one extra goroutine. This removes more overhead than what cometbft#3180 may introduce.

It also better highlights the point that maybe all broadcast usecases should consider using TrySend (And we potentially adapt channel buffers), ala the point in cometbft#3151 . (This would have significant system-wide impact, by significantly lowering the number of timers/scheduler overhead we have)




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

…ometbft#3480)

Remove the return channel and waitgroup from switch.Broadcast as its
unused. This was costing us some CPU overhead (2% of broadcast routine
as it stands right now, but broadcast routine time should be dropping)
and one extra goroutine. This removes more overhead than what cometbft#3180 may
introduce.

It also better highlights the point that maybe all broadcast usecases
should consider using TrySend (And we potentially adapt channel
buffers), ala the point in cometbft#3151 . (This would have significant
system-wide impact, by significantly lowering the number of
timers/scheduler overhead we have)

---

- [x] Tests written/updated - Simplifies code, the behavior being
removed was never tested!
- [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
- [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#3182 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>
@ValarDragon ValarDragon merged commit f1fae04 into osmo/v0.38.11 Aug 19, 2024
1 of 10 checks passed
@ValarDragon ValarDragon deleted the dev/backports branch August 19, 2024 18:06
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