Skip to content

Commit 4170e21

Browse files
clrytz
authored andcommitted
Align ArrayBuilder resizing with ArrayBuffer
If the requested size is larger than double the current array lenght, use the requested size. Set `VM_MaxArraySize` to `Int.MaxValue - 8`, which seems to be a common default. Use that in ArrayBuffer and ArrayBuilder. To check if a size of more than Int.MaxValue is requested, check for an Int overflow instead of using a Long.
1 parent 22453d0 commit 4170e21

File tree

5 files changed

+40
-35
lines changed

5 files changed

+40
-35
lines changed

library/src/scala/collection/IterableOnce.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,9 @@ object IterableOnce {
287287
@inline private[collection] def checkArraySizeWithinVMLimit(size: Int): Unit = {
288288
import scala.runtime.PStatics.VM_MaxArraySize
289289
if (size > VM_MaxArraySize) {
290-
throw new Exception(s"Size of array-backed collection exceeds VM array size limit of ${VM_MaxArraySize}")
290+
throw new Exception(s"Size of array-backed collection exceeds VM array size limit of $VM_MaxArraySize. Encountered size: $size")
291+
} else if (size < 0) {
292+
throw new Exception(s"Size of array-backed collection must exceed 0. Encountered size: $size")
291293
}
292294
}
293295
}

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

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,6 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
7070
array = ArrayBuffer.ensureSize(array, size0, n)
7171
}
7272

