Skip to content

Commit b67ca7d

Browse files
committed
Changed HashMap.getOrElseUpdate to only calculate the index once
Fixes https://issues.scala-lang.org/browse/SI-10049 Since `groupBy` uses this method extensively and suffered a measurable slowdown in `2.12.0`, this modification restores (and exceeds) its original speed. --- included benchmarks: (`ns/op` → smaller is better) `before (2.12.0):` ```java Benchmark (size) Mode Cnt Score Error Units s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 865.693 ± 7.869 ns/op s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 3095.657 ± 56.438 ns/op s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 28247.005 ± 470.513 ns/op s.c.mutable.HashMapBenchmark.get 10 avgt 20 679.448 ± 11.809 ns/op s.c.mutable.HashMapBenchmark.get 100 avgt 20 7240.178 ± 61.734 ns/op s.c.mutable.HashMapBenchmark.get 1000 avgt 20 95725.127 ± 2373.458 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 836.561 ± 20.085 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 7891.368 ± 56.808 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 97478.629 ± 1782.497 ns/op s.c.mutable.HashMapBenchmark.put 10 avgt 20 243.422 ± 2.915 ns/op s.c.mutable.HashMapBenchmark.put 100 avgt 20 5810.927 ± 60.054 ns/op s.c.mutable.HashMapBenchmark.put 1000 avgt 20 82175.539 ± 1690.296 ns/op ``` `after:` ```java Benchmark (size) Mode Cnt Score Error Units s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 627.007 ± 9.718 ns/op s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 2086.955 ± 19.042 ns/op s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 19515.234 ± 173.647 ns/op s.c.mutable.HashMapBenchmark.get 10 avgt 20 683.977 ± 11.843 ns/op s.c.mutable.HashMapBenchmark.get 100 avgt 20 7345.675 ± 41.092 ns/op s.c.mutable.HashMapBenchmark.get 1000 avgt 20 95085.926 ± 1702.997 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 503.208 ± 2.643 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 5526.483 ± 28.262 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 69265.900 ± 674.958 ns/op s.c.mutable.HashMapBenchmark.put 10 avgt 20 252.481 ± 7.597 ns/op s.c.mutable.HashMapBenchmark.put 100 avgt 20 5708.034 ± 110.360 ns/op s.c.mutable.HashMapBenchmark.put 1000 avgt 20 82051.378 ± 1432.009 ns/op ``` i.e. for the given benchmark conditions `~40%` faster `groupBy` and `getOrElseUpdate`
1 parent 5c93cd2 commit b67ca7d

File tree

1 file changed

+31
-0
lines changed

1 file changed

+31
-0
lines changed

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,37 @@ extends AbstractMap[A, B]
7272
else Some(e.value)
7373
}
7474

75+
override def getOrElseUpdate(key: A, defaultValue: => B): B = {
76+
val i = index(elemHashCode(key))
77+
val entry = findEntry(key, i)
78+
if (entry != null) entry.value
79+
else addEntry(createNewEntry(key, defaultValue), i)
80+
}
81+
82+
/* inlined HashTable.findEntry0 to preserve its visibility */
83+
private[this] def findEntry(key: A, h: Int): Entry = {
84+
var e = table(h).asInstanceOf[Entry]
85+
while (notFound(key, e))
86+
e = e.next
87+
e
88+
}
89+
private[this] def notFound(key: A, e: Entry): Boolean = (e != null) && !elemEquals(e.key, key)
90+
91+
/* inlined HashTable.addEntry0 to preserve its visibility */
92+
private[this] def addEntry(e: Entry, h: Int): B = {
93+
if (tableSize >= threshold) addEntry(e)
94+
else addEntry0(e, h)
95+
e.value
96+
}
97+
98+
/* extracted to make addEntry inlinable */
99+
private[this] def addEntry0(e: Entry, h: Int) {
100+
e.next = table(h).asInstanceOf[Entry]
101+
table(h) = e
102+
tableSize += 1
103+
nnSizeMapAdd(h)
104+
}
105+
75106
override def put(key: A, value: B): Option[B] = {
76107
val e = findOrAddEntry(key, value)
77108
if (e eq null) None

0 commit comments

Comments
 (0)