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

revm v1.9 bytecode hash #2677

Merged
merged 2 commits into from
Aug 9, 2022
Merged

Conversation

rakita
Copy link
Contributor

@rakita rakita commented Aug 9, 2022

Updating revm to v1.9

more on the change here: bluealloy/revm#165

It removes repetitive hashing and speeds up the fuzzing tests in the forge.

Additionally I moved the flag for limit_contract_code_size in CREATE/CREATE2 from Inspector override_spec to cfg and I set it to max value in foundry. Please check if this is okay or maybe in some places it should be consensus value 0x6000?

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

ty!

@mattsse
Copy link
Member

mattsse commented Aug 9, 2022

broke a test that compared used json object of cfg

updated

@mattsse mattsse added the T-perf Type: performance label Aug 9, 2022
@gakonst
Copy link
Member

gakonst commented Aug 9, 2022

@onbjerg time to run new bench on your box..

@mattsse mattsse merged commit 50fbe82 into foundry-rs:master Aug 9, 2022
@onbjerg
Copy link
Member

onbjerg commented Aug 9, 2022

Nightly:

solmate on  main is 📦 v6.5.0 via  v18.7.0
❯ hyperfine 'forge test --no-match-test "testSafeTransferFromToEOA"'
Benchmark 1: forge test --no-match-test "testSafeTransferFromToEOA"
  Time (mean ± σ):      2.989 s ±  0.194 s    [User: 34.802 s, System: 0.425 s]
  Range (min … max):    2.740 s …  3.283 s    10 runs

This PR:

solmate on  main is 📦 v6.5.0 via  v18.7.0 took 2s
❯ hyperfine 'forge test --no-match-test "testSafeTransferFromToEOA"'
Benchmark 1: forge test --no-match-test "testSafeTransferFromToEOA"
  Time (mean ± σ):      2.504 s ±  0.218 s    [User: 24.660 s, System: 0.429 s]
  Range (min … max):    2.189 s …  2.839 s    10 runs

🔥

@gakonst
Copy link
Member

gakonst commented Aug 9, 2022

Amazing. Was this also using RUSTFLAGS="-C target-cpu=native" for both?

@onbjerg
Copy link
Member

onbjerg commented Aug 9, 2022

None of them used that, can try with those flags

@gakonst
Copy link
Member

gakonst commented Aug 9, 2022

Yep, try RUSTFLAGS="-C target-cpu=native" cargo b --release

@onbjerg
Copy link
Member

onbjerg commented Aug 9, 2022

@gakonst Results were about the same for me, very minor/insignificant change

@rakita
Copy link
Contributor Author

rakita commented Aug 9, 2022

15-20% sounds right.

I don't see any major low hanging fruits that would give as more performance boosts.
calldataload is probably the biggest one not optimized, but that is just a few percentages of execution.
It is maybe worth looking at run_fuzz_test, it uses HashMap::insert a lot.

@mattsse
Copy link
Member

mattsse commented Aug 9, 2022

yeh definitely (invariant)fuzzing,
there's also a lot of Abi cloning involved that could be optimized

iFrostizz pushed a commit to iFrostizz/foundry that referenced this pull request Nov 9, 2022
* revm v1.9: bytecode hash

* update storage test json file

Co-authored-by: Matthias Seitz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-perf Type: performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants