Skip to content

Conversation

yihuang
Copy link

@yihuang yihuang commented Apr 17, 2024

Solution:

  • instead of only cache the module parameters, we can cache all the common parameters that's not changed during the whole block execution.

benchmark results:

$ go test -benchmem -v ./app/... -run "^$" -bench="BenchmarkERC20Transfer/memiavl-stm-16" -count 6 > /tmp/after
$ benchstat /tmp/before /tmp/after
goos: darwin
goarch: arm64
pkg: github.com/crypto-org-chain/cronos/v2/app
                                │ /tmp/before │            /tmp/after             │
                                │   sec/op    │   sec/op     vs base              │
ERC20Transfer/memiavl-stm-16-16   134.1m ± 2%   131.5m ± 1%  -1.97% (p=0.009 n=6)

                                │ /tmp/before  │             /tmp/after             │
                                │     B/op     │     B/op      vs base              │
ERC20Transfer/memiavl-stm-16-16   562.1Mi ± 0%   539.2Mi ± 0%  -4.06% (p=0.002 n=6)

                                │ /tmp/before │            /tmp/after             │
                                │  allocs/op  │  allocs/op   vs base              │
ERC20Transfer/memiavl-stm-16-16   7.705M ± 0%   7.398M ± 0%  -3.98% (p=0.002 n=6)

Closes: #XXX

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@yihuang yihuang requested a review from mmsqe April 17, 2024 06:44
@yihuang yihuang force-pushed the cache-block-params branch from 95ba799 to a8c138c Compare April 17, 2024 06:59
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 63.20%. Comparing base (5a7f857) to head (cbb2263).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #469      +/-   ##
===========================================
- Coverage    63.27%   63.20%   -0.07%     
===========================================
  Files          127      127              
  Lines         9617     9618       +1     
===========================================
- Hits          6085     6079       -6     
- Misses        2993     2996       +3     
- Partials       539      543       +4     
Files Coverage Δ
app/ante/eth.go 69.46% <100.00%> (-0.91%) ⬇️
app/app.go 76.86% <100.00%> (ø)
x/evm/keeper/gas.go 86.95% <100.00%> (-1.94%) ⬇️
x/evm/keeper/grpc_query.go 78.55% <100.00%> (+0.20%) ⬆️
x/evm/keeper/keeper.go 85.84% <100.00%> (ø)
x/evm/keeper/utils.go 79.22% <100.00%> (+0.84%) ⬆️
x/evm/types/key.go 0.00% <ø> (ø)
x/feemarket/keeper/keeper.go 51.85% <ø> (-1.72%) ⬇️
x/evm/keeper/state_transition.go 73.68% <92.30%> (-0.55%) ⬇️
x/evm/keeper/abci.go 77.77% <33.33%> (-22.23%) ⬇️
... and 5 more

Solution:
- instead of only cache the module parameters, we can cache all the
  common parameters that's not changed during the whole block execution.

benchmark results:

```
$ # test case in cronos repo
$ go test -benchmem -v ./app/... -run "^$" -bench="BenchmarkERC20Transfer/memiavl-stm-16" -count 6 > /tmp/after
$ benchstat /tmp/before /tmp/after                                                                                                                                                                             ~/src/cronos
goos: darwin
goarch: arm64
pkg: github.com/crypto-org-chain/cronos/v2/app
                                │ /tmp/before │            /tmp/after             │
                                │   sec/op    │   sec/op     vs base              │
ERC20Transfer/memiavl-stm-16-16   134.1m ± 2%   131.5m ± 1%  -1.97% (p=0.009 n=6)

                                │ /tmp/before  │             /tmp/after             │
                                │     B/op     │     B/op      vs base              │
ERC20Transfer/memiavl-stm-16-16   562.1Mi ± 0%   539.2Mi ± 0%  -4.06% (p=0.002 n=6)

                                │ /tmp/before │            /tmp/after             │
                                │  allocs/op  │  allocs/op   vs base              │
ERC20Transfer/memiavl-stm-16-16   7.705M ± 0%   7.398M ± 0%  -3.98% (p=0.002 n=6)
```

Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

fix lint

Update x/evm/keeper/config.go

Signed-off-by: yihuang <[email protected]>

cleanup

fix tests

fix lint

fix lint

fix integration test
@yihuang yihuang force-pushed the cache-block-params branch from 889686d to f2a68ae Compare April 17, 2024 09:01
@yihuang yihuang enabled auto-merge (squash) April 17, 2024 09:02
Co-authored-by: mmsqe <[email protected]>
Signed-off-by: yihuang <[email protected]>
@yihuang yihuang merged commit 8252ec9 into crypto-org-chain:develop Apr 18, 2024
@yihuang yihuang deleted the cache-block-params branch April 18, 2024 01:49
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