-
Notifications
You must be signed in to change notification settings - Fork 414
feat(test-benchmark): fixed precompile count support #1955
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
feat(test-benchmark): fixed precompile count support #1955
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #1955 +/- ##
================================================
Coverage 86.33% 86.33%
================================================
Files 538 538
Lines 34557 34557
Branches 3222 3222
================================================
Hits 29835 29835
Misses 4148 4148
Partials 574 574
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:
|
danceratopz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @LouisTsai-Csie, thanks for this! Just a flying review, I'll take a closer look tomorrow!
We really should introduce a precompile enum to help avoid the issues below.
|
I've explored a precompile-specific benchmark code generator also, but this has some challenges, or it does not give much benefits. But we could revisit the topic again if necessary. |
4dafaf2 to
7d045dd
Compare
spencer-tb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Can we create issues for the remaining enhancements?
At what --gas-benchmark-values do you get fails?
|
It is not related to the benchmark gas limit but how some of the test's implementation issue. I create an issue for that and will document details there. |
🗒️ Description
This is for the gas repricing analysis effort. Our version 1 benchmark infra only supports the opcode (
tests/benchmark/compute/instruction) scenario, and this adds support to the precompiles (tests/benchmark/compute/precompiles). Now we are able to support most of the precompiles, but still a subset of them are failling.The issue seems to related to insufficient gas consider the current block gas limit, but more in-depth analysis is required.
Other benchmark tests are running correctly under fill mode, we will later configure the
fixed_opcode_count.jsonconfiguration for this after PR #1946 done.Other enhancement:
IDENTITY,SHA256,RIPEMD160precompiles.gas_benchmark_valuetotx_gas_limitto consider eip-7825.🔗 Related Issues or PRs
None
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture