Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mill.scalalib.Dependency/showUpdates seems much slower due to forced non-parallel execution #3359

Closed
lefou opened this issue Aug 9, 2024 · 7 comments · Fixed by #3429
Closed
Milestone

Comments

@lefou
Copy link
Member

lefou commented Aug 9, 2024

I noticed this is a project where I regularily run:

> mill -j 0 mill.scalalib.Dependency/showUpdates --format PerDependency
...
[1/2] mill.scalalib.Dependency.updates > Resolving dependencies [99/109]: module.document.test
...
[1/2] mill.scalalib.Dependency.updates > Analyzing dependencies [59/1761]: module.document.test / org.fedorahosted.tennera:jgettext
...

The projects has a lot of modules and dependencies. The passed evaluator now seems to run with only one job. I spend some time to improve the performance of showUpdates, so it should ran much faster with parallelization turned on.

Related:

We probably needs some way to easily enable parallelism for evaluator commands. E.g. Tasks run with show will otherwise always run in single-job mode.

@lihaoyi
Copy link
Member

lihaoyi commented Aug 10, 2024

I guess we can configure the evaluator that gets passed to an evaluator command differently for the evaluator that runs it? Only the evaluator running show needs to be sequential, the evaluator that gets passed to it can follow the normal --jobs config

@lihaoyi
Copy link
Member

lihaoyi commented Aug 10, 2024

I'm actually not seeing the evaluator commands causing nested evaluations to run sequentially. Seems running show __.compile still seems to compile things in parallel, and effectiveThreadCount is still set to the number of cores :

lihaoyi mill$ git diff
diff --git a/main/eval/src/mill/eval/GroupEvaluator.scala b/main/eval/src/mill/eval/GroupEvaluator.scala
index 3ee01997a9..129526d572 100644
--- a/main/eval/src/mill/eval/GroupEvaluator.scala
+++ b/main/eval/src/mill/eval/GroupEvaluator.scala
@@ -41,6 +41,8 @@ private[mill] trait GroupEvaluator {
   val effectiveThreadCount: Int =
     this.threadCount.getOrElse(Runtime.getRuntime().availableProcessors())

+  pprint.log(effectiveThreadCount)
+
   /**
    * Synchronize evaluations of the same terminal task.
    * This isn't necessarily needed for normal Mill executions,
lihaoyi mill$ ./mill -i dev.run example/scalabuilds/9-realistic -i show __.compile
[2035/2035] dev.run
GroupEvaluator.scala:44 effectiveThreadCount: 10
[#04] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/mill-build/compile.dest/classes ...
[#04] [warn] /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/mill-build/generateScriptSources.dest/build/build.sc:14:32: trait JavaModuleTests in trait JavaModule is deprecated (since Mill 0.11.10): Use JavaTests instead
[#04] [warn]     _root_.mill.define.Discover[build_]
[#04] [warn]                                ^
[#04] [warn] one warning found
[#04] [info] done compiling
GroupEvaluator.scala:44 effectiveThreadCount: 10
GroupEvaluator.scala:44 effectiveThreadCount: 10
[#01] [info] compiling 1 Java source to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/qux/compile.dest/classes ...
[#01] [info] done compiling
[#02] [info] compiling 2 Scala sources to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/2.13.8/compile.dest/classes ...
[#00] [info] compiling 2 Scala sources to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/3.3.3/compile.dest/classes ...
[#02] [info] done compiling
[#08] [info] compiling 2 Scala sources to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/2.13.8/compile.dest/classes ...
[#02] [info] compiling 2 Scala sources to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/2.13.8/test/compile.dest/classes ...
[#08] [info] done compiling
[#03] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/2.13.8/test/compile.dest/classes ...
[#02] [info] done compiling
[#00] [info] done compiling
[#00] [info] compiling 2 Scala sources to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/3.3.3/compile.dest/classes ...
[#01] [info] compiling 2 Scala sources to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/3.3.3/test/compile.dest/classes ...
[#03] [info] done compiling
[#00] [info] done compiling
[#05] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/3.3.3/test/compile.dest/classes ...
[#01] [info] done compiling
[#05] [info] done compiling
{
  "bar[2.13.8].compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/2.13.8/compile.dest/zinc",
    "classes": "ref:v0:2e67d3d3:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/2.13.8/compile.dest/classes"
  },
  "bar[2.13.8].test.compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/2.13.8/test/compile.dest/zinc",
    "classes": "ref:v0:21464c17:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/2.13.8/test/compile.dest/classes"
  },
  "bar[3.3.3].compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/3.3.3/compile.dest/zinc",
    "classes": "ref:v0:2d45fce7:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/3.3.3/compile.dest/classes"
  },
  "bar[3.3.3].test.compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/3.3.3/test/compile.dest/zinc",
    "classes": "ref:v0:9fd3283d:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/bar/3.3.3/test/compile.dest/classes"
  },
  "foo[2.13.8].compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/2.13.8/compile.dest/zinc",
    "classes": "ref:v0:9c25402c:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/2.13.8/compile.dest/classes"
  },
  "foo[2.13.8].test.compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/2.13.8/test/compile.dest/zinc",
    "classes": "ref:v0:423b09f2:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/2.13.8/test/compile.dest/classes"
  },
  "foo[3.3.3].compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/3.3.3/compile.dest/zinc",
    "classes": "ref:v0:d6516f96:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/3.3.3/compile.dest/classes"
  },
  "foo[3.3.3].test.compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/3.3.3/test/compile.dest/zinc",
    "classes": "ref:v0:36c604e0:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/foo/3.3.3/test/compile.dest/classes"
  },
  "qux.compile": {
    "analysisFile": "/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/qux/compile.dest/zinc",
    "classes": "ref:v0:8006ecfd:/Users/lihaoyi/Github/mill/example/scalabuilds/9-realistic/out/qux/compile.dest/classes"
  }
}

@lefou
Copy link
Member Author

lefou commented Aug 11, 2024

I haven't tested with show but mill.scalalib.Dependency/showUpdates --format PerDependency. Maybe, there is some difference how these are implemented?

But as you can probably see, in my OP, there is no thread indication ([#01] prefix in progress output).

I'll invesigate ...

@lefou lefou changed the title Evaluator commands are now much slower due to forced non-parallel execution mill.scalalib.Dependency/showUpdates seems much slower due to forced non-parallel execution Aug 12, 2024
@lefou
Copy link
Member Author

lefou commented Aug 28, 2024

The log messages are misleading. Since we're now passing the non-parallel RunNow: ExcutionContext to the inner def evaluateTerminals, when executing the commands, the value of GroupEvaluator.effectiveThreadCount in effectively ignored. Hence, all tasks scheduled by the evaluator command are run sequentially.

I think we need to make the "evaluate command at the end" logic optional and only activate it for directly started evaluations. We might need a parameter in evaluate for it.

@lihaoyi
Copy link
Member

lihaoyi commented Aug 28, 2024

That sounds reasonable. It's likely there will be many cases where commands are pure enough to run in parallel, but we dont really have a way of distinguishing them in the build, so the decision would have to be made externally

@lefou
Copy link
Member Author

lefou commented Aug 28, 2024

Ideally, a command itself can give a hint.

lefou added a commit that referenced this issue Aug 28, 2024
Introduced a new `serialCommandExec` parameter in `evaluate` and added required binary compatibility shims.

Fix #3359
@lihaoyi
Copy link
Member

lihaoyi commented Aug 28, 2024

Yeah, some kind of parallelizableCommand or safeCommand or something could work

@lefou lefou closed this as completed in e5eb648 Aug 28, 2024
@lefou lefou added this to the 0.12.0 milestone Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants