Skip to content

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

Merged
melekes merged 3 commits intov1.xfrom
mergify/bp/v1.x/pr-3006
May 9, 2024
Merged

perf: Make every gossip thread use its own randomness instance, reducing contention (backport #3006)#3045
melekes merged 3 commits intov1.xfrom
mergify/bp/v1.x/pr-3006

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented May 9, 2024

Closes #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.


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
  • Title follows the Conventional Commits spec

This is an automatic backport of pull request #3006 done by [Mergify](https://mergify.com).

…ing contention (#3006)

Closes #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.

---

#### PR checklist

- [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 df28e73)

# Conflicts:
#	internal/consensus/reactor.go
@mergify mergify bot requested a review from a team as a code owner May 9, 2024 07:19
@mergify
Copy link
Contributor Author

mergify bot commented May 9, 2024

Cherry-pick of df28e73 has failed:

On branch mergify/bp/v1.x/pr-3006
Your branch is up to date with 'origin/v1.x'.

You are currently cherry-picking commit df28e73d3.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .changelog/unreleased/improvements/3006-use-thread-independent-randomness-in-gossip.md
	modified:   internal/bits/bit_array.go
	modified:   internal/bits/bit_array_test.go
	modified:   internal/rand/random.go
	modified:   internal/rand/random_test.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   internal/consensus/reactor.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot requested a review from a team May 9, 2024 07:19
@mergify mergify bot added the conflicts label May 9, 2024
@melekes melekes merged commit d33235e into v1.x May 9, 2024
@melekes melekes deleted the mergify/bp/v1.x/pr-3006 branch May 9, 2024 08:56
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.

3 participants