Improve Source Positions #5644

odersky · 2018-12-19T09:39:58Z

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.

odersky · 2019-01-04T16:09:06Z

Status so far:

Trees now produce full source positions.
Source positions are encoded by the uniqueId of the tree and its offset range.
Offset ranges are renamed from Position to Span
SourcePositions are no longer generated implicitly from the context position.

Still to be done:

Make the tests pass. There is some funky stuff going on in ReplTests where the same SourceFile gets created several time (i.e. copies have the same file component) but some copies have empty contents. These weird source files are in the context when we create trees in REPL tests. They seem to upset message renderings, adding an empty line. This problem did not get noticed before because by the time we rendered the messages we did have the source with the right contents in the context. But now that trees get source positions at the time they are created this does not work any longer.
Rename SourcePosition to Position, aligning uses of the term with Dotty backend, JS backend and Tasty.
Rename SourceFile to Source, avoiding the confusion with AbstractFile.

odersky · 2019-01-04T16:09:52Z

@nicolasstucki I am done with it for now. Please have a look to see what we can do for the tests.

odersky · 2019-01-04T17:28:53Z

Rebased. I was hoping that the recently changes to ReplCompiler had an effect, but no luck.

nicolasstucki · 2019-01-05T13:23:49Z

Rebased again due to conflicts

odersky · 2019-01-05T14:53:45Z

test performance please

dottybot · 2019-01-05T14:55:15Z

performance test scheduled: 1 job(s) in queue, 0 running.

odersky · 2019-01-05T16:25:37Z

compiler/src/dotty/tools/io/VirtualFile.scala

@@ -38,12 +38,6 @@ class VirtualFile(val name: String, override val path: String) extends AbstractF
    this.content = content
  }

-  override def hashCode: Int = path.hashCode


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.

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.

dottybot · 2019-01-05T16:42:28Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/5644/ to see the changes.

Benchmarks is based on merging with master (5b7d5b6)

odersky · 2019-01-06T09:39:01Z

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.

odersky · 2019-01-06T10:09:14Z

Rebased to master

nicolasstucki · 2019-01-06T18:02:15Z

Rebased

compiler/test/dotty/tools/dotc/parsing/ModifiersParsingTest.scala

compiler/test/dotty/tools/repl/TabcompleteTests.scala

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.

odersky · 2019-01-15T20:10:25Z

Superseded by #5713.

odersky added the stat:wip label Dec 19, 2018

odersky force-pushed the add-sourcepos-2 branch from 0050ca9 to c7f2f05 Compare December 19, 2018 11:27

nicolasstucki mentioned this pull request Dec 19, 2018

Fix #5503: Keep outermost inlined call #5641

Closed

odersky force-pushed the add-sourcepos-2 branch 2 times, most recently from 672199e to 63765a7 Compare December 23, 2018 11:56

odersky force-pushed the add-sourcepos-2 branch 3 times, most recently from 042615f to 457f814 Compare January 3, 2019 18:36

odersky assigned nicolasstucki Jan 4, 2019

odersky force-pushed the add-sourcepos-2 branch from 2ef00e1 to 211e812 Compare January 4, 2019 17:28

nicolasstucki force-pushed the add-sourcepos-2 branch 2 times, most recently from 7a92e02 to 216a89a Compare January 5, 2019 13:23

nicolasstucki force-pushed the add-sourcepos-2 branch from 7a0b4b4 to 48e8f92 Compare January 5, 2019 15:28

odersky commented Jan 5, 2019

View reviewed changes

odersky force-pushed the add-sourcepos-2 branch from 48e8f92 to 11fe765 Compare January 6, 2019 10:08

nicolasstucki force-pushed the add-sourcepos-2 branch 3 times, most recently from e8ff6b0 to c946591 Compare January 6, 2019 18:01

nicolasstucki reviewed Jan 6, 2019

View reviewed changes

compiler/test/dotty/tools/dotc/parsing/ModifiersParsingTest.scala Outdated Show resolved Hide resolved

compiler/test/dotty/tools/repl/TabcompleteTests.scala Outdated Show resolved Hide resolved

nicolasstucki force-pushed the add-sourcepos-2 branch from b4a1c5b to 9126db8 Compare January 7, 2019 13:04

nicolasstucki and others added 27 commits January 15, 2019 14:01

Add missing contents to vitual files

2150fdd

Avoid memoization of virtual source files with same path

fa08b7e

Use non-virtual files in dottydoc tests

57e893f

To avoid the trying to load virtual files based on their pickled names.

Update test to use Spans

6188f37

Update missing pos to span

ba52c01

Update implicit modifier test

ff7a87e

Now we keep only flags on the parameters (except the first one), so that parameter positions don't overlap.

Add missing spans in closures

d0d6393

Previously they where infered from the mods.

Comment debug assertion

5413f87

Remove unnecessary workaround for repl

1f7ab41

Fix rebase breakage

19cf827

Avoid creating source positions in backend interface

6491872

Optimize computation of Sourcefile#content

512d7c3

Make SourceFile equals depend on file

a2f437a

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.

Optimize equals + hashCode on PlainFile

cb3515b

Use interned strings for the comparison of their paths.

Reduce concurrent hashmap lookups

d1008fc

Reduce concurrent hashmap lookups in TreeIds by caching the last result in SourceInfo. This reduces lookups for typer/*.scala from 500K+ to 7.6K.

Refactor SourceFile operations

4421092

Deemphasize AbstractFile, drop TreeIds.

Optimize PlainFile.equals

c03e31b

Implement -Ydebug-pos

036546f

When set, prints full source positions, not just file + line.

Revise PositionPickler

7947349

The decision when to pickle a position is a time sink. Try to optimize this by not going through Positioned#{initialSpan,elemSource} functionality.

Fix rebase breakage

29c8f45

Optimize initialPos in Positioned

a7a8203

Single traversal to set id and span.

Optimize SourceFile.fromId

a3ee7c0

Optimize Symbol#source

66ebabd

It's called more often now that we generation source positions. Time for some caching.

Use SourceFile implicits instead of SourceInfo

8ed8723

No need to do the wrapping

Make virtual source files explicit

5a6b1ee

Sourcefile refactoring

84321eb

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.

Don't read contents when creating a source file

f5e226d

odersky force-pushed the add-sourcepos-2 branch from 8c7cd1b to f5e226d Compare January 15, 2019 13:01

odersky closed this Jan 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve Source Positions #5644

Improve Source Positions #5644

Uh oh!

odersky commented Dec 19, 2018 •

edited

Loading

Uh oh!

odersky commented Jan 4, 2019 •

edited

Loading

Uh oh!

odersky commented Jan 4, 2019

Uh oh!

odersky commented Jan 4, 2019

Uh oh!

nicolasstucki commented Jan 5, 2019

Uh oh!

odersky commented Jan 5, 2019

Uh oh!

dottybot commented Jan 5, 2019

Uh oh!

odersky Jan 5, 2019

Uh oh!

nicolasstucki Jan 5, 2019

Uh oh!

dottybot commented Jan 5, 2019

Uh oh!

odersky commented Jan 6, 2019

Uh oh!

odersky commented Jan 6, 2019

Uh oh!

nicolasstucki commented Jan 6, 2019

Uh oh!

Uh oh!

Uh oh!

odersky commented Jan 15, 2019

Uh oh!

Uh oh!

Improve Source Positions #5644

Improve Source Positions #5644

Uh oh!

Conversation

odersky commented Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odersky commented Jan 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odersky commented Jan 4, 2019

Uh oh!

odersky commented Jan 4, 2019

Uh oh!

nicolasstucki commented Jan 5, 2019

Uh oh!

odersky commented Jan 5, 2019

Uh oh!

dottybot commented Jan 5, 2019

Uh oh!

odersky Jan 5, 2019

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Jan 5, 2019

Choose a reason for hiding this comment

Uh oh!

dottybot commented Jan 5, 2019

Uh oh!

odersky commented Jan 6, 2019

Uh oh!

odersky commented Jan 6, 2019

Uh oh!

nicolasstucki commented Jan 6, 2019

Uh oh!

Uh oh!

Uh oh!

odersky commented Jan 15, 2019

Uh oh!

Uh oh!

odersky commented Dec 19, 2018 •

edited

Loading

odersky commented Jan 4, 2019 •

edited

Loading