-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve Source Positions #5644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Source Positions #5644
Conversation
0050ca9
to
c7f2f05
Compare
672199e
to
63765a7
Compare
042615f
to
457f814
Compare
Status so far:
Still to be done:
|
@nicolasstucki I am done with it for now. Please have a look to see what we can do for the tests. |
2ef00e1
to
211e812
Compare
Rebased. I was hoping that the recently changes to ReplCompiler had an effect, but no luck. |
7a92e02
to
216a89a
Compare
Rebased again due to conflicts |
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
7a0b4b4
to
48e8f92
Compare
@@ -38,12 +38,6 @@ class VirtualFile(val name: String, override val path: String) extends AbstractF | |||
this.content = content | |||
} | |||
|
|||
override def hashCode: Int = path.hashCode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a shame to lose that. Should virtual files not be structurally equal? I fear if they are not we get a ripple down effect with e.g. too many source file entries in contexts since duplicates are no longer discovered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already noticed that this will not work as interested. I will roll back this change and use unique paths for all the virtual source files created by the repl. Currently all paths are <console>
or rs$line$i
which the wrong memorized source file.
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/5644/ to see the changes. Benchmarks is based on merging with master (5b7d5b6) |
Performance test is a lot worse, which is what I feared. There's a price to pay for carrying around all these source positions. We have to see how to optimize that. |
48e8f92
to
11fe765
Compare
Rebased to master |
e8ff6b0
to
c946591
Compare
Rebased |
compiler/test/dotty/tools/dotc/parsing/ModifiersParsingTest.scala
Outdated
Show resolved
Hide resolved
b4a1c5b
to
9126db8
Compare
To avoid the trying to load virtual files based on their pickled names.
Now we keep only flags on the parameters (except the first one), so that parameter positions don't overlap.
Previously they where infered from the mods.
Used to depend on file's path, but that does not work for VirtualFiles that might be different even though they have the same path.
Use interned strings for the comparison of their paths.
Reduce concurrent hashmap lookups in TreeIds by caching the last result in SourceInfo. This reduces lookups for typer/*.scala from 500K+ to 7.6K.
Deemphasize AbstractFile, drop TreeIds.
When set, prints full source positions, not just file + line.
The decision when to pickle a position is a time sink. Try to optimize this by not going through Positioned#{initialSpan,elemSource} functionality.
Single traversal to set id and span.
It's called more often now that we generation source positions. Time for some caching.
No need to do the wrapping
Don't check whether a sourcefile exists when creating it - it might lead to spurious errors. Do it instead at the point where we create a CompilationUnit from a source file.
8c7cd1b
to
f5e226d
Compare
Superseded by #5713. |
The goal of this PR is to be able to produce the full source position of a tree without
relying on context in Inlined nodes. Such context is fragile because it is very hard to preserve
during staging and other operations on Tasty trees. On the other hand, we do not want to spend one extra field per tree. So the idea is to use the
uniqueId
field of a tree as an index to determine the source file in which it was created.