From f8584d61ffce2033848ebea4c9fc635a6c950243 Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Thu, 21 Nov 2024 19:57:14 +0100 Subject: [PATCH 1/3] [compiler] Avoid computing zinc stamps for JDK 9+ module .sig files/paths #SCL-22939 fixed - These paths really do not like being accessed through the regular Java IO means, throwing ClosedFileSystemException instances when accessed concurrently, like we do in the Scala compile server. - https://github.com/sbt/zinc/issues/1383 --- .../scala/local/zinc/StampReader.scala | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/zinc/StampReader.scala b/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/zinc/StampReader.scala index 01f65916809..de556dafc33 100644 --- a/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/zinc/StampReader.scala +++ b/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/zinc/StampReader.scala @@ -1,8 +1,28 @@ package org.jetbrains.jps.incremental.scala.local.zinc -import sbt.internal.inc.{PlainVirtualFileConverter, Stamps} -import xsbti.compile.analysis.ReadStamps +import sbt.internal.inc.{FarmHash, PlainVirtualFileConverter, Stamper, Stamps} +import xsbti.VirtualFileRef +import xsbti.compile.analysis.{ReadStamps, Stamp} private[local] object StampReader { - val Instance: ReadStamps = Stamps.timeWrapBinaryStamps(PlainVirtualFileConverter.converter) + + private def avoidSigs(ref: VirtualFileRef): Stamp = { + if (ref.id.endsWith(".sig")) { + val path = PlainVirtualFileConverter.converter.toPath(ref) + if (path.getClass.getName == "jdk.nio.zipfs.ZipPath") + return FarmHash.fromLong(path.##.toLong) + } + fallback(ref) + } + + private def fallback(ref: VirtualFileRef): Stamp = + Stamper.forHashInRootPaths(PlainVirtualFileConverter.converter).apply(ref) + + private val uncached: ReadStamps = Stamps.uncachedStamps( + avoidSigs, + Stamper.forContentHash, + avoidSigs + ) + + val Instance: ReadStamps = Stamps.timeWrapBinaryStamps(uncached, PlainVirtualFileConverter.converter) } From 8c70197d492d9f931044a918006ca5c3440487cc Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Fri, 22 Nov 2024 14:41:26 +0100 Subject: [PATCH 2/3] [compiler] Optimize zinc stamp recomputation after JPS annotation processing #SCL-21674 - The process global stamper instance is reused. Either the stamp value is already cached, or it will be recomputed if the file actually changed on disk (during annotation processing). - This stamp is then compared to the original stamp at the beginning of the compute stamps step. - The stamp data structures are only modified if these two stamps differ. --- .../incremental/scala/local/LocalServer.scala | 16 +++-- .../ZincAnnotationProcessingTest.scala | 62 +++++++++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 scala/compiler-integration/test/org/jetbrains/plugins/scala/compiler/ZincAnnotationProcessingTest.scala diff --git a/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala b/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala index e2942921990..d3c435b9bc0 100644 --- a/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala +++ b/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala @@ -1,9 +1,9 @@ package org.jetbrains.jps.incremental.scala.local -import org.jetbrains.jps.incremental.scala.local.zinc.AnalysisStoreFactory +import org.jetbrains.jps.incremental.scala.local.zinc.{AnalysisStoreFactory, StampReader} import org.jetbrains.jps.incremental.scala.{Client, CompileServerBundle, DelegateClient, ExitCode, Server} import org.jetbrains.plugins.scala.compiler.data.{CompilationData, CompilerData, DocumentCompilationArguments, SbtData} -import sbt.internal.inc.{Analysis, PlainVirtualFileConverter, Stamper} +import sbt.internal.inc.{Analysis, PlainVirtualFileConverter} import xsbti.compile.{AnalysisContents, AnalysisStore} import java.io.File @@ -60,12 +60,16 @@ final class LocalServer extends Server { analysisStore.get().toScala.filter(_.getAnalysis.isInstanceOf[Analysis]).foreach { analysisContents => val analysis = analysisContents.getAnalysis.asInstanceOf[Analysis] - var newStamps = analysis.stamps - val stamper = Stamper.forHashInRootPaths(PlainVirtualFileConverter.converter) + val originalStamps = analysis.stamps + var newStamps = originalStamps + val stamper = StampReader.Instance outputFiles.foreach { classFile => val virtualFile = PlainVirtualFileConverter.converter.toVirtualFile(classFile) - val stamp = stamper(virtualFile) - newStamps = newStamps.markProduct(virtualFile, stamp) + val oldStamp = originalStamps.product(virtualFile) + val stamp = stamper.product(virtualFile) + if (oldStamp != stamp) { + newStamps = newStamps.markProduct(virtualFile, stamp) + } } val newAnalysis = analysis.copy(stamps = newStamps) val newAnalysisContents = AnalysisContents.create(newAnalysis, analysisContents.getMiniSetup) diff --git a/scala/compiler-integration/test/org/jetbrains/plugins/scala/compiler/ZincAnnotationProcessingTest.scala b/scala/compiler-integration/test/org/jetbrains/plugins/scala/compiler/ZincAnnotationProcessingTest.scala new file mode 100644 index 00000000000..ade057929e7 --- /dev/null +++ b/scala/compiler-integration/test/org/jetbrains/plugins/scala/compiler/ZincAnnotationProcessingTest.scala @@ -0,0 +1,62 @@ +package org.jetbrains.plugins.scala.compiler + +import com.intellij.openapi.vfs.VfsUtil +import junit.framework.TestCase.{assertEquals, assertTrue} +import org.jetbrains.plugins.scala.DependencyManagerBase.RichStr +import org.jetbrains.plugins.scala.base.libraryLoaders.{IvyManagedLoader, LibraryLoader} +import org.jetbrains.plugins.scala.compiler.CompilerMessagesUtil.{assertCompilingScalaSources, assertNoErrorsOrWarnings} +import org.jetbrains.plugins.scala.compiler.data.IncrementalityType +import org.jetbrains.plugins.scala.extensions.inWriteAction +import org.jetbrains.plugins.scala.{ScalaVersion, SlowTests} +import org.junit.experimental.categories.Category + +import java.nio.file.Files +import scala.jdk.CollectionConverters._ + +@Category(Array(classOf[SlowTests])) +class ZincAnnotationProcessingTest extends ScalaCompilerTestBase { + + override protected def supportedIn(version: ScalaVersion): Boolean = version == ScalaVersion.Latest.Scala_3 + + override protected def incrementalityType: IncrementalityType = IncrementalityType.SBT + + override protected def additionalLibraries: Seq[LibraryLoader] = + Seq(IvyManagedLoader("org.jetbrains" % "annotations" % "26.0.1")) + + def testAnnotationProcessing(): Unit = { + addFileToProjectSources("A.scala", + """import org.jetbrains.annotations.NotNull + | + |class A(@NotNull x: String) + |""".stripMargin) + addFileToProjectSources("B.scala", "class B") + + val messages1 = compiler.make().asScala.toSeq + assertNoErrorsOrWarnings(messages1) + assertCompilingScalaSources(messages1, 2) + + val classFileNames = Seq("A", "B") + + val firstClassFiles = classFileNames.map(findClassFile) + val firstTimestamps = firstClassFiles.map(Files.getLastModifiedTime(_).toMillis) + assertTrue("There should be exactly two class files", firstClassFiles.sizeIs == 2) + + inWriteAction { + VfsUtil.saveText(getSourceRootDir.findChild("B.scala"), "class B { def x = 5 }") + } + + val messages2 = compiler.make().asScala.toSeq + assertNoErrorsOrWarnings(messages2) + assertCompilingScalaSources(messages2, 1) + + val secondClassFiles = classFileNames.map(findClassFile) + val secondTimestamps = secondClassFiles.map(Files.getLastModifiedTime(_).toMillis) + assertTrue("There should be exactly two class files", firstClassFiles.sizeIs == 2) + + (firstTimestamps, secondTimestamps) match { + case (Seq(aBefore, bBefore), Seq(aAfter, bAfter)) => + assertEquals("A.scala was recompiled when it shouldn't have been", aBefore, aAfter) + assertTrue("B.scala was not recompiled when it should have been", bBefore < bAfter) + } + } +} From 567270b48a6ad3f369d1ce5cb06d06a47c51d35f Mon Sep 17 00:00:00 2001 From: Vasil Vasilev Date: Fri, 22 Nov 2024 20:49:53 +0100 Subject: [PATCH 3/3] [compiler] Use an iterator in a while instead of capturing variables in a lambda when recomputing zinc stamps after annotation processing in JPS #SCL-21674 --- .../incremental/scala/local/LocalServer.scala | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala b/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala index d3c435b9bc0..83eadc3e91d 100644 --- a/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala +++ b/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/LocalServer.scala @@ -58,22 +58,30 @@ final class LocalServer extends Server { override def computeStamps(outputFiles: Seq[Path], analysisFile: Path, client: Client): Right[Server.ServerError, ExitCode] = { val analysisStore = AnalysisStoreFactory.createAnalysisStore(analysisFile) - analysisStore.get().toScala.filter(_.getAnalysis.isInstanceOf[Analysis]).foreach { analysisContents => - val analysis = analysisContents.getAnalysis.asInstanceOf[Analysis] - val originalStamps = analysis.stamps - var newStamps = originalStamps - val stamper = StampReader.Instance - outputFiles.foreach { classFile => - val virtualFile = PlainVirtualFileConverter.converter.toVirtualFile(classFile) - val oldStamp = originalStamps.product(virtualFile) - val stamp = stamper.product(virtualFile) - if (oldStamp != stamp) { - newStamps = newStamps.markProduct(virtualFile, stamp) + analysisStore.get().toScala match { + case Some(analysisContents) => + analysisContents.getAnalysis match { + case analysis: Analysis => + val originalStamps = analysis.stamps + var newStamps = originalStamps + val stamper = StampReader.Instance + val converter = PlainVirtualFileConverter.converter + val iterator = outputFiles.iterator + while (iterator.hasNext) { + val classFile = iterator.next() + val virtualFile = converter.toVirtualFile(classFile) + val oldStamp = originalStamps.product(virtualFile) + val stamp = stamper.product(virtualFile) + if (oldStamp != stamp) { + newStamps = newStamps.markProduct(virtualFile, stamp) + } + } + val newAnalysis = analysis.copy(stamps = newStamps) + val newAnalysisContents = AnalysisContents.create(newAnalysis, analysisContents.getMiniSetup) + analysisStore.set(newAnalysisContents) + case _ => } - } - val newAnalysis = analysis.copy(stamps = newStamps) - val newAnalysisContents = AnalysisContents.create(newAnalysis, analysisContents.getMiniSetup) - analysisStore.set(newAnalysisContents) + case _ => } Right(ExitCode.Ok)