feat(tests): add test for worst-case prefetcher performance#2657
feat(tests): add test for worst-case prefetcher performance#2657LouisTsai-Csie merged 16 commits intoethereum:forks/amsterdamfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2657 +/- ##
===================================================
+ Coverage 86.24% 86.25% +0.01%
===================================================
Files 599 599
Lines 36984 37032 +48
Branches 3795 3795
===================================================
+ Hits 31895 31943 +48
Misses 4525 4525
Partials 564 564
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jochem-brouwer
left a comment
There was a problem hiding this comment.
I somehow did not see that this approach solves the same problem but with less gas (I did not realize this also tricks the prefetcher). One final comment for verification.
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Leave some style suggestion comment, please take a look!
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
| tx_gas = min(gas_available, tx_gas_limit) | ||
| target = pre.deploy_contract( | ||
| code=runtime_code, | ||
| storage=Storage(storage_data), |
There was a problem hiding this comment.
This will likely have the problem that either this code will not fit in initcode, or the predeploy code to execute will use more than 2**24 gas. @LouisTsai-Csie we should likely create a general helper here for the predeploy in execute
There was a problem hiding this comment.
We already have this helper, it is not clear though:
In the test_sstore_variants benchmark, we delegate to this helper then initialize the storage, later execute the benchmark code. This would not go out of gas, but not sure if it would take too much time or not.
|
Here is fixture output of the |
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Thanks for the update, i did a final check, after these changes let's merge.
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
Co-authored-by: 蔡佳誠 Louis Tsai <72684086+LouisTsai-Csie@users.noreply.github.com>
|
Everything addressed. Ready for one last review! |
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Thank you! I will merge it for now, and refactor in PR #2672, as the current storage initialization would break at high gas configuration.
🗒️ Description
Add
test_sload_bloated_prefetch_missbenchmark that defeats client prefetchers by making each transaction's SLOAD range depend on state written by its predecessor. A minimal first tx writes an offset to slot 0 via calldata; subsequent max-gas txs read the previous offset, SLOAD from it, and write a new offset for the next tx.🔗 Related Issues or PRs
N/A.
✅ Checklist
just staticCute Animal Picture