Skip to content

Commit

Permalink
Fix for view isEmpty.
Browse files Browse the repository at this point in the history
Views have been inheriting the very inefficient isEmpty
from Traversable, and since isEmpty is not specifically
forwarded to the underlying collections, views miss out on all
the important optimizations in those collections which tend to
be implemented via method override.  Not to mention, they miss
out on correctness, because calling foreach has a habit of
forcing the first element of the view.
  • Loading branch information
paulp committed Aug 13, 2012
1 parent 8cfba0f commit caf7eb6
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 20 deletions.
3 changes: 2 additions & 1 deletion src/library/scala/collection/IterableLike.scala
Expand Up @@ -171,7 +171,7 @@ self =>
* fewer elements than size. * fewer elements than size.
*/ */
def sliding(size: Int): Iterator[Repr] = sliding(size, 1) def sliding(size: Int): Iterator[Repr] = sliding(size, 1)

/** Groups elements in fixed size blocks by passing a "sliding window" /** Groups elements in fixed size blocks by passing a "sliding window"
* over them (as opposed to partitioning them, as is done in grouped.) * over them (as opposed to partitioning them, as is done in grouped.)
* @see [[scala.collection.Iterator]], method `sliding` * @see [[scala.collection.Iterator]], method `sliding`
Expand Down Expand Up @@ -293,6 +293,7 @@ self =>


override /*TraversableLike*/ def view = new IterableView[A, Repr] { override /*TraversableLike*/ def view = new IterableView[A, Repr] {
protected lazy val underlying = self.repr protected lazy val underlying = self.repr
override def isEmpty = self.isEmpty
override def iterator = self.iterator override def iterator = self.iterator
} }


Expand Down
1 change: 1 addition & 0 deletions src/library/scala/collection/SeqLike.scala
Expand Up @@ -627,6 +627,7 @@ trait SeqLike[+A, +Repr] extends Any with IterableLike[A, Repr] with GenSeqLike[


override def view = new SeqView[A, Repr] { override def view = new SeqView[A, Repr] {
protected lazy val underlying = self.repr protected lazy val underlying = self.repr
override def isEmpty = self.isEmpty
override def iterator = self.iterator override def iterator = self.iterator
override def length = self.length override def length = self.length
override def apply(idx: Int) = self.apply(idx) override def apply(idx: Int) = self.apply(idx)
Expand Down
5 changes: 3 additions & 2 deletions src/library/scala/collection/TraversableLike.scala
Expand Up @@ -86,7 +86,7 @@ trait TraversableLike[+A, +Repr] extends Any
def repr: Repr = this.asInstanceOf[Repr] def repr: Repr = this.asInstanceOf[Repr]


final def isTraversableAgain: Boolean = true final def isTraversableAgain: Boolean = true

/** The underlying collection seen as an instance of `$Coll`. /** The underlying collection seen as an instance of `$Coll`.
* By default this is implemented as the current collection object itself, * By default this is implemented as the current collection object itself,
* but this can be overridden. * but this can be overridden.
Expand Down Expand Up @@ -174,7 +174,7 @@ trait TraversableLike[+A, +Repr] extends Any
* *
* @usecase def ++:[B](that: TraversableOnce[B]): $Coll[B] * @usecase def ++:[B](that: TraversableOnce[B]): $Coll[B]
* @inheritdoc * @inheritdoc
* *
* Example: * Example:
* {{{ * {{{
* scala> val x = List(1) * scala> val x = List(1)
Expand Down Expand Up @@ -655,6 +655,7 @@ trait TraversableLike[+A, +Repr] extends Any
def view = new TraversableView[A, Repr] { def view = new TraversableView[A, Repr] {
protected lazy val underlying = self.repr protected lazy val underlying = self.repr
override def foreach[U](f: A => U) = self foreach f override def foreach[U](f: A => U) = self foreach f
override def isEmpty = self.isEmpty
} }


/** Creates a non-strict view of a slice of this $coll. /** Creates a non-strict view of a slice of this $coll.
Expand Down
13 changes: 11 additions & 2 deletions src/library/scala/collection/TraversableViewLike.scala
Expand Up @@ -59,7 +59,7 @@ trait ViewMkString[+A] {
* $viewInfo * $viewInfo
* *
* All views for traversable collections are defined by creating a new `foreach` method. * All views for traversable collections are defined by creating a new `foreach` method.
* *
* @author Martin Odersky * @author Martin Odersky
* @version 2.8 * @version 2.8
* @since 2.8 * @since 2.8
Expand Down Expand Up @@ -162,7 +162,7 @@ trait TraversableViewLike[+A,
// if (b.isInstanceOf[NoBuilder[_]]) newFlatMapped(f).asInstanceOf[That] // if (b.isInstanceOf[NoBuilder[_]]) newFlatMapped(f).asInstanceOf[That]
// else super.flatMap[B, That](f)(bf) // else super.flatMap[B, That](f)(bf)
} }
override def flatten[B](implicit asTraversable: A => /*<:<!!!*/ GenTraversableOnce[B]) = override def flatten[B](implicit asTraversable: A => /*<:<!!!*/ GenTraversableOnce[B]) =
newFlatMapped(asTraversable) newFlatMapped(asTraversable)
private[this] implicit def asThis(xs: Transformed[A]): This = xs.asInstanceOf[This] private[this] implicit def asThis(xs: Transformed[A]): This = xs.asInstanceOf[This]


Expand Down Expand Up @@ -193,6 +193,15 @@ trait TraversableViewLike[+A,
override def span(p: A => Boolean): (This, This) = (newTakenWhile(p), newDroppedWhile(p)) override def span(p: A => Boolean): (This, This) = (newTakenWhile(p), newDroppedWhile(p))
override def splitAt(n: Int): (This, This) = (newTaken(n), newDropped(n)) override def splitAt(n: Int): (This, This) = (newTaken(n), newDropped(n))


// Without this, isEmpty tests go back to the Traversable default, which
// involves starting a foreach, which can force the first element of the
// view. This is just a backstop - it's overridden at all the "def view"
// instantiation points in the collections where the Coll type is known.
override def isEmpty = underlying match {
case x: GenTraversableOnce[_] => x.isEmpty
case _ => super.isEmpty
}

override def scanLeft[B, That](z: B)(op: (B, A) => B)(implicit bf: CanBuildFrom[This, B, That]): That = override def scanLeft[B, That](z: B)(op: (B, A) => B)(implicit bf: CanBuildFrom[This, B, That]): That =
newForced(thisSeq.scanLeft(z)(op)).asInstanceOf[That] newForced(thisSeq.scanLeft(z)(op)).asInstanceOf[That]


Expand Down
1 change: 1 addition & 0 deletions src/library/scala/collection/immutable/Stream.scala
Expand Up @@ -916,6 +916,7 @@ self =>


override def view = new StreamView[A, Stream[A]] { override def view = new StreamView[A, Stream[A]] {
protected lazy val underlying = self.repr protected lazy val underlying = self.repr
override def isEmpty = self.isEmpty
override def iterator = self.iterator override def iterator = self.iterator
override def length = self.length override def length = self.length
override def apply(idx: Int) = self.apply(idx) override def apply(idx: Int) = self.apply(idx)
Expand Down
1 change: 1 addition & 0 deletions src/library/scala/collection/mutable/IndexedSeqLike.scala
Expand Up @@ -53,6 +53,7 @@ trait IndexedSeqLike[A, +Repr] extends scala.collection.IndexedSeqLike[A, Repr]
*/ */
override def view = new IndexedSeqView[A, Repr] { override def view = new IndexedSeqView[A, Repr] {
protected lazy val underlying = self.repr protected lazy val underlying = self.repr
override def isEmpty = self.isEmpty
override def iterator = self.iterator override def iterator = self.iterator
override def length = self.length override def length = self.length
override def apply(idx: Int) = self.apply(idx) override def apply(idx: Int) = self.apply(idx)
Expand Down
29 changes: 15 additions & 14 deletions src/library/scala/collection/parallel/ParIterableLike.scala
Expand Up @@ -171,9 +171,9 @@ self: ParIterableLike[T, Repr, Sequential] =>


/** The task support object which is responsible for scheduling and /** The task support object which is responsible for scheduling and
* load-balancing tasks to processors. * load-balancing tasks to processors.
* *
* @see [[scala.collection.parallel.TaskSupport]] * @see [[scala.collection.parallel.TaskSupport]]
*/ */
def tasksupport = { def tasksupport = {
val ts = _tasksupport val ts = _tasksupport
if (ts eq null) { if (ts eq null) {
Expand All @@ -188,18 +188,18 @@ self: ParIterableLike[T, Repr, Sequential] =>
* A task support object can be changed in a parallel collection after it * A task support object can be changed in a parallel collection after it
* has been created, but only during a quiescent period, i.e. while there * has been created, but only during a quiescent period, i.e. while there
* are no concurrent invocations to parallel collection methods. * are no concurrent invocations to parallel collection methods.
* *
* Here is a way to change the task support of a parallel collection: * Here is a way to change the task support of a parallel collection:
* *
* {{{ * {{{
* import scala.collection.parallel._ * import scala.collection.parallel._
* val pc = mutable.ParArray(1, 2, 3) * val pc = mutable.ParArray(1, 2, 3)
* pc.tasksupport = new ForkJoinTaskSupport( * pc.tasksupport = new ForkJoinTaskSupport(
* new scala.concurrent.forkjoin.ForkJoinPool(2)) * new scala.concurrent.forkjoin.ForkJoinPool(2))
* }}} * }}}
* *
* @see [[scala.collection.parallel.TaskSupport]] * @see [[scala.collection.parallel.TaskSupport]]
*/ */
def tasksupport_=(ts: TaskSupport) = _tasksupport = ts def tasksupport_=(ts: TaskSupport) = _tasksupport = ts


def seq: Sequential def seq: Sequential
Expand Down Expand Up @@ -848,6 +848,7 @@ self: ParIterableLike[T, Repr, Sequential] =>
override def seq = self.seq.view override def seq = self.seq.view
def splitter = self.splitter def splitter = self.splitter
def size = splitter.remaining def size = splitter.remaining
override def isEmpty = size == 0
} }


override def toArray[U >: T: ClassTag]: Array[U] = { override def toArray[U >: T: ClassTag]: Array[U] = {
Expand Down Expand Up @@ -877,13 +878,13 @@ self: ParIterableLike[T, Repr, Sequential] =>
override def toSet[U >: T]: immutable.ParSet[U] = toParCollection[U, immutable.ParSet[U]](() => immutable.ParSet.newCombiner[U]) override def toSet[U >: T]: immutable.ParSet[U] = toParCollection[U, immutable.ParSet[U]](() => immutable.ParSet.newCombiner[U])


override def toMap[K, V](implicit ev: T <:< (K, V)): immutable.ParMap[K, V] = toParMap[K, V, immutable.ParMap[K, V]](() => immutable.ParMap.newCombiner[K, V]) override def toMap[K, V](implicit ev: T <:< (K, V)): immutable.ParMap[K, V] = toParMap[K, V, immutable.ParMap[K, V]](() => immutable.ParMap.newCombiner[K, V])

override def toVector: Vector[T] = to[Vector] override def toVector: Vector[T] = to[Vector]


override def to[Col[_]](implicit cbf: CanBuildFrom[Nothing, T, Col[T @uncheckedVariance]]): Col[T @uncheckedVariance] = if (cbf().isCombiner) { override def to[Col[_]](implicit cbf: CanBuildFrom[Nothing, T, Col[T @uncheckedVariance]]): Col[T @uncheckedVariance] = if (cbf().isCombiner) {
toParCollection[T, Col[T]](() => cbf().asCombiner) toParCollection[T, Col[T]](() => cbf().asCombiner)
} else seq.to(cbf) } else seq.to(cbf)

/* tasks */ /* tasks */


protected trait StrictSplitterCheckTask[R, Tp] extends Task[R, Tp] { protected trait StrictSplitterCheckTask[R, Tp] extends Task[R, Tp] {
Expand Down
3 changes: 2 additions & 1 deletion src/library/scala/collection/parallel/ParSeqLike.scala
Expand Up @@ -44,7 +44,7 @@ trait ParSeqLike[+T, +Repr <: ParSeq[T], +Sequential <: Seq[T] with SeqLike[T, S
extends scala.collection.GenSeqLike[T, Repr] extends scala.collection.GenSeqLike[T, Repr]
with ParIterableLike[T, Repr, Sequential] { with ParIterableLike[T, Repr, Sequential] {
self => self =>

type SuperParIterator = IterableSplitter[T] type SuperParIterator = IterableSplitter[T]


/** A more refined version of the iterator found in the `ParallelIterable` trait, /** A more refined version of the iterator found in the `ParallelIterable` trait,
Expand Down Expand Up @@ -330,6 +330,7 @@ self =>
def apply(idx: Int) = self(idx) def apply(idx: Int) = self(idx)
override def seq = self.seq.view override def seq = self.seq.view
def splitter = self.splitter def splitter = self.splitter
override def isEmpty = size == 0
} }


/* tasks */ /* tasks */
Expand Down

0 comments on commit caf7eb6

Please sign in to comment.