73-
// TODO 3.T: should be `protected`, perhaps `protected[this]`
74-
/** Ensure that the internal array has at least `n` additional cells more than `size0`. */
75-
private[mutable] def ensureAdditionalSize(n: Int): Unit = {
76-
// `.toLong` to ensure `Long` arithmetic is used and prevent `Int` overflow
77-
array = ArrayBuffer.ensureSize(array, size0, size0.toLong + n)
78-
}
79-
8073
/** Uses the given size to resize internal storage, if necessary.
8174
*
8275
* @param size Expected maximum number of elements.
@@ -147,10 +140,10 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
147140

148141
def addOne(elem: A): this.type = {
149142
mutationCount += 1
150-
ensureAdditionalSize(1)
151-
val oldSize = size0
152-
size0 = oldSize + 1
153-
this(oldSize) = elem
143+
val newSize = size0 + 1
144+
ensureSize(newSize)
145+
size0 = newSize
146+
this(size0 - 1) = elem
154147
this
155148
}
156149

@@ -161,7 +154,7 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
161154
val elemsLength = elems.size0
162155
if (elemsLength > 0) {
163156
mutationCount += 1
164-
ensureAdditionalSize(elemsLength)
157+
ensureSize(size0 + elemsLength)
165158
Array.copy(elems.array, 0, array, length, elemsLength)
166159
size0 = length + elemsLength
167160
}
@@ -173,7 +166,7 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
173166
def insert(@deprecatedName("n", "2.13.0") index: Int, elem: A): Unit = {
174167
checkWithinBounds(index, index)
175168
mutationCount += 1
176-
ensureAdditionalSize(1)
169+
ensureSize(size0 + 1)
177170
Array.copy(array, index, array, index + 1, size0 - index)
178171
size0 += 1
179172
this(index) = elem
@@ -191,7 +184,7 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
191184
val elemsLength = elems.size
192185
if (elemsLength > 0) {
193186
mutationCount += 1
194-
ensureAdditionalSize(elemsLength)
187+
ensureSize(size0 + elemsLength)
195188
val len = size0
196189
Array.copy(array, index, array, index + elemsLength, len - index)
197190
// if `elems eq this`, this copy is safe because
@@ -314,24 +307,30 @@ object ArrayBuffer extends StrictOptimizedSeqFactory[ArrayBuffer] {
314307

315308
def empty[A]: ArrayBuffer[A] = new ArrayBuffer[A]()
316309

310+
import scala.runtime.PStatics.VM_MaxArraySize
317311
/**
318312
* @param arrayLen the length of the backing array
319313
* @param targetLen the minimum length to resize up to
320-
* @return -1 if no resizing is needed, or the size for the new array otherwise
314+
* @return -1 if no resizing is needed, the greater of targetLen and 2 * arrayLen if neither exceeds VM_MaxArraySize,
315+
* or VM_MaxArraySize otherwise.
321316
*/
322-
private def resizeUp(arrayLen: Long, targetLen: Long): Int = {
323-
if (targetLen <= arrayLen) -1
317+
private[mutable] def resizeUp(arrayLen: Int, targetLen: Int): Int = {
318+
// Hybrid
319+
if (targetLen > 0 && targetLen <= arrayLen) -1
324320
else {
325-
if (targetLen > Int.MaxValue) throw new Exception(s"Collections cannot have more than ${Int.MaxValue} elements")
326-
IterableOnce.checkArraySizeWithinVMLimit(targetLen.toInt) // safe because `targetSize <= Int.MaxValue`
321+
IterableOnce.checkArraySizeWithinVMLimit(targetLen) // safe because `targetSize <= Int.MaxValue`
327322

328-
val newLen = math.max(targetLen, math.max(arrayLen * 2, DefaultInitialSize))
329-
math.min(newLen, scala.runtime.PStatics.VM_MaxArraySize).toInt
323+
math.min(
324+
if (targetLen > (Int.MaxValue >> 1)) VM_MaxArraySize
325+
else math.max(targetLen, math.max(arrayLen << 1, DefaultInitialSize)),
326+
VM_MaxArraySize
327+
)
330328
}
331329
}
330+
332331
// if necessary, copy (curSize elements of) the array to a new array of capacity n.
333332
// Should use Array.copyOf(array, resizeEnsuring(array.length))?
334-
private def ensureSize(array: Array[AnyRef], curSize: Int, targetSize: Long): Array[AnyRef] = {
333+
private def ensureSize(array: Array[AnyRef], curSize: Int, targetSize: Int): Array[AnyRef] = {
335334
val newLen = resizeUp(array.length, targetSize)
336335
if (newLen < 0) array
337336
else {

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package scala.collection
1414
package mutable
1515

16+
import scala.collection.mutable.ArrayBuffer.resizeUp
1617
import scala.reflect.ClassTag
1718

1819
/** A builder class for arrays.
@@ -34,15 +35,11 @@ sealed abstract class ArrayBuilder[T]
3435
override def knownSize: Int = size
3536

3637
protected[this] final def ensureSize(size: Int): Unit = {
37-
if (capacity < size || capacity == 0) {
38-
var newsize = if (capacity == 0) 16 else capacity * 2
39-
while (newsize < size) newsize *= 2
40-
resize(newsize)
41-
}
38+
val newLen = resizeUp(capacity, size)
39+
if (newLen > 0) resize(newLen)
4240
}
4341

44-
override final def sizeHint(size: Int): Unit =
45-
if (capacity < size) resize(size)
42+
override final def sizeHint(size: Int): Unit = if (capacity < size) resize(size)
4643

4744
def clear(): Unit = size = 0
4845

@@ -491,17 +488,23 @@ object ArrayBuilder {
491488
protected def elems: Array[Unit] = throw new UnsupportedOperationException()
492489

493490
def addOne(elem: Unit): this.type = {
494-
size += 1
491+
val newSize:Int = size + 1
492+
ensureSize(newSize)
493+
size = newSize
495494
this
496495
}
497496

498497
override def addAll(xs: IterableOnce[Unit]): this.type = {
499-
size += xs.iterator.size
498+
val newSize:Int = size + xs.iterator.size
499+
ensureSize(newSize)
500+
size = newSize
500501
this
501502
}
502503

503504
override def addAll(xs: Array[_ <: Unit], offset: Int, length: Int): this.type = {
504-
size += length
505+
val newSize: Int = size + length
506+
ensureSize(newSize)
507+
size = newSize
505508
this
506509
}
507510

@@ -522,3 +525,4 @@ object ArrayBuilder {
522525
override def toString = "ArrayBuilder.ofUnit"
523526
}
524527
}
528+

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ sealed class PriorityQueue[A](implicit val ord: Ordering[A])
8989
def p_size0_=(s: Int) = size0 = s
9090
def p_array = array
9191
def p_ensureSize(n: Int) = super.ensureSize(n)
92-
def p_ensureAdditionalSize(n: Int) = super.ensureAdditionalSize(n)
92+
def p_ensureAdditionalSize(n: Int) = super.ensureSize(size0 + n)
9393
def p_swap(a: Int, b: Int): Unit = {
9494
val h = array(a)
9595
array(a) = array(b)

library/src/scala/runtime/PStatics.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ package scala.runtime
1515
// things that should be in `Statics`, but can't be yet for bincompat reasons
1616
// TODO 3.T: move to `Statics`
1717
private[scala] object PStatics {
18-
final val VM_MaxArraySize = 2147483645 // == `Int.MaxValue - 2`, hotspot limit
18+
final val VM_MaxArraySize:Int = 2147483639 // `Int.MaxValue - 8` traditional soft limit to maximize compatibility with diverse JVMs.
1919
}

0 commit comments

Comments
 (0)