From 54166df6ffe3783386e1c78e9ea92a35b820d820 Mon Sep 17 00:00:00 2001 From: Jonathan Schuster Date: Sun, 1 Jan 2017 20:19:53 -0500 Subject: [PATCH 1/3] Rename split_off to split_off_left (as discussed in PR #189) --- src/par_iter/chain.rs | 4 ++-- src/par_iter/collect/consumer.rs | 2 +- src/par_iter/filter.rs | 4 ++-- src/par_iter/filter_map.rs | 4 ++-- src/par_iter/find.rs | 4 ++-- src/par_iter/find_first_last/mod.rs | 8 ++------ src/par_iter/find_first_last/test.rs | 17 +++++++++-------- src/par_iter/flat_map.rs | 10 +++++----- src/par_iter/fold.rs | 4 ++-- src/par_iter/for_each.rs | 4 ++-- src/par_iter/internal.rs | 7 +++++-- src/par_iter/map.rs | 4 ++-- src/par_iter/mod.rs | 6 +++--- src/par_iter/noop.rs | 2 +- src/par_iter/reduce.rs | 2 +- src/par_iter/weight.rs | 4 ++-- 16 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/par_iter/chain.rs b/src/par_iter/chain.rs index 34b33778c..f7fd24f30 100644 --- a/src/par_iter/chain.rs +++ b/src/par_iter/chain.rs @@ -29,8 +29,8 @@ impl ParallelIterator for ChainIter fn drive_unindexed(self, consumer: C) -> C::Result where C: UnindexedConsumer { - let a = self.a.drive_unindexed(consumer.split_off()); - let b = self.b.drive_unindexed(consumer.split_off()); + let a = self.a.drive_unindexed(consumer.split_off_left()); + let b = self.b.drive_unindexed(consumer.split_off_left()); consumer.to_reducer().reduce(a, b) } diff --git a/src/par_iter/collect/consumer.rs b/src/par_iter/collect/consumer.rs index 37e21d0f0..aa1b84779 100644 --- a/src/par_iter/collect/consumer.rs +++ b/src/par_iter/collect/consumer.rs @@ -90,7 +90,7 @@ impl<'c, ITEM: Send + 'c> Folder for CollectFolder<'c, ITEM> { /// Pretend to be unindexed for `special_collect_into`, /// but we should never actually get used that way... impl<'c, ITEM: Send + 'c> UnindexedConsumer for CollectConsumer<'c, ITEM> { - fn split_off(&self) -> Self { + fn split_off_left(&self) -> Self { unreachable!("CollectConsumer must be indexed!") } fn to_reducer(&self) -> Self::Reducer { diff --git a/src/par_iter/filter.rs b/src/par_iter/filter.rs index cc5458a18..396337254 100644 --- a/src/par_iter/filter.rs +++ b/src/par_iter/filter.rs @@ -104,8 +104,8 @@ impl<'f, ITEM, C, FILTER_OP: 'f> UnindexedConsumer for FilterConsumer<'f, where C: UnindexedConsumer, FILTER_OP: Fn(&ITEM) -> bool + Sync { - fn split_off(&self) -> Self { - FilterConsumer::new(self.base.split_off(), &self.filter_op) + fn split_off_left(&self) -> Self { + FilterConsumer::new(self.base.split_off_left(), &self.filter_op) } fn to_reducer(&self) -> Self::Reducer { diff --git a/src/par_iter/filter_map.rs b/src/par_iter/filter_map.rs index be4dd8e2e..af9549b54 100644 --- a/src/par_iter/filter_map.rs +++ b/src/par_iter/filter_map.rs @@ -107,8 +107,8 @@ impl<'f, ITEM, MAPPED_ITEM, C, FILTER_OP> UnindexedConsumer where C: UnindexedConsumer, FILTER_OP: Fn(ITEM) -> Option + Sync + 'f { - fn split_off(&self) -> Self { - FilterMapConsumer::new(self.base.split_off(), &self.filter_op) + fn split_off_left(&self) -> Self { + FilterMapConsumer::new(self.base.split_off_left(), &self.filter_op) } fn to_reducer(&self) -> Self::Reducer { diff --git a/src/par_iter/find.rs b/src/par_iter/find.rs index c43180f6e..85c082aa0 100644 --- a/src/par_iter/find.rs +++ b/src/par_iter/find.rs @@ -40,7 +40,7 @@ impl<'f, ITEM, FIND_OP: 'f> Consumer for FindConsumer<'f, FIND_OP> } fn split_at(self, _index: usize) -> (Self, Self, Self::Reducer) { - (self.split_off(), self, FindReducer) + (self.split_off_left(), self, FindReducer) } fn into_folder(self) -> Self::Folder { @@ -61,7 +61,7 @@ impl<'f, ITEM, FIND_OP: 'f> UnindexedConsumer for FindConsumer<'f, FIND_OP where ITEM: Send, FIND_OP: Fn(&ITEM) -> bool + Sync { - fn split_off(&self) -> Self { + fn split_off_left(&self) -> Self { FindConsumer::new(self.find_op, self.found) } diff --git a/src/par_iter/find_first_last/mod.rs b/src/par_iter/find_first_last/mod.rs index c2c721cb7..6e0cf902f 100644 --- a/src/par_iter/find_first_last/mod.rs +++ b/src/par_iter/find_first_last/mod.rs @@ -99,7 +99,7 @@ impl<'f, ITEM, FIND_OP> Consumer for FindConsumer<'f, FIND_OP> fn split_at(self, _index: usize) -> (Self, Self, Self::Reducer) { let dir = self.match_position; - (self.split_off(), + (self.split_off_left(), self, FindReducer { match_position: dir }) } @@ -127,7 +127,7 @@ impl<'f, ITEM, FIND_OP> UnindexedConsumer for FindConsumer<'f, FIND_OP> where ITEM: Send, FIND_OP: Fn(&ITEM) -> bool + Sync { - fn split_off(&self) -> Self { + fn split_off_left(&self) -> Self { // Upper bound for one consumer will be lower bound for the other. This // overlap is okay, because only one of the bounds will be used for // comparing against best_found; the other is kept only to be able to @@ -139,10 +139,6 @@ impl<'f, ITEM, FIND_OP> UnindexedConsumer for FindConsumer<'f, FIND_OP> // consumer to stop working when the other finds a better match, but the // reducer ensures that the best answer is still returned (see the test // above). - // - // This code assumes that the caller of split_off will use the result as - // the *left* side of this iterator, and the remainder of self as the - // *right* side. let old_lower_bound = self.lower_bound.get(); let median = old_lower_bound + ((self.upper_bound - old_lower_bound) / 2); self.lower_bound.set(median); diff --git a/src/par_iter/find_first_last/test.rs b/src/par_iter/find_first_last/test.rs index 08e3989ae..c284001c8 100644 --- a/src/par_iter/find_first_last/test.rs +++ b/src/par_iter/find_first_last/test.rs @@ -9,18 +9,19 @@ fn same_range_first_consumers_return_correct_answer() { // We save a consumer that will be far to the right of the main consumer (and therefore not // sharing an index range with that consumer) for fullness testing - let consumer = far_right_consumer.split_off(); + let consumer = far_right_consumer.split_off_left(); // split until we have an indivisible range let bits_in_usize = usize::min_value().count_zeros(); + for _ in 0..bits_in_usize { - consumer.split_off(); + consumer.split_off_left(); } let reducer = consumer.to_reducer(); // the left and right folders should now have the same range, having // exhausted the resolution of usize - let left_folder = consumer.split_off().into_folder(); + let left_folder = consumer.split_off_left().into_folder(); let right_folder = consumer.into_folder(); let left_folder = left_folder.consume(0).consume(1); @@ -42,20 +43,20 @@ fn same_range_last_consumers_return_correct_answer() { // We save a consumer that will be far to the left of the main consumer (and therefore not // sharing an index range with that consumer) for fullness testing - let far_left_consumer = consumer.split_off(); + let far_left_consumer = consumer.split_off_left(); // split until we have an indivisible range let bits_in_usize = usize::min_value().count_zeros(); for _ in 0..bits_in_usize { - consumer.split_off(); + consumer.split_off_left(); } let reducer = consumer.to_reducer(); - // due to the exact calculation in split_off, the very last consumer has a + // due to the exact calculation in split_off_left, the very last consumer has a // range of width 2, so we use the second-to-last consumer instead to get // the same boundary on both folders - let consumer = consumer.split_off(); - let left_folder = consumer.split_off().into_folder(); + let consumer = consumer.split_off_left(); + let left_folder = consumer.split_off_left().into_folder(); let right_folder = consumer.into_folder(); let right_folder = right_folder.consume(2).consume(3); assert_eq!(left_folder.boundary, right_folder.boundary); diff --git a/src/par_iter/flat_map.rs b/src/par_iter/flat_map.rs index e2b825108..cc7ca5eb7 100644 --- a/src/par_iter/flat_map.rs +++ b/src/par_iter/flat_map.rs @@ -73,8 +73,8 @@ impl<'m, ITEM, MAPPED_ITEM, C, MAP_OP> Consumer for FlatMapConsumer<'m, C, } fn split_at(self, _index: usize) -> (Self, Self, C::Reducer) { - (FlatMapConsumer::new(self.base.split_off(), self.map_op), - FlatMapConsumer::new(self.base.split_off(), self.map_op), + (FlatMapConsumer::new(self.base.split_off_left(), self.map_op), + FlatMapConsumer::new(self.base.split_off_left(), self.map_op), self.base.to_reducer()) } @@ -96,8 +96,8 @@ impl<'m, ITEM, MAPPED_ITEM, C, MAP_OP> UnindexedConsumer for FlatMapConsum MAP_OP: Fn(ITEM) -> MAPPED_ITEM + Sync, MAPPED_ITEM: IntoParallelIterator { - fn split_off(&self) -> Self { - FlatMapConsumer::new(self.base.split_off(), self.map_op) + fn split_off_left(&self) -> Self { + FlatMapConsumer::new(self.base.split_off_left(), self.map_op) } fn to_reducer(&self) -> Self::Reducer { @@ -122,7 +122,7 @@ impl<'m, ITEM, MAPPED_ITEM, C, MAP_OP> Folder for FlatMapFolder<'m, C, MAP fn consume(self, item: ITEM) -> Self { let map_op = self.map_op; let par_iter = map_op(item).into_par_iter(); - let result = par_iter.drive_unindexed(self.base.split_off()); + let result = par_iter.drive_unindexed(self.base.split_off_left()); // We expect that `previous` is `None`, because we drive // the cost up so high, but just in case. diff --git a/src/par_iter/fold.rs b/src/par_iter/fold.rs index ba9d3c9b0..a9bd472f3 100644 --- a/src/par_iter/fold.rs +++ b/src/par_iter/fold.rs @@ -106,8 +106,8 @@ impl<'r, U, ITEM, C, IDENTITY, FOLD_OP> UnindexedConsumer IDENTITY: Fn() -> U + Sync, U: Send { - fn split_off(&self) -> Self { - FoldConsumer { base: self.base.split_off(), ..*self } + fn split_off_left(&self) -> Self { + FoldConsumer { base: self.base.split_off_left(), ..*self } } fn to_reducer(&self) -> Self::Reducer { diff --git a/src/par_iter/for_each.rs b/src/par_iter/for_each.rs index 00321ce66..0eba94f80 100644 --- a/src/par_iter/for_each.rs +++ b/src/par_iter/for_each.rs @@ -29,7 +29,7 @@ impl<'f, OP, ITEM> Consumer for ForEachConsumer<'f, OP> } fn split_at(self, _index: usize) -> (Self, Self, NoopReducer) { - (self.split_off(), self.split_off(), NoopReducer) + (self.split_off_left(), self.split_off_left(), NoopReducer) } fn into_folder(self) -> Self { @@ -53,7 +53,7 @@ impl<'f, OP, ITEM> Folder for ForEachConsumer<'f, OP> impl<'f, OP, ITEM> UnindexedConsumer for ForEachConsumer<'f, OP> where OP: Fn(ITEM) + Sync { - fn split_off(&self) -> Self { + fn split_off_left(&self) -> Self { ForEachConsumer { op: self.op } } diff --git a/src/par_iter/internal.rs b/src/par_iter/internal.rs index 60391f8c2..5500efab6 100644 --- a/src/par_iter/internal.rs +++ b/src/par_iter/internal.rs @@ -102,7 +102,10 @@ pub trait Reducer { /// A stateless consumer can be freely copied. pub trait UnindexedConsumer: Consumer { - fn split_off(&self) -> Self; + // The result of split_off_left should be used for the left side of the + // data it consumes, and the remaining consumer for the right side + // (this matters for methods like find_first). + fn split_off_left(&self) -> Self; fn to_reducer(&self) -> Self::Reducer; } @@ -270,7 +273,7 @@ fn bridge_unindexed_producer_consumer(mut splitter: Splitter, consumer.into_folder().complete() } else if let Some(right_producer) = splitter.try_unindexed(&mut producer) { let (reducer, left_consumer, right_consumer) = - (consumer.to_reducer(), consumer.split_off(), consumer); + (consumer.to_reducer(), consumer.split_off_left(), consumer); let (left_result, right_result) = join(|| bridge_unindexed_producer_consumer(splitter, producer, left_consumer), || bridge_unindexed_producer_consumer(splitter, right_producer, right_consumer)); diff --git a/src/par_iter/map.rs b/src/par_iter/map.rs index 19cbf8a25..19111c1ec 100644 --- a/src/par_iter/map.rs +++ b/src/par_iter/map.rs @@ -256,8 +256,8 @@ impl<'m, ITEM, C, MAP_OP> UnindexedConsumer for MapConsumer<'m, C, MAP_OP> where C: UnindexedConsumer, MAP_OP: MapOp { - fn split_off(&self) -> Self { - MapConsumer::new(self.base.split_off(), &self.map_op) + fn split_off_left(&self) -> Self { + MapConsumer::new(self.base.split_off_left(), &self.map_op) } fn to_reducer(&self) -> Self::Reducer { diff --git a/src/par_iter/mod.rs b/src/par_iter/mod.rs index 135573f81..6189d5d17 100644 --- a/src/par_iter/mod.rs +++ b/src/par_iter/mod.rs @@ -629,9 +629,9 @@ pub trait ParallelIterator: Sized { /// statically. This can be used by consumers to trigger special fast /// paths. Therefore, if `Some(_)` is returned, this iterator must only /// use the (indexed) `Consumer` methods when driving a consumer, such - /// as `split_at()`. Calling `UnindexedConsumer::split_off()` or other - /// `UnindexedConsumer` methods -- or returning an inaccurate value -- - /// may result in panics. + /// as `split_at()`. Calling `UnindexedConsumer::split_off_left()` or + /// other `UnindexedConsumer` methods -- or returning an inaccurate + /// value -- may result in panics. /// /// This is hidden & considered internal for now, until we decide /// whether it makes sense for a public API. Right now it is only used diff --git a/src/par_iter/noop.rs b/src/par_iter/noop.rs index e4a12b018..6683ce903 100644 --- a/src/par_iter/noop.rs +++ b/src/par_iter/noop.rs @@ -37,7 +37,7 @@ impl Folder for NoopConsumer { } impl UnindexedConsumer for NoopConsumer { - fn split_off(&self) -> Self { + fn split_off_left(&self) -> Self { NoopConsumer } diff --git a/src/par_iter/reduce.rs b/src/par_iter/reduce.rs index 567cbb877..3f59a3791 100644 --- a/src/par_iter/reduce.rs +++ b/src/par_iter/reduce.rs @@ -81,7 +81,7 @@ impl<'r, REDUCE_OP, ITEM> UnindexedConsumer for ReduceConsumer<'r, REDUCE_ where REDUCE_OP: ReduceOp, ITEM: Send { - fn split_off(&self) -> Self { + fn split_off_left(&self) -> Self { ReduceConsumer { reduce_op: self.reduce_op } } diff --git a/src/par_iter/weight.rs b/src/par_iter/weight.rs index 3751d19af..053d9f274 100644 --- a/src/par_iter/weight.rs +++ b/src/par_iter/weight.rs @@ -169,8 +169,8 @@ impl Consumer for WeightConsumer impl UnindexedConsumer for WeightConsumer where C: UnindexedConsumer { - fn split_off(&self) -> Self { - WeightConsumer::new(self.base.split_off(), self.weight) + fn split_off_left(&self) -> Self { + WeightConsumer::new(self.base.split_off_left(), self.weight) } fn to_reducer(&self) -> Self::Reducer { From 7febef0c5479b3d779620a6474c94cc432fa5ce7 Mon Sep 17 00:00:00 2001 From: Jonathan Schuster Date: Sun, 1 Jan 2017 20:43:05 -0500 Subject: [PATCH 2/3] Rename split_off in README.md, too --- src/par_iter/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/par_iter/README.md b/src/par_iter/README.md index 4c361eec6..54d60f7cb 100644 --- a/src/par_iter/README.md +++ b/src/par_iter/README.md @@ -41,10 +41,10 @@ modes (which is why there are two): accepts an index where the split should be performed. All iterators can work in this mode. The resulting halves thus have an idea about how much data they expect to consume. - - in the `UnindexedConsumer` trait, splitting is done with `split`. - There is no index: the resulting halves must be prepared to - process any amount of data, and they don't know where that data - falls in the overall stream. + - in the `UnindexedConsumer` trait, splitting is done with + `split_off_left`. There is no index: the resulting halves must be + prepared to process any amount of data, and they don't know where that + data falls in the overall stream. - Not all consumers can operate in this mode. It works for `for_each` and `reduce`, for example, but it does not work for `collect_into`, since in that case the position of each item is From dc0f576ed634cdfda82bcc8da3c317d3eadbbea2 Mon Sep 17 00:00:00 2001 From: Jonathan Schuster Date: Mon, 2 Jan 2017 17:44:58 -0500 Subject: [PATCH 3/3] Do one less split_off_left in chain's unindexed driver --- src/par_iter/chain.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/par_iter/chain.rs b/src/par_iter/chain.rs index f7fd24f30..856ced1b8 100644 --- a/src/par_iter/chain.rs +++ b/src/par_iter/chain.rs @@ -29,9 +29,10 @@ impl ParallelIterator for ChainIter fn drive_unindexed(self, consumer: C) -> C::Result where C: UnindexedConsumer { + let reducer = consumer.to_reducer(); let a = self.a.drive_unindexed(consumer.split_off_left()); - let b = self.b.drive_unindexed(consumer.split_off_left()); - consumer.to_reducer().reduce(a, b) + let b = self.b.drive_unindexed(consumer); + reducer.reduce(a, b) } fn opt_len(&mut self) -> Option {