Skip to content

Commit d7d9612

Browse files
committed
Reduce allocations in groupBy
- Avoid lambda allocation in getOrElseUpdate call - Avoid intermediate Tuple2 instances when iterating through the mutable map. - Avoid allocating Map1-4 during building the result if the size of the grouped exceeds 4. Intead build a HashMap directly. ``` ./benchdb list --limit 2 && ./benchdb results --run 5 --run 6 --pivot size && ./benchdb results --run 5 --run 6 --pivot size --metric "·gc.alloc.rate.norm" ┏━━━━┳━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━┓ ┃ ID ┃ Timestamp ┃ Msg ┃ ┣━━━━╋━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━┫ ┃ 6 ┃ 2020-08-24 07:14:36 ┃ PR 8948 ┃ ┃ 5 ┃ 2020-08-24 07:06:47 ┃ PR 8948 Baseline ┃ ┗━━━━┻━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━┛ 2 test runs found (limit reached). ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━┳━━━━━┳━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━┓ ┃ (size) ┃ ┃ ┃ ┃ ┃ 128 ┃ 512 ┃ 2048 ┃ 8192 ┃ ┃ ┃ Benchmark ┃ (hashCodeCost) ┃ (maxNumGroups) ┃ Mode ┃ Cnt ┃ Score ┃ Error ┃ Score ┃ Error ┃ Score ┃ Error ┃ Score ┃ Error ┃ Units ┃ ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━╋━━━━━━╋━━━━━╋━━━━━━━━━━╋━━━━━━━━━╋━━━━━━━━━━━╋━━━━━━━━━╋━━━━━━━━━━━━╋━━━━━━━━━━╋━━━━━━━━━━━━╋━━━━━━━━━━╋━━━━━━━┫ ┃ #5:scala.collection.GroupByBenchmark.buildArrayBuffer ┃ 32 ┃ 8 ┃ avgt ┃ 30 ┃ 8631.963 ┃ 116.790 ┃ 30969.939 ┃ 456.711 ┃ 119820.013 ┃ 1741.671 ┃ 480813.306 ┃ 7268.723 ┃ ns/op ┃ ┃ #6:scala.collection.GroupByBenchmark.buildArrayBuffer ┃ 32 ┃ 8 ┃ avgt ┃ 30 ┃ 8287.717 ┃ 135.038 ┃ 30672.551 ┃ 540.409 ┃ 119356.530 ┃ 1592.624 ┃ 477570.346 ┃ 6365.010 ┃ ns/op ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━┻━━━━━━┻━━━━━┻━━━━━━━━━━┻━━━━━━━━━┻━━━━━━━━━━━┻━━━━━━━━━┻━━━━━━━━━━━━┻━━━━━━━━━━┻━━━━━━━━━━━━┻━━━━━━━━━━┻━━━━━━━┛ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┳━━━━━━┳━━━━━┳━━━━━━━━━━┳━━━━━━━┳━━━━━━━━━━━┳━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━┳━━━━━━━┓ ┃ (size) ┃ ┃ ┃ ┃ ┃ 128 ┃ 512 ┃ 2048 ┃ 8192 ┃ ┃ ┃ Benchmark [·gc.alloc.rate.norm] ┃ (hashCodeCost) ┃ (maxNumGroups) ┃ Mode ┃ Cnt ┃ Score ┃ Error ┃ Score ┃ Error ┃ Score ┃ Error ┃ Score ┃ Error ┃ Units ┃ ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━╋━━━━━━╋━━━━━╋━━━━━━━━━━╋━━━━━━━╋━━━━━━━━━━━╋━━━━━━━╋━━━━━━━━━━━╋━━━━━━━━╋━━━━━━━━━━━━╋━━━━━━━╋━━━━━━━┫ ┃ #5:scala.collection.GroupByBenchmark.buildArrayBuffer ┃ 32 ┃ 8 ┃ avgt ┃ 30 ┃ 2192.014 ┃ 0.022 ┃ 13712.051 ┃ 0.076 ┃ 50949.533 ┃ 56.381 ┃ 198544.803 ┃ 1.213 ┃ B/op ┃ ┃ #6:scala.collection.GroupByBenchmark.buildArrayBuffer ┃ 32 ┃ 8 ┃ avgt ┃ 30 ┃ 1896.014 ┃ 0.021 ┃ 5224.051 ┃ 0.077 ┃ 17768.204 ┃ 0.305 ┃ 67176.786 ┃ 1.180 ┃ B/op ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━┻━━━━━━┻━━━━━┻━━━━━━━━━━┻━━━━━━━┻━━━━━━━━━━━┻━━━━━━━┻━━━━━━━━━━━┻━━━━━━━━┻━━━━━━━━━━━━┻━━━━━━━┻━━━━━━━┛ ```
1 parent 9d499d7 commit d7d9612

File tree

2 files changed

+21
-9
lines changed

2 files changed

+21
-9
lines changed

src/library/scala/collection/TraversableLike.scala

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -451,17 +451,23 @@ trait TraversableLike[+A, +Repr] extends Any
451451
}
452452

453453
def groupBy[K](f: A => K): immutable.Map[K, Repr] = {
454-
val m = mutable.Map.empty[K, Builder[A, Repr]]
455-
for (elem <- this) {
456-
val key = f(elem)
457-
val bldr = m.getOrElseUpdate(key, newBuilder)
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())
458462
bldr += elem
459463
}
460-
val b = immutable.Map.newBuilder[K, Repr]
461-
for ((k, v) <- m)
462-
b += ((k, v.result))
463-
464-
b.result
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()))
469+
}
470+
m1.result()
465471
}
466472

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

src/library/scala/collection/immutable/HashMap.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,12 @@ object HashMap extends ImmutableMapFactory[HashMap] with BitOperations.Int {
11711171
this
11721172
}
11731173

1174+
private[collection] def +=(elem: (A, B), keyHashCode: Int): this.type = {
1175+
val hash = improve(keyHashCode)
1176+
rootNode = addOne(rootNode, elem, hash, 0)
1177+
this
1178+
}
1179+
11741180
override def ++=(xs: TraversableOnce[(A, B)]): this.type = xs match {
11751181
case hm: HashMap[A, B] =>
11761182
if (rootNode eq EmptyHashMap) {

0 commit comments

Comments
 (0)