Optimization: Introduce CompactAnnotation #24679
Conversation
odersky
force-pushed
the
opt-annots
branch
3 times, most recently
from
9af8000 to
66aa6bb
Compare
This one takes a type argument instead of as a tree argument. For now it's reserved for retains-like annotations. CompactAnnotations don't need tree maps and annotated types containing them hash properly.
We don't need the constructor anymore since `RetainingAnnotation` has equivalent functionality. And we don't need the deconstructor since there is already `CapturingOrRetainsType`.
|
I did some benchmarking: I compiled repeatedly the contents of typer/*.scala. Results are here: 
Benchmark setup: 2 compiler instances with 20 runs each. Classpath is kept in memory between runs. Tests were run on an M4 Macbook Pro. lines: Total number of lines The gist is:
We should drill deeper and find out where the other losses came from. |
|
I noticed that some of the versions in the benchmarks produced warnings and other did not, so I reran them with warnings off. Here are the new numbers. 
So, we lose about 4% from 3.3 to 3.6, another 2.5% from 3.6 to 3.7, and another 4% between 3.7 and 3.8. And this PR makes no difference to the 3.8 baseline. |
|
I ran the new benchmarks on this PR. Each benchmark was run for 60-180 iterations (depending on the size of the benchmark). I took the last 20 iterations of each to ensure the JVM was fully warmed up. They ran on the lampscalatst4 server, which has 4 Intel cores fixed at 2.6 GHz. We need more runs to know if the differences are significant or not, but here are already temporary results. Big Benchmarks
Small Benchmarks
|
| val mtpsWithArgs = mtps.zip(argss) | ||
| val argMap = mtpsWithArgs.toMap | ||
| val deps = mutable.HashMap[Tree, List[Tree]]().withDefaultValue(Nil) | ||
| val deps = mutable.LinkedHashMap[Tree, List[Tree]]().withDefaultValue(Nil) |
There was a problem hiding this comment.
Just wondering, why do we choose to use mutable.LinkedHashMap now?
|
@odersky unfortunately, scaladoc now renders CC-related annotations incorrectly/not at all in places. I'll try to push a fix before the release cutoff. |
|
@bracevac So should we hold-off merging this until the scaladoc is fixed? |
|
@odersky yes, otherwise, the API will not be rendered correctly. |
4 participants
This one takes a type argument instead of as a tree argument. This makes mapping such annotations a lot faster and also safer. In fact, in retrospect, most annotations could be represented as
CompactAnnotations.CompactAnnotationis extended byRetainingAnnotation, which is reserved for@retains,@retainsByNameand@retainsCap. We make sure that all annotations with these classes are represented asRetainingAnnotations.For now there are no
CompactAnnotations other than@RetainingAnnotations but this could be changed in the future.