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

Return value/use extra::test::black_box in benchmarks that are optimised to nothing #12118

Closed
huonw opened this issue Feb 9, 2014 · 5 comments · Fixed by #16651
Closed

Return value/use extra::test::black_box in benchmarks that are optimised to nothing #12118

huonw opened this issue Feb 9, 2014 · 5 comments · Fixed by #16651
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@huonw
Copy link
Member

huonw commented Feb 9, 2014

As #8261 describes one has to be careful with benchmarks to avoid them being optimised to remove the section of code that's actually of interest; many in tree benchmarks are not this careful.
#12105 adds two ways to get around this:

  • Return a value from the closure

    // speed of 1 allocation
    bh.iter(|| ~1);
  • Call the noop extra::test::black_box function on some result of a calculation, e.g.

    // speed of 100 allocations
    use extra::test;
    
    bh.iter(|| { for _ in range(0, 100) { test::black_box(~1) } }) 

In both cases, the value is unused, but LLVM can't be sure and so cannot remove optimise out the computation of the value.


The following are the benchmarks that took less than 10ns on the linux-64-opt buildbot (for the landing of #12105), and so are strong candidates for benchmarks that have been optimised to uselessness (there are probably others which aren't completely optimised to nothing, but still aren't benchmarking what they are purporting to):

  • collections
    • bitv::tests::bench_big_bitv_big
    • bitv::tests::bench_big_bitv_small
    • bitv::tests::bench_bitv_big
    • bitv::tests::bench_bitv_set_big
    • bitv::tests::bench_bitv_set_small
    • bitv::tests::bench_bitv_small
    • bitv::tests::bench_small_bitv_small
    • bitv::tests::bench_uint_small
    • smallintmap::bench::find_seq_100
    • smallintmap::bench::find_seq_10_000
    • treemap::bench::find_seq_100
    • treemap::bench::find_seq_10_000
  • arena
    • test::bench_pod
  • std
    • io::buffered::test::bench_buffered_reader
    • io::buffered::test::bench_buffered_writer
    • rt::global_heap::bench::alloc_owned_big
    • rt::global_heap::bench::alloc_owned_small
    • str::bench::bench_with_capacity
    • str::bench::is_utf8_100_ascii
    • str::bench::is_utf8_100_multibyte
    • util::bench::match_option_some
    • util::bench::match_vec_pattern
    • util::bench::trait_static_method_call
    • util::bench::trait_vtable_method_call
    • vec::bench::contains_last_element
    • vec::bench::ends_with_diff_one_element_at_beginning
    • vec::bench::ends_with_same_vector
    • vec::bench::ends_with_single_element
    • vec::bench::push
    • vec::bench::starts_with_diff_one_element_at_end
    • vec::bench::starts_with_same_vector
    • vec::bench::starts_with_single_element
    • vec::bench::zero_1kb_fixed_repeat
    • vec::bench::zero_1kb_mut_iter
    • vec::bench::zero_1kb_set_memory
@huonw huonw added the E-mentor label Feb 9, 2014
@huonw
Copy link
Member Author

huonw commented Feb 9, 2014

I'm tagging this easy because I think it should be relatively easy to choose a few benchmarks from the list above to progressively fix. And mentor because I'm very happy to assist.

(Some of the benchmarks above may not be dead-code eliminated, but never the less are so small that the benchmarker doesn't have enough resolution to give meaningful results: changing it to something like for _ in range(0, 100) { ... } would probably be appropriate.)

@lpy
Copy link
Contributor

lpy commented Feb 9, 2014

Could I take this issue?

@huonw
Copy link
Member Author

huonw commented Feb 9, 2014

Sure! (Don't feel required to do all of them at once.)

@huonw
Copy link
Member Author

huonw commented Feb 15, 2014

#12212 reduced the list to

  • collections
    • bitv::tests::bench_big_bitv_big
    • bitv::tests::bench_big_bitv_small
    • bitv::tests::bench_bitv_big
    • bitv::tests::bench_bitv_set_big
    • bitv::tests::bench_bitv_set_small
    • bitv::tests::bench_bitv_small
    • bitv::tests::bench_small_bitv_small
    • bitv::tests::bench_uint_small
  • arena
    • test::bench_pod
  • std
    • mem::bench::match_option_some
    • mem::bench::match_vec_pattern
    • mem::bench::trait_static_method_call
    • mem::bench::trait_vtable_method_call
    • vec::bench::ends_with_diff_one_element_at_beginning
    • vec::bench::ends_with_single_element
    • vec::bench::push
    • vec::bench::starts_with_single_element

And I think everything except the collections::bitv ones are legitimately fast (and so only the bitv ones need fixing).

@vks
Copy link
Contributor

vks commented Aug 21, 2014

I think most of those are fixed by now. There are only a few left in bitv where nothing is returned.

I'll prepare a pull request.

vks added a commit to vks/rust that referenced this issue Aug 21, 2014
This makes sure that the benchmarked code does not get optimized away.
Also fixed a typo.

Fixes rust-lang#12118.
@vks vks mentioned this issue Aug 21, 2014
bors added a commit that referenced this issue Aug 22, 2014
Fixes #12118.

(I sneaked in an unrelated one-character whitespace fix I spotted while reviewing some benchmarks, if that is not okay, I can create a separate pull request for that.)
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
style: rename crates to kebab-case

Ref: rust-lang#12102
I updated all the folders names as well as the crates names in each `Cargo.toml` to use kebab-case.

This is my first ra PR. In case I missed something, I am ready/available to fix it until it is ready to merge.

Thank you.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
3 participants