Skip to content

Commit

Permalink
Turn on parallelism by default, ensure Commands always run serially a…
Browse files Browse the repository at this point in the history
…t the end (#3265)

* Remove the hardcoded setting of `threadCount = Some(1)`

* Split `Task`s and `Command`s into two separate batches, to only run
`Command`s serially after all `Task`s have run

* Introduce a `MILL_TEST_DEST_FOLDER` environment variable in our
standard `TestModule`. This is something we needed to do to avoid
collisions between parallel test modules in Mill's own build (e.g.
everyone stomping over `target/worksources`) and will be something that
Mill users will need as well

Commands are typically the less well-behaved tasks, whereas Targets and
others are meant to be pure functional and thread safe without side
effects. So running commands serially at the end is a decent tradeoff
the do the right thing by default, and if someone wants parallelism they
can always use a task (e.g. `.testCached` instead of `.test`)

Commands called from other tasks, e.g. those tested by
`example/misc/5-module-run-task`, are run in parallel together with the
main group. We assume that these commands are well behaved, as their
results will potentially be cached (as part of downstream tasks), and so
I run them as part of the primary task graph.

(An alternative could have been to run these sequentially as well, but
given the nature of commands-run-as-part-of-tasks I think that would be
unnecessarily conservative)

I disable the thread-number log prefixes in the integration tests, since
all they would do there is add clutter.

Pull request: #3265
  • Loading branch information
