Merge pull request #9786 from NthPortal/topic/ArrayBuffer-cleanup

scala · Oct 28, 2021 · 8fde4e3 · 8fde4e3
2 parents 8ee33bb + ca88d99
commit 8fde4e3
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 18 deletions.
diff --git a/src/library/scala/collection/mutable/ArrayBuffer.scala b/src/library/scala/collection/mutable/ArrayBuffer.scala
@@ -66,7 +66,6 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
 
   /** Ensure that the internal array has at least `n` cells. */
   protected def ensureSize(n: Int): Unit = {
-    mutationCount += 1
     array = ArrayBuffer.ensureSize(array, size0, n)
   }
 
@@ -86,7 +85,6 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
    *  This allows releasing some unused memory.
    */
   def trimToSize(): Unit = {
-    mutationCount += 1
     resize(length)
   }
 
@@ -136,10 +134,11 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
   }
 
   def addOne(elem: A): this.type = {
-    val i = size0
-    ensureSize(size0 + 1)
-    size0 += 1
-    this(i) = elem
+    mutationCount += 1
+    val oldSize = size0
+    ensureSize(oldSize + 1)
+    size0 = oldSize + 1
+    this(oldSize) = elem
     this
   }
 
@@ -149,6 +148,7 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
       case elems: ArrayBuffer[_] =>
         val elemsLength = elems.size0
         if (elemsLength > 0) {
+          mutationCount += 1
           ensureSize(length + elemsLength)
           Array.copy(elems.array, 0, array, length, elemsLength)
           size0 = length + elemsLength
@@ -160,6 +160,7 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
 
   def insert(@deprecatedName("n", "2.13.0") index: Int, elem: A): Unit = {
     checkWithinBounds(index, index)
+    mutationCount += 1
     ensureSize(size0 + 1)
     Array.copy(array, index, array, index + 1, size0 - index)
     size0 += 1
@@ -177,6 +178,7 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
       case elems: collection.Iterable[A] =>
         val elemsLength = elems.size
         if (elemsLength > 0) {
+          mutationCount += 1
           val len = size0
           val newSize = len + elemsLength
           ensureSize(newSize)

diff --git a/test/benchmarks/src/main/scala/scala/collection/mutable/ArrayBufferBenchmark.scala b/test/benchmarks/src/main/scala/scala/collection/mutable/ArrayBufferBenchmark.scala
@@ -63,27 +63,56 @@ class ArrayBufferBenchmark {
     bh.consume(b1)
   }
 
-  // append collection with known size
+  // append `Iterable` with known size
   @Benchmark def addAll2(bh: Blackhole): Unit = {
-    val b1 = ref.clone()
-    b1.addAll(set)
-    bh.consume(b1)
+    val b = ref.clone()
+    b.addAll(set)
+    bh.consume(b)
   }
 
-  // append collection without known size
+  // append `Iterable` without known size
   @Benchmark def addAll3(bh: Blackhole): Unit = {
-    val b1 = ref.clone()
-    b1.addAll(list)
-    bh.consume(b1)
+    val b = ref.clone()
+    b.addAll(list)
+    bh.consume(b)
   }
 
-  // append `IterableOnce` with no known size
+  // append `IterableOnce` without known size
   @Benchmark def addAll4(bh: Blackhole): Unit = {
+    val b = ref.clone()
+    b.addAll(list.iterator)
+    bh.consume(b)
+  }
+
+  // insert `ArrayBuffer`
+  @Benchmark def insertAll1(bh: Blackhole): Unit = {
     val b1 = ref.clone()
-    b1.addAll(list.iterator)
+    val b2 = ref.clone()
+    b1.insertAll(size / 2, b2)
     bh.consume(b1)
   }
 
+  // insert `Iterable` with known size
+  @Benchmark def insertAll2(bh: Blackhole): Unit = {
+    val b = ref.clone()
+    b.insertAll(size / 2, set)
+    bh.consume(b)
+  }
+
+  // insert `Iterable` without known size
+  @Benchmark def insertAll3(bh: Blackhole): Unit = {
+    val b = ref.clone()
+    b.insertAll(size / 2, list)
+    bh.consume(b)
+  }
+
+  // insert `IterableOnce` without known size
+  @Benchmark def insertAll4(bh: Blackhole): Unit = {
+    val b = ref.clone()
+    b.insertAll(size / 2, list.iterator)
+    bh.consume(b)
+  }
+
   @Benchmark def flatMapInPlace1(bh: Blackhole): Unit = {
     val b   = ref.clone()
     val seq = scala.Seq(0, 0)

diff --git a/test/junit/scala/collection/mutable/MutationTrackingTest.scala b/test/junit/scala/collection/mutable/MutationTrackingTest.scala
@@ -245,9 +245,9 @@ package MutationTrackingTestImpl {
     def clearAndShrink(): Unit = checkThrows { _ clearAndShrink 2 }
 
     @Test
-    def trimToSize(): Unit = checkThrows { _.trimToSize() }
+    def trimToSize(): Unit = checkFine { _.trimToSize() }
 
     @Test
-    def sizeHint(): Unit = checkThrows { _ sizeHint 16 }
+    def sizeHint(): Unit = checkFine { _ sizeHint 16 }
   }
 }