Optimize immutable.HashMap builder (inspired by the 2.13 implementation)#8726
Conversation
e397489 to
f2d37cc
Compare
06af356 to
9ffb971
Compare
|
|
||
| protected def filter0(p: ((A, B)) => Boolean, negate: Boolean, level: Int, buffer: Array[HashMap[A, B @uV]], offset0: Int): HashMap[A, B] = null | ||
|
|
||
| protected def elemHashCode(key: A) = key.## |
There was a problem hiding this comment.
User defined element hashing was removed in 2.13 with a migration suggestion to use use wrapper classes for keys. But I don't think we can remove this in a minor release. Did you intend to include this diff in this PR?
There was a problem hiding this comment.
I didn't change the functionality her, just moved it to be in the object, as I needed the hashing from the builder
Is your concern the binary compat, or the functionality?
If the former, the methods can remain but be unused?
There was a problem hiding this comment.
The functionality. Admittedly it does not have test coverage but it is used a few times in the wild: https://github.com/search?q=%22override+def+elemHashCode%22&type=Code
There was a problem hiding this comment.
i have restored the extension point
|
Echoing my comments on #8722, this change will be targetted to 2.12.12. |
| trie | ||
| } | ||
| case _ => hs | ||
| } |
There was a problem hiding this comment.
We should call ScalaRuntime.releaseFence (which will itself need to be backported but made private[scala]) here.
There was a problem hiding this comment.
@mkeskells I just merged #8779 so you could now add s.c.i.VM.releaseFence() to this and #8722 in the same places as 2.13.x.
828b587 to
7c147e2
Compare
|
squashed |
7c147e2 to
b830eab
Compare
b830eab to
0e310c3
Compare
0e310c3 to
0a18c32
Compare
The builder allows TrieHashMap to be mutated in place until it is visible this reduces the memory churn while building Maps
0a18c32 to
9460890
Compare
The builder allows
TrieHashMapto be mutated in place until it is visible this reduces the memory churn while buildingMaps.