Merge pull request #10722 from som-snytt/issue/12971-array-builder

Defer array resize on empty

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))?

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
   }
 

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)

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)
+  }
 }