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: Amortize clearing unsorted cache entries (Juno genesis fix) (backport #12885) #12961

Merged
merged 3 commits into from
Aug 19, 2022

Commits on Aug 18, 2022

  1. perf: Amortize clearing unsorted cache entries (Juno genesis fix) (#1…

    …2885)
    
    This change fixes a bounty by the Juno team.  Juno's invariant checks took 10 hours during their most recent chain halt. This PR cuts that down to 30 seconds.  See https://github.com/CosmosContracts/bounties#improve-speed-of-invariant-checks.
    
    The root problem is deep in the `can-withdraw` invariant check, which calls this repeatedly: https://github.com/cosmos/cosmos-sdk/blob/main/x/distribution/keeper/store.go#L337.  Iterators have a chain of parents and in this case creates an iterator from the `cachekv` store.  For the genesis file, it has a cache of 500,000+ unsorted entries, which are sorted as strings here: https://github.com/cosmos/cosmos-sdk/blob/main/store/cachekv/store.go#L314.  Each delegation from `can-withdraw` uses this cache and many of the cache checks miss or are a very small range.  This means very few entries get removed from the unsorted cache and they have to be re-sorted on the next call.  With a full cache it takes about 180ms on my machine to sort them.
    
    This change introduce a minimum number of entries that will get processed and removed from the unsorted list. It's set at the same value that directs the code to sort them in the first place.  This ensures the unsorted values get removed in a relative short amount of time, and amortizes the cost to ensure an individual check does not have to process the entire cache.
    
    ## Benchmarks
    On running the benchmarks included in this change produces:
    ```shell
    name                    old time/op    new time/op    delta
    LargeUnsortedMisses-32     21.2s ± 9%      0.0s ± 1%   -99.91%  (p=0.000 n=20+17)
    
    name                    old alloc/op   new alloc/op   delta
    LargeUnsortedMisses-32    1.64GB ± 0%    0.00GB ± 0%   -99.83%  (p=0.000 n=19+19)
    
    name                    old allocs/op  new allocs/op  delta
    LargeUnsortedMisses-32     20.0k ± 0%     41.1k ± 0%  +105.23%  (p=0.000 n=19+20)
    ```
    
    ## Invariant checks results
    This is what the invariant checks for Juno look like with this change (on a Hetzner AX101):
    
    ```shell
    INF starting node with ABCI Tendermint in-process
    4:11PM INF Starting multiAppConn service impl=multiAppConn module=proxy
    4:11PM INF Starting localClient service connection=query impl=localClient module=abci-client
    4:11PM INF Starting localClient service connection=snapshot impl=localClient module=abci-client
    4:11PM INF Starting localClient service connection=mempool impl=localClient module=abci-client
    4:11PM INF Starting localClient service connection=consensus impl=localClient module=abci-client
    4:11PM INF Starting EventBus service impl=EventBus module=events
    4:11PM INF Starting PubSub service impl=PubSub module=pubsub
    4:11PM INF Starting IndexerService service impl=IndexerService module=txindex
    4:11PM INF ABCI Handshake App Info hash= height=0 module=consensus protocol-version=0 software-version=v9.0.0-36-g8fd6f16
    4:11PM INF ABCI Replay Blocks appHeight=0 module=consensus stateHeight=0 storeHeight=0
    4:12PM INF asserting crisis invariants inv=1/11 module=x/crisis name=gov/module-account
    4:12PM INF asserting crisis invariants inv=2/11 module=x/crisis name=distribution/nonnegative-outstanding
    4:12PM INF asserting crisis invariants inv=3/11 module=x/crisis name=distribution/can-withdraw
    4:12PM INF asserting crisis invariants inv=4/11 module=x/crisis name=distribution/reference-count
    4:12PM INF asserting crisis invariants inv=5/11 module=x/crisis name=distribution/module-account
    4:12PM INF asserting crisis invariants inv=6/11 module=x/crisis name=bank/nonnegative-outstanding
    4:12PM INF asserting crisis invariants inv=7/11 module=x/crisis name=bank/total-supply
    4:12PM INF asserting crisis invariants inv=8/11 module=x/crisis name=staking/module-accounts
    4:12PM INF asserting crisis invariants inv=9/11 module=x/crisis name=staking/nonnegative-power
    4:12PM INF asserting crisis invariants inv=10/11 module=x/crisis name=staking/positive-delegation
    4:12PM INF asserting crisis invariants inv=11/11 module=x/crisis name=staking/delegator-shares
    4:12PM INF asserted all invariants duration=28383.559601 height=4136532 module=x/crisis
    ```
    
    ## Alternatives
    There is another PR which fixes this problem for the Juno genesis file #12886. However, because of its concurrent nature, it happens to hit a large range relatively early, clearing the unsorted entries and allowing the rest of the checks to not sort it.
    
    (cherry picked from commit 4fc1f73)
    
    # Conflicts:
    #	CHANGELOG.md
    blazeroni authored and mergify[bot] committed Aug 18, 2022
    Configuration menu
    Copy the full SHA
    ffd7b32 View commit details
    Browse the repository at this point in the history
  2. fix conflict

    julienrbrt committed Aug 18, 2022
    Configuration menu
    Copy the full SHA
    47ec1c9 View commit details
    Browse the repository at this point in the history

Commits on Aug 19, 2022

  1. Configuration menu
    Copy the full SHA
    e378e8e View commit details
    Browse the repository at this point in the history