feat(l1): add target and script to run multiple benchmark runs #5368

MegaRedHand · 2025-11-14T22:10:05Z

This PR improves tooling/import_benchmark by adding a target that runs multiple repetitions of the same benchmark.

Copilot

Pull Request Overview

This PR adds automation for running multiple benchmark iterations by introducing a shell script and a new Makefile target. The goal is to streamline the process of running repeated import benchmarks with automatic log file numbering.

Adds benchmark.sh script to automate multiple benchmark runs with sequential log numbering
Introduces run-benches Makefile target as a convenient wrapper for the script
Enables running 3 benchmark iterations by default using the existing benchmark infrastructure

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
tooling/import_benchmark/benchmark.sh	New shell script that runs multiple benchmark iterations with automatic log file numbering and cleanup
tooling/import_benchmark/Makefile	Adds `run-benches` target to invoke the benchmark script with 3 repetitions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-14T22:12:15Z

tooling/import_benchmark/benchmark.sh

+
+NETWORK=$1
+NUM_REPETITIONS=$2
+


Missing input validation for required parameters. The script will fail with cryptic errors if NETWORK or NUM_REPETITIONS are not provided. Add validation to check if both parameters are supplied and NUM_REPETITIONS is a positive integer.

Suggested change

NETWORK=$1

NUM_REPETITIONS=$2

# Input validation

