From 6565618a15b0e038e3c015d8425ad8f87b7f1092 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Fri, 3 Jul 2020 11:33:04 -0700 Subject: [PATCH 1/3] Cache compiled map during build load The continuous command recompiles the setting graph into a CompiledMap data structure so that it can determine which files it needs to transitively monitor during watch. Generating the CompiledMap can be very slow for large projects (5 seconds or so on my computer in the sbt project) and this startup cost is paid every time the user enters a watch with `~`. To avoid this, we can cache the compile map that is generated during the initial settings evaluation. The only real drawback I can see is that the compiled map is guaranteed to remain in memory so long as the BuildStructure instance that holds it is alive. Given the performance benefit, this seems like a worthwhile tradeoff. --- .../main/scala/sbt/internal/util/Settings.scala | 11 +++++++++-- .../src/test/scala/SettingsExample.scala | 3 ++- .../src/test/scala/SettingsTest.scala | 2 +- .../scala/sbt/internal/BuildStructure.scala | 12 ++++++++++++ main/src/main/scala/sbt/internal/Load.scala | 17 ++++++++++++----- .../main/scala/sbt/internal/SettingsGraph.scala | 3 +-- main/src/test/scala/PluginCommandTest.scala | 6 ++++-- .../src/test/scala/sbt/internal/TestBuild.scala | 2 +- .../sbt/internal/server/SettingQueryTest.scala | 15 ++++++++++++--- 9 files changed, 54 insertions(+), 17 deletions(-) diff --git a/internal/util-collection/src/main/scala/sbt/internal/util/Settings.scala b/internal/util-collection/src/main/scala/sbt/internal/util/Settings.scala index 4a1a31fa9..b575eb1ec 100644 --- a/internal/util-collection/src/main/scala/sbt/internal/util/Settings.scala +++ b/internal/util-collection/src/main/scala/sbt/internal/util/Settings.scala @@ -198,17 +198,24 @@ trait Init[ScopeType] { compile(dMap) } + @deprecated("Use makeWithCompiledMap", "1.4.0") def make(init: Seq[Setting[_]])( implicit delegates: ScopeType => Seq[ScopeType], scopeLocal: ScopeLocal, display: Show[ScopedKey[_]] - ): Settings[ScopeType] = { + ): Settings[ScopeType] = makeWithCompiledMap(init)._2 + + def makeWithCompiledMap(init: Seq[Setting[_]])( + implicit delegates: ScopeType => Seq[ScopeType], + scopeLocal: ScopeLocal, + display: Show[ScopedKey[_]] + ): (CompiledMap, Settings[ScopeType]) = { val cMap = compiled(init)(delegates, scopeLocal, display) // order the initializations. cyclic references are detected here. val ordered: Seq[Compiled[_]] = sort(cMap) // evaluation: apply the initializations. try { - applyInits(ordered) + (cMap, applyInits(ordered)) } catch { case rru: RuntimeUndefined => throw Uninitialized(cMap.keys.toSeq, delegates, rru.undefined, true) diff --git a/internal/util-collection/src/test/scala/SettingsExample.scala b/internal/util-collection/src/test/scala/SettingsExample.scala index bcdc3916d..5399f79c1 100644 --- a/internal/util-collection/src/test/scala/SettingsExample.scala +++ b/internal/util-collection/src/test/scala/SettingsExample.scala @@ -64,7 +64,8 @@ case class SettingsUsage(val settingsExample: SettingsExample) { // "compiles" and applies the settings. // This can be split into multiple steps to access intermediate results if desired. // The 'inspect' command operates on the output of 'compile', for example. - val applied: Settings[Scope] = make(mySettings)(delegates, scopeLocal, showFullKey) + val applied: Settings[Scope] = + makeWithCompiledMap(mySettings)(delegates, scopeLocal, showFullKey)._2 // Show results. /* for(i <- 0 to 5; k <- Seq(a, b)) { diff --git a/internal/util-collection/src/test/scala/SettingsTest.scala b/internal/util-collection/src/test/scala/SettingsTest.scala index 95083f225..bbfa2a848 100644 --- a/internal/util-collection/src/test/scala/SettingsTest.scala +++ b/internal/util-collection/src/test/scala/SettingsTest.scala @@ -200,7 +200,7 @@ object SettingsTest extends Properties("settings") { def evaluate(settings: Seq[Setting[_]]): Settings[Scope] = try { - make(settings)(delegates, scopeLocal, showFullKey) + makeWithCompiledMap(settings)(delegates, scopeLocal, showFullKey)._2 } catch { case e: Throwable => e.printStackTrace(); throw e } diff --git a/main/src/main/scala/sbt/internal/BuildStructure.scala b/main/src/main/scala/sbt/internal/BuildStructure.scala index 157428801..a349b9a86 100644 --- a/main/src/main/scala/sbt/internal/BuildStructure.scala +++ b/main/src/main/scala/sbt/internal/BuildStructure.scala @@ -32,7 +32,19 @@ final class BuildStructure( val streams: State => Streams, val delegates: Scope => Seq[Scope], val scopeLocal: ScopeLocal, + private[sbt] val compiledMap: Map[ScopedKey[_], Def.Compiled[_]], ) { + @deprecated("Used the variant that takes a compiledMap", "1.4.0") + def this( + units: Map[URI, LoadedBuildUnit], + root: URI, + settings: Seq[Setting[_]], + data: Settings[Scope], + index: StructureIndex, + streams: State => Streams, + delegates: Scope => Seq[Scope], + scopeLocal: ScopeLocal, + ) = this(units, root, settings, data, index, streams, delegates, scopeLocal, Map.empty) val extra: BuildUtil[ResolvedProject] = BuildUtil(root, units, index.keyIndex, data) diff --git a/main/src/main/scala/sbt/internal/Load.scala b/main/src/main/scala/sbt/internal/Load.scala index c3f52dcfe..a502e2364 100755 --- a/main/src/main/scala/sbt/internal/Load.scala +++ b/main/src/main/scala/sbt/internal/Load.scala @@ -251,12 +251,16 @@ private[sbt] object Load { val delegates = timed("Load.apply: config.delegates", log) { config.delegates(loaded) } - val data = timed("Load.apply: Def.make(settings)...", log) { + val (cMap, data) = timed("Load.apply: Def.make(settings)...", log) { // When settings.size is 100000, Def.make takes around 10s. if (settings.size > 10000) { log.info(s"resolving key references (${settings.size} settings) ...") } - Def.make(settings)(delegates, config.scopeLocal, Project.showLoadingKey(loaded)) + Def.makeWithCompiledMap(settings)( + delegates, + config.scopeLocal, + Project.showLoadingKey(loaded) + ) } Project.checkTargets(data) foreach sys.error val index = timed("Load.apply: structureIndex", log) { @@ -271,7 +275,8 @@ private[sbt] object Load { index, streams, delegates, - config.scopeLocal + config.scopeLocal, + cMap ) (rootEval, bs) } @@ -338,7 +343,8 @@ private[sbt] object Load { implicit display: Show[ScopedKey[_]] ): BuildStructure = { val transformed = finalTransforms(newSettings) - val newData = Def.make(transformed)(structure.delegates, structure.scopeLocal, display) + val (cMap, newData) = + Def.makeWithCompiledMap(transformed)(structure.delegates, structure.scopeLocal, display) def extra(index: KeyIndex) = BuildUtil(structure.root, structure.units, index, newData) val newIndex = structureIndex(newData, transformed, extra, structure.units) val newStreams = mkStreams(structure.units, structure.root, newData) @@ -350,7 +356,8 @@ private[sbt] object Load { index = newIndex, streams = newStreams, delegates = structure.delegates, - scopeLocal = structure.scopeLocal + scopeLocal = structure.scopeLocal, + compiledMap = cMap, ) } diff --git a/main/src/main/scala/sbt/internal/SettingsGraph.scala b/main/src/main/scala/sbt/internal/SettingsGraph.scala index ecf8266d1..ba1ce3a4b 100644 --- a/main/src/main/scala/sbt/internal/SettingsGraph.scala +++ b/main/src/main/scala/sbt/internal/SettingsGraph.scala @@ -41,8 +41,7 @@ private[sbt] object SettingsGraph { f(extracted, compile(extracted.structure)) } - private[sbt] def compile(structure: BuildStructure): CompiledMap = - compiled(structure.settings)(structure.delegates, structure.scopeLocal, (_: ScopedKey[_]) => "") + private[sbt] def compile(structure: BuildStructure): CompiledMap = structure.compiledMap private[sbt] final class Arguments( val scopedKey: ScopedKey[_], val extracted: Extracted, diff --git a/main/src/test/scala/PluginCommandTest.scala b/main/src/test/scala/PluginCommandTest.scala index 7fd524ac2..0ea4c5303 100644 --- a/main/src/test/scala/PluginCommandTest.scala +++ b/main/src/test/scala/PluginCommandTest.scala @@ -102,7 +102,8 @@ object FakeState { val delegates: (Scope) => Seq[Scope] = _ => Nil val scopeLocal: Def.ScopeLocal = _ => Nil - val data: Settings[Scope] = Def.make(settings)(delegates, scopeLocal, Def.showFullKey) + val (cMap, data: Settings[Scope]) = + Def.makeWithCompiledMap(settings)(delegates, scopeLocal, Def.showFullKey) val extra: KeyIndex => BuildUtil[_] = (keyIndex) => BuildUtil(base.toURI, Map.empty, keyIndex, data) val structureIndex: StructureIndex = @@ -140,7 +141,8 @@ object FakeState { structureIndex, streams, delegates, - scopeLocal + scopeLocal, + cMap, ) val attributes = AttributeMap.empty ++ AttributeMap( diff --git a/main/src/test/scala/sbt/internal/TestBuild.scala b/main/src/test/scala/sbt/internal/TestBuild.scala index a5e02ab7e..48bf993b9 100644 --- a/main/src/test/scala/sbt/internal/TestBuild.scala +++ b/main/src/test/scala/sbt/internal/TestBuild.scala @@ -241,7 +241,7 @@ abstract class TestBuild { throw e } } - val data = Def.make(settings)(env.delegates, const(Nil), display) + val data = Def.makeWithCompiledMap(settings)(env.delegates, const(Nil), display)._2 val keys = data.allKeys((s, key) => ScopedKey(s, key)) val keyMap = keys.map(k => (k.key.label, k.key)).toMap[String, AttributeKey[_]] val projectsMap = env.builds.map(b => (b.uri, b.projects.map(_.id).toSet)).toMap diff --git a/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala b/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala index 290b2f1bf..7e59b6d14 100644 --- a/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala +++ b/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala @@ -18,7 +18,6 @@ import scala.collection.mutable import xsbti.{ Logger => _, _ } import sbt.io.IO -import sbt.internal.util._ import sbt.internal.BuildStreams.{ Streams => _, _ } import sbt.internal.Load._ import sbt.util._ @@ -171,14 +170,24 @@ object SettingQueryTest extends org.specs2.mutable.Specification { val scopeLocal: ScopeLocal = EvaluateTask.injectStreams val display: Show[ScopedKey[_]] = Project showLoadingKey loadedBuild - val data: Settings[Scope] = Def.make(settings)(delegates, scopeLocal, display) + val (cMap, data) = Def.makeWithCompiledMap(settings)(delegates, scopeLocal, display) val extra: KeyIndex => BuildUtil[_] = index => BuildUtil(baseUri, units, index, data) val index: StructureIndex = structureIndex(data, settings, extra, units) val streams: State => Streams = mkStreams(units, baseUri, data) val structure: BuildStructure = - new BuildStructure(units, baseUri, settings, data, index, streams, delegates, scopeLocal) + new BuildStructure( + units, + baseUri, + settings, + data, + index, + streams, + delegates, + scopeLocal, + cMap + ) structure } From 4a8e7c07345e3d4c550aa5784a5d75a056285c7e Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Fri, 3 Jul 2020 11:55:52 -0700 Subject: [PATCH 2/3] Used cached compiled map in LintUnused Linting unused keys was adding a significant overhead to sbt project loading because Def.compiled is so slow. It was around 4 seconds in the sbt project on my computer. --- main/src/main/scala/sbt/internal/LintUnused.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/main/src/main/scala/sbt/internal/LintUnused.scala b/main/src/main/scala/sbt/internal/LintUnused.scala index bb549918e..57485ae67 100644 --- a/main/src/main/scala/sbt/internal/LintUnused.scala +++ b/main/src/main/scala/sbt/internal/LintUnused.scala @@ -117,9 +117,7 @@ object LintUnused { val extracted = Project.extract(state) val structure = extracted.structure val display = Def.showShortKey(None) // extracted.showKey - val actual = true - val comp = - Def.compiled(structure.settings, actual)(structure.delegates, structure.scopeLocal, display) + val comp = structure.compiledMap val cMap = Def.flattenLocals(comp) val used: Set[ScopedKey[_]] = cMap.values.flatMap(_.dependencies).toSet val unused: Seq[ScopedKey[_]] = cMap.keys.filter(!used.contains(_)).toSeq From 80cd0d5e6b7082b03f371b7f2d00ca44008229f9 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Fri, 3 Jul 2020 12:02:38 -0700 Subject: [PATCH 3/3] Rename SettingsGraph WatchTransitiveDependencies This is a more descriptive name and differentiates the object from `SettingGraph`. --- build.sbt | 1 + main/src/main/scala/sbt/Defaults.scala | 2 +- main/src/main/scala/sbt/internal/Continuous.scala | 8 ++++---- ...tingsGraph.scala => WatchTransitiveDependencies.scala} | 2 +- main/src/main/scala/sbt/nio/Settings.scala | 5 +++-- 5 files changed, 10 insertions(+), 8 deletions(-) rename main/src/main/scala/sbt/internal/{SettingsGraph.scala => WatchTransitiveDependencies.scala} (99%) diff --git a/build.sbt b/build.sbt index 3cf295a74..7faaeef45 100644 --- a/build.sbt +++ b/build.sbt @@ -963,6 +963,7 @@ lazy val mainProj = (project in file("main")) // the binary compatible version. exclude[IncompatibleMethTypeProblem]("sbt.internal.server.NetworkChannel.this"), exclude[IncompatibleSignatureProblem]("sbt.internal.DeprecatedContinuous.taskDefinitions"), + exclude[MissingClassProblem]("sbt.internal.SettingsGraph*") ) ) .configure( diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 2e6622174..2b3dd1e8a 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -747,7 +747,7 @@ object Defaults extends BuildCommon { cleanKeepGlobs ++= historyPath.value.map(_.toGlob).toVector, clean := Def.taskDyn(Clean.task(resolvedScoped.value.scope, full = true)).value, consoleProject := consoleProjectTask.value, - transitiveDynamicInputs := SettingsGraph.task.value, + transitiveDynamicInputs := WatchTransitiveDependencies.task.value, ) ++ sbt.internal.DeprecatedContinuous.taskDefinitions def generate(generators: SettingKey[Seq[Task[Seq[File]]]]): Initialize[Task[Seq[File]]] = diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala index f66c0c47d..f41c2d858 100644 --- a/main/src/main/scala/sbt/internal/Continuous.scala +++ b/main/src/main/scala/sbt/internal/Continuous.scala @@ -170,9 +170,9 @@ private[sbt] object Continuous extends DeprecatedContinuous { // Extract all of the globs that we will monitor during the continuous build. val inputs = { val configs = scopedKey.get(internalDependencyConfigurations).getOrElse(Nil) - val args = - new SettingsGraph.Arguments(scopedKey, extracted, compiledMap, logger, configs, state) - SettingsGraph.transitiveDynamicInputs(args) + import WatchTransitiveDependencies.{ Arguments => DArguments } + val args = new DArguments(scopedKey, extracted, compiledMap, logger, configs, state) + WatchTransitiveDependencies.transitiveDynamicInputs(args) } val repository = getRepository(state) @@ -240,7 +240,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { dynamicInputs: mutable.Set[DynamicInput], )(implicit extracted: Extracted, logger: Logger): Seq[Config] = { val commandKeys = commands.map(parseCommand(_, state)) - val compiledMap = SettingsGraph.compile(extracted.structure) + val compiledMap = WatchTransitiveDependencies.compile(extracted.structure) commandKeys.flatMap(_.map(getConfig(state, _, compiledMap, dynamicInputs))) } diff --git a/main/src/main/scala/sbt/internal/SettingsGraph.scala b/main/src/main/scala/sbt/internal/WatchTransitiveDependencies.scala similarity index 99% rename from main/src/main/scala/sbt/internal/SettingsGraph.scala rename to main/src/main/scala/sbt/internal/WatchTransitiveDependencies.scala index ba1ce3a4b..44fb45f3a 100644 --- a/main/src/main/scala/sbt/internal/SettingsGraph.scala +++ b/main/src/main/scala/sbt/internal/WatchTransitiveDependencies.scala @@ -21,7 +21,7 @@ import sbt.nio.file.Glob import scala.annotation.tailrec -private[sbt] object SettingsGraph { +private[sbt] object WatchTransitiveDependencies { private implicit class SourceOps(val source: Source) { def toGlob: Glob = { val filter = source.includeFilter -- source.excludeFilter diff --git a/main/src/main/scala/sbt/nio/Settings.scala b/main/src/main/scala/sbt/nio/Settings.scala index 5934f2cb1..49731d6e1 100644 --- a/main/src/main/scala/sbt/nio/Settings.scala +++ b/main/src/main/scala/sbt/nio/Settings.scala @@ -15,7 +15,7 @@ import sbt.Keys._ import sbt.internal.Clean.ToSeqPath import sbt.internal.Continuous.FileStampRepository import sbt.internal.util.AttributeKey -import sbt.internal.{ Clean, Continuous, DynamicInput, SettingsGraph } +import sbt.internal.{ Clean, Continuous, DynamicInput, WatchTransitiveDependencies } import sbt.nio.FileStamp.Formats._ import sbt.nio.FileStamper.{ Hash, LastModified } import sbt.nio.Keys._ @@ -108,7 +108,8 @@ private[sbt] object Settings { case transitiveDynamicInputs.key => scopedKey.scope.task.toOption.toSeq.map { key => val updatedKey = Def.ScopedKey(scopedKey.scope.copy(task = Zero), key) - transitiveDynamicInputs in scopedKey.scope := SettingsGraph.task(updatedKey).value + transitiveDynamicInputs in scopedKey.scope := + WatchTransitiveDependencies.task(updatedKey).value } case dynamicDependency.key => (dynamicDependency in scopedKey.scope := { () }) :: Nil case transitiveClasspathDependency.key =>