Skip to content

Commit

Permalink
Merge pull request #142 from bjaglin/classloading
Browse files Browse the repository at this point in the history
Fix 0.9.18 caching/performance regressions
  • Loading branch information
github-brice-jaglin authored Jul 7, 2020
2 parents 9ef7380 + 28e70a7 commit bffce56
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 35 deletions.
22 changes: 22 additions & 0 deletions src/main/scala/scalafix/internal/sbt/BlockingCache.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package scalafix.internal.sbt

import java.{util => ju}

/** A basic thread-safe cache without any eviction. */
class BlockingCache[K, V] {

// Number of keys is expected to be very small so the global lock should not be a bottleneck
private val underlying = ju.Collections.synchronizedMap(new ju.HashMap[K, V])

/**
* @param value By-name parameter evaluated when the key if missing. Value computation is guaranteed
* to be called only once per key across all invocations.
*/
def getOrElseUpdate(key: K, value: => V): V =
underlying.computeIfAbsent(
key,
new ju.function.Function[K, V] {
override def apply(k: K): V = value
}
)
}
104 changes: 69 additions & 35 deletions src/main/scala/scalafix/sbt/ScalafixPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import coursierapi.Repository
import sbt.KeyRanks.Invisible
import sbt.Keys._
import sbt._
import sbt.complete._
import sbt.internal.sbtscalafix.Caching._
import sbt.internal.sbtscalafix.{Compat, JLineAccess}
import sbt.plugins.JvmPlugin
Expand Down Expand Up @@ -123,6 +122,30 @@ object ScalafixPlugin extends AutoPlugin {
Invisible
)

private val scalafixInterfaceProvider: SettingKey[() => ScalafixInterface] =
SettingKey(
"scalafixInterfaceProvider",
"Implementation detail - do not use",
Invisible
)

private val scalafixCompletions: SettingKey[ScalafixCompletions] =
SettingKey(
"scalafixCompletions",
"Implementation detail - do not use",
Invisible
)

// Memoize ScalafixInterface instances initialized with a custom tool classpath across projects & configurations
// during task execution, to amortize the classloading cost when invoking scalafix concurrently on many targets
private val scalafixInterfaceCache
: TaskKey[BlockingCache[ToolClasspath, ScalafixInterface]] =
TaskKey(
"scalafixInterfaceCache",
"Implementation detail - do not use",
Invisible
)

override lazy val projectConfigurations: Seq[Configuration] =
Seq(ScalafixConfig)

Expand All @@ -147,7 +170,23 @@ object ScalafixPlugin extends AutoPlugin {
)
),
scalafixDependencies := Nil,
commands += ScalafixEnable.command
commands += ScalafixEnable.command,
scalafixInterfaceProvider := ScalafixInterface.fromToolClasspath(
scalafixScalaBinaryVersion.in(ThisBuild).value,
scalafixDependencies = scalafixDependencies.in(ThisBuild).value,
scalafixCustomResolvers = scalafixResolvers.in(ThisBuild).value
),
scalafixCompletions := new ScalafixCompletions(
workingDirectory = baseDirectory.in(ThisBuild).value.toPath,
// Unfortunately, local rules will not show up as completions in the parser, as that parser can only
// depend on settings, while local rules classpath must be looked up via tasks
loadedRules = () => scalafixInterfaceProvider.value().availableRules(),
terminalWidth = Some(JLineAccess.terminalWidth)
),
scalafixInterfaceCache := new BlockingCache[
ToolClasspath,
ScalafixInterface
]
)

