Optimize iterator adapters. #25035
Optimize iterator adapters. #25035
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @aturon |
|
Out of curiousity, do you have any numbers/examples of perf improvements from this change? |
|
Ah I see. This is just part of a bigger "do it because we can" issue. In the past we pushed back on this kind of thing (in the hopes of some kind of legit specialization based on things like ExactSizeIterator). Didn't realize we'd changed direction on that. |
| } else if n == 0 { | ||
| self.next() | ||
| } else { | ||
| self.next(); |
There was a problem hiding this comment.
Would this be more clear as:
let old_n = self.n;
self.n = 0;
self.iter.nth(old_n + n)There was a problem hiding this comment.
(Which would also allow merging with the previous branch)
There was a problem hiding this comment.
Good point (although I'm going to do two nth calls to avoid the overflow issue).
There was a problem hiding this comment.
So I got rid of that branch but had to add another (I forgot to check if next returned None).
|
This is neat stuff! I left a few comments/questions, but the biggest thing I'd like to see is an expansion of the unit tests in |
|
No. I don't really know the right way to benchmark this. Because these methods are asymptotically better, I can write benchmarks that give infinite speedups... |
|
@Stebalien Ah good point. I suppose I was wondering if this allowed certain adaptors to be inline'd and optimized to "the right thing" that previously weren't. But yeah should be a uniform improvement. |
|
Addressed comments. TODO: Test. |
|
I realized I missed the |
|
Added test cases. |
|
@aturon, Would you like me to squash? |
|
@Stebalien Tests look great -- go ahead and squash, and then I'll send to bors. Thanks again for your work on this! |
Specifically, make count, nth, and last call the corresponding methods on the underlying iterator where possible. This way, if the underlying iterator has an optimized count, nth, or last implementations (e.g. slice::Iter), these methods will propagate these optimizations. Additionally, change Skip::next to take advantage of a potentially optimized nth method on the underlying iterator.
|
Done (I think). |
|
@bors: r+ |
|
📌 Commit 3fcbc31 has been approved by |
|
⌛ Testing commit 3fcbc31 with merge 6afa669... |
Specifically, make count, nth, and last call the corresponding methods on the underlying iterator where possible. This way, if the underlying iterator has an optimized count, nth, or last implementations (e.g. slice::Iter), these methods will propagate these optimizations. Additionally, change Skip::next to take advantage of a potentially optimized nth method on the underlying iterator. This covers: * core::iter::Chain: count, last, nth * core::iter::Enumerate: count, nth * core::iter::Peekable: count, last, nth * core::iter::Skip: count, last, next (should call nth), nth * core::iter::Take: nth * core::iter::Fuse: count, last, nth of #24214.
Specifically, make count, nth, and last call the corresponding methods on the underlying iterator where possible. This way, if the underlying iterator has an optimized count, nth, or last implementations (e.g. slice::Iter), these methods will propagate these optimizations.
Additionally, change Skip::next to take advantage of a potentially optimized nth method on the underlying iterator.
This covers:
of #24214.