-
Notifications
You must be signed in to change notification settings - Fork 3
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
Ergs comparision CI #194
Merged
Merged
Ergs comparision CI #194
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
Ergs comparison results:
|
MarcosNicolau
approved these changes
Aug 26, 2024
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!
Oppen
approved these changes
Aug 26, 2024
Oppen
approved these changes
Aug 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a CI in order to test whether we are spending the same amount of ergs as the vm1.
This is done by using the benchmark-analyzer of the compiler tester with the interpreter tests
The idea is be the following:
Run:
cargo run --verbose --features lambda_vm --release --bin compiler-tester -- --path tests/solidity/complex/interpreter/test.json --mode 'Y+M3B3 0.8.26' --benchmark lambda_vm.json
Then run:
cargo run --verbose --release --bin compiler-tester -- --path tests/solidity/complex/interpreter/test.json --mode 'Y+M3B3 0.8.26' --benchmark vm1.json
This will generate 2 json files with data for each interpreter test, we care about ergs, running:
cargo run --release --bin benchmark-analyzer -- --reference vm1.json --candidate lambda_vm.json
Will output a table like this
╔═╡ Size (-%) ╞═════╡ EVMInterpreter M3B3 ╞═╗
║ Mean 0.000 ║
║ Best 0.000 ║
║ Worst 0.000 ║
║ Total 0.000 ║
╠═╡ Cycles (-%) ╞═══╡ EVMInterpreter M3B3 ╞═╣
║ Mean 4.834 ║
║ Best 7.022 ║
║ Worst -1.611 ║
║ Total 5.573 ║
╠═╡ Ergs (-%) ╞═════╡ EVMInterpreter M3B3 ╞═╣
║ Mean -3.166 ║
║ Best 0.000 ║
║ Worst -6.275 ║
║ Total -3.573 ║
╠═╡ Ergs/gas ╞══════╡ EVMInterpreter M3B3 ╞═╣
║ ADD 67.667 ║
║ MUL 39.400 ║
║ SUB 67.667 ║
.
.
.
║ SWAP16 66.000 ║
║ RETURN inf ║
║ REVERT inf ║
╠═╡ Ergs/gas (-%) ╞═╡ EVMInterpreter M3B3 ╞═╣
║ ADD 29.021 ║
║ MUL 31.119 ║
║ SUB 29.021 ║
.
.
.
║ SWAP16 38.700 ║
╚════════════════════════════════╝
We care about
ergs/gas
which indicates the relation between the ergs spent by the eravm and the gas spent by the interpreter, since the gas should be the same in both, this will tell us if the ergs spent is correct.What we need to check is that there are no opcodes in
ergs / gas %
, since that means that both values ofergs/gas
are the same.Another way would be checking the colors in
ergs/gas
, if all opcodes haveergs/gas
in grey then they are the same on both vms, however, on the pr comment colors are not showed.This CI will automatically post a comment on the PR with the results, and it should be manually checked if that output is correct before merging
Related to lambdaclass/era-compiler-tester#16