Skip to content

perf(txindex): search optimization (backport #3458)#3552

Merged
sergio-mena merged 1 commit intov0.38.x-experimentalfrom
mergify/bp/v0.38.x/pr-3458
Jul 25, 2024
Merged

perf(txindex): search optimization (backport #3458)#3552
sergio-mena merged 1 commit intov0.38.x-experimentalfrom
mergify/bp/v0.38.x/pr-3458

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Jul 24, 2024

This PR addresses several optimization points for TxIndex.Search:

  1. Remove expensive map access from sorting function
  2. Replace sort.Slice to sort.Sort with sorter approach
  3. Reduce heap allocations from extractEventSeqFromKey

benchstat result:

goos: darwin
goarch: amd64
pkg: github.com/cometbft/cometbft/state/txindex/kv
cpu: Intel(R) Core(TM) i9-10910 CPU @ 3.60GHz
                     │   old.txt    │              new.txt               │
                     │    sec/op    │   sec/op     vs base               │
TxSearchBigResult-20   3236.1m ± 7%   945.9m ± 9%  -70.77% (p=0.002 n=6)

                     │   old.txt    │               new.txt               │
                     │     B/op     │     B/op      vs base               │
TxSearchBigResult-20   426.1Mi ± 0%   311.6Mi ± 0%  -26.86% (p=0.002 n=6)

                     │   old.txt   │              new.txt               │
                     │  allocs/op  │  allocs/op   vs base               │
TxSearchBigResult-20   4.040M ± 0%   2.040M ± 0%  -49.51% (p=0.002 n=6)

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

@mergify mergify bot requested a review from a team as a code owner July 24, 2024 20:59
@mergify mergify bot added the conflicts label Jul 24, 2024
@mergify

This comment was marked as resolved.

@sergio-mena sergio-mena changed the base branch from v0.38.x to v0.38.x-experimental July 25, 2024 08:02
@sergio-mena sergio-mena requested a review from a team as a code owner July 25, 2024 08:02
@sergio-mena sergio-mena marked this pull request as draft July 25, 2024 08:04
@sergio-mena
Copy link
Collaborator

Blocked on #3555

@sergio-mena
Copy link
Collaborator

sergio-mena commented Jul 25, 2024

We need to backport #2855 first, as these changes heavily depend on that PR

EDIT: This is the PR for that #3556

This PR addresses several optimization points for TxIndex.Search:

1. Remove expensive map access from sorting function
2. Replace sort.Slice to sort.Sort with sorter approach
3. Reduce heap allocations from extractEventSeqFromKey

benchstat result:
```
goos: darwin
goarch: amd64
pkg: github.com/cometbft/cometbft/state/txindex/kv
cpu: Intel(R) Core(TM) i9-10910 CPU @ 3.60GHz
                     │   old.txt    │              new.txt               │
                     │    sec/op    │   sec/op     vs base               │
TxSearchBigResult-20   3236.1m ± 7%   945.9m ± 9%  -70.77% (p=0.002 n=6)

                     │   old.txt    │               new.txt               │
                     │     B/op     │     B/op      vs base               │
TxSearchBigResult-20   426.1Mi ± 0%   311.6Mi ± 0%  -26.86% (p=0.002 n=6)

                     │   old.txt   │              new.txt               │
                     │  allocs/op  │  allocs/op   vs base               │
TxSearchBigResult-20   4.040M ± 0%   2.040M ± 0%  -49.51% (p=0.002 n=6)
```

- [x] Tests written/updated
- [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

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
@sergio-mena sergio-mena force-pushed the mergify/bp/v0.38.x/pr-3458 branch from c5ad22e to c482b9e Compare July 25, 2024 12:48
@sergio-mena sergio-mena marked this pull request as ready for review July 25, 2024 12:48
@sergio-mena
Copy link
Collaborator

Ready for review

@sergio-mena sergio-mena self-assigned this Jul 25, 2024
Copy link

@cason cason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diffs fully match.

@sergio-mena sergio-mena merged commit 32e4512 into v0.38.x-experimental Jul 25, 2024
@sergio-mena sergio-mena deleted the mergify/bp/v0.38.x/pr-3458 branch July 25, 2024 16:32
@sergio-mena
Copy link
Collaborator

sergio-mena commented Jul 25, 2024

@ValarDragon if you pick up this PR for testing in Osmosis, don't forget you first need to take #3556 (a backport of #2855) to avoid massive conflicts (as explained here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants