Skip to content

perf(blockstore)!: Cache more in blockstore, which speedsup the gossip routines (backport #3342)#3482

Merged
melekes merged 1 commit intov1.xfrom
mergify/bp/v1.x/pr-3342
Jul 10, 2024
Merged

perf(blockstore)!: Cache more in blockstore, which speedsup the gossip routines (backport #3342)#3482
melekes merged 1 commit intov1.xfrom
mergify/bp/v1.x/pr-3342

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Jul 10, 2024

We speedup the gossip routines by using caches for LoadBlockPart, and making LoadBlockCommit no longer call Clone, as the caller does read-only operations to it.

This is a followup to #3003

This results in a net performance improvement, based on a 2hr sync on latest osmosis release, of:

  • 20% less RAM allocated over program lifetime. (76GB no longer allocated, 380 GB alloc total)
  • 50% speedup to gossipDataRoutine
  • ~10% speedup to gossipVotesRoutine
  • 1% less overall system CPU usage. (If GC proportional to bytes allocated, then 5% less)

I have checked that every usage of LoadBlockPart and LoadBlockCommit do not modify the return value, so returning this underlying cache copy is safe for the current codebase

I used an ! in the title, since this is technically API breaking, even though all current usages are safe.


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 #3342 done by [Mergify](https://mergify.com).

…p routines (#3342)

We speedup the gossip routines by using caches for LoadBlockPart, and
making LoadBlockCommit no longer call Clone, as the caller does
read-only operations to it.

This is a followup to #3003

This results in a net performance improvement, based on a 2hr sync on
latest osmosis release, of:
- 20% less RAM allocated over program lifetime. (76GB no longer
allocated, 380 GB alloc total)
- 50% speedup to gossipDataRoutine
- ~10% speedup to gossipVotesRoutine
- 1% less overall system CPU usage. (If GC proportional to bytes
allocated, then 5% less)

I have checked that every usage of `LoadBlockPart` and `LoadBlockCommit`
do not modify the return value, so returning this underlying cache copy
is safe for the current codebase

I used an `!` in the title, since this is technically API breaking, even
though all current usages are safe.

---

#### PR checklist

- [ ] 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 2c88a26)
@mergify mergify bot requested a review from a team as a code owner July 10, 2024 11:11
@mergify mergify bot requested a review from a team July 10, 2024 11:11
@melekes melekes merged commit c5ca1b6 into v1.x Jul 10, 2024
@melekes melekes deleted the mergify/bp/v1.x/pr-3342 branch July 10, 2024 11:30
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