From c0f3104b63b32f681cb11233d3d41efd09b888a7 Mon Sep 17 00:00:00 2001 From: Joonatan Saarhelo Date: Tue, 9 Apr 2024 15:19:21 +0100 Subject: [PATCH] fix: instruction count diff always N/A in VM perf comparison (#1608) Instruction count diff should now be correct. The problem wasn't found initially because counting instructions legitimately fails if the PR starts from a version where that wasn't supported yet. Also found and fixed a bug that hid performance improvements. --- .github/workflows/vm-perf-comparison.yml | 4 ++-- core/tests/vm-benchmark/src/compare_iai_results.rs | 2 +- core/tests/vm-benchmark/src/instruction_counts.rs | 12 +++++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/vm-perf-comparison.yml b/.github/workflows/vm-perf-comparison.yml index 555d13c9300..a6b2b71ce60 100644 --- a/.github/workflows/vm-perf-comparison.yml +++ b/.github/workflows/vm-perf-comparison.yml @@ -46,7 +46,7 @@ jobs: ci_run zk ci_run zk compiler system-contracts ci_run cargo bench --package vm-benchmark --bench iai | tee base-iai - ci_run cd core/tests/vm_benchmark && cargo run --release --bin instruction-counts | tee base-opcodes || touch base-opcodes + ci_run cargo run --package vm-benchmark --release --bin instruction-counts | tee base-opcodes || touch base-opcodes ci_run yarn workspace system-contracts clean - name: checkout PR @@ -58,7 +58,7 @@ jobs: ci_run zk ci_run zk compiler system-contracts ci_run cargo bench --package vm-benchmark --bench iai | tee pr-iai - ci_run cd core/tests/vm_benchmark && cargo run --release --bin instruction-counts | tee pr-opcodes || touch pr-opcodes + ci_run cargo run --package vm-benchmark --release --bin instruction-counts | tee pr-opcodes || touch pr-opcodes EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64) echo "speedup<<$EOF" >> $GITHUB_OUTPUT diff --git a/core/tests/vm-benchmark/src/compare_iai_results.rs b/core/tests/vm-benchmark/src/compare_iai_results.rs index 7ffadfc76be..b9b6440704c 100644 --- a/core/tests/vm-benchmark/src/compare_iai_results.rs +++ b/core/tests/vm-benchmark/src/compare_iai_results.rs @@ -25,7 +25,7 @@ fn main() { .intersection(&iai_after.keys().collect()) .flat_map(|&name| { let diff = percent_difference(iai_before[name], iai_after[name]); - if diff > 2. { + if diff.abs() > 2. { Some((name, format!("{:+.1}%", diff))) } else { None diff --git a/core/tests/vm-benchmark/src/instruction_counts.rs b/core/tests/vm-benchmark/src/instruction_counts.rs index 11f5d4c5ff6..a80d8a7ffd6 100644 --- a/core/tests/vm-benchmark/src/instruction_counts.rs +++ b/core/tests/vm-benchmark/src/instruction_counts.rs @@ -1,9 +1,19 @@ //! Runs all benchmarks and prints out the number of zkEVM opcodes each one executed. +use std::path::Path; + use vm_benchmark_harness::{cut_to_allowed_bytecode_size, get_deploy_tx, BenchmarkingVm}; fn main() { - for path in std::fs::read_dir("deployment_benchmarks").unwrap() { + // using source file location because this is just a script, the binary isn't meant to be reused + let benchmark_folder = Path::new(file!()) + .parent() + .unwrap() + .parent() + .unwrap() + .join("deployment_benchmarks"); + + for path in std::fs::read_dir(benchmark_folder).unwrap() { let path = path.unwrap().path(); let test_contract = std::fs::read(&path).expect("failed to read file");