Use WeakHashSet instead of HashSet for hash-consing types #12935
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thus making it a drop-in replacement for util.HashSet which will be useful in the next commit where we'll use it in Uniques. Also add `@constructorOnly` annotations for clarity. Also remove all existing references to WeakHashSet in the code base since it turns out they were all dead code.
This mimics what Scala 2 has been doing for a long time now and serves the same purpose: it considerably reduces peak memory usage when compiling some projects, for example previously compiling the Scalatest tests required a heap of at least 11 GB, but now it fits in about 4 GB. This required changing the implementation of WeakHashSet to have overridable `hash` and `isEqual` methods just like HashSet, it also required making various private methods protected since NamedTypeUniques and AppliedUniques contain an inlined implementation of `put`. This commit also changes the default load factor of a WeakHashSet from 0.75 to 0.5 to match the load factor we use for HashSets, though note that Scala 2 has always been using 0.75. For a history of the usage of WeakHashSet in Scala 2 see: - scala/scala#247 - scala/scala#2605 - scala/scala#2901
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/12935/ to see the changes. Benchmarks is based on merging with master (8163bff) |
odersky
approved these changes
Jul 6, 2021
|
Thank you for improvements 🎉 Here is GC logs of my large project(600000 lines scala code) Scala 3.0.1-RC2
Scala 3.0.2-RC1-bin-20210706-6011847-NIGHTLY
|
|
@xuwei-k Awesome, thank you for the report! Does your project also compile with Scala 2 and if so, do you get similar numbers there? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This mimics what Scala 2 has been doing for a long time now and serves
the same purpose: it considerably reduces peak memory usage when
compiling some projects, for example previously compiling the Scalatest
tests required a heap of at least 11 GB, but now it fits in about 4 GB.
This required changing the implementation of WeakHashSet to subclass util.MutableSet
and to have overridable
hashandisEqualmethods just like HashSet, it alsorequired making various private methods protected since NamedTypeUniques
and AppliedUniques contain an inlined implementation of
put.This PR also changes the default load factor of a WeakHashSet from
0.75 to 0.5 to match the load factor we use for HashSets, though note that
Scala 2 has always been using 0.75.
For a history of the usage of WeakHashSet in Scala 2 see: