From 2139ce31a39e4de1678821966862396ab03cb6f1 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 15 May 2025 11:49:47 -0700 Subject: [PATCH 01/13] Reduce StringBuffer in Regex --- library/src/scala/util/matching/Regex.scala | 35 ++++++++++----------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/library/src/scala/util/matching/Regex.scala b/library/src/scala/util/matching/Regex.scala index c19bc2a925b1..a90171243e3a 100644 --- a/library/src/scala/util/matching/Regex.scala +++ b/library/src/scala/util/matching/Regex.scala @@ -508,9 +508,10 @@ class Regex private[matching](val pattern: Pattern, groupNames: String*) extends * @return The target string after replacements. */ def replaceAllIn(target: CharSequence, replacer: Match => String): String = { - val it = new Regex.MatchIterator(target, this, groupNames).replacementData - it foreach (md => it replace replacer(md)) - it.replaced + val rit = new Regex.MatchIterator(target, this, groupNames).replacementData + for (matchdata <- rit; replacement = replacer(matchdata)) + rit.replace(replacement) + rit.replaced } /** @@ -535,11 +536,10 @@ class Regex private[matching](val pattern: Pattern, groupNames: String*) extends * @return The target string after replacements. */ def replaceSomeIn(target: CharSequence, replacer: Match => Option[String]): String = { - val it = new Regex.MatchIterator(target, this, groupNames).replacementData - for (matchdata <- it ; replacement <- replacer(matchdata)) - it replace replacement - - it.replaced + val rit = new Regex.MatchIterator(target, this, groupNames).replacementData + for (matchdata <- rit; replacement <- replacer(matchdata)) + rit.replace(replacement) + rit.replaced } /** Replaces the first match by a string. @@ -874,28 +874,27 @@ object Regex { /** Convert to an iterator that yields MatchData elements instead of Strings and has replacement support. */ private[matching] def replacementData = new AbstractIterator[Match] with Replacement { - def matcher = self.matcher + protected def matcher = self.matcher def hasNext = self.hasNext - def next() = { self.next(); new Match(source, matcher, _groupNames).force } + def next(): Match = { self.next(); new Match(source, matcher, _groupNames).force } } } - /** - * A trait able to build a string with replacements assuming it has a matcher. - * Meant to be mixed in with iterators. + /** Internal trait used by `replaceAllIn` and `replaceSomeIn`. */ private[matching] trait Replacement { protected def matcher: Matcher - private[this] val sb = new java.lang.StringBuffer + private[this] val sb = new java.lang.StringBuffer // StringBuffer for JDK 8 compatibility + // Appends the remaining input and returns the result text. def replaced = { - val newsb = new java.lang.StringBuffer(sb) - matcher.appendTail(newsb) - newsb.toString + matcher.appendTail(sb) + sb.toString } - def replace(rs: String) = matcher.appendReplacement(sb, rs) + // Appends the input prefix and the replacement text. + def replace(replacement: String) = matcher.appendReplacement(sb, replacement) } /** Quotes strings to be used literally in regex patterns. From e361894d2a7ad1d8d1348594f54a0a992190cc0e Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 16 May 2025 23:11:16 +0200 Subject: [PATCH 02/13] Comment on private ListBuffer.locate --- library/src/scala/collection/mutable/ListBuffer.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/library/src/scala/collection/mutable/ListBuffer.scala b/library/src/scala/collection/mutable/ListBuffer.scala index 273704592abd..118a42b53828 100644 --- a/library/src/scala/collection/mutable/ListBuffer.scala +++ b/library/src/scala/collection/mutable/ListBuffer.scala @@ -186,6 +186,7 @@ class ListBuffer[A] last0 = null } + // returns the `::` at `i - 1` (such that its `next` at position `i` can be mutated), or `null` if `i == 0`. private def locate(i: Int): Predecessor[A] = if (i == 0) null else if (i == len) last0 From 49b6967c9a245d2be7c094c4b106fbc35b4254da Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 19 May 2025 07:20:18 -0700 Subject: [PATCH 03/13] ListBuffer Predecessor --- .../scala/collection/mutable/ListBuffer.scala | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/library/src/scala/collection/mutable/ListBuffer.scala b/library/src/scala/collection/mutable/ListBuffer.scala index 118a42b53828..3edb286a9943 100644 --- a/library/src/scala/collection/mutable/ListBuffer.scala +++ b/library/src/scala/collection/mutable/ListBuffer.scala @@ -51,7 +51,7 @@ class ListBuffer[A] private[this] var aliased = false private[this] var len = 0 - private type Predecessor[A0] = ::[A0] /*| Null*/ + private type Predecessor = ::[A] /*| Null*/ def iterator: Iterator[A] = new MutationTracker.CheckedIterator(first.iterator, mutationCount) @@ -187,7 +187,7 @@ class ListBuffer[A] } // returns the `::` at `i - 1` (such that its `next` at position `i` can be mutated), or `null` if `i == 0`. - private def locate(i: Int): Predecessor[A] = + private def locate(i: Int): Predecessor = if (i == 0) null else if (i == len) last0 else { @@ -197,10 +197,10 @@ class ListBuffer[A] p = p.tail j -= 1 } - p.asInstanceOf[Predecessor[A]] + p.asInstanceOf[Predecessor] } - private def getNext(p: Predecessor[A]): List[A] = + private def getNext(p: Predecessor): List[A] = if (p == null) first else p.next def update(idx: Int, elem: A): Unit = { @@ -241,7 +241,7 @@ class ListBuffer[A] } // `fresh` must be a `ListBuffer` that only we have access to - private def insertAfter(prev: Predecessor[A], fresh: ListBuffer[A]): Unit = { + private def insertAfter(prev: Predecessor, fresh: ListBuffer[A]): Unit = { if (!fresh.isEmpty) { val follow = getNext(prev) if (prev eq null) first = fresh.first else prev.next = fresh.first @@ -289,7 +289,7 @@ class ListBuffer[A] throw new IllegalArgumentException("removing negative number of elements: " + count) } - private def removeAfter(prev: Predecessor[A], n: Int) = { + private def removeAfter(prev: Predecessor, n: Int) = { @tailrec def ahead(p: List[A], n: Int): List[A] = if (n == 0) p else ahead(p.tail, n - 1) val nx = ahead(getNext(prev), n) @@ -346,7 +346,7 @@ class ListBuffer[A] */ def filterInPlace(p: A => Boolean): this.type = { ensureUnaliased() - var prev: Predecessor[A] = null + var prev: Predecessor = null var cur: List[A] = first while (!cur.isEmpty) { val follow = cur.tail @@ -355,7 +355,7 @@ class ListBuffer[A] else prev.next = follow len -= 1 } else { - prev = cur.asInstanceOf[Predecessor[A]] + prev = cur.asInstanceOf[Predecessor] } cur = follow } From 9d4c513ff9200bd6413848604c83880484a9953d Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 19 May 2025 07:30:28 -0700 Subject: [PATCH 04/13] Naming guess --- .../src/scala/collection/mutable/ListBuffer.scala | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/src/scala/collection/mutable/ListBuffer.scala b/library/src/scala/collection/mutable/ListBuffer.scala index 3edb286a9943..241f1edc480b 100644 --- a/library/src/scala/collection/mutable/ListBuffer.scala +++ b/library/src/scala/collection/mutable/ListBuffer.scala @@ -187,7 +187,7 @@ class ListBuffer[A] } // returns the `::` at `i - 1` (such that its `next` at position `i` can be mutated), or `null` if `i == 0`. - private def locate(i: Int): Predecessor = + private def predecessor(i: Int): Predecessor = if (i == 0) null else if (i == len) last0 else { @@ -214,7 +214,7 @@ class ListBuffer[A] first = newElem } else { // `p` can not be `null` because the case where `idx == 0` is handled above - val p = locate(idx) + val p = predecessor(idx) val newElem = new :: (elem, p.tail.tail) if (last0 eq p.tail) { last0 = newElem @@ -228,7 +228,7 @@ class ListBuffer[A] if (idx < 0 || idx > len) throw CommonErrors.indexOutOfBounds(index = idx, max = len - 1) if (idx == len) addOne(elem) else { - val p = locate(idx) + val p = predecessor(idx) val nx = elem :: getNext(p) if(p eq null) first = nx else p.next = nx len += 1 @@ -259,7 +259,7 @@ class ListBuffer[A] else { val fresh = new ListBuffer[A].freshFrom(it) ensureUnaliased() - insertAfter(locate(idx), fresh) + insertAfter(predecessor(idx), fresh) } } } @@ -267,7 +267,7 @@ class ListBuffer[A] def remove(idx: Int): A = { ensureUnaliased() if (idx < 0 || idx >= len) throw CommonErrors.indexOutOfBounds(index = idx, max = len - 1) - val p = locate(idx) + val p = predecessor(idx) val nx = getNext(p) if(p eq null) { first = nx.tail @@ -284,7 +284,7 @@ class ListBuffer[A] if (count > 0) { ensureUnaliased() if (idx < 0 || idx + count > len) throw new IndexOutOfBoundsException(s"$idx to ${idx + count} is out of bounds (min 0, max ${len - 1})") - removeAfter(locate(idx), count) + removeAfter(predecessor(idx), count) } else if (count < 0) { throw new IllegalArgumentException("removing negative number of elements: " + count) } @@ -379,7 +379,7 @@ class ListBuffer[A] ensureUnaliased() val i = math.min(_from, _len) val n = math.min(_replaced, _len) - val p = locate(i) + val p = predecessor(i) removeAfter(p, math.min(n, _len - i)) insertAfter(p, fresh) } From ad91afc3294d52ba679bb1c8434cc30706784ad9 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 14 May 2025 18:23:01 -0700 Subject: [PATCH 05/13] Use List.fill instead of range --- library/src/scala/collection/Factory.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/src/scala/collection/Factory.scala b/library/src/scala/collection/Factory.scala index 595134eb54e4..4a05e6ce23bd 100644 --- a/library/src/scala/collection/Factory.scala +++ b/library/src/scala/collection/Factory.scala @@ -146,10 +146,10 @@ trait IterableFactory[+CC[_]] extends Serializable { def newBuilder[A]: Builder[A, CC[A]] /** Produces a $coll containing the results of some element computation a number of times. - * @param n the number of elements contained in the $coll. - * @param elem the element computation - * @return A $coll that contains the results of `n` evaluations of `elem`. - */ + * @param n the number of elements contained in the $coll. + * @param elem the element computation + * @return A $coll that contains the results of `n` evaluations of `elem`. + */ def fill[A](n: Int)(elem: => A): CC[A] = from(new View.Fill(n)(elem)) /** Produces a two-dimensional $coll containing the results of some element computation a number of times. From 71eb4a029da304632bfdae262531f9c86e18d342 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 20 May 2025 00:09:27 -0700 Subject: [PATCH 06/13] Move example doc --- library/src/scala/collection/IterableOnce.scala | 4 +++- library/src/scala/collection/immutable/List.scala | 11 ----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/library/src/scala/collection/IterableOnce.scala b/library/src/scala/collection/IterableOnce.scala index 36e71277604a..71bac9ca0052 100644 --- a/library/src/scala/collection/IterableOnce.scala +++ b/library/src/scala/collection/IterableOnce.scala @@ -435,6 +435,8 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] => * @return a $coll containing the elements greater than or equal to * index `from` extending up to (but not including) index `until` * of this $coll. + * @example + * `List('a', 'b', 'c', 'd', 'e').slice(1, 3) == List('b', 'c')` */ def slice(from: Int, until: Int): C @@ -1241,7 +1243,7 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] => * @param pf the partial function * @return an option value containing pf applied to the first * value for which it is defined, or `None` if none exists. - * @example `Seq("a", 1, 5L).collectFirst({ case x: Int => x*10 }) = Some(10)` + * @example `Seq("a", 1, 5L).collectFirst { case x: Int => x*10 } = Some(10)` */ def collectFirst[B](pf: PartialFunction[A, B]): Option[B] = { // Presumably the fastest way to get in and out of a partial function is for a sentinel function to return itself diff --git a/library/src/scala/collection/immutable/List.scala b/library/src/scala/collection/immutable/List.scala index d6651f417103..cee22bcc6d54 100644 --- a/library/src/scala/collection/immutable/List.scala +++ b/library/src/scala/collection/immutable/List.scala @@ -186,17 +186,6 @@ sealed abstract class List[+A] h } - /** @inheritdoc - * - * @example {{{ - * // Given a list - * val letters = List('a','b','c','d','e') - * - * // `slice` returns all elements beginning at index `from` and afterwards, - * // up until index `until` (excluding index `until`.) - * letters.slice(1,3) // Returns List('b','c') - * }}} - */ override def slice(from: Int, until: Int): List[A] = { val lo = scala.math.max(from, 0) if (until <= lo || isEmpty) Nil From cce867061621ef6cc820e1b01c070b0269313e6b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 14 Feb 2024 16:56:23 -0800 Subject: [PATCH 07/13] IterableOnce#copyToArray uses helper for count --- library/src/scala/collection/IterableOnce.scala | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/library/src/scala/collection/IterableOnce.scala b/library/src/scala/collection/IterableOnce.scala index 71bac9ca0052..5bed6e304bce 100644 --- a/library/src/scala/collection/IterableOnce.scala +++ b/library/src/scala/collection/IterableOnce.scala @@ -273,7 +273,9 @@ object IterableOnce { * @return the number of elements that will be copied to the destination array */ @inline private[collection] def elemsToCopyToArray(srcLen: Int, destLen: Int, start: Int, len: Int): Int = - math.max(math.min(math.min(len, srcLen), destLen - start), 0) + math.max(0, + math.min(if (start < 0) destLen else destLen - start, + math.min(len, srcLen))) /** Calls `copyToArray` on the given collection, regardless of whether or not it is an `Iterable`. */ @inline private[collection] def copyElemsToArray[A, B >: A](elems: IterableOnce[A], @@ -1035,7 +1037,11 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] => def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = { val it = iterator var i = start - val end = start + math.min(len, xs.length - start) + val srclen = knownSize match { + case -1 => xs.length + case k => k + } + val end = start + IterableOnce.elemsToCopyToArray(srclen, xs.length, start, len) while (i < end && it.hasNext) { xs(i) = it.next() i += 1 From 6dde5b04784c77cdfca0f3a5f0e620264f070f2a Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 12 May 2025 12:20:36 -0700 Subject: [PATCH 08/13] Improve param names --- .../src/scala/collection/IterableOnce.scala | 71 ++++++++++++------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/library/src/scala/collection/IterableOnce.scala b/library/src/scala/collection/IterableOnce.scala index 5bed6e304bce..6e1345031fe3 100644 --- a/library/src/scala/collection/IterableOnce.scala +++ b/library/src/scala/collection/IterableOnce.scala @@ -21,6 +21,8 @@ import scala.math.{Numeric, Ordering} import scala.reflect.ClassTag import scala.runtime.{AbstractFunction1, AbstractFunction2} +import IterableOnce.elemsToCopyToArray + /** * A template trait for collections which can be traversed either once only * or one or more times. @@ -264,18 +266,25 @@ object IterableOnce { @inline implicit def iterableOnceExtensionMethods[A](it: IterableOnce[A]): IterableOnceExtensionMethods[A] = new IterableOnceExtensionMethods[A](it) - /** Computes the number of elements to copy to an array from a source IterableOnce - * - * @param srcLen the length of the source collection - * @param destLen the length of the destination array - * @param start the index in the destination array at which to start copying elements to - * @param len the requested number of elements to copy (we may only be able to copy less than this) - * @return the number of elements that will be copied to the destination array - */ - @inline private[collection] def elemsToCopyToArray(srcLen: Int, destLen: Int, start: Int, len: Int): Int = - math.max(0, - math.min(if (start < 0) destLen else destLen - start, - math.min(len, srcLen))) + /** Computes the number of elements to copy to an array from a source IterableOnce. + * + * If `start` is less than zero, it is taken as zero. + * If any of the length inputs is less than zero, the computed result is zero. + * + * The result is the smaller of the remaining capacity in the destination and the requested count. + * + * @param srcLen the length of the source collection + * @param destLen the length of the destination array + * @param start the index in the destination array at which to start copying elements + * @param len the requested number of elements to copy (we may only be able to copy less than this) + * @return the number of elements that will be copied to the destination array + */ + @inline private[collection] def elemsToCopyToArray(srcLen: Int, destLen: Int, start: Int, len: Int): Int = { + val limit = math.min(len, srcLen) + val capacity = if (start < 0) destLen else destLen - start + val total = math.min(capacity, limit) + math.max(0, total) + } /** Calls `copyToArray` on the given collection, regardless of whether or not it is an `Iterable`. */ @inline private[collection] def copyElemsToArray[A, B >: A](elems: IterableOnce[A], @@ -988,28 +997,29 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] => /** Copies elements to an array, returning the number of elements written. * - * Fills the given array `xs` starting at index `start` with values of this $coll. + * Fills the given array `dest` starting at index `start` with values of this $coll. * * Copying will stop once either all the elements of this $coll have been copied, * or the end of the array is reached. * - * @param xs the array to fill. + * @param dest the array to fill. * @tparam B the type of the elements of the array. * @return the number of elements written to the array * * @note Reuse: $consumesIterator */ @deprecatedOverriding("This should always forward to the 3-arg version of this method", since = "2.13.4") - def copyToArray[B >: A](xs: Array[B]): Int = copyToArray(xs, 0, Int.MaxValue) + def copyToArray[B >: A](@deprecatedName("xs", since="2.13.17") dest: Array[B]): Int = + copyToArray(dest, start = 0, n = Int.MaxValue) /** Copies elements to an array, returning the number of elements written. * - * Fills the given array `xs` starting at index `start` with values of this $coll. + * Fills the given array `dest` starting at index `start` with values of this $coll. * * Copying will stop once either all the elements of this $coll have been copied, * or the end of the array is reached. * - * @param xs the array to fill. + * @param dest the array to fill. * @param start the starting index of xs. * @tparam B the type of the elements of the array. * @return the number of elements written to the array @@ -1017,33 +1027,40 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] => * @note Reuse: $consumesIterator */ @deprecatedOverriding("This should always forward to the 3-arg version of this method", since = "2.13.4") - def copyToArray[B >: A](xs: Array[B], start: Int): Int = copyToArray(xs, start, Int.MaxValue) + def copyToArray[B >: A](@deprecatedName("xs", since="2.13.17") dest: Array[B], start: Int): Int = + copyToArray(dest, start = start, n = Int.MaxValue) - /** Copy elements to an array, returning the number of elements written. + /** Copies elements to an array and returns the number of elements written. * - * Fills the given array `xs` starting at index `start` with at most `len` elements of this $coll. + * Fills the given array `dest` starting at index `start` with at most `n` elements of this $coll. * * Copying will stop once either all the elements of this $coll have been copied, - * or the end of the array is reached, or `len` elements have been copied. + * or the end of the array is reached, or `n` elements have been copied. + * + * If `start` is less than zero, it is taken as zero. * - * @param xs the array to fill. + * @param dest the array to fill. * @param start the starting index of xs. - * @param len the maximal number of elements to copy. + * @param n the maximal number of elements to copy. * @tparam B the type of the elements of the array. * @return the number of elements written to the array * * @note Reuse: $consumesIterator */ - def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = { + def copyToArray[B >: A]( + @deprecatedName("xs", since="2.13.17") dest: Array[B], + start: Int, + @deprecatedName("len", since="2.13.17") n: Int + ): Int = { val it = iterator var i = start val srclen = knownSize match { - case -1 => xs.length + case -1 => dest.length case k => k } - val end = start + IterableOnce.elemsToCopyToArray(srclen, xs.length, start, len) + val end = start + elemsToCopyToArray(srclen, dest.length, start, n) while (i < end && it.hasNext) { - xs(i) = it.next() + dest(i) = it.next() i += 1 } i - start From 5b76ce98b0441e1cd32b27d83ba7c943dd6c014c Mon Sep 17 00:00:00 2001 From: Emil Ejbyfeldt Date: Fri, 27 Jun 2025 10:44:35 +0200 Subject: [PATCH 09/13] Fix ArraySeq.ofFloat/ofDouble.equals for floating point This fixes scala/scala#13108 when comparing Seq[Float] with Seq[Float] it expected that Float.NaN != Float.NaN and 0.0 equals -0.0f. But before this patch this was violated by ArraySeq.ofFloat/ofDouble --- .../scala/collection/immutable/ArraySeq.scala | 18 ++++++++++++++++-- .../scala/collection/mutable/ArraySeq.scala | 16 ++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/library/src/scala/collection/immutable/ArraySeq.scala b/library/src/scala/collection/immutable/ArraySeq.scala index eafe9baa719f..55cfa4baa7ec 100644 --- a/library/src/scala/collection/immutable/ArraySeq.scala +++ b/library/src/scala/collection/immutable/ArraySeq.scala @@ -575,7 +575,14 @@ object ArraySeq extends StrictOptimizedClassTagSeqFactory[ArraySeq] { self => def apply(i: Int): Float = unsafeArray(i) override def hashCode = MurmurHash3.arraySeqHash(unsafeArray) override def equals(that: Any) = that match { - case that: ofFloat => Arrays.equals(unsafeArray, that.unsafeArray) + case that: ofFloat => + val array = unsafeArray + val thatArray = that.unsafeArray + (array eq thatArray) || array.length == thatArray.length && { + var i = 0 + while (i < array.length && array(i) == thatArray(i)) i += 1 + i >= array.length + } case _ => super.equals(that) } override def iterator: Iterator[Float] = new ArrayOps.ArrayIterator[Float](unsafeArray) @@ -610,7 +617,14 @@ object ArraySeq extends StrictOptimizedClassTagSeqFactory[ArraySeq] { self => def apply(i: Int): Double = unsafeArray(i) override def hashCode = MurmurHash3.arraySeqHash(unsafeArray) override def equals(that: Any) = that match { - case that: ofDouble => Arrays.equals(unsafeArray, that.unsafeArray) + case that: ofDouble => + val array = unsafeArray + val thatArray = that.unsafeArray + (array eq thatArray) || array.length == thatArray.length && { + var i = 0 + while (i < array.length && array(i) == thatArray(i)) i += 1 + i >= array.length + } case _ => super.equals(that) } override def iterator: Iterator[Double] = new ArrayOps.ArrayIterator[Double](unsafeArray) diff --git a/library/src/scala/collection/mutable/ArraySeq.scala b/library/src/scala/collection/mutable/ArraySeq.scala index 0537092d0b13..dee7bf4f2dd0 100644 --- a/library/src/scala/collection/mutable/ArraySeq.scala +++ b/library/src/scala/collection/mutable/ArraySeq.scala @@ -286,7 +286,13 @@ object ArraySeq extends StrictOptimizedClassTagSeqFactory[ArraySeq] { self => def update(index: Int, elem: Float): Unit = { array(index) = elem } override def hashCode = MurmurHash3.arraySeqHash(array) override def equals(that: Any) = that match { - case that: ofFloat => Arrays.equals(array, that.array) + case that: ofFloat => + val thatArray = that.array + (array eq thatArray) || array.length == thatArray.length && { + var i = 0 + while (i < array.length && array(i) == thatArray(i)) i += 1 + i >= array.length + } case _ => super.equals(that) } override def iterator: Iterator[Float] = new ArrayOps.ArrayIterator[Float](array) @@ -306,7 +312,13 @@ object ArraySeq extends StrictOptimizedClassTagSeqFactory[ArraySeq] { self => def update(index: Int, elem: Double): Unit = { array(index) = elem } override def hashCode = MurmurHash3.arraySeqHash(array) override def equals(that: Any) = that match { - case that: ofDouble => Arrays.equals(array, that.array) + case that: ofDouble => + val thatArray = that.array + (array eq thatArray) || array.length == thatArray.length && { + var i = 0 + while (i < array.length && array(i) == thatArray(i)) i += 1 + i >= array.length + } case _ => super.equals(that) } override def iterator: Iterator[Double] = new ArrayOps.ArrayIterator[Double](array) From c583aa3eb3fb2fdd450e91b3313f3d2abd4cd8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 7 Aug 2025 13:42:29 +0200 Subject: [PATCH 10/13] Fewer (and more predictable) branches in Range.isEmpty. Extract branches to be mostly dependent on `isInclusive` and `step >= 0`. These are often constant, and when they're not statically constant, they are probably predictable anyway. This commit upstreams https://github.com/scala-js/scala-js/commit/a5337ed336dcd831de330647f63048f4df6dda0d from Scala.js. --- library/src/scala/collection/immutable/Range.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/library/src/scala/collection/immutable/Range.scala b/library/src/scala/collection/immutable/Range.scala index 9a35153ace64..cd791f0faee3 100644 --- a/library/src/scala/collection/immutable/Range.scala +++ b/library/src/scala/collection/immutable/Range.scala @@ -91,10 +91,11 @@ sealed abstract class Range( def isInclusive: Boolean final override val isEmpty: Boolean = ( - (start > end && step > 0) - || (start < end && step < 0) - || (start == end && !isInclusive) - ) + if (isInclusive) + (if (step >= 0) start > end else start < end) + else + (if (step >= 0) start >= end else start <= end) + ) private[this] val numRangeElements: Int = { if (step == 0) throw new IllegalArgumentException("step cannot be 0.") From e8534eff58bbb77605f3d832a775543b47e2424f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 7 Aug 2025 13:52:53 +0200 Subject: [PATCH 11/13] Use unsigned arithmetics in Range, instead of Longs. Previously, `Range` used a number of intermediate operations on `Long`s to avoid overflow. We can streamline a lot of code by using unsigned `Int` arithmetics. In particular, there is only 1 division in the initialization path, instead of 3. Although the fields have not changed, the content of `numRangeElements` is more strict for overfull ranges. This means that deserializing an overfull range from a previous version would not be safe. This is why we bump the SerialVersionUID. This commit upstreams https://github.com/scala-js/scala-js/commit/d9722185beae7b66772b3bf9f91894a781613fe1 from Scala.js. --- .../scala/collection/immutable/Range.scala | 216 +++++++++++------- 1 file changed, 136 insertions(+), 80 deletions(-) diff --git a/library/src/scala/collection/immutable/Range.scala b/library/src/scala/collection/immutable/Range.scala index cd791f0faee3..6cc1ea09e2cb 100644 --- a/library/src/scala/collection/immutable/Range.scala +++ b/library/src/scala/collection/immutable/Range.scala @@ -57,7 +57,7 @@ import scala.util.hashing.MurmurHash3 * '''Note:''' this method does not use builders to construct a new range, * and its complexity is O(1). */ -@SerialVersionUID(3L) +@SerialVersionUID(4L) sealed abstract class Range( val start: Int, val end: Int, @@ -83,11 +83,6 @@ sealed abstract class Range( r.asInstanceOf[S with EfficientSplit] } - private[this] def gap = end.toLong - start.toLong - private[this] def isExact = gap % step == 0 - private[this] def hasStub = isInclusive || !isExact - private[this] def longLength = gap / step + ( if (hasStub) 1 else 0 ) - def isInclusive: Boolean final override val isEmpty: Boolean = ( @@ -97,27 +92,90 @@ sealed abstract class Range( (if (step >= 0) start >= end else start <= end) ) + if (step == 0) throw new IllegalArgumentException("step cannot be 0.") + + /** Number of elements in this range, if it is non-empty. + * + * If the range is empty, `numRangeElements` does not have a meaningful value. + * + * Otherwise, `numRangeElements` is interpreted in the range [1, 2^32], + * respecting modular arithmetics wrt. the unsigned interpretation. + * In other words, it is 0 if the mathematical value should be 2^32, and the + * standard unsigned int encoding of the mathematical value otherwise. + * + * This interpretation allows to represent all values with the correct + * modular arithmetics, which streamlines the usage sites. + */ private[this] val numRangeElements: Int = { - if (step == 0) throw new IllegalArgumentException("step cannot be 0.") - else if (isEmpty) 0 - else { - val len = longLength - if (len > scala.Int.MaxValue) -1 - else len.toInt - } + val stepSign = step >> 31 // if (step >= 0) 0 else -1 + val gap = ((end - start) ^ stepSign) - stepSign // if (step >= 0) (end - start) else -(end - start) + val absStep = (step ^ stepSign) - stepSign // if (step >= 0) step else -step + + /* If `absStep` is a constant 1, `div` collapses to being an alias of + * `gap`. Then `absStep * div` also collapses to `gap` and therefore + * `absStep * div != gap` constant-folds to `false`. + * + * Since most ranges are exclusive, that makes `numRangeElements` an alias + * of `gap`. Moreover, for exclusive ranges with step 1 and start 0 (which + * are the common case), it makes it an alias of `end` and the entire + * computation goes away. + */ + val div = Integer.divideUnsigned(gap, absStep) + if (isInclusive || (absStep * div != gap)) div + 1 else div } - final def length = if (numRangeElements < 0) fail() else numRangeElements + final def length: Int = + if (isEmpty) 0 + else if (numRangeElements > 0) numRangeElements + else fail() + + /** Computes the element of this range after `n` steps from `start`. + * + * `n` is interpreted as an unsigned integer. + * + * If the mathematical result is not within this Range, the result won't + * make sense, but won't error out. + */ + @inline + private[this] def locationAfterN(n: Int): Int = { + /* If `step >= 0`, we interpret `step * n` as an unsigned multiplication, + * and the addition as a mixed `(signed, unsigned) -> signed` operation. + * With those interpretation, they do not overflow, assuming the + * mathematical result is within this Range. + * + * If `step < 0`, we should compute `start - (-step * n)`, with the + * multiplication also interpreted as unsigned, and the subtraction as + * mixed. Again, using those interpretations, they do not overflow. + * But then modular arithmetics allow us to cancel out the two `-` signs, + * so we end up with the same formula. + */ + start + (step * n) + } - // This field has a sensible value only for non-empty ranges - private[this] val lastElement = step match { - case 1 => if (isInclusive) end else end-1 - case -1 => if (isInclusive) end else end+1 - case _ => - val remainder = (gap % step).toInt - if (remainder != 0) end - remainder - else if (isInclusive) end - else end - step + /** Last element of this non-empty range. + * + * For empty ranges, this value is nonsensical. + */ + private[this] val lastElement: Int = { + /* Since we can assume the range is non-empty, `(numRangeElements - 1)` + * is a valid unsigned value in the full int range. The general formula is + * therefore `locationAfterN(numRangeElements - 1)`. + * + * We special-case 1 and -1 so that, in the happy path where `step` is a + * constant 1 or -1, and we only use `foreach`, `numRangeElements` is dead + * code. + * + * When `step` is not constant, it is probably 1 or -1 anyway, so the + * single branch should be predictably true. + * + * `step == 1 || step == -1` + * equiv `(step + 1 == 2) || (step + 1 == 0)` + * equiv `((step + 1) & ~2) == 0` + */ + if (((step + 1) & ~2) == 0) + (if (isInclusive) end else end - step) + else + locationAfterN(numRangeElements - 1) } /** The last element of this range. This method will return the correct value @@ -171,7 +229,7 @@ sealed abstract class Range( // which means it will not fail fast for those cases where failing was // correct. private[this] def validateMaxLength(): Unit = { - if (numRangeElements < 0) + if (numRangeElements <= 0 && !isEmpty) fail() } private[this] def description = "%d %s %d by %s".format(start, if (isInclusive) "to" else "until", end, step) @@ -179,10 +237,14 @@ sealed abstract class Range( @throws[IndexOutOfBoundsException] final def apply(idx: Int): Int = { - validateMaxLength() - if (idx < 0 || idx >= numRangeElements) - throw CommonErrors.indexOutOfBounds(index = idx, max = numRangeElements - 1) - else start + (step * idx) + /* If length is not valid, numRangeElements <= 0, so the condition is always true. + * We push validateMaxLength() inside the then branch, out of the happy path. + */ + if (idx < 0 || idx >= numRangeElements || isEmpty) { + validateMaxLength() + val max = if (isEmpty) -1 else numRangeElements - 1 + throw CommonErrors.indexOutOfBounds(index = idx, max = max) + } else locationAfterN(idx) } /*@`inline`*/ final override def foreach[@specialized(Unit) U](f: Int => U): Unit = { @@ -230,6 +292,14 @@ sealed abstract class Range( case _ => super.sameElements(that) } + /** Is the non-negative value `n` greater or equal to the number of elements + * in this non-empty range? + * + * This method returns nonsensical results if `n < 0` or if `this.isEmpty`. + */ + @inline private[this] def greaterEqualNumRangeElements(n: Int): Boolean = + (n ^ Int.MinValue) > ((numRangeElements - 1) ^ Int.MinValue) // unsigned comparison + /** Creates a new range containing the first `n` elements of this range. * * @param n the number of elements to take. @@ -237,12 +307,8 @@ sealed abstract class Range( */ final override def take(n: Int): Range = if (n <= 0 || isEmpty) newEmptyRange(start) - else if (n >= numRangeElements && numRangeElements >= 0) this - else { - // May have more than Int.MaxValue elements in range (numRangeElements < 0) - // but the logic is the same either way: take the first n - new Range.Inclusive(start, locationAfterN(n - 1), step) - } + else if (greaterEqualNumRangeElements(n)) this + else new Range.Inclusive(start, locationAfterN(n - 1), step) /** Creates a new range containing all the elements of this range except the first `n` elements. * @@ -251,27 +317,17 @@ sealed abstract class Range( */ final override def drop(n: Int): Range = if (n <= 0 || isEmpty) this - else if (n >= numRangeElements && numRangeElements >= 0) newEmptyRange(end) - else { - // May have more than Int.MaxValue elements (numRangeElements < 0) - // but the logic is the same either way: go forwards n steps, keep the rest - copy(locationAfterN(n), end, step) - } + else if (greaterEqualNumRangeElements(n)) newEmptyRange(end) + else copy(locationAfterN(n), end, step) /** Creates a new range consisting of the last `n` elements of the range. * * $doesNotUseBuilders */ final override def takeRight(n: Int): Range = { - if (n <= 0) newEmptyRange(start) - else if (numRangeElements >= 0) drop(numRangeElements - n) - else { - // Need to handle over-full range separately - val y = last - val x = y - step.toLong*(n-1) - if ((step > 0 && x < start) || (step < 0 && x > start)) this - else Range.inclusive(x.toInt, y, step) - } + if (n <= 0 || isEmpty) newEmptyRange(start) + else if (greaterEqualNumRangeElements(n)) this + else copy(locationAfterN(numRangeElements - n), end, step) } /** Creates a new range consisting of the initial `length - n` elements of the range. @@ -279,14 +335,9 @@ sealed abstract class Range( * $doesNotUseBuilders */ final override def dropRight(n: Int): Range = { - if (n <= 0) this - else if (numRangeElements >= 0) take(numRangeElements - n) - else { - // Need to handle over-full range separately - val y = last - step.toInt*n - if ((step > 0 && y < start) || (step < 0 && y > start)) newEmptyRange(start) - else Range.inclusive(start, y.toInt, step) - } + if (n <= 0 || isEmpty) this + else if (greaterEqualNumRangeElements(n)) newEmptyRange(end) + else Range.inclusive(start, locationAfterN(numRangeElements - 1 - n), step) } // Advance from the start while we meet the given test @@ -340,8 +391,9 @@ sealed abstract class Range( * @return a new range consisting of a contiguous interval of values in the old range */ final override def slice(from: Int, until: Int): Range = - if (from <= 0) take(until) - else if (until >= numRangeElements && numRangeElements >= 0) drop(from) + if (isEmpty) this + else if (from <= 0) take(until) + else if (greaterEqualNumRangeElements(until) && until >= 0) drop(from) else { val fromValue = locationAfterN(from) if (from >= until) newEmptyRange(fromValue) @@ -351,10 +403,6 @@ sealed abstract class Range( // Overridden only to refine the return type final override def splitAt(n: Int): (Range, Range) = (take(n), drop(n)) - // Methods like apply throw exceptions on invalid n, but methods like take/drop - // are forgiving: therefore the checks are with the methods. - private[this] def locationAfterN(n: Int) = start + (step * n) - // When one drops everything. Can't ever have unchecked operations // like "end + 1" or "end - 1" because ranges involving Int.{ MinValue, MaxValue } // will overflow. This creates an exclusive range where start == end @@ -374,13 +422,13 @@ sealed abstract class Range( else new Range.Inclusive(start, end, step) final def contains(x: Int): Boolean = { - if (x == end && !isInclusive) false + if (isEmpty) false else if (step > 0) { - if (x < start || x > end) false + if (x < start || x > lastElement) false else (step == 1) || (Integer.remainderUnsigned(x - start, step) == 0) } else { - if (x < end || x > start) false + if (x > start || x < lastElement) false else (step == -1) || (Integer.remainderUnsigned(start - x, -step) == 0) } } @@ -483,7 +531,12 @@ sealed abstract class Range( final override def toString: String = { val preposition = if (isInclusive) "to" else "until" val stepped = if (step == 1) "" else s" by $step" - val prefix = if (isEmpty) "empty " else if (!isExact) "inexact " else "" + + def isInexact = + if (isInclusive) lastElement != end + else (lastElement + step) != end + + val prefix = if (isEmpty) "empty " else if (isInexact) "inexact " else "" s"${prefix}Range $start $preposition $end$stepped" } @@ -543,16 +596,19 @@ object Range { if (isEmpty) 0 else { - // Counts with Longs so we can recognize too-large ranges. - val gap: Long = end.toLong - start.toLong - val jumps: Long = gap / step - // Whether the size of this range is one larger than the - // number of full-sized jumps. - val hasStub = isInclusive || (gap % step != 0) - val result: Long = jumps + ( if (hasStub) 1 else 0 ) - - if (result > scala.Int.MaxValue) -1 - else result.toInt + val stepSign = step >> 31 // if (step >= 0) 0 else -1 + val gap = ((end - start) ^ stepSign) - stepSign // if (step >= 0) (end - start) else -(end - start) + val absStep = (step ^ stepSign) - stepSign // if (step >= 0) step else -step + + val div = Integer.divideUnsigned(gap, absStep) + if (isInclusive) { + if (div == -1) // max unsigned int + -1 // corner case: there are 2^32 elements, which would overflow to 0 + else + div + 1 + } else { + if (absStep * div != gap) div + 1 else div + } } } def count(start: Int, end: Int, step: Int): Int = @@ -576,12 +632,12 @@ object Range { */ def inclusive(start: Int, end: Int): Range.Inclusive = new Range.Inclusive(start, end, 1) - @SerialVersionUID(3L) + @SerialVersionUID(4L) final class Inclusive(start: Int, end: Int, step: Int) extends Range(start, end, step) { def isInclusive: Boolean = true } - @SerialVersionUID(3L) + @SerialVersionUID(4L) final class Exclusive(start: Int, end: Int, step: Int) extends Range(start, end, step) { def isInclusive: Boolean = false } @@ -635,7 +691,7 @@ object Range { * @param lastElement The last element included in the Range * @param initiallyEmpty Whether the Range was initially empty or not */ -@SerialVersionUID(3L) +@SerialVersionUID(4L) private class RangeIterator( start: Int, step: Int, From 76fa46c90011d69a442ba5661016a982f48b964e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 12 Sep 2025 15:46:40 -0700 Subject: [PATCH 12/13] HashMap nodes support toString --- library/src/scala/collection/immutable/HashMap.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/src/scala/collection/immutable/HashMap.scala b/library/src/scala/collection/immutable/HashMap.scala index e9257f1948fc..915f27089b56 100644 --- a/library/src/scala/collection/immutable/HashMap.scala +++ b/library/src/scala/collection/immutable/HashMap.scala @@ -1345,6 +1345,8 @@ private final class BitmapIndexedMapNode[K, +V]( override def hashCode(): Int = throw new UnsupportedOperationException("Trie nodes do not support hashing.") + override def toString = s"${getClass.getName}@${Integer.toHexString(System.identityHashCode(this))}" + override def concat[V1 >: V](that: MapNode[K, V1], shift: Int): BitmapIndexedMapNode[K, V1] = that match { case bm: BitmapIndexedMapNode[K, V] @unchecked => if (size == 0) return bm @@ -2090,6 +2092,8 @@ private final class HashCollisionMapNode[K, +V ]( override def hashCode(): Int = throw new UnsupportedOperationException("Trie nodes do not support hashing.") + override def toString = s"${getClass.getName}@${Integer.toHexString(System.identityHashCode(this))}" + override def cachedJavaKeySetHashCode: Int = size * hash } From ca0420f4c96801e7c9290ec0167e31863fd36e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=8E=E9=B8=A3?= Date: Fri, 3 Jan 2025 03:47:36 +0800 Subject: [PATCH 13/13] Improve Await.result/ready performance for completed futures Avoid evaluating `value0` twice on completed Futures for performance. --- library/src/scala/concurrent/Future.scala | 12 +++++++--- .../src/scala/concurrent/impl/Promise.scala | 4 ++-- library/src/scala/concurrent/package.scala | 24 +++++++++++++++---- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/library/src/scala/concurrent/Future.scala b/library/src/scala/concurrent/Future.scala index 4142d8400200..2701ee46f369 100644 --- a/library/src/scala/concurrent/Future.scala +++ b/library/src/scala/concurrent/Future.scala @@ -574,6 +574,12 @@ object Future { private[this] final val _addToBuilderFun: (Builder[Any, Nothing], Any) => Builder[Any, Nothing] = (b: Builder[Any, Nothing], e: Any) => b += e private[concurrent] final def addToBuilderFun[A, M] = _addToBuilderFun.asInstanceOf[Function2[Builder[A, M], A, Builder[A, M]]] + private[concurrent] def waitUndefinedError(): Nothing = + throw new IllegalArgumentException("Cannot wait for Undefined duration of time") + + private[concurrent] def timeoutError(delay: Duration): Nothing = + throw new TimeoutException(s"Future timed out after [$delay]") + /** A Future which is never completed. */ object never extends Future[Nothing] { @@ -583,7 +589,7 @@ object Future { override final def ready(atMost: Duration)(implicit permit: CanAwait): this.type = { import Duration.{Undefined, Inf, MinusInf} atMost match { - case u if u eq Undefined => throw new IllegalArgumentException("cannot wait for Undefined period") + case u if u eq Undefined => waitUndefinedError() case `Inf` => while(!Thread.interrupted()) { LockSupport.park(this) @@ -603,14 +609,14 @@ object Future { case _: FiniteDuration => // Drop out if 0 or less case x: Duration.Infinite => throw new MatchError(x) } - throw new TimeoutException(s"Future timed out after [$atMost]") + timeoutError(atMost) } @throws[TimeoutException] @throws[InterruptedException] override final def result(atMost: Duration)(implicit permit: CanAwait): Nothing = { ready(atMost) - throw new TimeoutException(s"Future timed out after [$atMost]") + timeoutError(atMost) } override final def onComplete[U](f: Try[Nothing] => U)(implicit executor: ExecutionContext): Unit = () diff --git a/library/src/scala/concurrent/impl/Promise.scala b/library/src/scala/concurrent/impl/Promise.scala index 89f1addb8aa8..bf1da294aea7 100644 --- a/library/src/scala/concurrent/impl/Promise.scala +++ b/library/src/scala/concurrent/impl/Promise.scala @@ -255,9 +255,9 @@ private[concurrent] object Promise { l.result } if (r ne null) r - else throw new TimeoutException("Future timed out after [" + atMost + "]") + else Future.timeoutError(atMost) } - } else throw new IllegalArgumentException("Cannot wait for Undefined duration of time") + } else Future.waitUndefinedError() @throws(classOf[TimeoutException]) @throws(classOf[InterruptedException]) diff --git a/library/src/scala/concurrent/package.scala b/library/src/scala/concurrent/package.scala index d648a1c90a15..e3bbb119d340 100644 --- a/library/src/scala/concurrent/package.scala +++ b/library/src/scala/concurrent/package.scala @@ -12,8 +12,9 @@ package scala -import scala.concurrent.duration.Duration import scala.annotation.implicitNotFound +import scala.concurrent.duration.Duration +import scala.util.Try /** This package object contains primitives for concurrent and parallel programming. * @@ -170,8 +171,11 @@ package concurrent { @throws(classOf[TimeoutException]) @throws(classOf[InterruptedException]) final def ready[T](awaitable: Awaitable[T], atMost: Duration): awaitable.type = awaitable match { - case f: Future[T] if f.isCompleted => awaitable.ready(atMost)(AwaitPermission) - case _ => blocking(awaitable.ready(atMost)(AwaitPermission)) + case f: Future[T] if f.isCompleted => + if (atMost eq Duration.Undefined) Future.waitUndefinedError() // preserve semantics, see scala/scala#10972 + else awaitable + case _ => + blocking(awaitable.ready(atMost)(AwaitPermission)) } /** @@ -197,8 +201,18 @@ package concurrent { @throws(classOf[TimeoutException]) @throws(classOf[InterruptedException]) final def result[T](awaitable: Awaitable[T], atMost: Duration): T = awaitable match { - case f: Future[T] if f.isCompleted => f.result(atMost)(AwaitPermission) - case _ => blocking(awaitable.result(atMost)(AwaitPermission)) + case FutureValue(v) => + if (atMost eq Duration.Undefined) Future.waitUndefinedError() // preserve semantics, see scala/scala#10972 + else v.get + case _ => + blocking(awaitable.result(atMost)(AwaitPermission)) + } + + private object FutureValue { + def unapply[T](a: Awaitable[T]): Option[Try[T]] = a match { + case f: Future[T] => f.value + case _ => None + } } } }