Add sbt-based bootstrap #1896
Merged
Add sbt-based bootstrap #1896
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review by @felixmulder for the sbt/tests related stuff and by @odersky for everything else :). |
|
Every day is a new adventure, apparently compiling dotty (with scalac) is enough to fill up the code cache of the JVM now? http://dotty-ci.epfl.ch/lampepfl/dotty/530/2 CodeCache: size=131072Kb used=124845Kb max_used=125852Kb free=6226Kb
bounds [0x00007f0cd0d0f000, 0x00007f0cd8d0f000, 0x00007f0cd8d0f000]
total_blobs=27273 nmethods=26706 adapters=462
compilation: disabled (not enough contiguous free space left)
OpenJDK 64-Bit Server VM warning: CodeCache is full. Compiler has been disabled.
OpenJDK 64-Bit Server VM warning: Try increasing the code cache size using -XX:ReservedCodeCacheSize= |
We decided to handle bootstrapping outside of the SBT build in the Scala build for this reason. We use |
|
I think I can handle it by restricting sbt task parallelism when I bootstrap (right now it's happily compiling/publishing everything at the same time) |
|
Oh, I think what's going is that it's compiling dotty twice at the same time, I thought sbt whole job was to not do that sort of things: [info] Compiling 296 Scala sources and 1 Java source to /drone/src/github.com/lampepfl/dotty/compiler/target/scala-2.11/classes...
...
[info] Compiling 296 Scala sources and 1 Java source to /drone/src/github.com/lampepfl/dotty/compiler/target/scala-2.11/classes... |
smarter
force-pushed
the
fix/bootstrap
branch
5 times, most recently
from
851a0b9 to
ab209a1
Compare
|
lampepfl/scala#4 was merged |
smarter
force-pushed
the
fix/bootstrap
branch
from
ab209a1 to
6d1c004
Compare
felixmulder
approved these changes
Jan 11, 2017
| TestFrameworks.JUnit, "-a", "-v", | ||
| "--run-listener=dotty.tools.ContextEscapeDetector" | ||
| TestFrameworks.JUnit, "-a", "-v"/*, | ||
| "--run-listener=dotty.tools.ContextEscapeDetector"*/ |
There was a problem hiding this comment.
Does this need to be disabled still?
There was a problem hiding this comment.
Until we have a published new backend and #1894 is merged, yes. (I'll wait for these things to happen before merging this PR)
| // val s = state.value | ||
| // val e = Project.extract(s) | ||
| // e.runAggregated(publishLocal in dotty, s) | ||
| // }) |
There was a problem hiding this comment.
comment in empty settings?
There was a problem hiding this comment.
Yeah I should comment the whole thing
If something needs to be fixed, fix it at the source. This prevented dotty-compiler-bootstrapped from using the dotty-library-bootstrapped clases instead of the dotty-library jar
This is necessary to run the tests with the bootstrapped projects and is just much better than hardcoding them anyway.
This adds two new project to the sbt build: dotty-library-bootstrapped and dotty-compiler bootstrapped. These projects use the same source files as dotty-library and dotty-compiler but are compiled using dotty itself. The main usecase for this is that we can now run the JUnit tests (which are _not_ just a subset of the tests run by partest, for example the REPL tests are only run through JUnit) with a bootstrapped compiler: $ sbt > publishLocal # Non-bootstrapped dotty needs to be published first > dotty-compiler-bootstrapped/test But this also allows one to experiment with a bootstrapped dotty much more easily in general. This revealed many issues in the compiler that are fixed in subsequent commits in this PR.
smarter
force-pushed
the
fix/bootstrap
branch
from
6d1c004 to
f629581
Compare
|
Rebased to master which contains #1912 which was the last blocker to get this in |
`partest` and `partest-only` are now run through `dotty-compiler-bootstrapped`. The old bootstrapping mechanism is deleted since it has been unmaintained and broken for several months and that I do not wish to maintain two bootstrapping mechanisms.
smarter
force-pushed
the
fix/bootstrap
branch
from
f629581 to
0facffb
Compare
odersky
reviewed
Jan 28, 2017
| } | ||
| _DottyPredefModuleRef | ||
| } | ||
| private[this] var _DottyPredefModuleRef: TermRef = _ |
There was a problem hiding this comment.
Aside: Following @Ichoran's example, I tend to use myDottyPredefModuleRef for private vars that have a getter DottyPredefModuleRef. But _... notation works as well.
There was a problem hiding this comment.
OK, let's standardize on my
| @@ -398,8 +398,13 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn | |||
| override def extraClasspath = | |||
| suiteRunner.fileManager.asInstanceOf[DottyFileManager].extraJarList ::: super.extraClasspath | |||
|
|
|||
|
|
|||
| // FIXME: Dotty deviation: error if return type is emitted: | |||
odersky
approved these changes
Jan 28, 2017
As reportd by dotty (same thing with showShop): cyclic reference involving implicit value showCar This happens when the right hand-side of value showCar's definition involves an implicit search. To avoid the error, give value showCar an explicit type.
Using changeOwnerAfter would be more appropriate but currently fails with an assertion in LambdaLift
The bug was that we declared case classes like:
case class CompFailed() extends NegTestState
but we used their companion objects like in:
case _ => CompFailed
Interestingly, this bug was caught by compiling this code with dotty,
instead of `failureStates` getting inferred to be of type `AnyRef`, it
ended up being a union of object types, this allows dotty to realize our
subsequent pattern match on `failureStates` cannot possibly succeed:
-- Error: /home/smarter/opt/dotty/compiler/test/dotty/partest/DPConsoleRunner.scala
353 | case CompFailedButWrongDiff() =>
| ^
| missing parameter type for parameter x$1 of expanded function x$1 =>
| x$1 @unchecked match
| {
| case CompFailedButWrongDiff() =>
| nextTestActionFailing(s"output differs")
| true
| case _ =>
| false
| }, expected = ?
-- Error: /home/smarter/opt/dotty/compiler/test/dotty/partest/DPConsoleRunner.scala
353 | case CompFailedButWrongDiff() =>
| ^^^^^^^^^^^^^^^^^^^^^^^^
|Pattern type CompFailedButWrongDiff is neither a subtype nor a supertype of selector type CompSucceeded | CompFailedButWrongNErr | CompFailed | CompFailedButWrongDiff'where: CompFailedButWrongDiff is a class in method runNegTest
| CompFailedButWrongDiff' is a object in method runNegTest
Type#member might return a denotation that doesn't "really exists" (as defined by TypeAssigner#reallyExists), in some circumstance this denotation can refer to a symbol in a class that is in the classpath but that is not used by this file, so using addDependency on the result of Type#member might add a false dependency. We avoid this by using Type#select instead which will internally do the right thing. This issue was discovered while compiling the bootstrapped projects which would sometimes force a full recompilation for no reason.
sbt runs up to `Runtime.getRuntime.availableProcessors` tasks in parallel, on our CI that means 40 tasks, compilation tasks are the one with the biggest footprint and the introduction of `dotty-library-bootstrapped` and `dotty-compiler-bootstrapped` in this PR means that sbt can now run more compilation tasks in parallel. To prevent the JVM from exploding, we increase: - The max heap size, from 1G to 4G - The max code cache size, from 240M to 512M - The max metaspace size, from 256M to 1G
This way you can run both the dotty-library-bootstrapped and dotty-compiler-bootstrapped tests with one command: sbt ;publishLocal;dotty-bootstrapped/test
smarter
force-pushed
the
fix/bootstrap
branch
from
0facffb to
fc2f931
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR might be the hardest I've worked on just because of all the deep and subtle issues in dotty it forced me to figure out (especially #1770, #1856, #1895), but is now finally ready!
(Well, almost, the backend needs to updated with lampepfl/scala#4 first and we need to get #1894 in, but other than that it should be complete).This adds two new project to the sbt build: dotty-library-bootstrapped and dotty-compiler-bootstrapped. These projects use the same source files as dotty-library and dotty-compiler but are compiled using dotty itself. The main usecase for this is that we can now run the JUnit tests (which are not just a subset of the tests run by partest, for example the REPL tests are only run through JUnit) with a bootstrapped compiler:
But this also allows one to experiment with a bootstrapped dotty in a much nicer way. This also replaces the previous bootstrap mechanism used for partest:
partestandpartest-onlywill now go throughdotty-compiler-bootstrapped. Since partest bootstrapping has been broken for several months now, I consider that a win (we could also keep the old mechanism around but only if someone volunteers to maintain it).