2.13 version of Map.hashCode optimizations #8446
Conversation
|
Split out of #8440 |
retronym
force-pushed
the
topic/map-hashcode-2.13.x
branch
2 times, most recently
from
fc66e18 to
32d00cd
Compare
| t0.## | ||
| } | ||
|
|
||
| onlyAllocates(256) { |
There was a problem hiding this comment.
This is higher than in the 2.12.x version because we use ChampBaseIterator which avoids using the JVM stack during iteration into the Trie.
diesalbla
added
the
library:collections
| ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.immutable.RedBlackTree.countInRange"), | ||
| ProblemFilters.exclude[DirectMissingMethodProblem]("scala.util.hashing.MurmurHash3.emptyMapHash"), | ||
| ProblemFilters.exclude[DirectMissingMethodProblem]("scala.util.hashing.MurmurHash3.tuple2Hash"), | ||
| ProblemFilters.exclude[MissingTypesProblem]("scala.collection.immutable.MapKeyValueTupleHashIterator"), |
There was a problem hiding this comment.
The class exists in the old and new version. Is this a false positive in MiMa?
There was a problem hiding this comment.
This class still exists, but it no longer extends Product2. The MissingTypesProblem entry ignores that problem as it is not in user facing code.
| /** Apply `f` to each key/value pair for its side effects | ||
| * Note: [U] parameter needed to help scalac's type inference. | ||
| */ |
There was a problem hiding this comment.
scaladoc comment can be omitted; it is identical to the inherited one
| f(curr.key, curr.value) | ||
| curr = curr.next | ||
| } | ||
| } |
There was a problem hiding this comment.
Isn't this going to present key-values in the wrong order, since ListMap is stored in reverse?
There was a problem hiding this comment.
You are right. This is okay for the hashCode computation (which is order-agnostic), but doesn't seem suitable for the general foreachEntry method.
There was a problem hiding this comment.
I've now removed this override other than in a private helper class used in ListMap.hashCode. I've also overridden foreachEntry in SeqMapN.
retronym
force-pushed
the
topic/map-hashcode-2.13.x
branch
from
32d00cd to
8d77d55
Compare
Cherry picking from scala#8411 with a 2.13.x twist. Because foreachEntry is part of the MapOps API, we can now centralize the optimization in MurmurHash3.mapHash, other than for the hand-inlined implementations ins MapN that seek to avoid even the allocation of the accumulator function.
retronym
force-pushed
the
topic/map-hashcode-2.13.x
branch
from
8d77d55 to
e6cb836
Compare
6 participants
Cherry picking from #8411 with a 2.13.x twist.
Because foreachEntry is part of the MapOps API, we can now centralize
the optimization in MurmurHash3.mapHash, other than for the
hand-inlined implementations ins MapN that seek to avoid even
the allocation of the accumulator function.