Skip to content

Commit

Permalink
Merge pull request #198 from schuster/rename_split_off
Browse files Browse the repository at this point in the history
Rename split_off to split_off_left (as discussed in PR #189)
  • Loading branch information
nikomatsakis authored Jan 3, 2017
2 parents dc60e60 + dc0f576 commit 32f9cce
Show file tree
Hide file tree
Showing 17 changed files with 49 additions and 48 deletions.
8 changes: 4 additions & 4 deletions src/par_iter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions src/par_iter/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ impl<A, B> ParallelIterator for ChainIter<A, B>
fn drive_unindexed<C>(self, consumer: C) -> C::Result
where C: UnindexedConsumer<Self::Item>
{
let a = self.a.drive_unindexed(consumer.split_off());
let b = self.b.drive_unindexed(consumer.split_off());
consumer.to_reducer().reduce(a, b)
let reducer = consumer.to_reducer();
let a = self.a.drive_unindexed(consumer.split_off_left());
let b = self.b.drive_unindexed(consumer);
reducer.reduce(a, b)
}

fn opt_len(&mut self) -> Option<usize> {
Expand Down
2 changes: 1 addition & 1 deletion src/par_iter/collect/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'c, ITEM: Send + 'c> Folder<ITEM> 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<ITEM> 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 {
Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ impl<'f, ITEM, C, FILTER_OP: 'f> UnindexedConsumer<ITEM> for FilterConsumer<'f,
where C: UnindexedConsumer<ITEM>,
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 {
Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/filter_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ impl<'f, ITEM, MAPPED_ITEM, C, FILTER_OP> UnindexedConsumer<ITEM>
where C: UnindexedConsumer<MAPPED_ITEM>,
FILTER_OP: Fn(ITEM) -> Option<MAPPED_ITEM> + 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 {
Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<'f, ITEM, FIND_OP: 'f> Consumer<ITEM> 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 {
Expand All @@ -61,7 +61,7 @@ impl<'f, ITEM, FIND_OP: 'f> UnindexedConsumer<ITEM> 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)
}

Expand Down
8 changes: 2 additions & 6 deletions src/par_iter/find_first_last/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<'f, ITEM, FIND_OP> Consumer<ITEM> 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 })
}
Expand Down Expand Up @@ -127,7 +127,7 @@ impl<'f, ITEM, FIND_OP> UnindexedConsumer<ITEM> 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
Expand All @@ -139,10 +139,6 @@ impl<'f, ITEM, FIND_OP> UnindexedConsumer<ITEM> 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);
Expand Down
17 changes: 9 additions & 8 deletions src/par_iter/find_first_last/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions src/par_iter/flat_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ impl<'m, ITEM, MAPPED_ITEM, C, MAP_OP> Consumer<ITEM> 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())
}

Expand All @@ -96,8 +96,8 @@ impl<'m, ITEM, MAPPED_ITEM, C, MAP_OP> UnindexedConsumer<ITEM> 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 {
Expand All @@ -122,7 +122,7 @@ impl<'m, ITEM, MAPPED_ITEM, C, MAP_OP> Folder<ITEM> 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.
Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ impl<'r, U, ITEM, C, IDENTITY, FOLD_OP> UnindexedConsumer<ITEM>
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 {
Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/for_each.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<'f, OP, ITEM> Consumer<ITEM> 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 {
Expand All @@ -53,7 +53,7 @@ impl<'f, OP, ITEM> Folder<ITEM> for ForEachConsumer<'f, OP>
impl<'f, OP, ITEM> UnindexedConsumer<ITEM> for ForEachConsumer<'f, OP>
where OP: Fn(ITEM) + Sync
{
fn split_off(&self) -> Self {
fn split_off_left(&self) -> Self {
ForEachConsumer { op: self.op }
}

Expand Down
7 changes: 5 additions & 2 deletions src/par_iter/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ pub trait Reducer<Result> {

/// A stateless consumer can be freely copied.
pub trait UnindexedConsumer<ITEM>: Consumer<ITEM> {
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;
}

Expand Down Expand Up @@ -270,7 +273,7 @@ fn bridge_unindexed_producer_consumer<P, C>(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));
Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ impl<'m, ITEM, C, MAP_OP> UnindexedConsumer<ITEM> for MapConsumer<'m, C, MAP_OP>
where C: UnindexedConsumer<MAP_OP::Output>,
MAP_OP: MapOp<ITEM>
{
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 {
Expand Down
6 changes: 3 additions & 3 deletions src/par_iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/par_iter/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl<ITEM> Folder<ITEM> for NoopConsumer {
}

impl<ITEM> UnindexedConsumer<ITEM> for NoopConsumer {
fn split_off(&self) -> Self {
fn split_off_left(&self) -> Self {
NoopConsumer
}

Expand Down
2 changes: 1 addition & 1 deletion src/par_iter/reduce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl<'r, REDUCE_OP, ITEM> UnindexedConsumer<ITEM> for ReduceConsumer<'r, REDUCE_
where REDUCE_OP: ReduceOp<ITEM>,
ITEM: Send
{
fn split_off(&self) -> Self {
fn split_off_left(&self) -> Self {
ReduceConsumer { reduce_op: self.reduce_op }
}

Expand Down
4 changes: 2 additions & 2 deletions src/par_iter/weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ impl<C, ITEM> Consumer<ITEM> for WeightConsumer<C>
impl<C, ITEM> UnindexedConsumer<ITEM> for WeightConsumer<C>
where C: UnindexedConsumer<ITEM>
{
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 {
Expand Down

0 comments on commit 32f9cce

Please sign in to comment.