Skip to content

Commit

Permalink
don't call no op compilations
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek committed Dec 9, 2024
1 parent 0f0c33f commit 9f7899a
Show file tree
Hide file tree
Showing 12 changed files with 192 additions and 168 deletions.
60 changes: 32 additions & 28 deletions metals/src/main/scala/scala/meta/internal/metals/Compilations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ final class Compilations(
downstreamTargets: PreviouslyCompiledDownsteamTargets,
bestEffortEnabled: Boolean,
)(implicit ec: ExecutionContext) {
private val fileSignatures = new SavedFileSignatures
private val fileChanges = new FileChanges(buildTargets, workspace)
private val compileTimeout: Timeout =
Timeout("compile", Duration(10, TimeUnit.MINUTES))
private val cascadeTimeout: Timeout =
Expand Down Expand Up @@ -97,6 +97,7 @@ final class Compilations(
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 @@ -105,48 +106,52 @@ final class Compilations(
def compileTargets(
targets: Seq[b.BuildTargetIdentifier]
): Future[Unit] = {
fileChanges.willCompile(targets.toList)
compileBatch(targets).ignoreValue
}

def compileFile(path: PathWithContent): Future[Option[b.CompileResult]] = {
if (fileSignatures.didSavedContentChanged(path)) {
def empty = new b.CompileResult(b.StatusCode.CANCELLED)
for {
targetOpt <- expand(path.path)
result <- targetOpt match {
case None => Future.successful(empty)
case Some(target) =>
compileBatch(target)
.map(res => res.getOrElse(target, empty))
}
_ <- compileWorksheets(Seq(path.path))
} yield Some(result)
} else Future.successful(None)
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 <- fileChanges.buildTargetToCompile(
path,
fingerprint,
assumeDidNotChange,
)
result <- targetOpt match {
case None => Future.successful(empty)
case Some(target) =>
compileBatch(target)
.map(res => res.getOrElse(target, empty))
}
_ <-
if (assumeDidNotChange && targetOpt.isEmpty) Future.successful(())
else compileWorksheets(Seq(path))
} yield Some(result)
}

def compileFiles(
pathsWithContent: Seq[PathWithContent],
paths: Seq[(AbsolutePath, Fingerprint)],
focusedDocumentBuildTarget: Option[BuildTargetIdentifier],
): Future[Unit] = {
val paths =
pathsWithContent.filter(fileSignatures.didSavedContentChanged).map(_.path)
for {
targets <- expand(paths)
targets <- fileChanges.buildTargetsToCompile(
paths,
focusedDocumentBuildTarget,
)
_ <- 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)
cascadeBatch(inverseDependencyLeaves).map(_ => ())
}

Expand All @@ -162,7 +167,6 @@ final class Compilations(
lastCompile = Set.empty
cascadeBatch.cancelAll()
compileBatch.cancelAll()
fileSignatures.cancel()
}

def recompileAll(): Future[Unit] = {
Expand Down
131 changes: 131 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,131 @@
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 previousCreateOrModify = TrieMap[AbsolutePath, String]()
private val dirtyBuildTargets = mutable.Set[BuildTargetIdentifier]()

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 =>
findBuildTargetIfShouldCompile(path, Some(fingerprint)).map(acc ++ _)
}
}
} yield {
val allToCompile =
if (focusedDocumentBuildTarget.exists(dirtyBuildTargets(_)))
toCompile ++ focusedDocumentBuildTarget
else toCompile
willCompile(allToCompile)
allToCompile
}

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,
): Future[Option[BuildTargetIdentifier]] = {
expand(path).map(
_.filter(bt =>
dirtyBuildTargets.contains(
bt
) || (!assumeDidNotChange && didContentChange(path, fingerprint, bt))
)
)
}

private def expand(
path: AbsolutePath
): Future[Option[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)
}

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 (previousCreateOrModify.getOrElse(path, null) == fingerprint.md5)
false
else {
previousCreateOrModify.put(path, fingerprint.md5)
true
}
}
}
.getOrElse(true)
}

