Skip to content

Commit

Permalink
Merge pull request #9036 from retronym/12027-vector-prepended
Browse files Browse the repository at this point in the history 
Restore special cases for small operands in {append,prepend}edAll
  • Loading branch information
@lrytz
lrytz committed Jun 12, 2020
2 parents 0ff65a0 + 0e8a5f8 commit 3da5a0d
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 3 deletions.
2 changes: 2 additions & 0 deletions build.sbt
View file
Expand Up @@ -327,6 +327,8 @@ val mimaFilterSettings = Seq {
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.immutable.RedBlackTree#TreeIterator.ordering"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.collection.SortedSet.scala$collection$SortedSet$$super=uals"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.collection.SortedMap.scala$collection$SortedMap$$super=uals"),

ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.immutable.Vector.prependedAll0"),
),
}

Expand Down
87 changes: 86 additions & 1 deletion src/library/scala/collection/immutable/Vector.scala
View file
Expand Up @@ -191,14 +191,33 @@ sealed abstract class Vector[+A] private[immutable] (private[immutable] final va
override def updated[B >: A](index: Int, elem: B): Vector[B] = super.updated(index, elem)
override def appended[B >: A](elem: B): Vector[B] = super.appended(elem)
override def prepended[B >: A](elem: B): Vector[B] = super.prepended(elem)
override def prependedAll[B >: A](prefix: collection.IterableOnce[B]): Vector[B] = super.prependedAll(prefix)
override def prependedAll[B >: A](prefix: collection.IterableOnce[B]): Vector[B] = {
val k = prefix.knownSize
if(k == 0) this
else prependedAll0(prefix, k)
}

override final def appendedAll[B >: A](suffix: collection.IterableOnce[B]): Vector[B] = {
val k = suffix.knownSize
if(k == 0) this
else appendedAll0(suffix, k)
}

protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val tinyAppendLimit = 4 + vectorSliceCount
if (k < tinyAppendLimit /*|| k < (this.size >>> Log2ConcatFaster)*/) {
var v: Vector[B] = this
val it = IndexedSeq.from(prefix).reverseIterator
while (it.hasNext) v = it.next() +: v
v
} else if (this.size < (k >>> Log2ConcatFaster) && prefix.isInstanceOf[Vector[_]]) {
var v = prefix.asInstanceOf[Vector[B]]
val it = this.iterator
while (it.hasNext) v = v :+ it.next()
v
} else super.prependedAll(prefix)
}

protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val tinyAppendLimit = 4 + vectorSliceCount
if(k > 0 && k < tinyAppendLimit) {
Expand All @@ -208,6 +227,11 @@ sealed abstract class Vector[+A] private[immutable] (private[immutable] final va
case _ => suffix.iterator.foreach(x => v = v.appended(x))
}
v
} else if (this.size < (k >>> Log2ConcatFaster) && suffix.isInstanceOf[Vector[_]]) {
var v = suffix.asInstanceOf[Vector[B]]
val ri = this.reverseIterator
while (ri.hasNext) v = v.prepended(ri.next())
v
} else new VectorBuilder[B].initFrom(this).addAll(suffix).result()
}

Expand Down Expand Up @@ -337,6 +361,9 @@ private object Vector0 extends BigVector[Nothing](empty1, empty1, 0) {
}
}

override protected[this]def prependedAll0[B >: Nothing](prefix: collection.IterableOnce[B], k: Int): Vector[B] =
Vector.from(prefix)

override protected[this]def appendedAll0[B >: Nothing](suffix: collection.IterableOnce[B], k: Int): Vector[B] =
Vector.from(suffix)

Expand Down Expand Up @@ -388,6 +415,12 @@ private final class Vector1[+A](_data1: Arr1) extends VectorImpl[A](_data1) {
protected[immutable] def vectorSlice(idx: Int): Array[_ <: AnyRef] = prefix1
protected[immutable] def vectorSlicePrefixLength(idx: Int): Int = prefix1.length

override protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val data1b = prepend1IfSpace(prefix1, prefix)
if(data1b ne null) new Vector1(data1b)
else super.prependedAll0(prefix, k)
}

override protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val data1b = append1IfSpace(prefix1, suffix)
if(data1b ne null) new Vector1(data1b)
Expand Down Expand Up @@ -477,6 +510,11 @@ private final class Vector2[+A](_prefix1: Arr1, private[immutable] val len1: Int
case 2 => length0
}

override protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val prefix1b = prepend1IfSpace(prefix1, prefix)
if(prefix1b ne null) copy(prefix1 = prefix1b, len1 = len1-prefix1.length+prefix1b.length, length0 = length0-prefix1.length+prefix1b.length)
else super.prependedAll0(prefix, k)
}
override protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val suffix1b = append1IfSpace(suffix1, suffix)
if(suffix1b ne null) copy(suffix1 = suffix1b, length0 = length0-suffix1.length+suffix1b.length)
Expand Down Expand Up @@ -588,6 +626,11 @@ private final class Vector3[+A](_prefix1: Arr1, private[immutable] val len1: Int
case 4 => length0
}

override protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val prefix1b = prepend1IfSpace(prefix1, prefix)
if(prefix1b ne null) copy(prefix1 = prefix1b, len1 = len1-prefix1.length+prefix1b.length, length0 = length0-prefix1.length+prefix1b.length)
else super.prependedAll0(prefix, k)
}
override protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val suffix1b = append1IfSpace(suffix1, suffix)
if(suffix1b ne null) copy(suffix1 = suffix1b, length0 = length0-suffix1.length+suffix1b.length)
Expand Down Expand Up @@ -719,6 +762,12 @@ private final class Vector4[+A](_prefix1: Arr1, private[immutable] val len1: Int
case 6 => length0
}

override protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val prefix1b = prepend1IfSpace(prefix1, prefix)
if(prefix1b ne null) copy(prefix1 = prefix1b, len1 = len1-prefix1.length+prefix1b.length, length0 = length0-prefix1.length+prefix1b.length)
else super.prependedAll0(prefix, k)
}

override protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val suffix1b = append1IfSpace(suffix1, suffix)
if(suffix1b ne null) copy(suffix1 = suffix1b, length0 = length0-suffix1.length+suffix1b.length)
Expand Down Expand Up @@ -870,6 +919,12 @@ private final class Vector5[+A](_prefix1: Arr1, private[immutable] val len1: Int
case 8 => length0
}

override protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val prefix1b = prepend1IfSpace(prefix1, prefix)
if(prefix1b ne null) copy(prefix1 = prefix1b, len1 = len1-prefix1.length+prefix1b.length, length0 = length0-prefix1.length+prefix1b.length)
else super.prependedAll0(prefix, k)
}

override protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val suffix1b = append1IfSpace(suffix1, suffix)
if(suffix1b ne null) copy(suffix1 = suffix1b, length0 = length0-suffix1.length+suffix1b.length)
Expand Down Expand Up @@ -1041,6 +1096,12 @@ private final class Vector6[+A](_prefix1: Arr1, private[immutable] val len1: Int
case 10 => length0
}

override protected[this] def prependedAll0[B >: A](prefix: collection.IterableOnce[B], k: Int): Vector[B] = {
val prefix1b = prepend1IfSpace(prefix1, prefix)
if(prefix1b ne null) copy(prefix1 = prefix1b, len1 = len1-prefix1.length+prefix1b.length, length0 = length0-prefix1.length+prefix1b.length)
else super.prependedAll0(prefix, k)
}

override protected[this] def appendedAll0[B >: A](suffix: collection.IterableOnce[B], k: Int): Vector[B] = {
val suffix1b = append1IfSpace(suffix1, suffix)
if(suffix1b ne null) copy(suffix1 = suffix1b, length0 = length0-suffix1.length+suffix1b.length)
Expand Down Expand Up @@ -1674,6 +1735,7 @@ private[immutable] object VectorInline {
final val BITS6 = BITS * 6
final val WIDTH6 = 1 << BITS6
final val LASTWIDTH = WIDTH << 1 // 1 extra bit in the last level to go up to Int.MaxValue (2^31-1) instead of 2^30:
final val Log2ConcatFaster = 5

type Arr1 = Array[AnyRef]
type Arr2 = Array[Array[AnyRef]]
Expand Down Expand Up @@ -1855,6 +1917,29 @@ private object VectorStatics {
ac.asInstanceOf[Array[T]]
}

final def prepend1IfSpace(prefix1: Arr1, xs: IterableOnce[_]): Arr1 = xs match {
case it: Iterable[_] =>
if(it.sizeCompare(WIDTH-prefix1.length) <= 0) {
it.size match {
case 0 => null
case 1 => copyPrepend(it.head.asInstanceOf[AnyRef], prefix1)
case s =>
val prefix1b = new Arr1(prefix1.length + s)
System.arraycopy(prefix1, 0, prefix1b, s, prefix1.length)
it.copyToArray(prefix1b.asInstanceOf[Array[Any]], 0)
prefix1b
}
} else null
case it =>
val s = it.knownSize
if(s > 0 && s <= WIDTH-prefix1.length) {
val prefix1b = new Arr1(prefix1.length + s)
System.arraycopy(prefix1, 0, prefix1b, s, prefix1.length)
it.iterator.copyToArray(prefix1b.asInstanceOf[Array[Any]], 0)
prefix1b
} else null
}

final def append1IfSpace(suffix1: Arr1, xs: IterableOnce[_]): Arr1 = xs match {
case it: Iterable[_] =>
if(it.sizeCompare(WIDTH-suffix1.length) <= 0) {
Expand Down
13 changes: 11 additions & 2 deletions test/junit/scala/collection/immutable/VectorTest.scala
View file
Expand Up @@ -34,16 +34,18 @@ class VectorTest {
}

@Test
def hasCorrectPrependedAll(): Unit = {
def hasCorrectAppenedAndPrependedAll(): Unit = {
val els = Vector(1 to 1000: _*)

for (i <- 0 until els.size) {
val (prefix, suffix) = els.splitAt(i)
validateDebug(prefix)
validateDebug(suffix)
validateDebug(prefix ++: suffix)
assertEquals(els, prefix ++: suffix)
assertEquals((prefix, suffix).toString, els, prefix ++: suffix)
assertEquals((prefix, suffix).toString, els, prefix :++ suffix)
assertEquals(els, prefix.toList ++: suffix)
assertEquals(els, prefix.toList :++ suffix)
}
}

Expand Down Expand Up @@ -412,6 +414,13 @@ class VectorTest {
assertEquals(0 until (size + appendSize), v3)
}
}

// scala/bug#12027
// This took forever in 2.13.2
@Test def testAppendWithTinyLhs(): Unit = {
(1 to 1000000).foldLeft(Vector.empty[Int]) { (v, i) => Vector(i) ++ v }
(1 to 1000000).foldLeft(Vector.empty[Int]) { (v, i) => v.prependedAll(Vector(i)) }
}
}

object VectorUtils {
Expand Down

0 comments on commit 3da5a0d

Please sign in to comment.