Skip to content
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

Divan benchmarking PoC #140

Merged
merged 9 commits into from
Dec 13, 2023
Merged

Divan benchmarking PoC #140

merged 9 commits into from
Dec 13, 2023

Conversation

CrockAgile
Copy link
Collaborator

@CrockAgile CrockAgile commented Nov 26, 2023

Closes #138

I ported the existing benchmarks to Divan as described in #138.

cargo bench -q --bench divan

Timer precision: 41 ns
divan                                  fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ examples                                          │               │               │               │         │
   ├─ generate_dna                     415 ns        │ 75.45 µs      │ 707 ns        │ 765.5 ns      │ 570652  │ 570652
   ├─ parse_infinite_nullable_grammar  999 ns        │ 2.995 ms      │ 106.4 µs      │ 146.1 µs      │ 34183   │ 34183
   │                                   1 Mitem/s     │ 31.04 Kitem/s │ 469.6 Kitem/s │ 335.2 Kitem/s │         │
   ├─ parse_polish_calculator          4.79 µs       │ 76.19 ms      │ 11.45 µs      │ 852.3 µs      │ 5867    │ 5867
   ╰─ parse_postal                     12.91 µs      │ 1.481 ms      │ 13.29 µs      │ 13.49 µs      │ 369045  │ 369045

TODO

  • write up pros / cons compared to Criterion
  • rename benchmark file
  • remove old benchmarks

@coveralls
Copy link

coveralls commented Nov 26, 2023

Coverage Status

coverage: 96.67%. remained the same
when pulling 8a77974 on divan-compare
into 67a7054 on main.

@nvzqz
Copy link
Contributor

nvzqz commented Dec 2, 2023

Hi, I noticed this PR and wanted to suggest a few improvements if I may 😄


parse_postal can instead be written as:

#[divan::bench]
fn parse_postal(bencher: divan::Bencher) {
    let input = divan::black_box(
        include_str!("../tests/fixtures/postal_address.terminated.input.bnf")
    );

    bencher.bench(|| {
        input.parse::<bnf::Grammar>().unwrap();
    });
}

This ensures each iteration gets an opaque input without the cost of storing duplicate inputs (Bencher::with_inputs allocates storage for all inputs in a sample).


Similarly, parse_infinite_nullable_grammar could be written as:

#[divan::bench]
fn parse_infinite_nullable_grammar(bencher: divan::Bencher) {
    let infinite_grammar: bnf::Grammar = "
            <a> ::= '' | <b>
            <b> ::= <a>"
        .parse()
        .unwrap();
    
    let parse_count: usize = {
        use rand::Rng;
        let mut rng: rand::rngs::StdRng = rand::SeedableRng::seed_from_u64(0);
        rng.gen_range(1..100);
    };

    bencher
        .counter(parse_count)
        .bench(|| {
            let _: Vec<_> = infinite_grammar.parse_input("").take(parse_count).collect();
        });
}

However, you probably meant to have each iteration use a random parse_count, in which case it could be rewritten to:

#[divan::bench]
fn parse_infinite_nullable_grammar(bencher: divan::Bencher) {
    use rand::Rng;

    let infinite_grammar: bnf::Grammar = "
            <a> ::= '' | <b>
            <b> ::= <a>"
        .parse()
        .unwrap();
    
    let mut rng: rand::rngs::StdRng = rand::SeedableRng::seed_from_u64(0);

    bencher
        .with_inputs(|| rng.gen_range(1..100))
        .count_inputs_as::<divan::counter::ItemsCount>()
        .bench_local_values(|parse_count| {
            let _: Vec<_> = infinite_grammar.parse_input("").take(parse_count).collect();
        });
}

Note that Bencher::count_inputs_as is in 0.1.4.

@CrockAgile
Copy link
Collaborator Author

@nvzqz wow thank you for the great notes! I applied your suggestions. thank you!

benches/divan.rs Outdated Show resolved Hide resolved
benches/divan.rs Outdated Show resolved Hide resolved
benches/divan.rs Outdated Show resolved Hide resolved
benches/divan.rs Outdated Show resolved Hide resolved
CrockAgile and others added 3 commits December 5, 2023 14:56
Co-authored-by: Nikolai Vazquez <hello@nikolaivazquez.com>
Co-authored-by: Nikolai Vazquez <hello@nikolaivazquez.com>
Copy link
Contributor

@nvzqz nvzqz left a comment

Choose a reason for hiding this comment

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

I released 0.1.5 which adds black_box_drop, which should make this code more legible 😄

benches/divan.rs Outdated Show resolved Hide resolved
benches/divan.rs Outdated Show resolved Hide resolved
Copy link
Owner

@shnewto shnewto left a comment

Choose a reason for hiding this comment

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

very cool!

@CrockAgile CrockAgile merged commit c5e3a5b into main Dec 13, 2023
10 checks passed
@CrockAgile CrockAgile deleted the divan-compare branch December 13, 2023 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compare Criterion & Divan
4 participants