Improve and make scripted parallel #440
Conversation
|
A future improvement (in another PR) could be to gzip the scripted logs directory to take less space. I don't know if this is worth it, though. The logs should be relatively short and hence not take a lot of space. Before implementing this feature, I would spend some research to double-check that size is a problem. Perhaps this would be a good spree ticket. |
| def finish(state: Option[IncInstance]): Unit = () | ||
|
|
||
| def initialState: Option[IncInstance] = { | ||
| initBuildStructure() |
There was a problem hiding this comment.
This is required so that batch execution reusing the same IncHandler works.
| @@ -1,5 +1,4 @@ | |||
| # Marked as pending, see https://github.com/sbt/sbt/issues/1543 | |||
| # | |||
There was a problem hiding this comment.
Empty comments are not parsed correctly, so that's why they're removed. The parser error was not happening before because parser errors were recognized as legit scripted errors, even though they are not (!).
This means that a pending test could have given a false positive.
jvican
force-pushed
the
independent-scripted-parallel
branch
from
45f0311
to
dd20e7c
Compare
These are the improvements that I've added to scripted:
1. Scripted is now parallel and does batch execution.
2. Scripted logs to both a file and the console (if `bufferLog == true`).
All the logs can be inspected locally by going to a directory in
`/tmp`. This directory is shown to the user at the end of the
execution.
3. Scripted UI has been improved.
3.1. Colors are used for `+` and `x`.
3.1. It shows the command that actually failed, not `Command failed {line 1}`.
3.2. It trims the stack traces of the wrapping exceptions
(corresponding to the scripted infrastructure). Only the stack traces
of the causing exceptions are shown (which are the ones we're
interested in and are usually assertion errors).
I think these improvements enhance the current dev workflow
considerably. I invite you to give them a try.
This change combined with sbt#429, gives a really fast execution of
scripted. Testing just one test is under 7 seconds in my machine (note
that in those 7 seconds we have to fetch the bridge, etc).
jvican
force-pushed
the
independent-scripted-parallel
branch
from
dd20e7c
to
6c2c325
Compare
|
This is really cool but also kind of a shame that these improvements are limited to the custom implementation of scripted used by zinc, and not available for regular users of scripted. It means for example that I cannot take advantage of this in Dotty where we run all the zinc scripted tests too to test our incremental compiler. |
|
@smarter You can indeed take advantage of this in your fork, wait for sbt/sbt#3657 to land. The batch and parallel execution was first merged in sbt/sbt. There are some small changes in this implementation that you won't benefit from. If I were you, I would depend via source dependency on this build and use the scripted implementation provided by the build. I think it's important that this implementation is specific to Zinc. It's only for development, it's not published, and this allows us to improve the implementation much faster, without being bound by making changes in other sbt modules and abiding by bincompat requirements. It's worth noting that the code in this implementation is simpler than the one in sbt/sbt because we can assume some things about our setup. |
|
Ah, I hadn't seen sbt/sbt#3657, cool! I still feel it'd be nice if zinc-scripted/sbt-scripted/scripted-plugin shared code since you end up having to duplicate things like #441 |
|
It's worth nothing that not sharing code between sbt's scripted and zinc's has been the status quo for a long time -- this PR only makes it more obvious. I'm not sure that sharing it would simplify anything. From my perspective, it would make things harder. For one, every change would need to go first through sbt-util and be released in a new artifact. For the other one, the work of abstracting over the different scripted backends is demanding and time-consuming, and so is changing it. In my opinion, another advantage of implementing the logic here is that I can test everything manually in the zinc build and make sure there's not a regression (as I've done with this PR). Making changes to an scripted implementation shared by all the sbt plugins in the world, sbt and zinc would not give me peace of mind. |
|
@eed3si9n Can you review this please? |
| val logFile = createScriptedLogFile(loggerName) | ||
| val logger = rebindLogger(batchLogger, logFile) | ||
|
|
||
| println(s"Running $label") |
There was a problem hiding this comment.
Why isn't this output to the logger?
These are the improvements that I've added to scripted:
bufferLog == true).All the logs can be inspected locally by going to a directory in
/tmp. This directory is shown to the user at the end of theexecution.
3.1. Colors are used for
+andx.3.1. It shows the command that actually failed, not
Command failed {line 1}.3.2. It trims the stack traces of the wrapping exceptions
(corresponding to the scripted infrastructure). Only the stack traces
of the causing exceptions are shown (which are the ones we're
interested in and are usually assertion errors).
I think these improvements enhance the current dev workflow
considerably. I invite you to give them a try.
This change combined with #429, gives a really fast execution of
scripted. Testing just one test is under 7 seconds in my machine (note
that in those 7 seconds we have to fetch the bridge, publish everything,
etc).