implement TrustedLen for Flatten/FlatMap if the U: IntoIterator == [T; N] #87168
Conversation
…; N] This only works if arrays are passed directly instead of array iterators because we need to be sure that they have not been advanced before Flatten does its size calculation.
|
r? @scottmcm (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The specialization now covers flattening over references to arrays in addition to owned arrays. |
Due to rust-lang#20400 the corresponding TrustedLen impls need a helper trait instead of directly adding `Item = &[T;N]` bounds. Since TrustedLen is a public trait this in turn means the helper trait needs to be public. Since it's just a workaround for a compiler deficit it's marked hidden, unstable and unsafe.
|
Thanks for this! I tried to do something similar back in #48544, but doing this with specialization avoids the complaints people had about that. |
| #[unstable(feature = "std_internals", issue = "none")] | ||
| // FIXME(#20400): Instead of this helper trait there should be multiple impl TrustedLen for Flatten<> | ||
| // blocks with different bounds on Iterator::Item but the compiler erroneously considers them overlapping | ||
| pub unsafe trait TrustedConstSize: IntoIterator {} |
There was a problem hiding this comment.
nit: I would have expected something like TrustedConstSizeIntoIterator : ConstSizeIntoIterator? Is there some reason this mentions only IntoIterator?
There was a problem hiding this comment.
ConstSizeIntoIterator is implemented for all IntoIterator anyway but returns a None for the non-const ones. So adding that bound wouldn't make a difference.
scottmcm
added
S-waiting-on-author
the8472
added
S-waiting-on-review
|
Thanks! Let me just check perf just in case: @bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit c3ac8d8 with merge e54a7e2fa88fac0e640991c5eae629c26bb69bf1... |
|
☀️ Try build successful - checks-actions |
|
Queued e54a7e2fa88fac0e640991c5eae629c26bb69bf1 with parent 6535449, future comparison URL. |
|
Finished benchmarking try commit (e54a7e2fa88fac0e640991c5eae629c26bb69bf1): comparison url. Summary: This benchmark run did not return any significant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. @bors rollup=never |
rustbot
removed
the
S-waiting-on-perf
|
@bors r+ rollup=iffy |
|
📌 Commit c3ac8d8 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
implement fold() on array::IntoIter to improve flatten().collect() perf With rust-lang#87168 flattening `array::IntoIter`s is now `TrustedLen`, the `FromIterator` implementation for `Vec` has a specialization for `TrustedLen` iterators which uses internal iteration. This implements one of the main internal iteration methods on `array::Into` to optimize the combination of those two features. This should address the main issue in rust-lang#87411 ``` # old test vec::bench_flat_map_collect ... bench: 2,244,024 ns/iter (+/- 18,903) # new test vec::bench_flat_map_collect ... bench: 172,863 ns/iter (+/- 2,141) ```
This only works if arrays are passed directly instead of array iterators
because we need to be sure that they have not been advanced before
Flatten does its size calculation.
resolves #87094