if [ $# -ne 2 ]; then

echo "Usage: $0 <NETWORK> <NUM_REPETITIONS>"

exit 1

fi

NETWORK=$1

NUM_REPETITIONS=$2

# Check that NUM_REPETITIONS is a positive integer

if ! [[ "$NUM_REPETITIONS" =~ ^[1-9][0-9]*$ ]]; then

echo "Error: NUM_REPETITIONS must be a positive integer."

exit 1

fi

Copilot · 2025-11-14T22:12:15Z

tooling/import_benchmark/benchmark.sh

+NUM_REPETITIONS=$2
+
+touch bench-0.log
+bench_id=$(ls bench-*.log | cut -d '-' -f 2 | cut -d '.' -f 1 | sort -n | tail -1)


The ls bench-*.log command will fail with an error if no bench-*.log files exist (even after creating bench-0.log in some scenarios where the touch fails). Additionally, using ls for scripting is fragile. Consider using: bench_id=$(find . -maxdepth 1 -name 'bench-*.log' -printf '%f\n' 2>/dev/null | cut -d '-' -f 2 | cut -d '.' -f 1 | sort -n | tail -1 || echo 0) or handle the case where no files exist more explicitly.

Suggested change

bench_id=$(ls bench-*.log | cut -d '-' -f 2 | cut -d '.' -f 1 | sort -n | tail -1)

bench_id=$(find . -maxdepth 1 -name 'bench-*.log' -printf '%f\n' 2>/dev/null | cut -d '-' -f 2 | cut -d '.' -f 1 | sort -n | tail -1)

bench_id=${bench_id:-0}

Copilot · 2025-11-14T22:12:15Z

tooling/import_benchmark/benchmark.sh

+touch bench-0.log
+bench_id=$(ls bench-*.log | cut -d '-' -f 2 | cut -d '.' -f 1 | sort -n | tail -1)
+
+cd ../..


The script changes directory to ../.. without error checking or explanation. This makes the script fragile if called from an unexpected location. Consider either: (1) using cd ../.. || exit 1 to fail fast if the directory doesn't exist, or (2) storing the original directory and using absolute paths, or (3) adding a comment explaining the expected directory structure.

Suggested change

cd ../..

cd ../.. || exit 1

Copilot · 2025-11-14T22:12:16Z

tooling/import_benchmark/benchmark.sh

+    bench_id=$((bench_id + 1))
+    rm -rf ~/.local/share/temp
+    cp -r ~/.local/share/ethrex_${NETWORK}_bench/ethrex ~/.local/share/temp
+    cargo r --release -- --network $NETWORK --datadir ~/.local/share/temp import-bench ~/.local/share/ethrex_${NETWORK}_bench/chain.rlp | tee bench-${bench_id}.log


The output log file bench-${bench_id}.log is written to the ethrex root directory (after cd ../..), but the initial log file discovery in line 9 looks for logs in the tooling/import_benchmark directory. This inconsistency means the auto-incrementing bench_id won't work correctly after the first run. The logs should either all be in the same directory, or the script should cd back to the original directory before writing logs.

Copilot · 2025-11-14T22:12:16Z

tooling/import_benchmark/Makefile

 run-bench: ## Runs a bench for the current pr. parameters -BENCH_ID: number for the log file where it will be saved -NETWORK: which network to acesss
 	rm -rf ~/.local/share/temp
 	cp -r ~/.local/share/ethrex_$(NETWORK)_bench/ethrex ~/.local/share/temp
 	cd ../.. && cargo r --release -- --network $(NETWORK) --datadir ~/.local/share/temp import-bench ~/.local/share/ethrex_$(NETWORK)_bench/chain.rlp | tee bench-$(BENCH_ID).log


The new run-benches target lacks documentation. Other targets in the Makefile have comments explaining their purpose, parameters, and requirements. Add a comment describing this target, its purpose, and that it runs 3 benchmark iterations by default.

Suggested change

cd ../.. && cargo r --release -- --network $(NETWORK) --datadir ~/.local/share/temp import-bench ~/.local/share/ethrex_$(NETWORK)_bench/chain.rlp | tee bench-$(BENCH_ID).log

# run-benches: ## Runs 3 benchmark iterations by default for the current PR.

# Parameters:

# - NETWORK: which network to access (hoodi, mainnet)

# Requirements:

# Assumes the environment is set up as for run-bench (Linux, valid db and chain.rlp in ~/.local/share/ethrex_NETWORK_bench/)

Copilot · 2025-11-14T22:12:16Z

tooling/import_benchmark/Makefile

 	cp -r ~/.local/share/ethrex_$(NETWORK)_bench/ethrex ~/.local/share/temp
 	cd ../.. && cargo r --release -- --network $(NETWORK) --datadir ~/.local/share/temp import-bench ~/.local/share/ethrex_$(NETWORK)_bench/chain.rlp | tee bench-$(BENCH_ID).log
+
+run-benches:


The number of repetitions (3) is hardcoded. Consider making it configurable via a Makefile variable like NUM_REPS ?= 3 to allow users to easily customize the number of benchmark runs without modifying the script or Makefile.

github-actions · 2025-11-14T22:52:33Z

Benchmark Block Execution Results Comparison Against Main

Command	Mean [s]	Min [s]	Max [s]	Relative
`base`	59.803 ± 0.191	59.508	60.099	1.00
`head`	60.018 ± 0.233	59.726	60.452	1.00 ± 0.01

fedacking · 2025-11-16T23:47:34Z

tooling/import_benchmark/Makefile

 	@printf "python3 parse_bench.py bench_num_1 bench_num_2: ## Parses the bench log files from [bench_num_1 to bench_num_2) to find average ggas\nRequirements\n\
 	 This script assumes we have the bench logs on the ethrex folder\n\n"

 run-bench: ## Runs a bench for the current pr. parameters -BENCH_ID: number for the log file where it will be saved -NETWORK: which network to acesss


Nitpick: I think that we should have the rm -> cp -> cargo r -- import-bench process defined in a single file, either the cargo or the make.

feat: add target and script to run multiple benchmark runs

710e4a4

Copilot AI review requested due to automatic review settings November 14, 2025 22:10

MegaRedHand requested a review from a team as a code owner November 14, 2025 22:10

MegaRedHand added performance tooling External and Internal tooling that makes it easier to test the client labels Nov 14, 2025

github-project-automation bot moved this to Todo in ethrex_performance Nov 14, 2025

github-project-automation bot added this to ethrex_performance Nov 14, 2025

github-actions bot assigned MegaRedHand Nov 14, 2025

github-actions bot added L1 Ethereum client and removed performance labels Nov 14, 2025

github-project-automation bot added this to ethrex_l1 Nov 14, 2025

Copilot started reviewing on behalf of MegaRedHand November 14, 2025 22:10 View session

Copilot finished reviewing on behalf of MegaRedHand November 14, 2025 22:11

feat: add argument for number of repetitions

a26e154

Copilot AI reviewed Nov 14, 2025

View reviewed changes

docs: add comment in help

d49e0b0

fedacking reviewed Nov 16, 2025

View reviewed changes

mpaulucci approved these changes Nov 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(l1): add target and script to run multiple benchmark runs #5368

feat(l1): add target and script to run multiple benchmark runs #5368

MegaRedHand commented Nov 14, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

github-actions bot commented Nov 14, 2025

Uh oh!

fedacking Nov 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-NETWORK=$1
-NUM_REPETITIONS=$2
+# Input validation
+if [ $# -ne 2 ]; then
+    echo "Usage: $0 <NETWORK> <NUM_REPETITIONS>"
+    exit 1
+fi
+NETWORK=$1
+NUM_REPETITIONS=$2
+# Check that NUM_REPETITIONS is a positive integer
+if ! [[ "$NUM_REPETITIONS" =~ ^[1-9][0-9]*$ ]]; then
+    echo "Error: NUM_REPETITIONS must be a positive integer."
+    exit 1
+fi

	bench_id=$(ls bench-*.log \| cut -d '-' -f 2 \| cut -d '.' -f 1 \| sort -n \| tail -1)
	bench_id=$(find . -maxdepth 1 -name 'bench-*.log' -printf '%f\n' 2>/dev/null \| cut -d '-' -f 2 \| cut -d '.' -f 1 \| sort -n \| tail -1)
	bench_id=${bench_id:-0}

-	cd ../.. && cargo r --release -- --network $(NETWORK) --datadir ~/.local/share/temp import-bench ~/.local/share/ethrex_$(NETWORK)_bench/chain.rlp | tee bench-$(BENCH_ID).log
+# run-benches: ## Runs 3 benchmark iterations by default for the current PR.
+# Parameters:
+#  - NETWORK: which network to access (hoodi, mainnet)
+# Requirements:
+#  Assumes the environment is set up as for run-bench (Linux, valid db and chain.rlp in ~/.local/share/ethrex_NETWORK_bench/)

feat(l1): add target and script to run multiple benchmark runs #5368

Are you sure you want to change the base?

feat(l1): add target and script to run multiple benchmark runs #5368

Conversation

MegaRedHand commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 14, 2025

Benchmark Block Execution Results Comparison Against Main

Uh oh!

fedacking Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MegaRedHand commented Nov 14, 2025 •

edited

Loading