Make source a field in Positioned nodes #9900
Conversation
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (d084b8b) |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (d084b8b) |
|
If we take take the average of the two benchmark runs we get a ~1% deterioration for 3 of the 4 compilation test and a ~1% improvement for the last (re2). So it looks like nothing much to worry about. |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (09eaed7) |
Tree ids cannot be garbage collected, so they are prone to overflow for long-running compilation sessions. Therefore, we should not rely on tree ids except for debugging.
odersky
force-pushed
the
fix-sourcepos
branch
from
7f305d5
to
5b39279
Compare
Instead, keep a separate weak hashmap if a tree-id option is set. Since tree ids are now only used for debugging, we do not need to maintain them as an inline field anymore.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Trees now have a source field instead of a uniqueId, so their size does not change. Unique ids can still be obtained via a weak hashmap if -Yshow-tree-ids or -Ydebug-tree-with-id is set. Since trees are no longer mappable to ints, we cannot use SparseArrays in pickling anymore. For now, we reverted back to an IdentityHashMap, which means all stored addresses are boxed. We could improve on this with a different custom data structure. Another improvement down the line would be to pass sources when we set the tree's span instead of right away. That would simplify tree creation infrastructure quite a lot since no implicit argument would have to be passed. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (c40ef6e) |
IntMap[Key] is conceptually like a Map[Key, Int]. For now, only `apply` and `update` are supported but `remove` is not supported. The map is implemented by means of a perfect hashing scheme that is itself implemented in the parent class `PerfectHashing`. This maps keys to indices in a dense interval. Once we have the index, we can associate keys and values that are simply stored in arrays.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
which is no longer needed
Size it so that it corresponds roughly with the initial size of the buffer.
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (c40ef6e) |
|
test performance please |
Code taken from scala.collection.mutable.AnyRefMap
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (19cf871) |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9900/ to see the changes. Benchmarks is based on merging with master (94970a0) |
|
That's the first instalment of the source position changes. We make source position a field and remove all usages of uniqueId except for debugging. This should fix #9738. There's scope for further changes. namely to pass sources when we also set the span of a tree. It turns out that's a lot trickier to do, so we will defer this to a separate PR. |
Co-authored-by: Nicolas Stucki <nicolas.stucki@gmail.com>
Co-authored-by: Nicolas Stucki <nicolas.stucki@gmail.com>
Fixes scala#13626 (regression introduced in scala#9900).
Fixes scala#13626 (regression introduced in scala#9900).
The purpose of the PR is to revise the source pos scheme so that no global data structures are needed. There seems to be no way other than to simply add the source file as a field to each tree.
Fixes #9738