-
Notifications
You must be signed in to change notification settings - Fork 415
tests(benchmark): add missing scenario and optimization #1768
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
tests(benchmark): add missing scenario and optimization #1768
Conversation
f9fdb17 to
01e4d6b
Compare
01e4d6b to
1b44dea
Compare
| ) | ||
|
|
||
|
|
||
| def test_pc_op( |
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.
We miss the benchmark for PC opcode
| @pytest.mark.parametrize("mem_alloc", [b"", b"ff", b"ff" * 32]) | ||
| @pytest.mark.parametrize("offset", [0, 31, 1024]) | ||
| def test_keccak( |
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.
Instead of finding the optimal length, add a new case that parametrized initial memory layout and access offset. Request by gas repricing effort.
| ], | ||
| ) | ||
| @pytest.mark.parametrize("balance", [0, 1]) | ||
| def test_block_full_of_ether_transfers( |
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.
Add a new scenario for ether transfer benchmark: whether the receiver is an empty account.
| # Create 256 dummy blocks to fill the blockhash window. | ||
| blocks = [Block()] * 256 | ||
|
|
||
| block_number = Op.AND(Op.GAS, 0xFF) if index is None else index |
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.
Adding extra BLOCKHASH operation for benchmarking, as it was one of the slowest operations.
Cases:
- Valid block index
- Invalid block index
- Dynamic block index
| empty_account: bool, | ||
| initial_balance: bool, | ||
| initial_storage: bool, |
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.
Add new test scenario for gas repricing effort:
- Accessing empty / non-empty account
- Accessing account contains zero / non-zero balance
- Accessing account contains zero / non-zero storage
marioevz
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.
Thanks!
In general, I liked the refactoring and changes, I've just left comments on multiple files just to make everything more polished.
c720a28 to
2b6823a
Compare
|
Sorry for adding complexity to this PR, I’ve added one more commit that optimizes the
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1768 +/- ##
============================================
Coverage 86.08% 86.08%
============================================
Files 743 743
Lines 44072 44072
Branches 3891 3891
============================================
Hits 37938 37938
Misses 5656 5656
Partials 478 478
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:
|
marioevz
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.
Excellent job on all these optimizations. LGTM, thanks!
* feat: add max_stack_height method to BaseFork and Frontier classes * refactor: replace hardcode stack limit to fork configuration
* feat: add max_stack_height method to BaseFork and Frontier classes * refactor: replace hardcode stack limit to fork configuration
🗒️ Description
Enhance the code generator for
BenchmarkTestwrapper:JumpLoopGenerator, which already supports in the other code generator, this could simplifytest_create.ExtCallGenerator, when calculating the max iterations, it should deduct the stack delta that comes fromsetupsection, or it might lead to stack overflow issue.STOPopcode. This could help simplify some cases liketest_codecopy&test_return_revert.ExtCallGenerator, there are two different account: target contract and loop contract. The loop contract will repeatedly calls into target contract viaSTATICCALL. For some operations that involvesCALLDATAinteractions, we should forward theCALLDATAfrom loop contract to target contract via (1) forward theCALLDATAfrom loop contract and (2) Configure the memory from CALLDATA in target contract.Test Case Enhancement:
ExtCallGenerator🔗 Related Issues or PRs
N/A.
✅ 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