Skip to content

perf(txindex): Lower allocation overhead of txIndex matchRange (backport #2839)#2882

Merged
melekes merged 1 commit intov1.xfrom
mergify/bp/v1.x/pr-2839
Apr 24, 2024
Merged

perf(txindex): Lower allocation overhead of txIndex matchRange (backport #2839)#2882
melekes merged 1 commit intov1.xfrom
mergify/bp/v1.x/pr-2839

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Apr 24, 2024

In Osmosis we see massive amounts of heap pressure/allocations coming from txIndex matchRange. (Screenshot below from ~1 hour of heap profiling)

image

This PR is expected to fully compatibly drop this down by a factor of 3. It:

  • Does not get Key() twice (160GB allocation saved)
  • Uses no heap allocations for isTagKey (120GB saved)
  • Does not string cast or do strings.Split in parsing the value (~400GB expected saved)
  • Reuses the big.Int (24GB saved)

The remaining RAM overhead from .Key() needs a cometbft-db API change. The remaining RAM overhead from extracting the value can be saved with an unsafe call for casting the output to string with no heap allocation, but we can do that in a separate PR.


PR checklist

  • Tests written/updated - All existing tests still apply
  • 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 #2839 done by [Mergify](https://mergify.com).

In Osmosis we see massive amounts of heap pressure/allocations coming
from txIndex matchRange. (Screenshot below from ~1 hour of heap
profiling)

![image](https://github.com/cometbft/cometbft/assets/6440154/bf2dfe89-56f0-4824-815b-c5822d20568b)

This PR is expected to fully compatibly drop this down by a factor of 3.
It:
- Does not get Key() twice (160GB allocation saved)
- Uses no heap allocations for isTagKey (120GB saved)
- Does not string cast or do strings.Split in parsing the value (~400GB
expected saved)
- Reuses the big.Int (24GB saved)

The remaining RAM overhead from .Key() needs a cometbft-db API change.
The remaining RAM overhead from extracting the value can be saved with
an unsafe call for casting the output to string with no heap allocation,
but we can do that in a separate PR.

---

#### PR checklist

- [x] Tests written/updated - All existing tests still apply
- [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

(cherry picked from commit e75267f)
@mergify mergify bot requested a review from a team as a code owner April 24, 2024 05:51
@mergify mergify bot requested a review from a team April 24, 2024 05:51
@melekes melekes merged commit 30db6e2 into v1.x Apr 24, 2024
@melekes melekes deleted the mergify/bp/v1.x/pr-2839 branch April 24, 2024 06:02
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