Skip to content

Address testing guide nits #19888

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

Merged
merged 2 commits into from
Jan 6, 2015
Merged

Address testing guide nits #19888

merged 2 commits into from
Jan 6, 2015

Conversation

steveklabnik
Copy link
Member

Fixes #19861

/cc @huonw

@alexcrichton
Copy link
Member

r? @huonw

@@ -571,3 +573,9 @@ test bench_xor_1000_ints ... bench: 1 ns/iter (+/- 0)

Copy link
Member

Choose a reason for hiding this comment

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

This benchmark is still incorrect: the text implies that this new benchmark is then fixed (i.e. accurately benchmarking the time it takes to xor 1000 uints), but it's not. It looks like we may need a new benchmark, or stronger black-boxing:

let mut n = 1000;
// `n` is unchanged, butthe optimiser is forced to assume that `n` may be mutated
test::black_box(&mut n);

b.iter(|| {
    test::black_box(range(0u, n).fold(...));
})

(or the version that does the range.fold without a semicolon.)

I haven't tested that, but one should see numbers on the order of the 375 ns/iter that it was before (the exact values isn't too important and varies based on the specific computer, but it should certainly be hundreds of nanoseconds, not ones).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to finish this PR up, did you ever come up with a good replacement?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, whoops, forgot to write it down:

    b.iter(|| {
        let mut n = 1000_u32;
        test::black_box(&mut n); // pretend to modify `n`
        range(0, n).fold(0, |a, b| a ^ b)
    })

@steveklabnik steveklabnik force-pushed the gh19861 branch 2 times, most recently from c5fbc79 to cd85f0a Compare December 27, 2014 11:11
@steveklabnik
Copy link
Member Author

@huonw added the example. what do you think?

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 5, 2015
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 6, 2015
@bors bors merged commit cd85f0a into rust-lang:master Jan 6, 2015
@steveklabnik steveklabnik deleted the gh19861 branch October 25, 2017 18:27
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.

New testing guide nits
4 participants