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

Add benchmarks for push, insert, extend, and pushpop #31

Merged
merged 2 commits into from Sep 11, 2016

Conversation

@nipunn1313
Copy link
Contributor

nipunn1313 commented Aug 27, 2016

Want to add these benchmarks so that we can more effectively detect regressions/improvements from PR's like #28 and #29.


This change is Reviewable

lib.rs Outdated
@@ -5,6 +5,8 @@
//! Small vectors in various sizes. These store a certain number of elements inline and fall back
//! to the heap for larger allocations.

#![feature(test)]

This comment has been minimized.

Copy link
@emilio

emilio Aug 28, 2016

Member

This won't compile in stable, will it?

This comment has been minimized.

Copy link
@nipunn1313

nipunn1313 Aug 28, 2016

Author Contributor

Correct. Let me work on factoring it into a conditionally compiled feature which is disabled by default.

fn bench_push(b: &mut Bencher) {
b.iter(|| {
let mut vec: SmallVec<[u64; 16]> = SmallVec::new();
for x in 0..100 {

This comment has been minimized.

Copy link
@emilio

emilio Aug 28, 2016

Member

Also, I think the compiler will be pretty much able to optimize these loops out. IIRC there was some builtin support for black box testing for this kind of things. I think we should be able to enforce that using it, or at least confirm it's not being optimized out.

This comment has been minimized.

Copy link
@nipunn1313

nipunn1313 Aug 28, 2016

Author Contributor

I believe if the closure returns a value then the optimizer will not remove it.

I was empirically able to determine that the optimizer was not removing the code either way (for now), but I will include the vec return value to ensure it.

@nipunn1313 nipunn1313 force-pushed the nipunn1313:bench branch from 232f805 to ff9e2f0 Aug 28, 2016
@nipunn1313 nipunn1313 force-pushed the nipunn1313:bench branch from ff9e2f0 to 951f1d0 Aug 28, 2016
@nipunn1313
Copy link
Contributor Author

nipunn1313 commented Aug 28, 2016

Looks like this on my machine (Mac book pro laptop)

Nipunns-MacBook-Pro:rust-smallvec nipunn$ cargo bench --verbose --features=benchmarks bench
       Fresh smallvec v0.2.0 (file:///Users/nipunn/src/rust-smallvec)
    Finished release [optimized] target(s) in 0.0 secs
     Running `/Users/nipunn/src/rust-smallvec/target/release/deps/smallvec-a59689f47ae774cb bench --bench`

running 4 tests
test bench::bench_extend  ... bench:         169 ns/iter (+/- 19)
test bench::bench_insert  ... bench:       1,201 ns/iter (+/- 267)
test bench::bench_push    ... bench:         295 ns/iter (+/- 39)
test bench::bench_pushpop ... bench:         357 ns/iter (+/- 117)

test result: ok. 0 passed; 0 failed; 0 ignored; 4 measured

Travis CI looked like this. I don't think we can really rely on its numbers though.

running 4 tests
test bench::bench_extend  ... bench:         182 ns/iter (+/- 3)
test bench::bench_insert  ... bench:       1,916 ns/iter (+/- 21)
test bench::bench_push    ... bench:         307 ns/iter (+/- 6)
test bench::bench_pushpop ... bench:         383 ns/iter (+/- 6)```
@nipunn1313
Copy link
Contributor Author

nipunn1313 commented Sep 11, 2016

Any update on this?

@jdm
Copy link
Member

jdm commented Sep 11, 2016

@bors-servo: r+
Thanks for doing this work!

@bors-servo
Copy link
Contributor

bors-servo commented Sep 11, 2016

📌 Commit 951f1d0 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Sep 11, 2016

Test exempted - status

@bors-servo bors-servo merged commit 951f1d0 into servo:master Sep 11, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
bors-servo added a commit that referenced this pull request Sep 11, 2016
Add benchmarks for push, insert, extend, and pushpop

Want to add these benchmarks so that we can more effectively detect regressions/improvements from PR's like #28 and #29.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/31)
<!-- Reviewable:end -->
@nipunn1313 nipunn1313 deleted the nipunn1313:bench branch Sep 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.