Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improvement: deduplicate compile requests #7006

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 26 additions & 43 deletions metals/src/main/scala/scala/meta/internal/metals/Compilations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ import ch.epfl.scala.{bsp4j => b}
final class Compilations(
buildTargets: BuildTargets,
classes: BuildTargetClasses,
workspace: () => AbsolutePath,
languageClient: MetalsLanguageClient,
refreshTestSuites: () => Unit,
afterSuccessfulCompilation: () => Unit,
isCurrentlyFocused: b.BuildTargetIdentifier => Boolean,
buildtargetInFocus: () => Option[b.BuildTargetIdentifier],
compileWorksheets: Seq[AbsolutePath] => Future[Unit],
onStartCompilation: () => Unit,
userConfiguration: () => UserConfiguration,
downstreamTargets: PreviouslyCompiledDownsteamTargets,
fileChanges: FileChanges,
bestEffortEnabled: Boolean,
)(implicit ec: ExecutionContext) {
private val compileTimeout: Timeout =
Expand Down Expand Up @@ -88,13 +88,16 @@ final class Compilations(
source: AbsolutePath,
compileInverseDependencies: Boolean,
): Future[Unit] =
expand(source).flatMap(targets =>
compilationFinished(targets.toSeq, compileInverseDependencies)
)
fileChanges
.expand(source)
.flatMap(targets =>
compilationFinished(targets.toSeq, compileInverseDependencies)
)

def compileTarget(
target: b.BuildTargetIdentifier
): Future[b.CompileResult] = {
fileChanges.willCompile(List(target))
compileBatch(target).map { results =>
results.getOrElse(target, new b.CompileResult(b.StatusCode.CANCELLED))
}
Expand All @@ -103,44 +106,44 @@ final class Compilations(
def compileTargets(
targets: Seq[b.BuildTargetIdentifier]
): Future[Unit] = {
fileChanges.willCompile(targets.toList)
compileBatch(targets).ignoreValue
}

def compileFile(path: AbsolutePath): Future[b.CompileResult] = {
def compileFile(
path: AbsolutePath,
fingerprint: Option[Fingerprint] = None,
assumeDidNotChange: Boolean = false,
): Future[Option[b.CompileResult]] = {
def empty = new b.CompileResult(b.StatusCode.CANCELLED)
for {
targetOpt <- expand(path)
targetOpt <- fileChanges.buildTargetToCompile(
path,
fingerprint,
assumeDidNotChange,
)
result <- targetOpt match {
case None => Future.successful(empty)
case Some(target) =>
compileBatch(target)
.map(res => res.getOrElse(target, empty))
}
_ <- compileWorksheets(Seq(path))
} yield result
} yield Some(result)
}

def compileFiles(
paths: Seq[AbsolutePath],
focusedDocumentBuildTarget: Option[BuildTargetIdentifier],
): Future[Unit] = {
def compileFiles(paths: Seq[(AbsolutePath, Fingerprint)]): Future[Unit] = {
for {
targets <- expand(paths)
targets <- fileChanges.buildTargetsToCompile(paths, buildtargetInFocus())
_ <- compileBatch(targets)
_ <- focusedDocumentBuildTarget match {
case Some(bt)
if !targets.contains(bt) &&
buildTargets.isInverseDependency(bt, targets.toList) =>
compileBatch(bt)
case _ => Future.successful(())
}
_ <- compileWorksheets(paths)
_ <- compileWorksheets(paths.map(_._1))
} yield ()
}

def cascadeCompile(targets: Seq[BuildTargetIdentifier]): Future[Unit] = {
val inverseDependencyLeaves =
targets.flatMap(buildTargets.inverseDependencyLeaves).distinct
fileChanges.willCompile(inverseDependencyLeaves.toList)
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
cascadeBatch(inverseDependencyLeaves).map(_ => ())
}

Expand Down Expand Up @@ -194,28 +197,8 @@ final class Compilations(
.ignoreValue
}

private def expand(
path: AbsolutePath
): Future[Option[b.BuildTargetIdentifier]] = {
val isCompilable =
(path.isScalaOrJava || path.isSbt) &&
!path.isDependencySource(workspace()) &&
!path.isInTmpDirectory(workspace())

if (isCompilable) {
val targetOpt = buildTargets.inverseSourcesBsp(path)
targetOpt.foreach {
case tgts if tgts.isEmpty => scribe.warn(s"no build target for: $path")
case _ =>
}

targetOpt
} else
Future.successful(None)
}

def expand(paths: Seq[AbsolutePath]): Future[Seq[b.BuildTargetIdentifier]] = {
val expansions = paths.map(expand)
val expansions = paths.map(fileChanges.expand)
Future.sequence(expansions).map(_.flatten)
}

Expand Down Expand Up @@ -291,7 +274,7 @@ final class Compilations(
targets,
() => {
refreshTestSuites()
if (targets.exists(isCurrentlyFocused)) {
if (targets.exists(buildtargetInFocus().contains)) {
languageClient.refreshModel()
}
},
Expand Down
141 changes: 141 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/FileChanges.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package scala.meta.internal.metals

import scala.collection.concurrent.TrieMap
import scala.collection.mutable
import scala.concurrent.ExecutionContext
import scala.concurrent.Future

import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.io.AbsolutePath

import ch.epfl.scala.bsp4j.BuildTargetIdentifier

class FileChanges(buildTargets: BuildTargets, workspace: () => AbsolutePath)(
implicit ec: ExecutionContext
) {
private val previousSignatures = TrieMap[AbsolutePath, String]()
private val dirtyBuildTargets = mutable.Set[BuildTargetIdentifier]()
kasiaMarek marked this conversation as resolved.
Show resolved Hide resolved

def addAllDirty(
targets: IterableOnce[BuildTargetIdentifier]
): mutable.Set[BuildTargetIdentifier] =
dirtyBuildTargets.addAll(targets)

def buildTargetsToCompile(
paths: Seq[(AbsolutePath, Fingerprint)],
focusedDocumentBuildTarget: Option[BuildTargetIdentifier],
): Future[Seq[BuildTargetIdentifier]] =
for {
toCompile <- paths.foldLeft(
Future.successful(Seq.empty[BuildTargetIdentifier])
) { case (toCompile, (path, fingerprint)) =>
toCompile.flatMap { acc =>
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
findBuildTargetIfShouldCompile(path, Some(fingerprint)).map(acc ++ _)
}
}
} yield {
val allToCompile =
if (focusedDocumentBuildTarget.exists(dirtyBuildTargets(_)))
toCompile ++ focusedDocumentBuildTarget
else toCompile
willCompile(allToCompile)
allToCompile
}

/**
* The `assumeDidNotChange` should be set when we want to assume that the file did not change
* if it is not present in the `previousSignatures` (opened for the first time),
* we set it to `true` for operations such as `didFocus` or `didOpen`.
*/
def buildTargetToCompile(
path: AbsolutePath,
fingerprint: Option[Fingerprint],
assumeDidNotChange: Boolean,
): Future[Option[BuildTargetIdentifier]] = {
for {
toCompile <- findBuildTargetIfShouldCompile(
path,
fingerprint,
assumeDidNotChange,
)
} yield {
willCompile(toCompile.toSeq)
toCompile
}
}

def willCompile(ids: Seq[BuildTargetIdentifier]): Unit =
buildTargets
.buildTargetTransitiveDependencies(ids.toList)
.foreach(dirtyBuildTargets.remove)

private def findBuildTargetIfShouldCompile(
path: AbsolutePath,
fingerprint: Option[Fingerprint],
assumeDidNotChange: Boolean = false,
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
): Future[Option[BuildTargetIdentifier]] = {
expand(path).map(
_.filter(bt =>
dirtyBuildTargets.contains(
bt
) || (!assumeDidNotChange && didContentChange(path, fingerprint, bt))
)
)
}

def expand(
path: AbsolutePath
): Future[Option[BuildTargetIdentifier]] = {
val isCompilable =
(path.isScalaOrJava || path.isSbt) &&
!path.isDependencySource(workspace()) &&
!path.isInTmpDirectory(workspace())
kasiaMarek marked this conversation as resolved.
Show resolved Hide resolved

if (isCompilable) {
val targetOpt = buildTargets.inverseSourcesBsp(path)
targetOpt.foreach {
case tgts if tgts.isEmpty => scribe.warn(s"no build target for: $path")
case _ =>
}

targetOpt
} else
Future.successful(None)
}

private def didContentChange(
path: AbsolutePath,
fingerprint: Option[Fingerprint],
buildTarget: BuildTargetIdentifier,
): Boolean = {
val didChange = didContentChange(path, fingerprint)
if (didChange) {
buildTargets
.allInverseDependencies(buildTarget)
.foreach { bt =>
if (bt != buildTarget) dirtyBuildTargets.add(bt)
}
}
didChange
}

private def didContentChange(
path: AbsolutePath,
fingerprint: Option[Fingerprint],
): Boolean = {
fingerprint
.map { fingerprint =>
synchronized {
if (previousSignatures.getOrElse(path, "") == fingerprint.md5)
false
else {
previousSignatures.put(path, fingerprint.md5)
true
}
}
}
.getOrElse(true)
}

def cancel(): Unit = previousSignatures.clear()
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@ trait IndexProviders {
def folder: AbsolutePath
def implementationProvider: ImplementationProvider
def resetService(): Unit
def fileChanges: FileChanges
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ case class Indexer(indexProviders: IndexProviders)(implicit rc: ReportContext) {
data.reset()
buildTargetClasses.clear()
data.addWorkspaceBuildTargets(importedBuild.workspaceBuildTargets)
fileChanges.addAllDirty(
importedBuild.workspaceBuildTargets
.getTargets()
.map(_.getId())
.asScala
)
data.addScalacOptions(
importedBuild.scalacOptions,
bspSession.map(_.mainConnection),
Expand Down
Loading
Loading