Optimization experiment: Drop Symbol class #16631
Conversation
odersky
force-pushed
the
perf2
branch
4 times, most recently
from
12ae395 to
a2f4c91
Compare
odersky
force-pushed
the
perf2
branch
3 times, most recently
from
be3e791 to
681f3bc
Compare
I.e. introduce a type member ThisName which is the result of `name`.
Also: Select chached data from lastDenot instead of initialDenot. It makes a difference for symbols that underwent a bringForward. For them, the initialDenot is from a previous run, so should be treated as stale. Also: Reset caches of symbols that are brought forward. This could avoid some memory leaks.
These are needed once we move to opaque types
They would be skipped when Symbol becomes an opaque type alias, since inside Symbols it is known that Symbol == SymDenotation
There are two separate problems with having `is` and `info` as extension methods in Symnbol. For `is` methods: There's a "data race" error when compiling the compiler with itself. It seems that both of the following conditions must hold for that error to come up: - the extension method must be overloaded - it must have a same-named real method in the target type that is reachable via an implicit conversion. We need to minimize and dig deeper on this one. For `info` the problem is different. We can't have both `info` and `info_=` as extension methods since assignment desugaring does not work for extension methods. We cannot have `info` alone either since then the conversion-based `info =` would not work anymore. We should add a fix that allows assignment desugaring of extension methods to the language; then this problem would go away.
We can't allow them in symbols since they would not kick in when Symbol and ClassSymbol are made opaque type aliases, because inside Symbols we know that these types are aliases of Denotations so no conversion would be applied then and we would possibly end up with a denotation at the wrong validity period.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/16631/ to see the changes. Benchmarks is based on merging with main (b8ad7b1) |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/16631/ to see the changes. Benchmarks is based on merging with main (b8ad7b1) |
|
So, performance results are inconclusive, unfortunately. Note that we did not get results for the dotty codebase itself -- I am not sure why that was. But for the other tests we are in the same ballpark as before. Another thing I noted by testing locally is how fast the M1 is. My local benchmarks are twice as fast as on the previous MacBook Pro with i7 processor. If all dependent classes are loaded we are now looking at 10K lines/sec for compiling all files in typer or all files in core. Before it was 5k. Despite absence of performance wins, I'd classify the experiment a success since it has shown that we can take a core class in a very complicated system that interacts with many other components in recursive dependencies and replace it with an opaque type alias. That is, after having fixed the bounds checking problem that was merged into 3.3.0. Problems discovered that need follow-up:
|
This is an experiment to see how much we could gain by merging Symbol with SymDenotation and thus saving one level of indirection in the common case.
The idea is to define Symbol as an opaque type alias of SymDenotation. We still need to select the right denotation associated with a symbol depending on period, but we can do that with basically the same code
as now, implemented as an extension method on Symbols. The advantage is that the test of
lastDenot.validForhits the Symbol itself if the denotation is the initial denotation. I gathered some statistics that show that this is the case 75-80% of the time.How much this will help in the end is of course completely unclear. We need to do the changes and run some benchmarks.