Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Problem: repeated tx sender recovery is wastful #1717

Merged
merged 7 commits into from
Mar 24, 2023

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Mar 16, 2023

Description

Solution:

  • only do once in ante handler, reuse the result

In our erc20 transfer benchmark, it improve performance by 40+%

$ cat /tmp/before.txt
goos: darwin
goarch: amd64
pkg: github.com/crypto-org-chain/cronos/v2/app
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkERC20Transfer/memdb-12         	       1	1204795758 ns/op	197702208 B/op	 3457302 allocs/op
BenchmarkERC20Transfer/memdb-12         	       1	1143134290 ns/op	197419672 B/op	 3455167 allocs/op
BenchmarkERC20Transfer/memdb-12         	       1	1160901738 ns/op	197426440 B/op	 3455267 allocs/op
BenchmarkERC20Transfer/leveldb-12       	       1	1150277598 ns/op	198896184 B/op	 3455196 allocs/op
BenchmarkERC20Transfer/leveldb-12       	       1	1114441908 ns/op	198857672 B/op	 3455059 allocs/op
BenchmarkERC20Transfer/leveldb-12       	       1	1127919136 ns/op	198873184 B/op	 3455154 allocs/op
PASS
ok  	github.com/crypto-org-chain/cronos/v2/app	12.740s
$ cat /tmp/after.txt
goos: darwin
goarch: amd64
pkg: github.com/crypto-org-chain/cronos/v2/app
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkERC20Transfer/memdb-12         	       2	 656319036 ns/op	195804392 B/op	 3416136 allocs/op
BenchmarkERC20Transfer/memdb-12         	       2	 661078979 ns/op	195809824 B/op	 3416138 allocs/op
BenchmarkERC20Transfer/memdb-12         	       2	 680140774 ns/op	195802376 B/op	 3416123 allocs/op
BenchmarkERC20Transfer/leveldb-12       	       2	 707129982 ns/op	197731348 B/op	 3415352 allocs/op
BenchmarkERC20Transfer/leveldb-12       	       2	 694035286 ns/op	197684224 B/op	 3415201 allocs/op
BenchmarkERC20Transfer/leveldb-12       	       2	 703206206 ns/op	197654692 B/op	 3415031 allocs/op
PASS
ok  	github.com/crypto-org-chain/cronos/v2/app	27.248s


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 a team as a code owner March 16, 2023 01:20
@yihuang yihuang requested review from MalteHerrmann and GAtom22 and removed request for a team March 16, 2023 01:20
Solution:
- only do once in ante handler, reuse the result

* Update CHANGELOG.md

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

* Update x/evm/types/msg.go

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

* return value

* fix unit test

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: mmsqe <[email protected]>
CHANGELOG.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #1717 (bfa7d6a) into main (f594627) will decrease coverage by 0.22%.
The diff coverage is 15.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1717      +/-   ##
==========================================
- Coverage   68.15%   67.94%   -0.22%     
==========================================
  Files         112      112              
  Lines       10175    10209      +34     
==========================================
+ Hits         6935     6936       +1     
- Misses       2832     2865      +33     
  Partials      408      408              
Impacted Files Coverage Δ
x/evm/types/msg.go 70.54% <0.00%> (-10.35%) ⬇️
x/evm/keeper/msg_server.go 90.90% <100.00%> (ø)
x/evm/keeper/state_transition.go 78.37% <100.00%> (+0.08%) ⬆️

@facs95 facs95 merged commit 2e6e863 into evmos:main Mar 24, 2023
@yihuang yihuang deleted the reuse-sender branch March 24, 2023 23:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants