-
Notifications
You must be signed in to change notification settings - Fork 450
perf(StdAssertions): avoid vm call for trivial conditions #693
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
Conversation
Avoid calling vm when the condition is trivial to check inline. This is a minor performance improvement as a couple of opcodes execute a lot faster than a full cheatcode pipeline (external call, EVM call bookkeeping, abi encoding/decoding, ...) that eventually does nothing.
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.
Pull Request Overview
This PR optimizes assertion functions by adding inline condition checks to avoid unnecessary VM calls when assertions would pass. The optimization adds conditional guards that only invoke the VM when an assertion is about to fail, improving performance for successful test cases.
- Adds inline condition checks to all basic assertion functions before calling VM methods
- Modifies
assertEq32andassertNotEq32functions to use direct VM calls instead of delegating to other assertion functions - Applies the optimization pattern consistently across boolean, numeric, and address comparison assertions
grandizzy
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.
nice finding, lgtm!
|
This makes sense, a further improvement could be exposing an error invoker directly and avoiding the duplicate assertion check |
yash-atreya
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!
It would make sense in theory but in practice the check is free and it would just be adding a lot more cheatcodes for no gain |
Avoid calling vm when the condition is trivial to check inline. This is a minor performance improvement as a couple of opcodes execute a lot faster than a full cheatcode pipeline (external call, EVM call bookkeeping, abi encoding/decoding, ...) that eventually does nothing.
For example, Uniswap's v4-core spends ~28% of the entire
forge testCPU time in a couple of trivialassertfunctions (uint256, bytes32, true, false), ~13% forforge coverage. This is without accounting for the actual CALL/abi coding etc.These numbers are skewed due to profiling overhead, however making this change does have ~5% overall test performance improvement, for no compilation time change.