Skip to content

Commit

Permalink
deduplicate reading file content
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek committed Dec 5, 2024
1 parent 2d9cd16 commit 0f0c33f
Show file tree
Hide file tree
Showing 10 changed files with 105 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,28 +108,30 @@ final class Compilations(
compileBatch(targets).ignoreValue
}

def compileFile(path: AbsolutePath): Future[Option[b.CompileResult]] = {
if(fileSignatures.didSavedContentChanged(path)) {
def compileFile(path: PathWithContent): Future[Option[b.CompileResult]] = {
if (fileSignatures.didSavedContentChanged(path)) {
def empty = new b.CompileResult(b.StatusCode.CANCELLED)
for {
targetOpt <- expand(path)
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))
_ <- compileWorksheets(Seq(path.path))
} yield Some(result)
} else Future.successful(None)
}

def compileFiles(
paths: Seq[AbsolutePath],
pathsWithContent: Seq[PathWithContent],
focusedDocumentBuildTarget: Option[BuildTargetIdentifier],
): Future[Unit] = {
val paths =
pathsWithContent.filter(fileSignatures.didSavedContentChanged).map(_.path)
for {
targets <- expand(paths.filter(fileSignatures.didSavedContentChanged))
targets <- expand(paths)
_ <- compileBatch(targets)
_ <- focusedDocumentBuildTarget match {
case Some(bt)
Expand Down Expand Up @@ -160,6 +162,7 @@ final class Compilations(
lastCompile = Set.empty
cascadeBatch.cancelAll()
compileBatch.cancelAll()
fileSignatures.cancel()
}

def recompileAll(): Future[Unit] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -750,8 +750,9 @@ abstract class MetalsLspService(
.getOrElse(current)
}

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

Expand Down Expand Up @@ -784,7 +785,10 @@ abstract class MetalsLspService(
Future
.sequence(
List(
maybeCompileOnDidFocus(path, prevBuildTarget),
maybeCompileOnDidFocus(
PathWithContent(path, content),
prevBuildTarget,
),
compilers.load(List(path)),
parser,
interactive,
Expand Down Expand Up @@ -821,20 +825,20 @@ abstract class MetalsLspService(
CompletableFuture.completedFuture(DidFocusResult.RecentlyActive)
} else {
worksheetProvider.onDidFocus(path)
maybeCompileOnDidFocus(path, prevBuildTarget).asJava
maybeCompileOnDidFocus(PathWithContent(path), prevBuildTarget).asJava
}
}

protected def maybeCompileOnDidFocus(
path: AbsolutePath,
path: PathWithContent,
prevBuildTarget: b.BuildTargetIdentifier,
): Future[DidFocusResult.Value] =
buildTargets.inverseSources(path) match {
buildTargets.inverseSources(path.path) match {
case Some(target) if prevBuildTarget != target =>
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
case _ if path.isWorksheet =>
case _ if path.path.isWorksheet =>
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
Expand Down Expand Up @@ -938,16 +942,22 @@ abstract class MetalsLspService(
}

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

Future
.sequence(
List(
Future(indexer.reindexWorkspaceSources(paths)),
compilations
.compileFiles(paths, Option(focusedDocumentBuildTarget.get())),
.compileFiles(
pathsWithContent,
Option(focusedDocumentBuildTarget.get()),
),
) ++ paths.map(f => Future(interactiveSemanticdbs.textDocument(f)))
)
.ignoreValue
Expand All @@ -958,7 +968,10 @@ abstract class MetalsLspService(
.sequence(
List(
compilations
.compileFiles(List(path), Option(focusedDocumentBuildTarget.get())),
.compileFiles(
List(PathWithContent.deleted(path)),
Option(focusedDocumentBuildTarget.get()),
),
Future {
diagnostics.didDelete(path)
testProvider.onFileDelete(path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,9 @@ class ProjectMetalsLspService(
for {
_ <- connect(ImportBuildAndIndex(session))
} {
focusedDocument().foreach(path => compilations.compileFile(path))
focusedDocument().foreach(path =>
compilations.compileFile(PathWithContent(path))
)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,55 @@ import scala.meta.io.AbsolutePath
class SavedFileSignatures {
private val previousCreateOrModify = TrieMap[AbsolutePath, String]()

def didSavedContentChanged(path: AbsolutePath): Boolean = {
try {
val md5 =
if (path.exists) Some(MD5.bytesToHex(FileIO.readAllBytes(path)))
else None
synchronized {
if (previousCreateOrModify.get(path) == md5) false
else {
md5 match {
case None => previousCreateOrModify.remove(path)
case Some(md5) => previousCreateOrModify.put(path, md5)
def didSavedContentChanged(pathWithContent: PathWithContent): Boolean = {
val path = pathWithContent.path
pathWithContent
.getSignature()
.map { md5 =>
synchronized {
if (previousCreateOrModify.get(path) == md5) false
else {
md5 match {
case None => previousCreateOrModify.remove(path)
case Some(md5) => previousCreateOrModify.put(path, md5)
}
true
}
true
}
}
} catch {
case e: IOException =>
scribe.warn(s"Failed to read contents of $path", e)
true
}
.getOrElse(true)
}

def cancel(): Unit = previousCreateOrModify.clear()
}

class PathWithContent(
val path: AbsolutePath,
optContent: Option[PathWithContent.Content],
) {
def getSignature(): Either[IOException, Option[String]] = {
optContent
.map(_.map(content => MD5.bytesToHex(content)))
.map(Right[IOException, Option[String]](_))
.getOrElse {
try {
if (path.exists)
Right(Some(MD5.bytesToHex(FileIO.readAllBytes(path))))
else Right(None)
} catch {
case e: IOException =>
scribe.warn(s"Failed to read contents of $path", e)
Left(e)
}
}
}
}

object PathWithContent {
// None if the file doesn't exist
type Content = Option[Array[Byte]]
def apply(path: AbsolutePath) = new PathWithContent(path, None)
def apply(path: AbsolutePath, content: Array[Byte]) =
new PathWithContent(path, Some(Some(content)))
def deleted(path: AbsolutePath) = new PathWithContent(path, Some(None))
}
5 changes: 3 additions & 2 deletions tests/slow/src/test/scala/tests/sbt/CancelCompileSuite.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tests

import scala.meta.internal.metals.MetalsServerConfig
import scala.meta.internal.metals.PathWithContent
import scala.meta.internal.metals.ServerCommands
import scala.meta.internal.metals.{BuildInfo => V}

Expand Down Expand Up @@ -58,7 +59,7 @@ class CancelCompileSuite
_ <- server.server.buildServerPromise.future
(compileReport, _) <- server.server.compilations
.compileFile(
workspace.resolve("c/src/main/scala/c/C.scala")
PathWithContent(workspace.resolve("c/src/main/scala/c/C.scala"))
)
.zip {
// wait until the compilation start
Expand All @@ -68,7 +69,7 @@ class CancelCompileSuite
_ = assertNoDiff(client.workspaceDiagnostics, "")
_ = assertEquals(compileReport.get.getStatusCode(), StatusCode.CANCELLED)
_ <- server.server.compilations.compileFile(
workspace.resolve("c/src/main/scala/c/C.scala")
PathWithContent(workspace.resolve("c/src/main/scala/c/C.scala"))
)
_ = assertNoDiff(
client.workspaceDiagnostics,
Expand Down
3 changes: 2 additions & 1 deletion tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import scala.meta.internal.metals.CreateSession
import scala.meta.internal.metals.Messages
import scala.meta.internal.metals.Messages.ImportBuildChanges
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.PathWithContent
import scala.meta.internal.metals.ServerCommands
import scala.meta.internal.metals.{BuildInfo => V}
import scala.meta.internal.process.SystemProcess
Expand Down Expand Up @@ -409,7 +410,7 @@ class SbtServerSuite
_ <- initialize(layout)
// make sure to compile once
_ <- server.server.compilations.compileFile(
workspace.resolve("target/Foo.scala")
PathWithContent(workspace.resolve("target/Foo.scala"))
)
} yield {
// Sleep 100 ms: that should be enough to see the compilation looping
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/src/main/scala/tests/TestingServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,10 @@ final case class TestingServer(

val compilations =
paths.map(path =>
fullServer.getServiceFor(path).compilations.compileFile(path)
fullServer
.getServiceFor(path)
.compilations
.compileFile(m.internal.metals.PathWithContent(path))
)

for {
Expand Down Expand Up @@ -1119,7 +1122,9 @@ final case class TestingServer(
codeLenses.trySuccess(lenses.toList)
else if (retries > 0) {
retries -= 1
server.compilations.compileFile(path)
server.compilations.compileFile(
m.internal.metals.PathWithContent(path)
)
} else {
val error = s"Could not fetch any code lenses in $maxRetries tries"
codeLenses.tryFailure(new NoSuchElementException(error))
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/src/test/scala/tests/BillLspSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import scala.concurrent.Future
import scala.meta.internal.metals.Directories
import scala.meta.internal.metals.Messages
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.PathWithContent
import scala.meta.internal.metals.RecursivelyDelete
import scala.meta.internal.metals.ServerCommands
import scala.meta.io.AbsolutePath
Expand Down Expand Up @@ -241,7 +242,7 @@ class BillLspSuite extends BaseLspSuite("bill") {
)
(compileReport, _) <- server.server.compilations
.compileFile(
workspace.resolve("src/com/App.scala")
PathWithContent(workspace.resolve("src/com/App.scala"))
)
.zip {
// wait until the compilation start
Expand All @@ -260,7 +261,7 @@ class BillLspSuite extends BaseLspSuite("bill") {
_ = assert(currentTrace.contains(s"buildTarget/compile - ($cancelId)"))
compileReport <- server.server.compilations
.compileFile(
workspace.resolve("src/com/App.scala")
PathWithContent(workspace.resolve("src/com/App.scala"))
)
_ = assertEquals(compileReport.get.getStatusCode(), StatusCode.OK)
} yield ()
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/src/test/scala/tests/CancelCompileLspSuite.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package tests

import scala.meta.internal.metals.PathWithContent
import scala.meta.internal.metals.ServerCommands

import ch.epfl.scala.bsp4j.StatusCode
Expand Down Expand Up @@ -48,7 +49,7 @@ class CancelCompileLspSuite extends BaseLspSuite("compile-cancel") {
_ <- server.server.buildServerPromise.future
(compileReport, _) <- server.server.compilations
.compileFile(
workspace.resolve("c/src/main/scala/c/C.scala")
PathWithContent(workspace.resolve("c/src/main/scala/c/C.scala"))
)
.zip {
// wait until the compilation start
Expand All @@ -58,7 +59,7 @@ class CancelCompileLspSuite extends BaseLspSuite("compile-cancel") {
_ = assertNoDiff(client.workspaceDiagnostics, "")
_ = assertEquals(compileReport.get.getStatusCode(), StatusCode.CANCELLED)
_ <- server.server.compilations.compileFile(
workspace.resolve("c/src/main/scala/c/C.scala")
PathWithContent(workspace.resolve("c/src/main/scala/c/C.scala"))
)
_ = assertNoDiff(
client.workspaceDiagnostics,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import scala.util.Success
import scala.meta.internal.metals.ClientCommands
import scala.meta.internal.metals.InitializationOptions
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.PathWithContent

class UnsupportedDebuggingLspSuite
extends BaseLspSuite("unsupported-debugging") {
Expand Down Expand Up @@ -62,7 +63,9 @@ class UnsupportedDebuggingLspSuite
)
_ <-
server.server.compilations
.compileFile(server.toPath("a/src/main/scala/Main.scala"))
.compileFile(
PathWithContent(server.toPath("a/src/main/scala/Main.scala"))
)
} yield {
val clientCommands = client.clientCommands.asScala.map(_.getCommand).toSet
assert(!clientCommands.contains(ClientCommands.RefreshModel.id))
Expand Down

0 comments on commit 0f0c33f

Please sign in to comment.