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: Make every gossip thread use its own randomness instance, reduc… #132

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

PaddyMc
Copy link

@PaddyMc PaddyMc commented Aug 19, 2024

…… (backport #77) (#82)

Closes cometbft#3005

As noted in that issue, we currently are doing extra CPU overhead and mutex contention for getting a random number. This PR removes this overhead by making every performance sensitive thread have its own rand instance.

In a subsequent PR, we can cleanup all the testing usages, and likely just entirely delete our internal randomness package. I didn't do that in this PR to keep it straightforward to verify.


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

(cherry picked from commit f55b9f4)

  • Add Changelgo

(cherry picked from commit ce04f04)

  • Fix changelog further

(cherry picked from commit bd34ce6)



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

…… (backport #77) (#82)

* perf: Make every gossip thread use its own randomness instance, reducing contention (cometbft#3006)

Closes cometbft#3005

As noted in that issue, we currently are doing extra CPU overhead and
mutex contention for getting a random number. This PR removes this
overhead by making every performance sensitive thread have its own rand
instance.

In a subsequent PR, we can cleanup all the testing usages, and likely
just entirely delete our internal randomness package. I didn't do that
in this PR to keep it straightforward to verify.

---

- [x] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit f55b9f4)

* Add Changelgo

(cherry picked from commit ce04f04)

* Fix changelog further

(cherry picked from commit bd34ce6)

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Dev Ojha <dojha@berkeley.edu>
Co-authored-by: PaddyMc <paddymchale@hotmail.com>
@ValarDragon ValarDragon merged commit 0af1eb5 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.

2 participants