lihaoyi authored Aug 8, 2024
1 parent 4783eee commit 044b2a4
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 112 deletions.
2 changes: 2 additions & 0 deletions build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ trait MillStableScalaModule extends MillPublishScalaModule with Mima {
ProblemFilter.exclude[ReversedMissingMethodProblem](
"mill.scalalib.JavaModule.mill$scalalib$JavaModule$$super$runMain"
),
// Terminal is sealed, not sure why MIMA still complains
ProblemFilter.exclude[ReversedMissingMethodProblem]("mill.eval.Terminal.task"),

// Not sure why mima is picking up this stuff which is private[mill]
ProblemFilter.exclude[Problem]("mill.resolve.*.resolve0"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
// with no changes
val initial = evalStdout("outer.inner.qux")
assert(
initial.out.linesIterator.toSeq == Seq(
initial.out.linesIterator.toSet == Set(
"running foo",
"running helperFoo",
"running bar",
Expand All @@ -32,7 +32,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("running foo", "running foo2"))
val mangledFoo = evalStdout("outer.inner.qux")
assert(
mangledFoo.out.linesIterator.toSeq == Seq(
mangledFoo.out.linesIterator.toSet == Set(
"running foo2",
"running helperFoo"
// The return value of foo did not change so qux is not invalidated
Expand All @@ -42,7 +42,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("; helperFoo }", "; helperFoo + 4 }"))
val mangledHelperFooCall = evalStdout("outer.inner.qux")
assert(
mangledHelperFooCall.out.linesIterator.toSeq == Seq(
mangledHelperFooCall.out.linesIterator.toSet == Set(
"running foo2",
"running helperFoo",
// The return value of foo changes from 1 to 1+4=5, so qux is invalidated
Expand All @@ -54,9 +54,9 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("running qux", "running qux2"))
val mangledQux = evalStdout("outer.inner.qux")
assert(
mangledQux.out.linesIterator.toSeq ==
mangledQux.out.linesIterator.toSet ==
// qux itself was changed, and so it is invalidated
Seq("running qux2", "running helperQux")
Set("running qux2", "running helperQux")
)

// Changing the body of some helper method that gets called by a T{...}
Expand All @@ -65,7 +65,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace(" 1 ", " 6 "))
val mangledHelperFooValue = evalStdout("outer.inner.qux")
assert(
mangledHelperFooValue.out.linesIterator.toSeq == Seq(
mangledHelperFooValue.out.linesIterator.toSet == Set(
"running foo2",
"running helperFoo",
// Because the return value of helperFoo/foo changes from 1+4=5 to 6+5=11, qux is invalidated
Expand All @@ -77,7 +77,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("running helperBar", "running helperBar2"))
val mangledHelperBar = evalStdout("outer.inner.qux")
assert(
mangledHelperBar.out.linesIterator.toSeq == Seq(
mangledHelperBar.out.linesIterator.toSet == Set(
"running bar",
"running helperBar2"
// We do not need to re-evaluate qux because the return value of bar did not change
Expand All @@ -87,7 +87,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("20", "70"))
val mangledHelperBarValue = evalStdout("outer.inner.qux")
assert(
mangledHelperBarValue.out.linesIterator.toSeq == Seq(
mangledHelperBarValue.out.linesIterator.toSet == Set(
"running bar",
"running helperBar2",
// Because the return value of helperBar/bar changes from 20 to 70, qux is invalidated
Expand All @@ -99,17 +99,17 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("running helperQux", "running helperQux2"))
val mangledBar = evalStdout("outer.inner.qux")
assert(
mangledBar.out.linesIterator.toSeq ==
mangledBar.out.linesIterator.toSet ==
// helperQux was changed, so qux needs to invalidate
Seq("running qux2", "running helperQux2")
Set("running qux2", "running helperQux2")
)

// Make sure changing `val`s in varying levels of nested modules conservatively invalidates
// all targets in inner modules, regardless of whether they are related or not
mangleFile(wsRoot / "build.sc", _.replace("val valueFoo = 0", "val valueFoo = 10"))
val mangledValFoo = evalStdout("outer.inner.qux")
assert(
mangledValFoo.out.linesIterator.toSeq == Seq(
mangledValFoo.out.linesIterator.toSet == Set(
"running foo2",
"running helperFoo",
"running bar",
Expand All @@ -122,7 +122,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("val valueBar = 0", "val valueBar = 10"))
val mangledValBar = evalStdout("outer.inner.qux")
assert(
mangledValBar.out.linesIterator.toSeq == Seq(
mangledValBar.out.linesIterator.toSet == Set(
"running bar",
"running helperBar2",
"running qux2",
Expand All @@ -133,7 +133,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
mangleFile(wsRoot / "build.sc", _.replace("val valueQux = 0", "val valueQux = 10"))
val mangledValQux = evalStdout("outer.inner.qux")
assert(
mangledValQux.out.linesIterator.toSeq == Seq(
mangledValQux.out.linesIterator.toSet == Set(
"running qux2",
"running helperQux2"
)
Expand All @@ -145,7 +145,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
)
val mangledValFooUsedInBar = evalStdout("outer.inner.qux")
assert(
mangledValFooUsedInBar.out.linesIterator.toSeq == Seq(
mangledValFooUsedInBar.out.linesIterator.toSet == Set(
"running foo2",
"running helperFoo",
"running bar",
Expand All @@ -161,7 +161,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
)
val mangledValBarUsedInQux = evalStdout("outer.inner.qux")
assert(
mangledValBarUsedInQux.out.linesIterator.toSeq == Seq(
mangledValBarUsedInQux.out.linesIterator.toSet == Set(
"running bar",
"running helperBar2",
"running qux2",
Expand Down Expand Up @@ -191,7 +191,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
"trait" - {
val initial = evalStdout("traitOuter.traitInner.inner")
assert(
initial.out.linesIterator.toSeq == Seq(
initial.out.linesIterator.toSet == Set(
"running foo",
"running helperFoo",
"running outer",
Expand All @@ -210,7 +210,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
)
val mangleTraitInnerValue = evalStdout("traitOuter.traitInner.inner")
assert(
mangleTraitInnerValue.out.linesIterator.toSeq == Seq(
mangleTraitInnerValue.out.linesIterator.toSet == Set(
"running inner",
"running helperTraitInner"
)
Expand All @@ -222,7 +222,7 @@ object CodeSigNestedTests extends IntegrationTestSuite {
)
val mangleTraitOuterValue = evalStdout("traitOuter.traitInner.inner")
assert(
mangleTraitOuterValue.out.linesIterator.toSeq == Seq(
mangleTraitOuterValue.out.linesIterator.toSet == Set(
"running outer",
"running helperTraitOuter",
"running inner",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import utest._
object CodeSigScalaModuleTests extends IntegrationTestSuite {
val tests: Tests = Tests {
def filterLines(out: String) = {
out.linesIterator.filter(!_.contains("[info]")).toSeq
out.linesIterator.filter(!_.contains("[info]")).toSet
}
val wsRoot = initWorkspace()
"single" - {
Expand All @@ -17,7 +17,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(initial.out) ==
Seq(
Set(
"Foo generating sources...",
"Foo compiling...",
"Foo Hello World",
Expand All @@ -28,7 +28,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {
val cached = evalStdout("foo.run")
assert(
filterLines(cached.out) ==
Seq(
Set(
"Foo Hello World",
"Foo running..."
)
Expand All @@ -38,7 +38,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {
// and any downstream targets
mangleFile(wsRoot / "build.sc", _.replace("Foo running...", "FOO RUNNING"))
val mangledFoo = evalStdout("foo.run")
assert(filterLines(mangledFoo.out) == Seq("Foo Hello World", "FOO RUNNING"))
assert(filterLines(mangledFoo.out) == Set("Foo Hello World", "FOO RUNNING"))

// Changing the body `foo.compile` invalidates `foo.compile`, and downstream
// `foo.run` runs regardless
Expand All @@ -47,7 +47,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(mangledFoo2.out) ==
Seq(
Set(
"FOO COMPILING",
"Foo Hello World",
"FOO RUNNING"
Expand All @@ -64,7 +64,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(mangledFoo3.out) ==
Seq(
Set(
"FOO GENERATING SOURCES",
"Foo Hello World",
"FOO RUNNING"
Expand All @@ -78,7 +78,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(mangledFoo4.out) ==
Seq(
Set(
"FOO GENERATING SOURCES",
"FOO COMPILING",
"Foo HELLO WORLD",
Expand All @@ -91,7 +91,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(mangledFoo5.out) ==
Seq(
Set(
"FOO COMPILING",
"Foo HELLO WORLD",
"FOO RUNNING"
Expand All @@ -111,7 +111,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {
val mangledFoo6 = evalStdout("foo.run")
assert(
filterLines(mangledFoo6.out) ==
Seq(
Set(
"Foo HELLO WORLD",
"FOO RUNNING"
)
Expand All @@ -128,7 +128,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(initial.out) ==
Seq(
Set(
"Foo generating sources...",
"Foo compiling...",
"Foo assembly...",
Expand All @@ -142,7 +142,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {
)

val cached = evalStdout("{foo,bar,qux}.assembly")
assert(filterLines(cached.out) == Seq())
assert(filterLines(cached.out) == Set())

// Changing the implementation of foo.compile or foo.generatedSources
// without changing its return value causes that specific target to
Expand All @@ -154,14 +154,14 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {
// evaluation can see the return value was not changed and avoid invalidation
mangleFile(wsRoot / "build.sc", _.replace("Foo compiling...", "FOO COMPILING"))
val mangledFoo2 = evalStdout("{foo,bar,qux}.assembly")
assert(filterLines(mangledFoo2.out) == Seq("FOO COMPILING"))
assert(filterLines(mangledFoo2.out) == Set("FOO COMPILING"))

mangleFile(
wsRoot / "build.sc",
_.replace("Foo generating sources...", "FOO generating sources")
)
val mangledFoo3 = evalStdout("{foo,bar,qux}.assembly")
assert(filterLines(mangledFoo3.out) == Seq("FOO generating sources"))
assert(filterLines(mangledFoo3.out) == Set("FOO generating sources"))

// Changing the implementation of foo.generatedSources in a way that changes
// its return value does cause downstream targets in foo and bar to invalidate,
Expand All @@ -174,7 +174,7 @@ object CodeSigScalaModuleTests extends IntegrationTestSuite {

assert(
filterLines(mangledFoo4.out) ==
Seq(
Set(
"FOO generating sources",
"FOO COMPILING",
"Foo assembly...",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import utest._

object ScriptsInvalidationTests extends IntegrationTestSuite {

def runTask(task: String): Vector[String] = {
def runTask(task: String): Set[String] = {
val res = evalStdout(task)
assert(res.isSuccess)
res.out.linesIterator.map(_.trim).toVector
res.out.linesIterator.map(_.trim).toSet
}

val tests: Tests = Tests {
Expand All @@ -17,7 +17,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {

val result = runTask("task")

val expected = Seq("a", "d", "b", "c")
val expected = Set("a", "d", "b", "c")

assert(result == expected)
}
Expand All @@ -39,7 +39,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
initWorkspace()

val result = runTask("task")
val expected = Seq("a", "d", "b", "c")
val expected = Set("a", "d", "b", "c")

assert(result == expected)
}
Expand All @@ -51,7 +51,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
os.write.over(workspacePath / inputD, newContent)

val result = runTask("task")
val expected = Seq("d2", "b")
val expected = Set("d2", "b")

assert(result == expected)
}
Expand All @@ -61,7 +61,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
initWorkspace()

val result = runTask("module.task")
val expected = Seq("a", "d", "b", "c", "task")
val expected = Set("a", "d", "b", "c", "task")

assert(result == expected)
}
Expand All @@ -73,7 +73,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
os.write.over(workspacePath / buildPath, newContent)

val result = runTask("module.task")
val expected = Seq("task2")
val expected = Set("task2")

assert(result == expected)
}
Expand All @@ -83,7 +83,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
initWorkspace()

val result = runTask("taskE")
val expected = Seq("a", "e", "taskE")
val expected = Set("a", "e", "taskE")

assert(result == expected)
}
Expand All @@ -95,7 +95,7 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
os.write.over(workspacePath / buildPath, newContent)

val result = runTask("taskE")
val expected = Seq("taskE2")
val expected = Set("taskE2")

assert(result == expected)
}
Expand All @@ -104,15 +104,15 @@ object ScriptsInvalidationTests extends IntegrationTestSuite {
initWorkspace()

val result = runTask("taskSymbols")
val expected = Seq("taskSymbols")
val expected = Set("taskSymbols")

assert(result == expected)
}
test("should handle ammonite files with symbols") {
initWorkspace()

val result = runTask("taskSymbolsInFile")
val expected = Seq("taskSymbolsInFile")
val expected = Set("taskSymbolsInFile")

assert(result == expected)
}
Expand Down
Loading

0 comments on commit 044b2a4

Please sign in to comment.