ci: run benchmark command

[AvivYossef-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[AvivYossef-starkware]

• bench_tools: add absolute limits to benchmark runs #9861 : 3 dependent PRs (#9526 , #9874 , #9876 )
• bench_tools: dont capture output when running a benchmark #9860
• ci: use benchtools to benchmark the committer #9707
• bench_tools: fix committer benchmark config #9807
• bench_tools: run and compare benchmark #9699
• bench_tools: support deserialization of criterion change #9698
• bench_tools: save specific benchmark result #9742
• bench_tools: add criterion benchmark names to benchmark config #9697
• bench_tools: add bench tools to allowed scopes #9696
• ci: run benchmark command #9636 👈 (View in Graphite)
• infra: copy dir rec #9802
• main-v0.14.1

This stack of pull requests is managed by Graphite. Learn more about stacking.

[avi-starkware]

bench_tools/src/runner.rs line 22 at r1 (raw file):

        if !local_path.exists() {
            panic!("Input directory does not exist: {}", local_dir);
        }

If input_dir is Some, then it is never used. This function just checks that the directory exists, but run_benchmarks doesn't do anything with that directory other than that, and it is not copied into bench.input_dir.

Code quote:

    if let Some(local_dir) = input_dir {
        let local_path = PathBuf::from(local_dir);
        if !local_path.exists() {
            panic!("Input directory does not exist: {}", local_dir);
        }

[github-actions]

Artifacts upload workflows:

• Started at 2025-10-22T06:16:10Z
• Started at 2025-10-22T06:32:29Z
• Started at 2025-10-22T06:42:18Z
• Started at 2025-10-22T06:45:08Z
• Started at 2025-10-22T06:55:57Z
• Started at 2025-10-22T07:06:18Z
• Started at 2025-10-22T07:06:19Z
• Started at 2025-10-22T12:13:08Z
• Started at 2025-10-22T12:23:21Z
• Started at 2025-10-22T12:25:49Z
• Started at 2025-10-22T12:54:57Z
• Started at 2025-10-23T09:36:54Z
• Started at 2025-10-23T10:08:05Z
• Started at 2025-10-23T11:31:27Z
• Started at 2025-10-23T11:31:27Z
• Started at 2025-10-23T12:12:20Z
• Started at 2025-10-28T06:31:57Z
• Started at 2025-10-28T07:18:22Z

[github-actions]

Benchmark movements: tree_computation_flow performance improved 😺 tree_computation_flow time: [13.345 ms 13.494 ms 13.660 ms] change: [-9.0732% -7.4423% -5.7610%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe

[AvivYossef-starkware]

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @meship-starkware)

---

bench_tools/src/runner.rs line 22 at r1 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

If input_dir is Some, then it is never used. This function just checks that the directory exists, but run_benchmarks doesn't do anything with that directory other than that, and it is not copied into bench.input_dir.

you absolutly right, thanks

[AvivYossef-starkware]

Reviewable status: 1 of 11 files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @meship-starkware)

---

Cargo.toml line 256 at r3 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

Note that adding new external dependencies is discouraged and requires special approval.

I can do something like that
, but fs extra is a popular crate and I prefer to use it.
I'll ask for permission

/// Recursively copies the contents of a directory to another directory.
fn copy_dir_contents(src: &Path, dst: &Path) -> std::io::Result<()> {
    for entry in fs::read_dir(src)? {
        let entry = entry?;
        let src_path = entry.path();
        let dst_path = dst.join(entry.file_name());

        if src_path.is_dir() {
            fs::create_dir_all(&dst_path)?;
            copy_dir_contents(&src_path, &dst_path)?;
        } else {
            fs::copy(&src_path, &dst_path)?;
        }
    }
    Ok(())
}

---

crates/bench_tools/src/runner.rs line 40 at r4 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

Three issues:

1. This will panic if local_dir == benchmark_input_dir which is unexpected behavior.
2. This recursive copy is blocking, so doesn't work well with your async runtime.
3. Is the new external dependency necessary?

1. Thanks, fixed
2. We removed async
3. We don't have to use it, but I think that we should.

---

crates/bench_tools/src/runner.rs line 67 at r4 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

why do you copy all files for each benchmark you run?
Why not just the files related to _bench?

Thanks,
fixed in #9742

[avi-starkware]

@avi-starkware reviewed 5 of 6 files at r6, all commit messages.
Reviewable status: 6 of 11 files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @meship-starkware)

---

Cargo.toml line 256 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

I can do something like that
, but fs extra is a popular crate and I prefer to use it.
I'll ask for permission

/// Recursively copies the contents of a directory to another directory.
fn copy_dir_contents(src: &Path, dst: &Path) -> std::io::Result<()> {
    for entry in fs::read_dir(src)? {
        let entry = entry?;
        let src_path = entry.path();
        let dst_path = dst.join(entry.file_name());

        if src_path.is_dir() {
            fs::create_dir_all(&dst_path)?;
            copy_dir_contents(&src_path, &dst_path)?;
        } else {
            fs::copy(&src_path, &dst_path)?;
        }
    }
    Ok(())
}

If you get approval, I won't block you from using it, but I don't think it is a good idea to introduce a new external dependency instead of adding this function. (Unless the external crate is somehow much more efficient, or has other benefits.)

Your call anyway - non-blocking

---

crates/bench_tools/src/runner.rs line 40 at r4 (raw file):

Previously, AvivYossef-starkware wrote…

1. Thanks, fixed
2. We removed async
3. We don't have to use it, but I think that we should.

Why is 1 fixed?
You need to handle the AlreadyExists error to avoid panicking in this case

[AvivYossef-starkware]

Reviewable status: 2 of 12 files reviewed, 2 unresolved discussions (waiting on @avi-starkware and @meship-starkware)

---

Cargo.toml line 256 at r3 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

If you get approval, I won't block you from using it, but I don't think it is a good idea to introduce a new external dependency instead of adding this function. (Unless the external crate is somehow much more efficient, or has other benefits.)

Your call anyway - non-blocking

I removed it

---

crates/bench_tools/src/runner.rs line 40 at r4 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

Why is 1 fixed?
You need to handle the AlreadyExists error to avoid panicking in this case

I forgot to push.
but now the new func does nothing if its he same

[graphite-app]

Merge activity

• Oct 29, 7:56 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

[avi-starkware]

@avi-starkware reviewed 3 of 6 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: 6 of 12 files reviewed, all discussions resolved (waiting on @meship-starkware)

[avi-starkware]

@avi-starkware reviewed 4 of 11 files at r2.
Reviewable status: 10 of 12 files reviewed, all discussions resolved (waiting on @meship-starkware)

[avi-starkware]

@avi-starkware reviewed 2 of 6 files at r7.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware)

[avi-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware)