Revive function specialization #10452
Merged
Conversation
This file contains hidden or 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
|
@Duhemm @felixmulder Thanks for your work on function specialization. To keep history simpler, are you OK with squash the commits to 3: one for each author? |
Thank you for taking this to the finish line! Yes, please squash all my commits the way you feel is the cleanest 👍 |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/10452/ to see the changes. Benchmarks is based on merging with master (31566b7) |
|
Go for it 👍 |
liufengyun
force-pushed
the
fun-spec2
branch
from
640886a to
252d30f
Compare
|
@smarter Could you have another look? If you find it OK, I can squash the experiment commits. Note: from the benchmark result http://dotty-bench.epfl.ch/10452/, the regression in scalap is caused by the code in 1b16557. |
smarter
approved these changes
Nov 24, 2020
compiler/src/dotty/tools/dotc/transform/SpecializeApplyMethods.scala
Outdated
Show resolved
Hide resolved
liufengyun
force-pushed
the
fun-spec2
branch
2 times, most recently
from
73f6a0b to
bee8aa8
Compare
Add phases and initial replacement for super Replace all existing combinations of Function1 with specialized version Do transformations on symbol level too Refactor transformations to be more idiomatic Add dispatch to specialized applys Add forwarding method for generic case Don't specialize Function1 tree when invalid to Write test to check for specialized apply Remove `DispatchToSpecializedApply` phase SpecializeFunction1: don't roll over parents, use mapConserve Rewrite to handle all specialized functions Don't remove parents not being specialized Add plain function tests to NameOps and Definitions Rewrite `SpecializeFunctions` from `DenotTransformer` to `InfoTransformer` Add `MiniPhaseTransform` to add specialized methods to FunctionN Add synthetic bridge when compiling FunctionN Fix ordering of specialized names and type parameterized apply Add parent types explicitly when specializing When a class directly extends a specialized function class, we need to replace the parent with the specialized interface. In other cases we don't replace it, even if the parent of a parent has a specialized apply - the symbols would propagate anyway. Make `ThisName` recursive on `self.ThisName` Make sure specialized functions get the correct name
Fix compilation errors
Don't change parents in `specializeFunctions`
Cleanup
Move type to JVM tag conversion to Definitions
Add more tests for function specialization
Pass by name should not introduce boxing
Cleanup
No specialization for Function3
Adapt to recent changes in transformers and phases
Address review comments
Optimize phase `SpecializeFunctions`
- Stop immediately if the type doesn't derive from `Function{0,1,2}`
- We don't need to check if any of the parents derives from
`Function{0,1,2}`, we can just check if the type derives from it.
- Fix isPlainFunctionClass Previous implementation is incorrect, as scala.Function1$ would qualify. - Create the symbol at the next phase Create the symbol at the next phase, so that it is a valid member of the corresponding function for all valid periods of its SymDenotations. Otherwise, the valid period will offset by 1, which causes a stale symbol in compiling stdlib. - Handle abstract apply and multiple applys - Don't specialize abstract apply - Fast specialization We avoid going through InfoTransformer, which will cause all symbols to be checked. The reason why it works is that the specialized base classes, i.e. Function0-2 already have all the relevant definitions. - Use StringBuilder instead of StringBuffer (thanks @smarter) StringBuffer is synchronized thus is slower.
liufengyun
force-pushed
the
fun-spec2
branch
from
bee8aa8 to
7fbec1c
Compare
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.
Revive function specialization
Supersede #9846