From a893b1c86d5e76b51517acbdda2fdc76be471faf Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Sat, 30 May 2026 21:12:10 -0400 Subject: [PATCH] [2.0.x] fix: Parallelize dependency resolution when no progress bar is rendered (#9274) `update` held a process-global lock around coursier resolution and artifact fetching whenever a logger was set OR coursier was not in fallback mode. That lock exists only to serialize coursier's interactive progress bar, which is rendered solely when no custom logger is supplied and coursier is not in fallback mode. The `loggerOpt.nonEmpty` clause therefore over-serialized the common non-interactive case (IntelliJ re-imports, CI, any non-TTY run, where sbt supplies a quiet debug-only logger), making `update` scale with the number of modules rather than the number of distinct artifacts. Narrow the predicate to `!hasCustomLogger && !fallbackMode` (Lock.progressBarActive), so resolution runs in parallel unless coursier is actually drawing its progress bar. Interactive terminals stay serialized; COURSIER_PROGRESS=false opts into parallelism there, and #5627 tracks fully parallel resolution with live progress bars. Safe: SbtCoursierCache is ConcurrentHashMap-backed, a CacheLogger is already invoked concurrently within a single resolution (parallel downloads), and the COURSIER_PROGRESS=false parallel path has been used in production for years. Fixes #5508. Co-authored-by: Brian Hotopp Co-authored-by: Claude Opus 4.8 (1M context) --- .../lmcoursier/internal/ArtifactsRun.scala | 5 +++- .../main/scala/lmcoursier/internal/Lock.scala | 8 +++++++ .../lmcoursier/internal/ResolutionRun.scala | 5 +++- .../scala/lmcoursier/internal/LockSpec.scala | 23 +++++++++++++++++++ notes/2.0.0/parallel-dependency-resolution.md | 21 +++++++++++++++++ 5 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 lm-coursier/src/test/scala/lmcoursier/internal/LockSpec.scala create mode 100644 notes/2.0.0/parallel-dependency-resolution.md diff --git a/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala b/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala index 291dbddae..9dd4e0db4 100644 --- a/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala +++ b/lm-coursier/src/main/scala/lmcoursier/internal/ArtifactsRun.scala @@ -41,7 +41,10 @@ object ArtifactsRun { } Lock.maybeSynchronized(needsLock = - params.loggerOpt.nonEmpty || !RefreshLogger.defaultFallbackMode + Lock.progressBarActive( + hasCustomLogger = params.loggerOpt.nonEmpty, + fallbackMode = RefreshLogger.defaultFallbackMode + ) ) { result(params, coursierLogger) } diff --git a/lm-coursier/src/main/scala/lmcoursier/internal/Lock.scala b/lm-coursier/src/main/scala/lmcoursier/internal/Lock.scala index 33a5cf7c8..6e3f789a2 100644 --- a/lm-coursier/src/main/scala/lmcoursier/internal/Lock.scala +++ b/lm-coursier/src/main/scala/lmcoursier/internal/Lock.scala @@ -3,6 +3,14 @@ package lmcoursier.internal private[lmcoursier] object Lock { private val lock = new Object + /* The lock guards coursier's interactive progress bar (ProgressBarRefreshDisplay), the only thing + * here that cannot be driven from more than one module at a time. coursier renders it only when no + * custom cache logger is supplied and it is not in fallback mode; otherwise (a custom logger, or + * the line-based fallback display) modules can be resolved in parallel. A CacheLogger is already + * invoked concurrently within a single resolution, so it must be thread-safe regardless. */ + def progressBarActive(hasCustomLogger: Boolean, fallbackMode: Boolean): Boolean = + !hasCustomLogger && !fallbackMode + /* Progress bars require us to only work on one module at the time. Without those we can go faster */ def maybeSynchronized[T](needsLock: Boolean)(f: => T): T = if (needsLock) lock.synchronized(f) diff --git a/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala b/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala index 840ebb498..49a5c81d7 100644 --- a/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala +++ b/lm-coursier/src/main/scala/lmcoursier/internal/ResolutionRun.scala @@ -189,7 +189,10 @@ object ResolutionRun { SbtCoursierCache.default.resolutionOpt(params.resolutionKey).map(Right(_)).getOrElse { val resOrError = Lock.maybeSynchronized(needsLock = - params.loggerOpt.nonEmpty || !RefreshLogger.defaultFallbackMode + Lock.progressBarActive( + hasCustomLogger = params.loggerOpt.nonEmpty, + fallbackMode = RefreshLogger.defaultFallbackMode + ) ) { val map = new mutable.HashMap[Configuration, Resolution] val either = params.orderedConfigs.foldLeft[Either[coursier.error.ResolutionError, Unit]]( diff --git a/lm-coursier/src/test/scala/lmcoursier/internal/LockSpec.scala b/lm-coursier/src/test/scala/lmcoursier/internal/LockSpec.scala new file mode 100644 index 000000000..fbeb7febd --- /dev/null +++ b/lm-coursier/src/test/scala/lmcoursier/internal/LockSpec.scala @@ -0,0 +1,23 @@ +package lmcoursier.internal + +import verify.* + +// progressBarActive decides whether resolution must be serialized. It is true only when coursier +// renders its interactive progress bar (no custom logger and not in fallback mode) -- the one thing +// that cannot be driven from more than one module at a time. +object LockSpec extends BasicTestSuite: + + test("a custom cache logger does not require the lock") { + assert(!Lock.progressBarActive(hasCustomLogger = true, fallbackMode = false)) + assert(!Lock.progressBarActive(hasCustomLogger = true, fallbackMode = true)) + } + + test("the fallback (line-based) display does not require the lock") { + assert(!Lock.progressBarActive(hasCustomLogger = false, fallbackMode = true)) + } + + test("only the interactive progress bar requires the lock") { + assert(Lock.progressBarActive(hasCustomLogger = false, fallbackMode = false)) + } + +end LockSpec diff --git a/notes/2.0.0/parallel-dependency-resolution.md b/notes/2.0.0/parallel-dependency-resolution.md new file mode 100644 index 000000000..4071b4f90 --- /dev/null +++ b/notes/2.0.0/parallel-dependency-resolution.md @@ -0,0 +1,21 @@ +### `update` resolves modules in parallel + +Dependency resolution (`update`) no longer holds a process-global lock while +coursier is *not* rendering its interactive progress bar -- that is, whenever a +custom `csrLogger` is set, or coursier is in fallback display mode (the default +under IntelliJ, CI, and any non-TTY environment, or with `COURSIER_PROGRESS=false`). + +Previously the lock was taken even for sbt's quiet default logger, so `update` +processed one module at a time and its duration grew with the number of modules +rather than the number of distinct artifacts. Large multi-module builds and +IDE/CI re-imports could spend minutes serializing resolution. + +The lock is still held while coursier draws its live per-module progress bars in +an interactive terminal, because that renderer is not safe to drive concurrently; +to parallelize there as well, run with `COURSIER_PROGRESS=false`. Fully parallel +resolution *with* live progress bars is tracked by [#5627][i5627]. + +This addresses [#5508][i5508]. + +[i5508]: https://github.com/sbt/sbt/issues/5508 +[i5627]: https://github.com/sbt/sbt/issues/5627