override def buildSettings: Seq[Def.Setting[_]] =
Expand All @@ -157,28 +196,10 @@ object ScalafixPlugin extends AutoPlugin {

lazy val stdoutLogger = Compat.ConsoleLogger(System.out)

private val scalafixInterface: Def.Initialize[() => ScalafixInterface] =
Def.setting {
ScalafixInterface.fromToolClasspath(
scalafixScalaBinaryVersion.in(ThisBuild).value,
scalafixDependencies = scalafixDependencies.in(ThisBuild).value,
scalafixCustomResolvers = scalafixResolvers.in(ThisBuild).value
)
}

private val parser: Def.Initialize[Parser[ShellArgs]] = Def.setting {
new ScalafixCompletions(
workingDirectory = baseDirectory.in(ThisBuild).value.toPath,
// Unfortunately, local rules will not show up as completions in the parser, as that parser can only
// depend on settings, while local rules classpath must be looked up via tasks
loadedRules = () => scalafixInterface.value().availableRules(),
terminalWidth = Some(JLineAccess.terminalWidth)
).parser
}

private def scalafixArgsFromShell(
shell: ShellArgs,
scalafixInterface: () => ScalafixInterface,
scalafixInterfaceCache: BlockingCache[ToolClasspath, ScalafixInterface],
projectDepsExternal: Seq[ModuleID],
baseDepsExternal: Seq[ModuleID],
baseResolvers: Seq[Repository],
Expand All @@ -198,18 +219,27 @@ object ScalafixPlugin extends AutoPlugin {
throw new ScalafixFailed(List(ScalafixError.CommandLineError))
}
val rulesDepsExternal = parsed.map(_.dependency)
val refreshClasspath =
(projectDepsInternal ++ projectDepsExternal ++ rulesDepsExternal).nonEmpty
val projectDepsInternal0 = projectDepsInternal.filter {
case directory if directory.isDirectory =>
directory.**(AllPassFilter).get.exists(_.isFile)
case file if file.isFile => true
case _ => false
}
val customToolClasspath =
(projectDepsInternal0 ++ projectDepsExternal ++ rulesDepsExternal).nonEmpty
val interface =
if (refreshClasspath)
scalafixInterface().withArgs(
ToolClasspath(
projectDepsInternal.map(_.toURI.toURL),
baseDepsExternal ++ projectDepsExternal ++ rulesDepsExternal,
baseResolvers
)
if (customToolClasspath) {
val toolClasspath = ToolClasspath(
projectDepsInternal0.map(_.toURI.toURL),
baseDepsExternal ++ projectDepsExternal ++ rulesDepsExternal,
baseResolvers
)
scalafixInterfaceCache.getOrElseUpdate(
toolClasspath,
// costly: triggers artifact resolution & classloader creation
scalafixInterface().withArgs(toolClasspath)
)
else
} else
// if there is nothing specific to the project or the invocation, reuse the default
// interface which already has the baseDepsExternal loaded
scalafixInterface()
Expand All @@ -222,7 +252,7 @@ object ScalafixPlugin extends AutoPlugin {
private def scalafixAllInputTask(): Def.Initialize[InputTask[Unit]] =
// workaround https://github.com/sbt/sbt/issues/3572 by invoking directly what Def.inputTaskDyn would via macro
InputTask
.createDyn(InputTask.initParserAsInput(parser))(
.createDyn(InputTask.initParserAsInput(scalafixCompletions(_.parser)))(
Def.task(shellArgs => scalafixAllTask(shellArgs, thisProject.value))
)

Expand All @@ -246,7 +276,7 @@ object ScalafixPlugin extends AutoPlugin {
): Def.Initialize[InputTask[Unit]] =
// workaround https://github.com/sbt/sbt/issues/3572 by invoking directly what Def.inputTaskDyn would via macro
InputTask
.createDyn(InputTask.initParserAsInput(parser))(
.createDyn(InputTask.initParserAsInput(scalafixCompletions(_.parser)))(
Def.task(shellArgs => scalafixTask(shellArgs, config))
)

Expand Down Expand Up @@ -276,7 +306,8 @@ object ScalafixPlugin extends AutoPlugin {
val scalafixConf = scalafixConfig.in(config).value.map(_.toPath)
val (shell, mainInterface0) = scalafixArgsFromShell(
shellArgs,
scalafixInterface.value,
scalafixInterfaceProvider.value,
scalafixInterfaceCache.value,
projectDepsExternal,
scalafixDependencies.in(ThisBuild).value,
scalafixResolvers.in(ThisBuild).value,
Expand Down Expand Up @@ -306,7 +337,10 @@ object ScalafixPlugin extends AutoPlugin {
}
private def scalafixHelp: Def.Initialize[Task[Unit]] =
Def.task {
scalafixInterface.value().withArgs(Arg.ParsedArgs(List("--help"))).run()
scalafixInterfaceProvider
.value()
.withArgs(Arg.ParsedArgs(List("--help")))
.run()
()
}

Expand Down

0 comments on commit bffce56

Please sign in to comment.