Skip to content

Customize <FlatMap as Iterator>::fold #44577

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Sep 17, 2017
Merged

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Sep 14, 2017

FlatMap can use internal iteration for its fold, which shows a
performance advantage in the new benchmarks:

test iter::bench_flat_map_chain_ref_sum ... bench:   4,354,111 ns/iter (+/- 108,871)
test iter::bench_flat_map_chain_sum     ... bench:     468,167 ns/iter (+/- 2,274)
test iter::bench_flat_map_ref_sum       ... bench:     449,616 ns/iter (+/- 6,257)
test iter::bench_flat_map_sum           ... bench:     348,010 ns/iter (+/- 1,227)

... where the "ref" benches are using by_ref() that isn't optimized.
So this change shows a decent advantage on its own, but much more when
combined with a chain iterator that also optimizes fold.

`FlatMap` can use internal iteration for its `fold`, which shows a
performance advantage in the new benchmarks:

    test iter::bench_flat_map_chain_ref_sum ... bench:   4,354,111 ns/iter (+/- 108,871)
    test iter::bench_flat_map_chain_sum     ... bench:     468,167 ns/iter (+/- 2,274)
    test iter::bench_flat_map_ref_sum       ... bench:     449,616 ns/iter (+/- 6,257)
    test iter::bench_flat_map_sum           ... bench:     348,010 ns/iter (+/- 1,227)

... where the "ref" benches are using `by_ref()` that isn't optimized.
So this change shows a decent advantage on its own, but much more when
combined with a `chain` iterator that also optimizes `fold`.
@alexcrichton alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 14, 2017
@alexcrichton
Copy link
Member

Thanks! Could you add some tests as well to ensure that the specialization here is correct? (in addition to the benchmarks)

@alexcrichton alexcrichton added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 14, 2017
@alexcrichton alexcrichton self-assigned this Sep 14, 2017
@cuviper
Copy link
Member Author

cuviper commented Sep 15, 2017

OK, I added a basic test -- did you want something more exhaustive than that?

@alexcrichton
Copy link
Member

@bors: r+

Nah looks great!

@bors
Copy link
Collaborator

bors commented Sep 15, 2017

📌 Commit 351f56a has been approved by alexcrichton

@alexcrichton
Copy link
Member

@bors: rollup

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 16, 2017
Customize `<FlatMap as Iterator>::fold`

`FlatMap` can use internal iteration for its `fold`, which shows a
performance advantage in the new benchmarks:

    test iter::bench_flat_map_chain_ref_sum ... bench:   4,354,111 ns/iter (+/- 108,871)
    test iter::bench_flat_map_chain_sum     ... bench:     468,167 ns/iter (+/- 2,274)
    test iter::bench_flat_map_ref_sum       ... bench:     449,616 ns/iter (+/- 6,257)
    test iter::bench_flat_map_sum           ... bench:     348,010 ns/iter (+/- 1,227)

... where the "ref" benches are using `by_ref()` that isn't optimized.
So this change shows a decent advantage on its own, but much more when
combined with a `chain` iterator that also optimizes `fold`.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 16, 2017
Customize `<FlatMap as Iterator>::fold`

`FlatMap` can use internal iteration for its `fold`, which shows a
performance advantage in the new benchmarks:

    test iter::bench_flat_map_chain_ref_sum ... bench:   4,354,111 ns/iter (+/- 108,871)
    test iter::bench_flat_map_chain_sum     ... bench:     468,167 ns/iter (+/- 2,274)
    test iter::bench_flat_map_ref_sum       ... bench:     449,616 ns/iter (+/- 6,257)
    test iter::bench_flat_map_sum           ... bench:     348,010 ns/iter (+/- 1,227)

... where the "ref" benches are using `by_ref()` that isn't optimized.
So this change shows a decent advantage on its own, but much more when
combined with a `chain` iterator that also optimizes `fold`.
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 17, 2017
Customize `<FlatMap as Iterator>::fold`

`FlatMap` can use internal iteration for its `fold`, which shows a
performance advantage in the new benchmarks:

    test iter::bench_flat_map_chain_ref_sum ... bench:   4,354,111 ns/iter (+/- 108,871)
    test iter::bench_flat_map_chain_sum     ... bench:     468,167 ns/iter (+/- 2,274)
    test iter::bench_flat_map_ref_sum       ... bench:     449,616 ns/iter (+/- 6,257)
    test iter::bench_flat_map_sum           ... bench:     348,010 ns/iter (+/- 1,227)

... where the "ref" benches are using `by_ref()` that isn't optimized.
So this change shows a decent advantage on its own, but much more when
combined with a `chain` iterator that also optimizes `fold`.
bors added a commit that referenced this pull request Sep 17, 2017
@bors bors merged commit 351f56a into rust-lang:master Sep 17, 2017
@cuviper cuviper deleted the flat_map-fold branch September 26, 2017 06:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants