Skip to content

Add exhaust which simply exhausts the iterator #131

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

Closed
wants to merge 2 commits into from

Conversation

tbu-
Copy link

@tbu- tbu- commented Jun 20, 2016

This is useful for iterators which are simply used for their side
effects.

The new function is added to make intent clearer for places where you'd
previously call count or similar to exhaust the iterator.

@phimuemue
Copy link
Member

Isn't it possible to override the behaviour of count, so that it does not necessarily repeatedly call next until the end? (E.g. done in https://github.com/rust-lang/rust/blob/7ac11cad3fe85163dd8b0ca1f63af492509f9bfe/src/libcollections/vec.rs)

I think it would be safer to explicitly call next until the iterator is exhausted.

@tbu-
Copy link
Author

tbu- commented Aug 22, 2016

That's only allowed if your iterator doesn't have side effects. E.g. the count implementation of the result of .map on any iterator still traverses the whole iterator. If you think that this should be different, you could ask the standard library to allow iterators to include such optimizations.

@bluss
Copy link
Member

bluss commented Aug 22, 2016

Changing map's count that way has been attempted once but was declined because of it breaking code in the wild.

@bluss
Copy link
Member

bluss commented Aug 22, 2016

@tbu- I am neutral on this PR, I'm not so convinced of the name. An older version of itertools had exactly this with the name "drain", but that name had to go when libstd used it for something different, so the method was removed. I sort of think .count() is good enough.

@tbu-
Copy link
Author

tbu- commented Aug 22, 2016

I'd just like a method that makes my intent clear if I want to traverse the whole iterator doing nothing but observing the side-effects.

@bluss
Copy link
Member

bluss commented Sep 11, 2016

I'd like to merge this but change it to a for loop, so that it really runs through the iterator without the possibility of a buggy adaptor omitting side effects. What do you think?

@tbu-
Copy link
Author

tbu- commented Sep 11, 2016

Then it might not benefit from optimizations of the count implementation.

@bluss
Copy link
Member

bluss commented Sep 11, 2016

For the slice iterator, it should optimize out anyway I think. And when you use exhaust, you have some side effect that you want to carry out, so there's not much use for the optimizations?

@tbu-
Copy link
Author

tbu- commented Sep 11, 2016

I'm not a fan of defensive programming, because it might hide bugs. I can change it to just iterate if you want.

@bluss
Copy link
Member

bluss commented Sep 11, 2016

Interesting viewpoint. I'm not a super fan of iterator-specific implementations of methods like count, I'd prefer if they just all had the same implementation.

These examples optimize out when exhaust is a for loop. Microbenchmarks, so it's a simplified view of course.

#[bench]                                                                         
fn exhaust_slice(b: &mut Bencher) {                                              
    let v = black_box(vec![0; 1024]);                                            
    b.iter(|| {                                                                  
        v.iter().exhaust()                                                       
    });                                                                          
}                                                                                

#[bench]                                                                         
fn exhaust_range(b: &mut Bencher) {                                              
    let r = black_box(0..1024);                                                  
    b.iter(|| {                                                                  
        r.clone().exhaust()                                                      
    });                                                                          
}                                                                                

#[bench]                                                                         
fn exhaust_map(b: &mut Bencher) {                                                
    let r = black_box(0..1024);                                                  
    b.iter(|| {                                                                  
        r.clone().map(|x| x * x * x).exhaust()                                   
    });                                                                          
}   
test exhaust_map                              ... bench:           1 ns/iter (+/- 0)
test exhaust_range                            ... bench:           1 ns/iter (+/- 0)
test exhaust_slice                            ... bench:           1 ns/iter (+/- 0)

@bluss
Copy link
Member

bluss commented Sep 13, 2016

I'd support using a for loop. If you think that's a bad idea, maybe we should wait.

tbu- added 2 commits January 27, 2017 21:49
This is useful for iterators which are simply used for their side
effects.

The new function is added to make intent clearer for places where you'd
previously call `count` or similar to exhaust the iterator.
@tbu-
Copy link
Author

tbu- commented Jan 27, 2017

@bluss Updated (at last!). Still interested?

@bluss
Copy link
Member

bluss commented Jan 29, 2017

Well I have definitely made a turn around when it comes to special casing of iterator methods (Which should be evident from my work on special cases of .fold() and so on). Exhaust should not use count since it does not want to be tied to the limits of the usize type, but it can otherwise use whatever iterator method works best.

I think this method would be much more meaningful in libcore than in our library here, to really clean up the iterator style. Does including it here gain or lose its chances to be inlucded in libcore? I don't know. Adding the best parts of itertools to core is long overdue.

@tbu-
Copy link
Author

tbu- commented Jan 30, 2017

OK, I'll make a pull request to Rust core. Or should I write an RFC first?

@bluss
Copy link
Member

bluss commented Jan 30, 2017

Maybe try it out in the internals forum first. Methods that really add value (new powers) are easier to argue for though.

@gnzlbg
Copy link

gnzlbg commented Jan 30, 2017

I just wanted something like this today, a way to execute an iterator chain for side-effects, that is guaranteed to not perform any tricks (e.g. count or last would not need to traverse an ExactSizedIterator, even if they do and that's unlikely to change I'd still not rely on it and have a more explicit method).

@tbu-
Copy link
Author

tbu- commented Jan 30, 2017

count needs to do that apparantly, check the Map iterator in the standard library. Otherwise, it could just call count on the underlying iterator.

@bluss
Copy link
Member

bluss commented Jan 6, 2018

closing since we didn't get anywhere with this. It looks like there is an opening in libstd, so we should go for it there instead. See the above linked issue in rust-lang/rust.

@bluss bluss closed this Jan 6, 2018
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