-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cache also private members #9648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Use a full cache instead of an LRU cache. For `typer/*.scala`, this reduced computed member searches on normal ClassDenotations from 520K to 170K. Cache hit rate improves to 97.5%, from 92.5%. (Without member caching there are almost 7Mio computed members for the same code base).
The two implementation classes of LinearTable had so much in common that they could be combined. Since the implementation is now fixed, we can use the usual mutable interface for a table.
When going from dense to hashed, we should expand by a multiple greater than two, or we will have to reallocate immediately afterwards.
We get occasional failures of the form java.lang.AssertionError: assertion failed: asTerm called on not-a-Term val <none> while compiling tests/pos/projections.scala Fatal compiler crash when compiling: tests/pos/projections.scala: assertion failed: asTerm called on not-a-Term val <none> dotty.DottyPredef$.assertFail(DottyPredef.scala:17) dotty.tools.dotc.core.Symbols$Symbol.asTerm(Symbols.scala:156) when compiling pos/projections.scala with FromTasty tests. It was first observed when we parallelized position pickling, so it might have something to do with that.
NoDenotation is a frequent result, so we should keep post processing it as simple as possible. As a side effect this now forces less, which makes some previously failing tests pass. I updated i3253.scala so that it fails again and moved i9052 to pos.
This means we have to do extra work for a nonPrivateMember that hits a private member. But that's probably not a very common case. On the other hand, every find member operation needs one less scope lookup this way.
This reverts commit f8d6ac4.
2bdee9e
to
44a07d6
Compare
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9648/ to see the changes. Benchmarks is based on merging with master (2e58a66) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
myMemberCache
is not copied when we create a new ClassDenotation
from an existing one (See copyCaches
). I assume that is OK, as when we copy a ClassDenotation
, usually its member cache becomes invalid.
Co-authored-by: Fengyun Liu <liu@fengy.me>
Yes, not copying member caches is intentional since then we do not have to worry about validity. |
Keep private members in member caches as well.
This means we have to do extra work for a nonPrivateMember call
that hits a private member. But that's probably not a very
common case. On the other hand, every regular findMember operation
needs one less scope lookup this way.