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

Implement fold on hash::table::{Iter,IterMut}. #37362

Closed
wants to merge 1 commit into from

Conversation

cristicbz
Copy link
Contributor

Also forwards fold to the wrapped iterator for HashMap and HashSet iterators to actually gain something from the change.

There are more places where this could be applied (eg. Drain, IntoIter), but I wanted to first decide whether this is a good idea or not. This PR could also do with some additional tests. A simple benchmark which sums up 1000, 10000, 100000 elements in a HashSet sees the following gains:

Before:

fl•~» ~/e/master_compiler/stage2/bin/rustc --test -C opt-level=3 x.rs && ./x --bench

running 3 tests
test large_set  ... bench:     340,945 ns/iter (+/- 10,672)
test medium_set ... bench:      54,366 ns/iter (+/- 1,998)
test small_set  ... bench:       1,965 ns/iter (+/- 68)

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

After:

fl•~» ~/e/fold_all_compiler/stage2/bin/rustc --test -C opt-level=3 x.rs && ./x --bench

running 3 tests
test large_set  ... bench:     313,788 ns/iter (+/- 11,935)
test medium_set ... bench:      45,260 ns/iter (+/- 1,631)
test small_set  ... bench:       1,748 ns/iter (+/- 34)

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

Note that these gains only work if you explicitly call fold. Calling sum calls cloned which doesn't forward the fold call to the inner iterator (but will after #37315).

Benchmark code

extern crate test;

use test::Bencher;
use std::collections::HashSet;

fn small_set(b: &mut Bencher) {
    let set: HashSet<u32> = (1..1000).collect();
    b.iter(|| {
        test::black_box(set.iter().fold(0u32, |a, &b| a.wrapping_add(b)));
    });
}

fn medium_set(b: &mut Bencher) {
    let set: HashSet<u32> = (1..10000).collect();
    b.iter(|| {
        test::black_box(set.iter().fold(0u32, |a, &b| a.wrapping_add(b)));
    });
}

fn large_set(b: &mut Bencher) {
    let set: HashSet<u32> = (1..100000).collect();
    b.iter(|| {
        test::black_box(set.iter().fold(0u32, |a, &b| a.wrapping_add(b)));
    });
}

fn main() {}

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@cristicbz
Copy link
Contributor Author

Hmm, I was based on an old master, recompiling and running benchmarks now, I'll update.

Also forwards `fold` to the wrapped iterator for `HashMap` and `HashSet`
iterators to actually gain something from the change.
@cristicbz
Copy link
Contributor Author

After rebasing, I can't reproduce the wins, closing this PR

@cristicbz cristicbz closed this Oct 23, 2016
@bluss
Copy link
Member

bluss commented Oct 23, 2016

Thanks for exploring this

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.

4 participants