def cancel(): Unit = previousCreateOrModify.clear()
}
Original file line number Diff line number Diff line change
Expand Up @@ -744,15 +744,9 @@ abstract class MetalsLspService(
// In some cases like peeking definition didOpen might be followed up by close
// and we would lose the notion of the focused document
recentlyOpenedFiles.add(path)
val prevBuildTarget = focusedDocumentBuildTarget.getAndUpdate { current =>
buildTargets
.inverseSources(path)
.getOrElse(current)
}

val content = FileIO.readAllBytes(path)
// Update md5 fingerprint from file contents on disk
fingerprints.add(path, new String(content, charset))
val fingerprint = fingerprints.add(path, FileIO.slurp(path, charset))
// Update in-memory buffer contents from LSP client
buffers.put(path, params.getTextDocument.getText)

Expand Down Expand Up @@ -785,10 +779,7 @@ abstract class MetalsLspService(
Future
.sequence(
List(
maybeCompileOnDidFocus(
PathWithContent(path, content),
prevBuildTarget,
),
compilations.compileFile(path, Some(fingerprint)),
compilers.load(List(path)),
parser,
interactive,
Expand All @@ -811,11 +802,6 @@ abstract class MetalsLspService(
uri: String
): CompletableFuture[DidFocusResult.Value] = {
val path = uri.toAbsolutePath
val prevBuildTarget = focusedDocumentBuildTarget.getAndUpdate { current =>
buildTargets
.inverseSources(path)
.getOrElse(current)
}
scalaCli.didFocus(path)
// Don't trigger compilation on didFocus events under cascade compilation
// because save events already trigger compile in inverse dependencies.
Expand All @@ -825,29 +811,16 @@ abstract class MetalsLspService(
CompletableFuture.completedFuture(DidFocusResult.RecentlyActive)
} else {
worksheetProvider.onDidFocus(path)
maybeCompileOnDidFocus(PathWithContent(path), prevBuildTarget).asJava
compilations
.compileFile(path, assumeDidNotChange = true)
.map(
_.map(_ => DidFocusResult.Compiled)
.getOrElse(DidFocusResult.AlreadyCompiled)
)
.asJava
}
}

protected def maybeCompileOnDidFocus(
path: PathWithContent,
prevBuildTarget: b.BuildTargetIdentifier,
): Future[DidFocusResult.Value] =
buildTargets.inverseSources(path.path) match {
case Some(target) if prevBuildTarget != target =>
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
case _ if path.path.isWorksheet =>
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
case Some(_) =>
Future.successful(DidFocusResult.AlreadyCompiled)
case None =>
Future.successful(DidFocusResult.NoBuildTarget)
}

def pause(): Unit = pauseables.pause()

def unpause(): Unit = pauseables.unpause()
Expand Down Expand Up @@ -942,11 +915,10 @@ abstract class MetalsLspService(
}

protected def onChange(paths: Seq[AbsolutePath]): Future[Unit] = {
val pathsWithContent =
val pathsWithFingerPrints =
paths.map { path =>
val content = FileIO.readAllBytes(path)
fingerprints.add(path, new String(content, charset))
PathWithContent(path, content)
val fingerprint = fingerprints.add(path, FileIO.slurp(path, charset))
(path, fingerprint)
}

Future
Expand All @@ -955,7 +927,7 @@ abstract class MetalsLspService(
Future(indexer.reindexWorkspaceSources(paths)),
compilations
.compileFiles(
pathsWithContent,
pathsWithFingerPrints,
Option(focusedDocumentBuildTarget.get()),
),
) ++ paths.map(f => Future(interactiveSemanticdbs.textDocument(f)))
Expand All @@ -969,7 +941,7 @@ abstract class MetalsLspService(
List(
compilations
.compileFiles(
List(PathWithContent.deleted(path)),
List((path, null)),
Option(focusedDocumentBuildTarget.get()),
),
Future {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ final class MutableMd5Fingerprints extends Md5Fingerprints {
path: AbsolutePath,
text: String,
md5: Option[String] = None,
): Unit = {
add(path, Fingerprint(text, md5.getOrElse(MD5.compute(text))))
): Fingerprint = {
val fingerprint = Fingerprint(text, md5.getOrElse(MD5.compute(text)))
add(path, fingerprint)
fingerprint
}

private def add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,9 +635,7 @@ class ProjectMetalsLspService(
for {
_ <- connect(ImportBuildAndIndex(session))
} {
focusedDocument().foreach(path =>
compilations.compileFile(PathWithContent(path))
)
focusedDocument().foreach(path => compilations.compileFile(path))
}
}
}
Expand Down
Loading

0 comments on commit 9f7899a

Please sign in to comment.