Skip to content

Conversation

@AvivYossef-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator

@avi-starkware avi-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avi-starkware reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @meship-starkware)


crates/bench_tools/src/types/benchmark_config.rs line 13 at r1 (raw file):

    /// If None, assumes a single benchmark with the same name as the config.
    /// Used for regression checking to know which criterion directories to check.
    pub criterion_benchmark_names: Option<&'static [&'static str]>,

I don't understand this new config

Code quote:

    /// Optional list of Criterion benchmark names that this benchmark suite produces.
    /// If None, assumes a single benchmark with the same name as the config.
    /// Used for regression checking to know which criterion directories to check.
    pub criterion_benchmark_names: Option<&'static [&'static str]>,

@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_bench_tools_to_allowed_scopes branch from 8d9209a to 80a909b Compare October 23, 2025 09:36
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch 2 times, most recently from 618a391 to 0b8e2f7 Compare October 23, 2025 10:08
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_bench_tools_to_allowed_scopes branch from 80a909b to 0a6ad5b Compare October 23, 2025 10:08
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from 0b8e2f7 to a6d3b63 Compare October 23, 2025 11:31
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_bench_tools_to_allowed_scopes branch 2 times, most recently from 60d4519 to 72a806b Compare October 23, 2025 12:12
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from a6d3b63 to 3d11228 Compare October 23, 2025 12:12
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from 3d11228 to de34bd8 Compare October 28, 2025 06:31
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_bench_tools_to_allowed_scopes branch 2 times, most recently from 9a7d354 to b6817f7 Compare October 28, 2025 08:21
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from de34bd8 to 1762d43 Compare October 28, 2025 08:21
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_bench_tools_to_allowed_scopes branch from b6817f7 to d0ebb5f Compare October 29, 2025 07:18
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from 1762d43 to 39b0efd Compare October 29, 2025 07:18
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_bench_tools_to_allowed_scopes branch from d0ebb5f to c0e50e1 Compare October 29, 2025 07:40
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from 39b0efd to 8e601c4 Compare October 29, 2025 07:40
@graphite-app graphite-app bot changed the base branch from aviv/add_bench_tools_to_allowed_scopes to graphite-base/9697 October 29, 2025 11:30
@AvivYossef-starkware AvivYossef-starkware force-pushed the aviv/add_criterion_benchmark_names branch from 8e601c4 to 4084f6c Compare October 29, 2025 12:19
@graphite-app graphite-app bot changed the base branch from graphite-base/9697 to main-v0.14.1 October 29, 2025 12:19
@graphite-app
Copy link

graphite-app bot commented Oct 29, 2025

Merge activity

  • Oct 29, 12:19 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

Copy link
Contributor Author

@AvivYossef-starkware AvivYossef-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


crates/bench_tools/src/types/benchmark_config.rs line 13 at r1 (raw file):

Previously, avi-starkware (Avi Cohen) wrote…

I don't understand this new config

look here

BenchmarkConfig {
        name: "dummy_benchmark",
        package: "bench_tools",
        cmd_args: &["bench", "-p", "bench_tools", "--bench", "dummy_bench"],
        input_dir: Some("crates/bench_tools/data/dummy_bench_input"),
        criterion_benchmark_names: Some(&[
            "dummy_sum_100",
            "dummy_sum_1000",
            "dummy_process_small_input",
            "dummy_process_large_input",
        ]),
    },

when running cargo bench -p bench_tools --bench dummy_bench
4 benches are actually run.
It happens because of adding a bench function like that

c.bench_function("dummy_process_large_input", |b| {
        b.iter(|| black_box(process_input(&large_input)))
    });

Copy link
Collaborator

@avi-starkware avi-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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

@AvivYossef-starkware AvivYossef-starkware added this pull request to the merge queue Oct 30, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 30, 2025
@AvivYossef-starkware AvivYossef-starkware added this pull request to the merge queue Oct 30, 2025
Merged via the queue into main-v0.14.1 with commit 753f61c Oct 30, 2025
17 of 34 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants