diff --git a/src/library/scala/collection/mutable/ArrayBuffer.scala b/src/library/scala/collection/mutable/ArrayBuffer.scala index 046cd6f6602c..2c386d1b25e9 100644 --- a/src/library/scala/collection/mutable/ArrayBuffer.scala +++ b/src/library/scala/collection/mutable/ArrayBuffer.scala @@ -316,20 +316,12 @@ object ArrayBuffer extends StrictOptimizedSeqFactory[ArrayBuffer] { * - `max(targetLen, arrayLen * 2, DefaultInitialSize)`. * - Throws an exception if `targetLen` exceeds `VM_MaxArraySize` or is negative (overflow). */ - private[mutable] def resizeUp(arrayLen: Int, targetLen: Int): Int = { - def checkArrayLengthLimit(): Unit = - if (targetLen > VM_MaxArraySize) - throw new Exception(s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $targetLen; current length: $arrayLen") - else if (targetLen < 0) - throw new Exception(s"Overflow while resizing array of array-backed collection. Requested length: $targetLen; current length: $arrayLen; increase: ${targetLen - arrayLen}") - - if (targetLen > 0 && targetLen <= arrayLen) -1 - else { - checkArrayLengthLimit() - if (arrayLen > VM_MaxArraySize / 2) VM_MaxArraySize - else math.max(targetLen, math.max(arrayLen * 2, DefaultInitialSize)) - } - } + private[mutable] def resizeUp(arrayLen: Int, targetLen: Int): Int = + if (targetLen < 0) throw new Exception(s"Overflow while resizing array of array-backed collection. Requested length: $targetLen; current length: $arrayLen; increase: ${targetLen - arrayLen}") + else if (targetLen <= arrayLen) -1 + else if (targetLen > VM_MaxArraySize) throw new Exception(s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $targetLen; current length: $arrayLen") + else if (arrayLen > VM_MaxArraySize / 2) VM_MaxArraySize + else math.max(targetLen, math.max(arrayLen * 2, DefaultInitialSize)) // if necessary, copy (curSize elements of) the array to a new array of capacity n. // Should use Array.copyOf(array, resizeEnsuring(array.length))? diff --git a/src/library/scala/collection/mutable/ArrayBuilder.scala b/src/library/scala/collection/mutable/ArrayBuilder.scala index 0ad9b082c427..1425d1f88cf5 100644 --- a/src/library/scala/collection/mutable/ArrayBuilder.scala +++ b/src/library/scala/collection/mutable/ArrayBuilder.scala @@ -25,7 +25,7 @@ sealed abstract class ArrayBuilder[T] extends ReusableBuilder[T, Array[T]] with Serializable { protected[this] var capacity: Int = 0 - protected[this] def elems: Array[T] + protected[this] def elems: Array[T] // may not be allocated at size = capacity = 0 protected var size: Int = 0 /** Current number of elements. */ @@ -45,14 +45,23 @@ sealed abstract class ArrayBuilder[T] protected[this] def resize(size: Int): Unit - /** Add all elements of an array */ - def addAll(xs: Array[_ <: T]): this.type = addAll(xs, 0, xs.length) + /** Add all elements of an array. */ + def addAll(xs: Array[_ <: T]): this.type = doAddAll(xs, 0, xs.length) - /** Add a slice of an array */ + /** Add a slice of an array. */ def addAll(xs: Array[_ <: T], offset: Int, length: Int): this.type = { - ensureSize(this.size + length) - Array.copy(xs, offset, elems, this.size, length) - size += length + val offset1 = offset.max(0) + val length1 = length.max(0) + val effectiveLength = length1.min(xs.length - offset1) + doAddAll(xs, offset1, effectiveLength) + } + + private def doAddAll(xs: Array[_ <: T], offset: Int, length: Int): this.type = { + if (length > 0) { + ensureSize(this.size + length) + Array.copy(xs, offset, elems, this.size, length) + size += length + } this } diff --git a/test/junit/scala/collection/mutable/ArrayBufferTest.scala b/test/junit/scala/collection/mutable/ArrayBufferTest.scala index 13b5d536f9d3..311786f163d2 100644 --- a/test/junit/scala/collection/mutable/ArrayBufferTest.scala +++ b/test/junit/scala/collection/mutable/ArrayBufferTest.scala @@ -392,6 +392,9 @@ class ArrayBufferTest { // check termination and correctness assertTrue(7 < ArrayBuffer.DefaultInitialSize) // condition of test + assertEquals(-1, resizeUp(7, 0)) + assertEquals(-1, resizeUp(Int.MaxValue, 0)) + assertEquals(ArrayBuffer.DefaultInitialSize, resizeUp(-1, 0)) // no check of arrayLen assertEquals(ArrayBuffer.DefaultInitialSize, resizeUp(7, 10)) assertEquals(VM_MaxArraySize, resizeUp(Int.MaxValue / 2, Int.MaxValue / 2 + 1)) // `MaxValue / 2` cannot be doubled assertEquals(VM_MaxArraySize / 2 * 2, resizeUp(VM_MaxArraySize / 2, VM_MaxArraySize / 2 + 1)) // `VM_MaxArraySize / 2` can be doubled @@ -406,12 +409,13 @@ class ArrayBufferTest { try op catch { case e: InvocationTargetException => throw e.getCause } def checkExceedsMaxInt(targetLen: Int): Unit = { assertThrows[Exception](rethrow(resizeUp(0, targetLen)), - _ == s"Overflow while resizing array of array-backed collection. Requested length: $targetLen; current length: 0; increase: $targetLen") + _ == s"Overflow while resizing array of array-backed collection. Requested length: $targetLen; current length: 0; increase: $targetLen") } def checkExceedsVMArrayLimit(targetLen: Int): Unit = assertThrows[Exception](rethrow(resizeUp(0, targetLen)), - _ == s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $targetLen; current length: 0") + _ == s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $targetLen; current length: 0") + checkExceedsMaxInt(-1) checkExceedsMaxInt(Int.MaxValue + 1: @nowarn) checkExceedsVMArrayLimit(Int.MaxValue) checkExceedsVMArrayLimit(Int.MaxValue - 1) diff --git a/test/junit/scala/collection/mutable/ArrayBuilderTest.scala b/test/junit/scala/collection/mutable/ArrayBuilderTest.scala index 567f3167fa8b..37a38b957a93 100644 --- a/test/junit/scala/collection/mutable/ArrayBuilderTest.scala +++ b/test/junit/scala/collection/mutable/ArrayBuilderTest.scala @@ -32,4 +32,11 @@ class ArrayBuilderTest { // expect an exception when trying to grow larger than maximum size by addAll(array) assertThrows[Exception](ab.addAll(arr)) } + + // avoid allocating "default size" for empty, and especially avoid doubling capacity for empty + @Test def `addAll allocates elems more lazily`: Unit = { + val builder = ArrayBuilder.make[String] + (1 to 100).foreach(_ => builder.addAll(Array.empty[String])) + assertEquals(0, builder.knownSize) + } }