Skip to content

Commit cdfcf96

Browse files
committed
Further performance improvements to groupBy
- Special case result sizes of 0-4 and avoid mutable.HashMap allocation. - Expose entriesIterator through a private[collection] method, rather than via subclassing, to avoid changing the JIT of code using mutable.HashMap that might currently rely on Class Heirarchy Analysis noticing that it effectively final. - Fuse some lambdas into `object grouper`.
1 parent 21a473b commit cdfcf96

File tree

6 files changed

+82
-24
lines changed

6 files changed

+82
-24
lines changed

build.sbt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,7 @@ val mimaFilterSettings = Seq {
492492
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.runtime.Settings.async"),
493493

494494
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.api.Internals#InternalApi.markForAsyncTransform"),
495+
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.mutable.HashMap.entriesIterator0"),
495496
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.io.ZipArchive.RootEntry"),
496497
)
497498
}

src/library/scala/collection/TraversableLike.scala

Lines changed: 71 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import scala.annotation.unchecked.{uncheckedVariance => uV}
2020
import parallel.ParIterable
2121
import scala.collection.immutable.{::, List, Nil}
2222
import scala.language.higherKinds
23+
import scala.runtime.AbstractFunction0
2324

2425
/** A template trait for traversable collections of type `Traversable[A]`.
2526
*
@@ -451,23 +452,77 @@ trait TraversableLike[+A, +Repr] extends Any
451452
}
452453

453454
def groupBy[K](f: A => K): immutable.Map[K, Repr] = {
454-
object m extends mutable.HashMap[K, Builder[A, Repr]] {
455-
override def entriesIterator: Iterator[mutable.DefaultEntry[K, Builder[A, Repr]]] =
456-
super.entriesIterator
457-
}
458-
val newBuilderFunction = () => newBuilder
459-
for (elem <- this.seq) {
460-
val key = f(elem)
461-
val bldr = m.getOrElseUpdate(key, newBuilderFunction())
462-
bldr += elem
463-
}
464-
val it = m.entriesIterator
465-
val m1 = if (m.size > 4) immutable.HashMap.newBuilder[K, Repr] else immutable.Map.newBuilder[K, Repr]
466-
while (it.hasNext) {
467-
val entry = it.next()
468-
m1.+=((entry.key, entry.value.result()))
455+
object grouper extends AbstractFunction0[Builder[A, Repr]] with Function1[A, Unit] {
456+
var k0, k1, k2, k3: K = null.asInstanceOf[K]
457+
var v0, v1, v2, v3 = (null : Builder[A, Repr])
458+
var size = 0
459+
var hashMap: mutable.HashMap[K, Builder[A, Repr]] = null
460+
override def apply(): mutable.Builder[A, Repr] = {
461+
size += 1
462+
newBuilder
463+
}
464+
def apply(elem: A): Unit = {
465+
val key = f(elem)
466+
val bldr = builderFor(key)
467+
bldr += elem
468+
}
469+
def builderFor(key: K): Builder[A, Repr] =
470+
size match {
471+
case 0 =>
472+
k0 = key
473+
v0 = apply()
474+
v0
475+
case 1 =>
476+
if (k0 == key) v0
477+
else {k1 = key; v1 = apply(); v1 }
478+
case 2 =>
479+
if (k0 == key) v0
480+
else if (k1 == key) v1
481+
else {k2 = key; v2 = apply(); v2 }
482+
case 3 =>
483+
if (k0 == key) v0
484+
else if (k1 == key) v1
485+
else if (k2 == key) v2
486+
else {k3 = key; v3 = apply(); v3 }
487+
case 4 =>
488+
if (k0 == key) v0
489+
else if (k1 == key) v1
490+
else if (k2 == key) v2
491+
else if (k3 == key) v3
492+
else {
493+
hashMap = new mutable.HashMap
494+
hashMap += ((k0, v0))
495+
hashMap += ((k1, v1))
496+
hashMap += ((k2, v2))
497+
hashMap += ((k3, v3))
498+
val bldr = apply()
499+
hashMap(key) = bldr
500+
bldr
501+
}
502+
case _ =>
503+
hashMap.getOrElseUpdate(key, apply())
504+
}
505+
506+
def result(): immutable.Map[K, Repr] =
507+
size match {
508+
case 0 => immutable.Map.empty
509+
case 1 => new immutable.Map.Map1(k0, v0.result())
510+
case 2 => new immutable.Map.Map2(k0, v0.result(), k1, v1.result())
511+
case 3 => new immutable.Map.Map3(k0, v0.result(), k1, v1.result(), k2, v2.result())
512+
case 4 => new immutable.Map.Map4(k0, v0.result(), k1, v1.result(), k2, v2.result(), k3, v3.result())
513+
case _ =>
514+
val it = hashMap.entriesIterator0
515+
val m1 = immutable.HashMap.newBuilder[K, Repr]
516+
while (it.hasNext) {
517+
val entry = it.next()
518+
m1.+=((entry.key, entry.value.result()))
519+
}
520+
m1.result()
521+
}
522+
469523
}
470-
m1.result()
524+
this.seq.foreach(grouper)
525+
grouper.result()
471526
}
472527

473528
def forall(p: A => Boolean): Boolean = {

src/library/scala/collection/mutable/HashMap.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ extends AbstractMap[A, B]
172172
def next() = iter.next().value
173173
}
174174

175+
private[collection] def entriesIterator0: Iterator[DefaultEntry[A, B]] = entriesIterator
176+
175177
/** Toggles whether a size map is used to track hash map statistics.
176178
*/
177179
def useSizeMap(t: Boolean) = if (t) {

src/partest/scala/tools/partest/StubErrorMessageTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ trait StubErrorMessageTest extends StoreReporterDirectTest {
5151
if (extraUserCode == "") compileCode(userCode)
5252
else compileCode(userCode, extraUserCode)
5353
import scala.reflect.internal.util.Position
54-
filteredInfos.map { report =>
54+
filteredInfos.sortBy(_.pos.point).foreach { report =>
5555
print(if (report.severity == storeReporter.ERROR) "error: " else "")
5656
println(Position.formatMessage(report.pos, report.msg, true))
5757
}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1+
error: newSource1.scala:13: type arguments [stuberrors.D] do not conform to method foo's type parameter bounds [T <: stuberrors.A]
2+
b.foo[D]
3+
^
14
error: newSource1.scala:13: Symbol 'type stuberrors.A' is missing from the classpath.
25
This symbol is required by 'class stuberrors.D'.
36
Make sure that type A is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
47
A full rebuild may help if 'D.class' was compiled against an incompatible version of stuberrors.
58
b.foo[D]
69
^
7-
error: newSource1.scala:13: type arguments [stuberrors.D] do not conform to method foo's type parameter bounds [T <: stuberrors.A]
8-
b.foo[D]
9-
^

test/files/run/StubErrorTypeDef.check

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
error: newSource1.scala:4: overriding type D in class B with bounds <: stuberrors.A;
2-
type D has incompatible type
3-
new B { type D = E }
4-
^
51
error: newSource1.scala:4: Symbol 'type stuberrors.A' is missing from the classpath.
62
This symbol is required by 'type stuberrors.B.D'.
73
Make sure that type A is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
84
A full rebuild may help if 'B.class' was compiled against an incompatible version of stuberrors.
95
new B { type D = E }
106
^
7+
error: newSource1.scala:4: overriding type D in class B with bounds <: stuberrors.A;
8+
type D has incompatible type
9+
new B { type D = E }
10+
^
1111
error: newSource1.scala:4: Symbol 'type stuberrors.A' is missing from the classpath.
1212
This symbol is required by 'class stuberrors.E'.
1313
Make sure that type A is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.

0 commit comments

Comments
 (0)