From 7d577350250b1a36ea1e3095b278a4c8679626ba Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Mon, 30 Sep 2024 16:44:11 +0200 Subject: [PATCH 1/4] Cache Scalafix instances globally --- .../goyeau/mill/scalafix/ScalafixCache.scala | 24 +++++++++++++++++++ .../goyeau/mill/scalafix/ScalafixModule.scala | 5 ++-- 2 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala diff --git a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala new file mode 100644 index 0000000..2fa3797 --- /dev/null +++ b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala @@ -0,0 +1,24 @@ +package com.goyeau.mill.scalafix + +import com.goyeau.mill.scalafix.CoursierUtils +import coursier.core.Repository +import scalafix.interfaces.Scalafix + +import scala.collection.mutable +import scala.ref.SoftReference +import scala.jdk.CollectionConverters._ + +private[scalafix] object ScalafixCache { + + private val cache = mutable.Map.empty[(String, Seq[Repository]), SoftReference[Scalafix]] + + def getOrElseCreate(scalaVersion: String, repositories: Seq[Repository]) = + cache.get((scalaVersion, repositories)) match { + case Some(SoftReference(value)) => value + case _ => + val newResult = + Scalafix.fetchAndClassloadInstance(scalaVersion, repositories.map(CoursierUtils.toApiRepository).asJava) + cache.update((scalaVersion, repositories), SoftReference(newResult)) + newResult + } +} diff --git a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala index c55eefe..e63c61c 100644 --- a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala +++ b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala @@ -7,7 +7,6 @@ import mill.api.{Logger, PathRef, Result} import mill.scalalib.{Dep, ScalaModule} import mill.define.Command -import scalafix.interfaces.Scalafix import scalafix.interfaces.ScalafixError.* import scala.compat.java8.OptionConverters.* @@ -77,8 +76,8 @@ object ScalafixModule { wd: os.Path ): Result[Unit] = if (sources.nonEmpty) { - val scalafix = Scalafix - .fetchAndClassloadInstance(scalaVersion, repositories.map(CoursierUtils.toApiRepository).asJava) + val scalafix = ScalafixCache + .getOrElseCreate(scalaVersion, repositories) .newArguments() .withParsedArguments(args.asJava) .withWorkingDirectory(wd.toNIO) From f959deb796ffbeae31fdc0380ac54f5d59491f41 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Mon, 30 Sep 2024 17:15:23 +0200 Subject: [PATCH 2/4] Use ConcurrentHashMap for better thread safety --- .../src/com/goyeau/mill/scalafix/ScalafixCache.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala index 2fa3797..8c27e44 100644 --- a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala +++ b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala @@ -4,21 +4,21 @@ import com.goyeau.mill.scalafix.CoursierUtils import coursier.core.Repository import scalafix.interfaces.Scalafix -import scala.collection.mutable -import scala.ref.SoftReference +import java.util.concurrent.ConcurrentHashMap import scala.jdk.CollectionConverters._ +import scala.ref.SoftReference private[scalafix] object ScalafixCache { - private val cache = mutable.Map.empty[(String, Seq[Repository]), SoftReference[Scalafix]] + private val cache = new ConcurrentHashMap[(String, Seq[Repository]), SoftReference[Scalafix]] def getOrElseCreate(scalaVersion: String, repositories: Seq[Repository]) = cache.get((scalaVersion, repositories)) match { - case Some(SoftReference(value)) => value + case SoftReference(value) => value case _ => val newResult = Scalafix.fetchAndClassloadInstance(scalaVersion, repositories.map(CoursierUtils.toApiRepository).asJava) - cache.update((scalaVersion, repositories), SoftReference(newResult)) + cache.put((scalaVersion, repositories), SoftReference(newResult)) newResult } } From c8798c6d6f987580d221520a9d8db55cad3c9aa6 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Mon, 30 Sep 2024 22:19:12 +0200 Subject: [PATCH 3/4] Use `compute` to avoid concurrent initialization of the same key --- .../goyeau/mill/scalafix/ScalafixCache.scala | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala index 8c27e44..72681a9 100644 --- a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala +++ b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala @@ -13,12 +13,14 @@ private[scalafix] object ScalafixCache { private val cache = new ConcurrentHashMap[(String, Seq[Repository]), SoftReference[Scalafix]] def getOrElseCreate(scalaVersion: String, repositories: Seq[Repository]) = - cache.get((scalaVersion, repositories)) match { - case SoftReference(value) => value - case _ => - val newResult = - Scalafix.fetchAndClassloadInstance(scalaVersion, repositories.map(CoursierUtils.toApiRepository).asJava) - cache.put((scalaVersion, repositories), SoftReference(newResult)) - newResult - } + cache.compute( + (scalaVersion, repositories), + { + case (_, v @ SoftReference(_)) => v + case _ => + SoftReference( + Scalafix.fetchAndClassloadInstance(scalaVersion, repositories.map(CoursierUtils.toApiRepository).asJava) + ) + } + )() } From 617f307d177829a80df5f08566a8314ff8b9c80c Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Mon, 30 Sep 2024 23:11:33 +0200 Subject: [PATCH 4/4] Cache also `withToolClasspath` using two-layered cache --- .../goyeau/mill/scalafix/ScalafixCache.scala | 47 ++++++++++++++----- .../goyeau/mill/scalafix/ScalafixModule.scala | 8 +--- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala index 72681a9..58aeebc 100644 --- a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala +++ b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixCache.scala @@ -2,7 +2,10 @@ package com.goyeau.mill.scalafix import com.goyeau.mill.scalafix.CoursierUtils import coursier.core.Repository +import mill.Agg +import mill.scalalib.Dep import scalafix.interfaces.Scalafix +import scalafix.interfaces.ScalafixArguments import java.util.concurrent.ConcurrentHashMap import scala.jdk.CollectionConverters._ @@ -10,17 +13,35 @@ import scala.ref.SoftReference private[scalafix] object ScalafixCache { - private val cache = new ConcurrentHashMap[(String, Seq[Repository]), SoftReference[Scalafix]] - - def getOrElseCreate(scalaVersion: String, repositories: Seq[Repository]) = - cache.compute( - (scalaVersion, repositories), - { - case (_, v @ SoftReference(_)) => v - case _ => - SoftReference( - Scalafix.fetchAndClassloadInstance(scalaVersion, repositories.map(CoursierUtils.toApiRepository).asJava) - ) - } - )() + private val scalafixCache = new Cache[(String, java.util.List[coursierapi.Repository]), Scalafix](createFunction = { + case (scalaVersion, repositories) => + Scalafix.fetchAndClassloadInstance(scalaVersion, repositories) + }) + + private val scalafixArgumentsCache = + new Cache[(String, Seq[Repository], Agg[Dep]), ScalafixArguments](createFunction = { + case (scalaVersion, repositories, scalafixIvyDeps) => + val repos = repositories.map(CoursierUtils.toApiRepository).asJava + val deps = scalafixIvyDeps.map(CoursierUtils.toCoordinates).iterator.toSeq.asJava + scalafixCache + .getOrElseCreate((scalaVersion, repos)) + .newArguments() + .withToolClasspath(Seq.empty.asJava, deps, repos) + }) + + def getOrElseCreate(scalaVersion: String, repositories: Seq[Repository], scalafixIvyDeps: Agg[Dep]) = + scalafixArgumentsCache.getOrElseCreate((scalaVersion, repositories, scalafixIvyDeps)) + + private class Cache[A, B <: AnyRef](createFunction: A => B) { + private val cache = new ConcurrentHashMap[A, SoftReference[B]] + + def getOrElseCreate(a: A) = + cache.compute( + a, + { + case (_, v @ SoftReference(_)) => v + case _ => SoftReference(createFunction(a)) + } + )() + } } diff --git a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala index e63c61c..db2a6c2 100644 --- a/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala +++ b/mill-scalafix/src/com/goyeau/mill/scalafix/ScalafixModule.scala @@ -77,8 +77,7 @@ object ScalafixModule { ): Result[Unit] = if (sources.nonEmpty) { val scalafix = ScalafixCache - .getOrElseCreate(scalaVersion, repositories) - .newArguments() + .getOrElseCreate(scalaVersion, repositories, scalafixIvyDeps) .withParsedArguments(args.asJava) .withWorkingDirectory(wd.toNIO) .withConfig(scalafixConfig.map(_.toNIO).asJava) @@ -86,11 +85,6 @@ object ScalafixModule { .withScalaVersion(scalaVersion) .withScalacOptions(scalacOptions.asJava) .withPaths(sources.map(_.toNIO).asJava) - .withToolClasspath( - Seq.empty.asJava, - scalafixIvyDeps.map(CoursierUtils.toCoordinates).iterator.toSeq.asJava, - repositories.map(CoursierUtils.toApiRepository).asJava - ) log.info(s"Rewriting and linting ${sources.size} Scala sources against ${scalafix.rulesThatWillRun.size} rules") val errors = scalafix.run()