Skip to content

Commit 29142a9

Browse files
NthPortallrytz
authored andcommitted
Optimise factories for Array-based collections
Optimise `from` methods on factories for `Array`-based collections by having them call `copyToArray` on the source collection. Deprecate overriding the overloads of `copyToArray` with fewer args, and remove overrides of those methods from the standard library. Fix incorrect doc about termination of `copyToArray` for infinite collections. Add helper method similar to `Array.from` for calling `copyToArray` on an `IterableOnce`.
1 parent 7829ae7 commit 29142a9

File tree

8 files changed

+23
-55
lines changed

8 files changed

+23
-55
lines changed

library/src/scala/collection/IterableOnce.scala

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,16 @@ object IterableOnce {
265265
*/
266266
@inline private[collection] def elemsToCopyToArray(srcLen: Int, destLen: Int, start: Int, len: Int): Int =
267267
math.max(math.min(math.min(len, srcLen), destLen - start), 0)
268+
269+
/** Calls `copyToArray` on the given collection, regardless of whether or not it is an `Iterable`. */
270+
@inline private[collection] def copyElemsToArray[A, B >: A](elems: IterableOnce[A],
271+
xs: Array[B],
272+
start: Int = 0,
273+
len: Int = Int.MaxValue): Int =
274+
elems match {
275+
case src: Iterable[A] => src.copyToArray[B](xs, start, len)
276+
case src => src.iterator.copyToArray[B](xs, start, len)
277+
}
268278
}
269279

270280
/** This implementation trait can be mixed into an `IterableOnce` to get the basic methods that are shared between
@@ -819,9 +829,10 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
819829
* @tparam B the type of the elements of the array.
820830
* @return the number of elements written to the array
821831
*
822-
* $willNotTerminateInf
832+
* @note Reuse: $consumesIterator
823833
*/
824-
def copyToArray[B >: A](xs: Array[B]): Int = copyToArray(xs, 0)
834+
@deprecatedOverriding("This should always forward to the 3-arg version of this method", since = "2.13.4")
835+
def copyToArray[B >: A](xs: Array[B]): Int = copyToArray(xs, 0, Int.MaxValue)
825836

826837
/** Copy elements to an array, returning the number of elements written.
827838
*
@@ -835,18 +846,10 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
835846
* @tparam B the type of the elements of the array.
836847
* @return the number of elements written to the array
837848
*
838-
* $willNotTerminateInf
849+
* @note Reuse: $consumesIterator
839850
*/
840-
def copyToArray[B >: A](xs: Array[B], start: Int): Int = {
841-
val xsLen = xs.length
842-
val it = iterator
843-
var i = start
844-
while (i < xsLen && it.hasNext) {
845-
xs(i) = it.next()
846-
i += 1
847-
}
848-
i - start
849-
}
851+
@deprecatedOverriding("This should always forward to the 3-arg version of this method", since = "2.13.4")
852+
def copyToArray[B >: A](xs: Array[B], start: Int): Int = copyToArray(xs, start, Int.MaxValue)
850853

851854
/** Copy elements to an array, returning the number of elements written.
852855
*
@@ -862,8 +865,6 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
862865
* @return the number of elements written to the array
863866
*
864867
* @note Reuse: $consumesIterator
865-
*
866-
* $willNotTerminateInf
867868
*/
868869
def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
869870
val it = iterator

library/src/scala/collection/immutable/ArraySeq.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,6 @@ sealed abstract class ArraySeq[+A]
237237

238238
override protected[this] def className = "ArraySeq"
239239

240-
override def copyToArray[B >: A](xs: Array[B], start: Int = 0): Int = copyToArray[B](xs, start, length)
241-
242240
override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
243241
val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
244242
if(copied > 0) {

library/src/scala/collection/immutable/WrappedString.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,6 @@ final class WrappedString(private val self: String) extends AbstractSeq[Char] wi
8888
case _ => super.lastIndexOf(elem, end)
8989
}
9090

