Fix bootstrap, take 3 #1477

odersky · 2016-08-25T15:03:23Z

With these fixes we now can measure times of bootstrapping dotty.

The command I used was (in directory src/dotty/tools):

src/dotty/tools> timed-boostrap-repeated

where I have

alias timed-bootstrap-repeated  time java -Xms1g -Xmx3g dotty.tools.dotc.Bench \#runs 3 -d /classes *.scala */*.scala */*/*.scala */*/*/*.scala */*/*/*/*.scala */*/*/*/*/*.scala

and /classes is on my classpath. Then run the same command again, this time the class files will be picked up from /classes, so we are testing the dotty-generated compiler.

Each command runs the dotty compiler 3 times on its own sources, keeping classes loaded. Here are the timings I got on my 4-year old MacBook:

/Users/odersky/workspace/dotty/src/dotty/tools> timed-bootstrap-repeated 
dotc/typer/RefChecks.scala:68: warning: match may not be exhaustive.
It would fail on the following input: (_, _)
      for ((_, m1 :: m2 :: _) <- (clazz.info member nme.applyDynamic).alternatives groupBy (_.symbol.typeParams.length)) {
           ^
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 48156ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 26351ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 27519ms
282.311u 7.846s 1:43.01 281.6%  0+0k 326+462io 0pf+0w
/Users/odersky/workspace/dotty/src/dotty/tools> timed-bootstrap-repeated
dotc/typer/RefChecks.scala:68: warning: match may not be exhaustive.
It would fail on the following input: (_, _)
      for ((_, m1 :: m2 :: _) <- (clazz.info member nme.applyDynamic).alternatives groupBy (_.symbol.typeParams.length)) {
           ^
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 56052ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 32422ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 32225ms
304.655u 6.576s 2:01.60 255.9%  0+0k 347+511io 0pf+0w
/Users/odersky/workspace/dotty/src/dotty/tools> cloc .
     341 text files.
     303 unique files.                                          
      66 files ignored.

http://cloc.sourceforge.net v 1.56  T=2.0 s (139.5 files/s, 38909.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Scala                          261           9180          14354          53092
XSLT                             1             30             16            468
XML                             14              0              0            342
CSS                              1             51             21            207
Lisp                             2              6              0             52
-------------------------------------------------------------------------------
SUM:                           279           9267          14391          54161
-------------------------------------------------------------------------------

So overall 20-25% slower after warm-up. That's the gap we need to close.

odersky · 2016-08-25T15:06:11Z

@DarkDimius I added the quckfix you suggested with true || in the last commit. Can you add a commit with a permanent fix?

DarkDimius · 2016-08-25T15:31:49Z

The single failed test is related to true ||

DarkDimius · 2016-08-25T15:39:01Z

I can't reproduce the benchmark on my machine because of #1478. Don't know why it happens.

smarter · 2016-08-25T19:47:55Z

src/dotty/tools/dotc/core/SymDenotations.scala

@@ -56,6 +56,7 @@ trait SymDenotations { this: Context =>
    stillValid(owner) && (
      !owner.isClass
      || owner.isRefinementClass
+      || owner.is(Scala2x)


This reminds me of 4b0cc8a which fixed an issue with stale Java symbol and outer fields, are you sure that the stale outer field here is not incorrectly generated by Dotty?

DarkDimius · 2016-08-26T09:25:49Z

@odersky, I'm not sure how you managed to make the mixed bootstrap work.
What I'm observing is that because inference of public type of overridden members in scalac and dotty works differently, I can't run dotty partially compiled by scalac and partially compiled by dotty. Issues are similar to:

Exception in thread "main" java.lang.NoSuchMethodError:
 dotty.tools.dotc.ast.tpd$.cpy()Ldotty/tools/dotc/ast/tpd$TypedTreeCopier;
    at dotty.tools.backend.jvm.LabelDefs.transformDefDef(LabelDefs.scala:161)
    at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.goDefDef(TreeTransform.scala:831)

// dotty.tools.backend.jvm.LabelDefs was compiled by scalac against scalac compiled tpd
// when dotty will recompile tpd, it will not reify the return type here and will create
// (Untyped)TreeCopier getter

smarter · 2016-08-26T09:28:53Z

Maybe we should add explicit type signatures where needed to workaround this.

DarkDimius · 2016-08-26T09:33:56Z

Instead of using the partially bootstrapped dotty, I can safely use dotty that is entirelly compiled by dotty.

To obtain it, you need to run in sbt:

> test:runMain dotc.build

This will create a dotty.jar in the root folder of project. Using it, I get 17 seconds vs 21 second on my machine:

dark@tsf-452-wpa-6-123 ~/workspace/dotty/src/dotty/tools/dotc (fix-bootstrap-3) $ timed-bootstrap-bench
typer/RefChecks.scala:68: warning: match may not be exhaustive.
It would fail on the following input: (_, _)
      for ((_, m1 :: m2 :: _) <- (clazz.info member nme.applyDynamic).alternatives groupBy (_.symbol.typeParams.length)) {
           ^
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 35069ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 18554ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 16999ms

dark@tsf-452-wpa-6-123 ~/workspace/dotty/src/dotty/tools/dotc (fix-bootstrap-3) $ timed-bootstrap-bench-dotty
typer/RefChecks.scala:68: warning: match may not be exhaustive.
It would fail on the following input: (_, _)
      for ((_, m1 :: m2 :: _) <- (clazz.info member nme.applyDynamic).alternatives groupBy (_.symbol.typeParams.length)) {
           ^
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 39777ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 23114ms
one warning found
there were 4 deprecation warning(s); re-run with -deprecation for details
time elapsed: 21051ms

odersky · 2016-08-26T09:35:32Z

This can be avoided by compiling dotty in language:Scala2 mode.

Correction: No we do not do that (yet). Not sure what's the best way to handle this.

On Fri, Aug 26, 2016 at 11:28 AM, Guillaume Martres <
notifications@github.com> wrote:

Maybe we should add explicit type signatures where needed to workaround
this.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1477 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwlVqV1eVLUDhtbrat9alpkKA4O7M7Dks5qjrHXgaJpZM4JtKzs
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@smarter in #1477:
Maybe we should add explicit type signatures where needed to workaround
this."}],"action":{"name":"View Pull Request","url":"https://
github.com//pull/1477#issuecomment-242678840"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

odersky · 2016-08-26T09:43:00Z

OK, now just have to fix that single test failure.

DarkDimius · 2016-08-26T09:44:23Z

@odersky, the test failure has to do with the true ||. I'll take care of it.

DarkDimius · 2016-08-29T13:31:30Z

a quick heads-up. By quickly porting some optimizations(copy propagation and de-inlining) from linker I was able to get dotty compile time down to 19 sec(from 21sec) compared to 17 sec when compiled by scalac.

I'm not done yet, but hopefully I'll be able to push a version of linker capable of compiling dotty into a version faster than compiled by scalac until ScalaWorld.

odersky · 2016-08-29T13:48:04Z

Excellent. I can't wait! (but i will)

On Mon, Aug 29, 2016 at 3:31 PM, Dmitry Petrashko notifications@github.com
wrote:

a quick heads-up. By quickly porting some optimizations(copy propagation
and de-inlining) from linker I was able to get dotty compile time down to
19 sec(from 21sec) compared to 17 sec when compiled by scalac.

I'm not done yet, but hopefully I'll be able to push a version of linker
capable of compiling dotty into a version faster than compiled by scalac
until ScalaWorld.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1477 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwlVtZQmyyCEpzfU2EVfTNqdMsz5_-mks5qkt80gaJpZM4JtKzs
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@DarkDimius in #1477: a
quick heads-up. By quickly porting some optimizations(copy propagation and
de-inlining) from linker I was able to get dotty compile time down to 19
sec(from 21sec) compared to 17 sec when compiled by scalac.\r\n\r\nI'm not
done yet, but hopefully I'll be able to push a version of linker capable of
compiling dotty into a version faster than compiled by scalac until
ScalaWorld."}],"action":{"name":"View Pull Request","url":"https://
github.com//pull/1477#issuecomment-243124355"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

smarter · 2016-08-29T14:04:30Z

Could the regression have the same root cause as http://scala-lang.org/blog/2016/07/08/trait-method-performance.html ?

odersky · 2016-08-29T14:27:38Z

On Mon, Aug 29, 2016 at 4:04 PM, Guillaume Martres <notifications@github.com

wrote:

Could the regression have the same root cause as
http://scala-lang.org/blog/2016/07/08/trait-method-performance.html ?

I don't think so, since most trait uses are collection related and we still
rely on 2.11 collections.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1477 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwlVmfu5u-R_UzlCYfXFxlSKZGdD5IHks5qkubwgaJpZM4JtKzs
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@smarter in #1477:
Could the regression have the same root cause as
http://scala-lang.org/blog/2016/07/08/trait-method-performance.html
?"}],"action":{"name":"View Pull Request","url":"https://
github.com//pull/1477#issuecomment-243133291"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

DarkDimius · 2016-08-29T14:59:16Z

I don't think so. We don't use many traits in compiler and in both cases
the benchmarks are using scala2.11.5 collections.

On 29 August 2016 16:04:52 Guillaume Martres notifications@github.com wrote:

Could the regression have the same root cause as
http://scala-lang.org/blog/2016/07/08/trait-method-performance.html ?

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1477 (comment)

DarkDimius · 2016-09-01T08:06:25Z

Current status: I needed to change label-defs a lot and it somehow introduced a substantial slowdown of generated code by around 2-3x(40 secs compilation time). Trying to figure out why.

odersky · 2016-09-17T13:06:34Z

I would like make a fix to this our first priority.

nicolasstucki · 2016-09-18T11:09:42Z

/rebuild

nicolasstucki · 2016-09-18T11:46:06Z

@odersky it seams that we fixed the the issue with run/redundantParents.scala in another PR.

1. There may be calls to super on non-this. 2. there may be calls to in-dirrect super-traits.

Need to do time travel to find out whether members were created as static symbols. After LambdaLift and Flatten most symbols are static anyway.

Wehn compiling dotty repeatedly using dotc.Bench, we got a stale symbol error involving an outer field of a Scala class. These can always be assumed to be current, as we would not think to recompile Scala2 classes mixed with dotty classes in the same session (we'd need two concurrent compilers for that).

odersky · 2016-09-19T09:15:51Z

Indeed, looks like it works now. I rebased to current master. Once the tests pass, I think we should merge.

odersky · 2016-09-19T14:29:44Z

In fact, no, it does not work. With the current commits I get when I try to run the dotty-compiled compiler again:

/Users/odersky/workspace/dotty/src/dotty/tools> timed-bootstrap-repeated -d /classes
Exception in thread "main" java.lang.IncompatibleClassChangeError: Interface method reference: dotty.tools.dotc.config.PropertiesTrait.initial$propFilename()Ljava/lang/String;, is in an indirect superinterface of dotty.tools.dotc.config.WrappedProperties$AccessContr
at dotty.tools.dotc.config.WrappedProperties$AccessControl$.(WrappedProperties.scala:31)
at dotty.tools.dotc.config.WrappedProperties$AccessControl$.(WrappedProperties.scala)
at dotty.tools.dotc.config.PathResolver$Environment$.scalaExtDirs(PathResolver.scala:52)
at dotty.tools.dotc.config.PathResolver$Defaults$.scalaExtDirs(PathResolver.scala:116)
at dotty.tools.dotc.config.ScalaSettings.(ScalaSettings.scala:14)
at dotty.tools.dotc.core.Contexts$ContextBase.(Contexts.scala:527)
at dotty.tools.dotc.Driver.initCtx(Driver.scala:36)
at dotty.tools.dotc.Driver.process(Driver.scala:88)
at dotty.tools.dotc.Driver.process(Driver.scala:105)
at dotty.tools.dotc.Driver.main(Driver.scala:132)
at dotty.tools.dotc.Bench.main(Bench.scala)

odersky · 2016-09-19T16:23:55Z

And, I should note, if I remove the last commit "Remove quick fix", the bootstrap passes. So the quickfix is needed, but it should be studied wether it does the right thing.

odersky · 2016-09-19T16:29:31Z

Advice to everyone: When compiler boostrapping, don't trust any runner to do what you think it does. You are most often wrong, and see something else tested. To reproduce, I put all needed jars on my CLASSPATH, then cd to src/dotty/tools, then I run something like

java -Xms1g -Xmx3g dotty.tools.dotc.Main -d /classes **/*.scala

where the output directory /classesis the first entry of my classpath. Then I run the command again. This way I am sure I get the bootstrapped compiler and nothing else.

nicolasstucki · 2016-09-21T05:28:20Z

Which are the 'needed jars'?

odersky · 2016-09-21T06:25:55Z

On Wed, Sep 21, 2016 at 7:28 AM, Nicolas Stucki notifications@github.com
wrote:

Which are the 'needed jars'?

All tye dependencies of dotc. (which are not many). Here's my classpath,
which contains a lot of stuff not needed by dotc:

setenv CLASSPATH
/classes:/Users/odersky/workspace/dotty/classes:Users/odersky/workspace/dotty/target/scala-2.11/classes:/Users/odersky/workspace/dotty/testclasses:Users/odersky/workspace/dotty/target/scala-2.11/testclasses:/Users/odersky/workspace/dotty/interfaces/target:/Users/odersky/bin/scala-2.11.5/lib/scala-library.jar:/Users/odersky/bin/scala-2.11.5/lib/scala-reflect.jar:/Users/odersky/lib/scala-compiler-2.11.5.jar:/Users/odersky/scala/lib/scala-xml.jar:/Users/odersky/workspace/scala/build/pack/lib/jline.jar:/Users/odersky/.ivy2/local/ch.epfl.lamp/dotty-bridge/0.1.1-SNAPSHOT/jars/dotty-bridge.jar:/Users/odersky/.ivy2/cache/org.scala-sbt/api/jars/api-0.13.11.jar:/Users/odersky/.ivy2/cache/org.scala-sbt/interface/jars/interface-0.13.11.jar:/Users/odersky/.ivy2/cache/com.googlecode.java-diff-utils/diffutils/jars/diffutils-1.3.0.jar

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1477 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwlVpSiwrffYHgZbAYRXFVaMk4RCfJWks5qsMB2gaJpZM4JtKzs
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@nicolasstucki in
#1477: Which are the 'needed jars'?"}],"action":{"name":"View Pull
Request","url":"#1477 (comment)
248515267"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

Add comment what it is supposed to achieve and change the implementation to follow the comment.

odersky · 2016-09-21T18:19:03Z

I think I fixed the problem at the root, by changing CollectSuperCalls. Bootstrap works now.

odersky added the In progress label Aug 25, 2016

This was referenced Aug 25, 2016

Fix handling of superCalls. #1476

Closed

Fix SymDenot.isStatic & isStaticOwner after lambdaLift. #1475

Closed

smarter reviewed Aug 25, 2016
View reviewed changes

DarkDimius and others added 3 commits September 19, 2016 11:07

Fix handling of superCalls.

07ea487

1. There may be calls to super on non-this. 2. there may be calls to in-dirrect super-traits.

Fix select static

75358be

Need to do time travel to find out whether members were created as static symbols. After LambdaLift and Flatten most symbols are static anyway.

odersky force-pushed the fix-bootstrap-3 branch from 101d85e to dfa4ea4 Compare September 19, 2016 09:14

Fix CollectSuper

c9d670f

Add comment what it is supposed to achieve and change the implementation to follow the comment.

odersky force-pushed the fix-bootstrap-3 branch from dfa4ea4 to c9d670f Compare September 21, 2016 17:15

odersky merged commit a83d436 into scala:master Sep 21, 2016

odersky removed the In progress label Sep 21, 2016

allanrenucci deleted the fix-bootstrap-3 branch December 14, 2017 19:19

Fix bootstrap, take 3 #1477

Fix bootstrap, take 3 #1477

Uh oh!

Conversation

odersky commented Aug 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odersky commented Aug 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DarkDimius commented Aug 25, 2016

Uh oh!

DarkDimius commented Aug 25, 2016

Uh oh!

smarter Aug 25, 2016

Choose a reason for hiding this comment

Uh oh!

DarkDimius commented Aug 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smarter commented Aug 26, 2016

Uh oh!

DarkDimius commented Aug 26, 2016

Uh oh!

odersky commented Aug 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odersky commented Aug 26, 2016

Uh oh!

DarkDimius commented Aug 26, 2016

Uh oh!

DarkDimius commented Aug 29, 2016

Uh oh!

odersky commented Aug 29, 2016

Uh oh!

smarter commented Aug 29, 2016

Uh oh!

odersky commented Aug 29, 2016

Uh oh!

DarkDimius commented Aug 29, 2016

Uh oh!

DarkDimius commented Sep 1, 2016

Uh oh!

odersky commented Sep 17, 2016

Uh oh!

nicolasstucki commented Sep 18, 2016

Uh oh!

nicolasstucki commented Sep 18, 2016

Uh oh!

odersky commented Sep 19, 2016

Uh oh!

odersky commented Sep 19, 2016

Uh oh!

odersky commented Sep 19, 2016

Uh oh!

odersky commented Sep 19, 2016

Uh oh!

nicolasstucki commented Sep 21, 2016

Uh oh!

odersky commented Sep 21, 2016

Uh oh!

odersky commented Sep 21, 2016

Uh oh!

Uh oh!

odersky commented Aug 25, 2016 •

edited

Loading

odersky commented Aug 25, 2016 •

edited

Loading

DarkDimius commented Aug 26, 2016 •

edited

Loading

odersky commented Aug 26, 2016 •

edited

Loading