-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Optimize basetype caches #9608
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
Optimize basetype caches #9608
Conversation
Their base types change, so they should get new denotations.
Phase-ids should not change base type caches. Instead we make sure that class denotations that contain base type caches are copied whenever base types might change.
1bfab07
to
6729e1c
Compare
test performance please |
performance test scheduled: 14 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9608/ to see the changes. Benchmarks is based on merging with master (1431be2) |
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.
LGTM
valid && createdAt.runId == ctx.runId | ||
// Note: We rely on the fact that whenever base types of classes change, | ||
// the affected classes will get new denotations with new basedata caches. | ||
// So basedata caches can become invalid only if the run changes. |
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.
I'm not familiar with code in this area. I'm wondering will the cache be always valid for classes defined in libraries, regardless of runs?
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.
Yes, but libraries might be redefined in source in the next run, so you need to re-check.
We get a ~5% improvement for dotc and a 8% improvement for re2. No change for scalap and 1-2% worse for stdlib. That could be in the noise though. Still I wonder why stdlib does not do better here. Maybe a low cache hit rate for base type caches? That would make sense as we have lots of small methods with their own type parameters. With a low hit rate, maybe fewer cache invalidations (which is what this PR achieves) means larger caches that are under-utilized. But that's speculation. |
Main optimization: Invalidate base type caches only if runId changes
Phase-ids should not change base type caches. Instead we make sure
that class denotations that contain base type caches are copied
whenever base types might change.