91-
override def copyToArray[B >: Char](xs: Array[B], start: Int): Int =
92-
copyToArray(xs, start, length)
93-
9491
override def copyToArray[B >: Char](xs: Array[B], start: Int, len: Int): Int =
9592
(xs: Any) match {
9693
case chs: Array[Char] =>

library/src/scala/collection/mutable/ArrayBuffer.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,6 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
219219

220220
override protected[this] def stringPrefix = "ArrayBuffer"
221221

222-
override def copyToArray[B >: A](xs: Array[B], start: Int): Int = copyToArray[B](xs, start, length)
223-
224222
override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
225223
val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
226224
if(copied > 0) {
@@ -258,8 +256,7 @@ object ArrayBuffer extends StrictOptimizedSeqFactory[ArrayBuffer] {
258256
val k = coll.knownSize
259257
if (k >= 0) {
260258
val array = new Array[AnyRef](k max DefaultInitialSize)
261-
val it = coll.iterator
262-
for (i <- 0 until k) array(i) = it.next().asInstanceOf[AnyRef]
259+
IterableOnce.copyElemsToArray(coll, array.asInstanceOf[Array[Any]])
263260
new ArrayBuffer[B](array, k)
264261
}
265262
else new ArrayBuffer[B] ++= coll

library/src/scala/collection/mutable/ArrayBuilder.scala

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,11 @@ sealed abstract class ArrayBuilder[T]
5959

6060
override def addAll(xs: IterableOnce[T]): this.type = {
6161
val k = xs.knownSize
62-
if(k > 0) {
62+
if (k > 0) {
6363
ensureSize(this.size + k)
64-
xs match {
65-
case xs: Iterable[T] => xs.copyToArray(elems, this.size)
66-
case _ => xs.iterator.copyToArray(elems, this.size)
67-
}
64+
IterableOnce.copyElemsToArray(xs, elems, this.size)
6865
size += k
69-
} else if(k < 0) super.addAll(xs)
66+
} else if (k < 0) super.addAll(xs)
7067
this
7168
}
7269
}

library/src/scala/collection/mutable/ArrayDeque.scala

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -527,14 +527,9 @@ object ArrayDeque extends StrictOptimizedSeqFactory[ArrayDeque] {
527527
val s = coll.knownSize
528528
if (s >= 0) {
529529
val array = alloc(s)
530-
val it = coll.iterator
531-
var i = 0
532-
while (it.hasNext) {
533-
array(i) = it.next().asInstanceOf[AnyRef]
534-
i += 1
535-
}
530+
IterableOnce.copyElemsToArray(coll, array.asInstanceOf[Array[Any]])
536531
new ArrayDeque[B](array, start = 0, end = s)
537-
} else empty[B] ++= coll
532+
} else new ArrayDeque[B]() ++= coll
538533
}
539534

540535
def newBuilder[A]: Builder[A, ArrayDeque[A]] =

library/src/scala/collection/mutable/ArraySeq.scala

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import java.util.Arrays
1818
import scala.collection.Stepper.EfficientSplit
1919
import scala.collection.convert.impl._
2020
import scala.reflect.ClassTag
21-
import scala.runtime.ScalaRunTime
2221
import scala.util.hashing.MurmurHash3
2322

2423
/**
@@ -74,8 +73,6 @@ sealed abstract class ArraySeq[T]
7473
/** Clones this object, including the underlying Array. */
7574
override def clone(): ArraySeq[T] = ArraySeq.make(array.clone()).asInstanceOf[ArraySeq[T]]
7675

77-
override def copyToArray[B >: T](xs: Array[B], start: Int): Int = copyToArray[B](xs, start, length)
78-
7976
override def copyToArray[B >: T](xs: Array[B], start: Int, len: Int): Int = {
8077
val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
8178
if(copied > 0) {
@@ -110,19 +107,7 @@ object ArraySeq extends StrictOptimizedClassTagSeqFactory[ArraySeq] { self =>
110107
private[this] val EmptyArraySeq = new ofRef[AnyRef](new Array[AnyRef](0))
111108
def empty[T : ClassTag]: ArraySeq[T] = EmptyArraySeq.asInstanceOf[ArraySeq[T]]
112109

113-
def from[A : ClassTag](it: scala.collection.IterableOnce[A]): ArraySeq[A] = {
114-
val n = it.knownSize
115-
if (n > -1) {
116-
val elements = scala.Array.ofDim[A](n)
117-
val iterator = it.iterator
118-
var i = 0
119-
while (i < n) {
120-
ScalaRunTime.array_update(elements, i, iterator.next())
121-
i = i + 1
122-
}
123-
make(elements)
124-
} else make(ArrayBuffer.from(it).toArray)
125-
}
110+
def from[A : ClassTag](it: scala.collection.IterableOnce[A]): ArraySeq[A] = make(Array.from[A](it))
126111

127112
def newBuilder[A : ClassTag]: Builder[A, ArraySeq[A]] = ArrayBuilder.make[A].mapResult(make)
128113

library/src/scala/collection/mutable/PriorityQueue.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,6 @@ sealed class PriorityQueue[A](implicit val ord: Ordering[A])
348348
pq
349349
}
350350

351-
override def copyToArray[B >: A](xs: Array[B], start: Int): Int = copyToArray(xs, start, length)
352-
353351
override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
354352
val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
355353
if (copied > 0) {

0 commit comments

Comments
 (0)