SD-226 Be lazier in Fields info transform for better performance#5412
SD-226 Be lazier in Fields info transform for better performance#5412adriaanm merged 2 commits intoscala:2.12.0from
Conversation
Only mixin fields + accessors into class infos of classes that are either in the current run, or appear in a superclass chain of a class in the current run. This is analagous to what happens in the mixin phase.
|
|
I have some additional optimizations in https://github.com/scala/scala/compare/2.12.0...retronym:opt/settings?expand=1 which in aggregate improve performance further to: |
| } else tp | ||
|
|
||
|
|
||
| case tp@ClassInfoType(parents, oldDecls, clazz) if !needsMixin(clazz) => tp |
There was a problem hiding this comment.
Thinking about this some more, we probably need to guard this with globalPhase.isAfterRefchecks, as we only populate the map of separated compiled superclasses in the refchecks tree transform.
If something at or before refchecks where to run exitingFields(cls.info), we would get here before that map was populated and falsely skip this info transform.
There was a problem hiding this comment.
I've added a commit to conservatively approximate this as false.
There was a problem hiding this comment.
I'm not sure this will fly, since we also need to hit the next case if there's a module or lazy val that needs expansion, for example. <-- scratch that, but would be good to rename needsMixin, since it's more than just about mixing in vals now
There was a problem hiding this comment.
Renamed to classNeedsInfoTransform
We don't hit this code path during bootstrapping, but we could conceivably hit it with macros or compiler plugins peering into the future through atPhase before refchecks as run. Also rename a method to reflect the generality of the info transform (it does more than mixin, now.)
8569b69 to
e07585c
Compare
|
I've just repeated some benchmarking after rebasing/restarring on the synchronized performance fix and the other major open PRs: After that we still see a big improvement from the laziness here: compile time changes by 0.82x for better-files, 0.91x for scalap. |
|
LGTM |
Only mixin fields + accessors into class infos of classes that are
either in the current run, or appear in a superclass chain of a class
in the current run.
This is analagous to what happens in the mixin phase.
Review by @adriaanm
I'll post some benchmark results below.