From a9225cc9eec5242ead3c27a9446f5181a4bc32b0 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Tue, 26 Sep 2023 16:42:28 +0200 Subject: [PATCH] Optimize ScalaJSModule and cache `IRFileCache` (#2783) This PR contains various optimizations in `ScalaJSModule` and `ScalaJSWorkerImpl`. Changes: - Limit the conversions between types and try to propagate the values to the worker to avoid allocations - Avoid manually splitting `sjsir` files and `jar` files manually and let the Scala.js toolchain do it with `PathIRContainer.fromClasspath(runClasspath)` passing the full runClasspath - Avoid allocating extra lists - Cache the `IRFileCache.Cache` together with the linker so it can be reused in multiple invocations of `fastLinkJS` Pull request: https://github.com/com-lihaoyi/mill/pull/2783 --- .../src/mill/scalajslib/ScalaJSModule.scala | 23 +++---- .../scalajslib/worker/ScalaJSWorker.scala | 13 ++-- .../worker/api/ScalaJSWorkerApi.scala | 6 +- .../scalajslib/worker/ScalaJSWorkerImpl.scala | 69 +++++++++---------- 4 files changed, 49 insertions(+), 62 deletions(-) diff --git a/scalajslib/src/mill/scalajslib/ScalaJSModule.scala b/scalajslib/src/mill/scalajslib/ScalaJSModule.scala index cc656617cd6..c0412bbb043 100644 --- a/scalajslib/src/mill/scalajslib/ScalaJSModule.scala +++ b/scalajslib/src/mill/scalajslib/ScalaJSModule.scala @@ -89,7 +89,8 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer => // we need to use the scala-library of the currently running mill resolveDependencies( repositoriesTask(), - (commonDeps ++ envDeps).map(Lib.depToBoundDep(_, mill.main.BuildInfo.scalaVersion, "")), + (commonDeps.iterator ++ envDeps) + .map(Lib.depToBoundDep(_, mill.main.BuildInfo.scalaVersion, "")), ctx = Some(T.log) ) } @@ -119,7 +120,7 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer => worker = ScalaJSWorkerExternalModule.scalaJSWorker(), toolsClasspath = scalaJSToolsClasspath(), runClasspath = runClasspath(), - mainClass = finalMainClassOpt().toOption, + mainClass = finalMainClassOpt(), forceOutJs = forceOutJs, testBridgeInit = false, isFullLinkJS = isFullLinkJS, @@ -160,7 +161,7 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer => worker: ScalaJSWorker, toolsClasspath: Agg[PathRef], runClasspath: Agg[PathRef], - mainClass: Option[String], + mainClass: Either[String, String], forceOutJs: Boolean, testBridgeInit: Boolean, isFullLinkJS: Boolean, @@ -175,16 +176,9 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer => os.makeDir.all(ctx.dest) - val classpath = runClasspath.map(_.path) - val sjsirFiles = classpath - .filter(path => os.exists(path) && os.isDir(path)) - .flatMap(os.walk(_)) - .filter(_.ext == "sjsir") - val libraries = classpath.filter(_.ext == "jar") worker.link( toolsClasspath = toolsClasspath, - sources = sjsirFiles, - libraries = libraries, + runClasspath = runClasspath, dest = outputPath.toIO, main = mainClass, forceOutJs = forceOutJs, @@ -279,7 +273,7 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer => scalaVersion = scalaVersion(), scalaBinaryVersion = ZincWorkerUtil.scalaBinaryVersion(scalaVersion()), platform = ScalaPlatform.JS, - jars = scalaCompilerClasspath().map(_.path.toNIO.toUri.toString).iterator.toSeq, + jars = scalaCompilerClasspath().iterator.map(_.path.toNIO.toUri.toString).toSeq, jvmBuildTarget = None ) )) @@ -296,8 +290,7 @@ trait TestScalaJSModule extends ScalaJSModule with TestModule { ivy"org.scala-js::scalajs-library:${scalaJSVersion()}", ivy"org.scala-js::scalajs-test-bridge:${scalaJSVersion()}" ) - .map(_.withDottyCompat(scalaVersion())) - .map(bind) + .map(dep => bind(dep.withDottyCompat(scalaVersion()))) }) } @@ -306,7 +299,7 @@ trait TestScalaJSModule extends ScalaJSModule with TestModule { worker = ScalaJSWorkerExternalModule.scalaJSWorker(), toolsClasspath = scalaJSToolsClasspath(), runClasspath = scalaJSTestDeps() ++ runClasspath(), - mainClass = None, + mainClass = Left("No main class specified or found"), forceOutJs = false, testBridgeInit = true, isFullLinkJS = false, diff --git a/scalajslib/src/mill/scalajslib/worker/ScalaJSWorker.scala b/scalajslib/src/mill/scalajslib/worker/ScalaJSWorker.scala index 0158f13addb..8deda69d200 100644 --- a/scalajslib/src/mill/scalajslib/worker/ScalaJSWorker.scala +++ b/scalajslib/src/mill/scalajslib/worker/ScalaJSWorker.scala @@ -149,10 +149,9 @@ private[scalajslib] class ScalaJSWorker extends AutoCloseable { def link( toolsClasspath: Agg[mill.PathRef], - sources: Agg[os.Path], - libraries: Agg[os.Path], + runClasspath: Agg[mill.PathRef], dest: File, - main: Option[String], + main: Either[String, String], forceOutJs: Boolean, testBridgeInit: Boolean, isFullLinkJS: Boolean, @@ -164,10 +163,9 @@ private[scalajslib] class ScalaJSWorker extends AutoCloseable { outputPatterns: api.OutputPatterns )(implicit ctx: Ctx.Home): Result[api.Report] = { bridge(toolsClasspath).link( - sources = sources.items.map(_.toIO).toArray, - libraries = libraries.items.map(_.toIO).toArray, + runClasspath = runClasspath.iterator.map(_.path.toNIO).toSeq, dest = dest, - main = main.orNull, + main = main, forceOutJs = forceOutJs, testBridgeInit = testBridgeInit, isFullLinkJS = isFullLinkJS, @@ -186,8 +184,7 @@ private[scalajslib] class ScalaJSWorker extends AutoCloseable { def run(toolsClasspath: Agg[mill.PathRef], config: api.JsEnvConfig, report: api.Report)( implicit ctx: Ctx.Home ): Unit = { - val dest = - bridge(toolsClasspath).run(toWorkerApi(config), toWorkerApi(report)) + bridge(toolsClasspath).run(toWorkerApi(config), toWorkerApi(report)) } def getFramework( diff --git a/scalajslib/worker-api/src/mill/scalajslib/worker/api/ScalaJSWorkerApi.scala b/scalajslib/worker-api/src/mill/scalajslib/worker/api/ScalaJSWorkerApi.scala index 6de7b74f86d..0c39abbaf20 100644 --- a/scalajslib/worker-api/src/mill/scalajslib/worker/api/ScalaJSWorkerApi.scala +++ b/scalajslib/worker-api/src/mill/scalajslib/worker/api/ScalaJSWorkerApi.scala @@ -1,13 +1,13 @@ package mill.scalajslib.worker.api import java.io.File +import java.nio.file.Path private[scalajslib] trait ScalaJSWorkerApi { def link( - sources: Array[File], - libraries: Array[File], + runClasspath: Seq[Path], dest: File, - main: String, + main: Either[String, String], forceOutJs: Boolean, testBridgeInit: Boolean, isFullLinkJS: Boolean, diff --git a/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala b/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala index e51d274bcaf..fe7050aa710 100644 --- a/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala +++ b/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala @@ -1,24 +1,19 @@ package mill.scalajslib.worker -import scala.concurrent.{Await, Future} +import scala.concurrent.Await import scala.concurrent.duration.Duration import java.io.File +import java.nio.file.Path import mill.scalajslib.worker.api._ import mill.scalajslib.worker.jsenv._ import org.scalajs.ir.ScalaJSVersions -import org.scalajs.linker.{ - PathIRContainer, - PathIRFile, - PathOutputDirectory, - PathOutputFile, - StandardImpl -} +import org.scalajs.linker.{PathIRContainer, PathOutputDirectory, PathOutputFile, StandardImpl} import org.scalajs.linker.interface.{ ESFeatures => ScalaJSESFeatures, ESVersion => ScalaJSESVersion, ModuleKind => ScalaJSModuleKind, OutputPatterns => ScalaJSOutputPatterns, - Report => ScalaJSReport, + Report => _, ModuleSplitStyle => _, _ } @@ -46,15 +41,16 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { case _ => true } private object ScalaJSLinker { - private val cache = mutable.Map.empty[LinkerInput, SoftReference[Linker]] - def reuseOrCreate(input: LinkerInput): Linker = cache.get(input) match { - case Some(SoftReference(linker)) => linker + private val irFileCache = StandardImpl.irFileCache() + private val cache = mutable.Map.empty[LinkerInput, SoftReference[(Linker, IRFileCache.Cache)]] + def reuseOrCreate(input: LinkerInput): (Linker, IRFileCache.Cache) = cache.get(input) match { + case Some(SoftReference((linker, irFileCacheCache))) => (linker, irFileCacheCache) case _ => - val newLinker = createLinker(input) - cache.update(input, SoftReference(newLinker)) - newLinker + val newResult = createLinker(input) + cache.update(input, SoftReference(newResult)) + newResult } - private def createLinker(input: LinkerInput): Linker = { + private def createLinker(input: LinkerInput): (Linker, IRFileCache.Cache) = { val semantics = input.isFullLinkJS match { case true => Semantics.Defaults.optimized case false => Semantics.Defaults @@ -101,7 +97,7 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { else withESVersion_1_5_minus(scalaJSESFeatures) val useClosure = input.isFullLinkJS && input.moduleKind != ModuleKind.ESModule - var partialConfig = StandardConfig() + val partialConfig = StandardConfig() .withOptimizer(input.optimizer) .withClosureCompilerIfAvailable(useClosure) .withSemantics(semantics) @@ -150,14 +146,15 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { ) else withModuleSplitStyle - StandardImpl.clearableLinker(withOutputPatterns) + val linker = StandardImpl.clearableLinker(withOutputPatterns) + val irFileCacheCache = irFileCache.newCache + (linker, irFileCacheCache) } } def link( - sources: Array[File], - libraries: Array[File], + runClasspath: Seq[Path], dest: File, - main: String, + main: Either[String, String], forceOutJs: Boolean, testBridgeInit: Boolean, isFullLinkJS: Boolean, @@ -172,7 +169,7 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { // the new mode is not supported and in tests we always use legacy = false val useLegacy = forceOutJs || !minorIsGreaterThanOrEqual(3) import scala.concurrent.ExecutionContext.Implicits.global - val linker = ScalaJSLinker.reuseOrCreate(LinkerInput( + val (linker, irFileCacheCache) = ScalaJSLinker.reuseOrCreate(LinkerInput( isFullLinkJS = isFullLinkJS, optimizer = optimizer, sourceMap = sourceMap, @@ -182,23 +179,23 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { outputPatterns = outputPatterns, dest = dest )) - val cache = StandardImpl.irFileCache().newCache - val sourceIRsFuture = Future.sequence(sources.toSeq.map(f => PathIRFile(f.toPath()))) - val irContainersPairs = PathIRContainer.fromClasspath(libraries.toIndexedSeq.map(_.toPath())) - val libraryIRsFuture = irContainersPairs.flatMap(pair => cache.cached(pair._1)) + val irContainersAndPathsFuture = PathIRContainer.fromClasspath(runClasspath) val logger = new ScalaConsoleLogger - val mainInitializer = Option(main).map { cls => - ModuleInitializer.mainMethodWithArgs(cls, "main") - } val testInitializer = if (testBridgeInit) - Some(ModuleInitializer.mainMethod(TAI.ModuleClassName, TAI.MainMethodName)) - else None - val moduleInitializers = mainInitializer.toList ::: testInitializer.toList + ModuleInitializer.mainMethod(TAI.ModuleClassName, TAI.MainMethodName) :: Nil + else Nil + val moduleInitializers = main match { + case Right(main) => + ModuleInitializer.mainMethodWithArgs(main, "main") :: + testInitializer + case _ => + testInitializer + } val resultFuture = (for { - sourceIRs <- sourceIRsFuture - libraryIRs <- libraryIRsFuture + (irContainers, _) <- irContainersAndPathsFuture + irFiles <- irFileCacheCache.cached(irContainers) report <- if (useLegacy) { val jsFileName = "out.js" @@ -212,7 +209,7 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { .withSourceMap(PathOutputFile(sourceMapFile)) .withSourceMapURI(java.net.URI.create(sourceMapFile.getFileName.toString)) } - linker.link(sourceIRs ++ libraryIRs, moduleInitializers, linkerOutput, logger).map { + linker.link(irFiles, moduleInitializers, linkerOutput, logger).map { file => Report( publicModules = Seq(Report.Module( @@ -227,7 +224,7 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { } else { val linkerOutput = PathOutputDirectory(dest.toPath()) linker.link( - sourceIRs ++ libraryIRs, + irFiles, moduleInitializers, linkerOutput, logger