From 5f822e1e9ec84caf52147774ac00016f42f7b485 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 10 Oct 2024 17:49:49 +0200 Subject: [PATCH] Redesign best effort compilation to not have to remove successful analyses Also better cleanup directories --- .../scala/bloop/BloopClassFileManager.scala | 42 ++---- backend/src/main/scala/bloop/Compiler.scala | 140 ++++++++++-------- .../inc/bloop/BloopZincCompiler.scala | 7 +- docs/contributing-guide.md | 25 ++++ .../bloop/engine/tasks/CompileTask.scala | 100 +++++-------- 5 files changed, 161 insertions(+), 153 deletions(-) diff --git a/backend/src/main/scala/bloop/BloopClassFileManager.scala b/backend/src/main/scala/bloop/BloopClassFileManager.scala index 103bbaa374..53c8dcf9bc 100644 --- a/backend/src/main/scala/bloop/BloopClassFileManager.scala +++ b/backend/src/main/scala/bloop/BloopClassFileManager.scala @@ -219,36 +219,20 @@ final class BloopClassFileManager( clientTracer.traceTaskVerbose("copy new products to external classes dir") { _ => val config = ParallelOps.CopyConfiguration(5, CopyMode.ReplaceExisting, Set.empty, Set.empty) - val clientExternalBestEffortDir = - clientExternalClassesDir.underlying.resolve("META-INF/best-effort") - - // Deletes all previous best-effort artifacts to get rid of all of the outdated ones. - // Since best effort compilation is not affected by incremental compilation, - // all relevant files are always produced by the compiler. Because of this, - // we can always delete all previous files and copy newly created ones - // without losing anything in the process. - val deleteClientExternalBestEffortDir = - Task { - if (Files.exists(clientExternalBestEffortDir)) { - BloopPaths.delete(AbsolutePath(clientExternalBestEffortDir)) - } + + ParallelOps + .copyDirectories(config)( + newClassesDir, + clientExternalClassesDir.underlying, + inputs.ioScheduler, + enableCancellation = false, + inputs.logger + ) + .map { walked => + readOnlyCopyDenylist.++=(walked.target) () - }.memoize - - deleteClientExternalBestEffortDir *> - ParallelOps - .copyDirectories(config)( - newClassesDir, - clientExternalClassesDir.underlying, - inputs.ioScheduler, - enableCancellation = false, - inputs.logger - ) - .map { walked => - readOnlyCopyDenylist.++=(walked.target) - () - } - .flatMap(_ => deleteAfterCompilation) + } + .flatMap(_ => deleteAfterCompilation) } } ) diff --git a/backend/src/main/scala/bloop/Compiler.scala b/backend/src/main/scala/bloop/Compiler.scala index af63951e59..299404d5ac 100644 --- a/backend/src/main/scala/bloop/Compiler.scala +++ b/backend/src/main/scala/bloop/Compiler.scala @@ -5,7 +5,6 @@ import java.io.PrintWriter import java.io.StringWriter import java.nio.file.Files import java.nio.file.Path -import java.nio.file.Paths import java.util.Optional import java.util.concurrent.Executor @@ -49,7 +48,6 @@ import xsbti.T2 import xsbti.VirtualFileRef import xsbti.compile._ - case class CompileInputs( scalaInstance: ScalaInstance, compilerCache: CompilerCache, @@ -295,11 +293,6 @@ object Compiler { ) } - val previousWasBestEffort = compileInputs.previousCompilerResult match { - case Failed(_, _, _, _, Some(BestEffortProducts(_, _, _))) => true - case _ => !firstCompilation - } - val isFatalWarningsEnabled: Boolean = compileInputs.scalacOptions.exists(_ == "-Xfatal-warnings") def getInputs(compilers: Compilers): Inputs = { @@ -325,7 +318,8 @@ object Compiler { val compilerCache = new FreshCompilerCache val cacheFile = compileInputs.baseDirectory.resolve("cache").toFile val incOptions = { - val disableIncremental = java.lang.Boolean.getBoolean("bloop.zinc.disabled") + val disableIncremental = + java.lang.Boolean.getBoolean("bloop.zinc.disabled") // Don't customize class file manager bc we pass our own to the zinc APIs directly IncOptions.create().withEnabled(!disableIncremental) } @@ -354,7 +348,7 @@ object Compiler { def cancel(): Unit = { // Complete all pending promises when compilation is cancelled - logger.debug(s"Cancelling compilation from ${readOnlyClassesDirPath} to ${newClassesDirPath}") + logger.info(s"Cancelling compilation from ${readOnlyClassesDirPath} to ${newClassesDirPath}") compileInputs.cancelPromise.trySuccess(()) // Always report the compilation of a project no matter if it's completed @@ -385,6 +379,9 @@ object Compiler { reporter.reportStartCompilation(previousProblems, wasPreviousSuccessful) val fileManager = newFileManager + val shouldAttemptRestartingCompilationForBestEffort = + firstCompilation && !isBestEffortDep && previousAnalysis.isDefined + // Manually skip redundant best-effort compilations. This is necessary because compiler // phases supplying the data needed to skip compilations in zinc remain unimplemented for now. val noopBestEffortResult = compileInputs.previousCompilerResult match { @@ -470,7 +467,8 @@ object Compiler { fileManager, cancelPromise, tracer, - classpathOptions + classpathOptions, + !(isBestEffortMode && isBestEffortDep) ) .materialize .doOnCancel(Task(cancel())) @@ -483,11 +481,9 @@ object Compiler { () => elapsed, reporter, backgroundTasksWhenNewSuccessfulAnalysis, - allInvalidatedClassFilesForProject, - allInvalidatedExtraCompileProducts, previousSuccessfulProblems, errorCause = None, - previousWasBestEffort + shouldAttemptRestartingCompilationForBestEffort ) case Success(result) => // Report end of compilation only after we have reported all warnings from previous runs @@ -553,16 +549,25 @@ object Compiler { val clientClassesDir = clientClassesObserver.classesDir clientLogger.debug(s"Triggering background tasks for $clientClassesDir") val updateClientState = - updateExternalClassesDirWithReadOnly( - clientClassesDir, - clientTracer, - clientLogger, - compileInputs, - readOnlyClassesDir, - readOnlyCopyDenylist, - allInvalidatedClassFilesForProject, - allInvalidatedExtraCompileProducts - ) + Task + .gatherUnordered( + List( + deleteClientExternalBestEffortDirTask(clientClassesDir), + deleteBestEffortDir() + ) + ) + .flatMap { _ => + updateExternalClassesDirWithReadOnly( + clientClassesDir, + clientTracer, + clientLogger, + compileInputs, + readOnlyClassesDir, + readOnlyCopyDenylist, + allInvalidatedClassFilesForProject, + allInvalidatedExtraCompileProducts + ) + } val writeAnalysisIfMissing = { if (compileOut.analysisOut.exists) Task.unit @@ -586,10 +591,9 @@ object Compiler { } .flatMap(clientClassesObserver.nextAnalysis) - deleteBestEffortDir() *> Task + Task .gatherUnordered( List( - deleteBestEffortDir, deleteNewClassesDir, updateClientState, writeAnalysisIfMissing, @@ -598,7 +602,8 @@ object Compiler { ) .flatMap(_ => publishClientAnalysis) .onErrorHandleWith(err => { - clientLogger.debug("Caught error in background tasks"); clientLogger.trace(err); + clientLogger.debug("Caught error in background tasks"); + clientLogger.trace(err); Task.raiseError(err) }) .doOnFinish(_ => Task(clientReporter.reportEndCompilation())) @@ -642,10 +647,19 @@ object Compiler { ): Task[Unit] = { val clientClassesDir = clientClassesObserver.classesDir val successBackgroundTasks = - deleteBestEffortDir() *> Task.gatherUnordered( - backgroundTasksWhenNewSuccessfulAnalysis - .map(f => f(clientClassesDir, clientReporter, clientTracer)) - ) + Task + .gatherUnordered( + List( + deleteBestEffortDir(), + deleteClientExternalBestEffortDirTask(clientClassesDir) + ) + ) + .flatMap { _ => + Task.gatherUnordered( + backgroundTasksWhenNewSuccessfulAnalysis + .map(f => f(clientClassesDir, clientReporter, clientTracer)) + ) + } val persistTask = persistAnalysis(analysisForFutureCompilationRuns, compileOut.analysisOut) val initialTasks = List(persistTask, successBackgroundTasks) @@ -724,11 +738,9 @@ object Compiler { () => elapsed, reporter, backgroundTasksWhenNewSuccessfulAnalysis, - allInvalidatedClassFilesForProject, - allInvalidatedExtraCompileProducts, previousSuccessfulProblems, errorCause = Some(cause), - previousWasBestEffort + shouldAttemptRestartingCompilationForBestEffort ) case Failure(_: xsbti.CompileCancelled) => handleCancellation @@ -747,7 +759,7 @@ object Compiler { t.printStackTrace() val sw = new StringWriter() t.printStackTrace(new PrintWriter(sw)) - logger.info(sw.toString()) + logger.error(sw.toString()) val backgroundTasks = toBackgroundTasks(backgroundTasksForFailedCompilation.toList) val failedProblems = findFailedProblems(reporter, None) @@ -954,14 +966,11 @@ object Compiler { elapsed: () => Long, reporter: ZincReporter, backgroundTasksWhenNewSuccessfulAnalysis: mutable.ListBuffer[CompileBackgroundTasks.Sig], - allInvalidatedClassFilesForProject: mutable.HashSet[File], - allInvalidatedExtraCompileProducts: mutable.HashSet[File], previousSuccessfulProblems: List[ProblemPerPhase], errorCause: Option[xsbti.CompileFailed], - previousWasBestEffort: Boolean + shouldAttemptRestartingCompilation: Boolean ): Result = { val uniqueInputs = compileInputs.uniqueInputs - val readOnlyClassesDir = compileOut.internalReadOnlyClassesDir.underlying val newClassesDir = compileOut.internalNewClassesDir.underlying reporter.processEndCompilation( @@ -978,7 +987,7 @@ object Compiler { ) val products = CompileProducts( - readOnlyClassesDir, + newClassesDir, // let's not use readonly dir newClassesDir, noOpPreviousResult, noOpPreviousResult, @@ -995,9 +1004,13 @@ object Compiler { ): Task[Unit] = { val clientClassesDir = clientClassesObserver.classesDir val successBackgroundTasks = - backgroundTasksWhenNewSuccessfulAnalysis - .map(f => f(clientClassesDir, clientReporter, clientTracer)) - val allClientSyncTasks = Task.gatherUnordered(successBackgroundTasks.toList).flatMap { _ => + deleteClientExternalBestEffortDirTask(clientClassesDir).flatMap { _ => + Task.gatherUnordered( + backgroundTasksWhenNewSuccessfulAnalysis + .map(f => f(clientClassesDir, clientReporter, clientTracer)) + ) + } + val allClientSyncTasks = successBackgroundTasks.flatMap { _ => // Only start this task after the previous IO tasks in the external dir are done Task { // Delete everything outside of betasty and semanticdb @@ -1005,8 +1018,11 @@ object Compiler { BloopClassFileManager.supportedCompileProducts.filter(_ != ".betasty") :+ ".class" Files .walk(clientClassesDir.underlying) - .filter(path => if (Files.exists(path)) Files.isRegularFile(path) else false) - .filter(path => deletedCompileProducts.exists(path.toString.endsWith(_))) + .filter(path => + Files.isRegularFile(path) && deletedCompileProducts.exists( + path.toString.endsWith(_) + ) + ) .forEach(path => if (Files.exists(path)) Files.delete(path)) }.map(_ => ()) } @@ -1015,22 +1031,12 @@ object Compiler { } } - val recompile = - if ( - !previousWasBestEffort && !(compileOut.internalReadOnlyClassesDir.exists && BloopPaths - .list(compileOut.internalReadOnlyClassesDir) - .length == 0) - ) { - if (compileOut.analysisOut.exists) BloopPaths.delete(compileOut.analysisOut) - BloopPaths.delete(compileOut.internalReadOnlyClassesDir) - Files.createDirectories(Paths.get(compileOut.internalReadOnlyClassesDir.toString)) - BloopPaths.delete(compileOut.internalNewClassesDir) - Files.createDirectories(Paths.get(compileOut.internalNewClassesDir.toString)) - true - } else false + if (shouldAttemptRestartingCompilation) { + BloopPaths.delete(compileOut.internalNewClassesDir) + } val newHash = - if (previousWasBestEffort) + if (!shouldAttemptRestartingCompilation) BestEffortUtils.hashResult( products.newClassesDir, compileInputs.sources, @@ -1043,7 +1049,7 @@ object Compiler { None, elapsed(), backgroundTasksExecution, - Some(BestEffortProducts(products, newHash, recompile)) + Some(BestEffortProducts(products, newHash, shouldAttemptRestartingCompilation)) ) } @@ -1211,4 +1217,20 @@ object Compiler { } } } + + // Deletes all previous best-effort artifacts to get rid of all of the outdated ones. + // Since best effort compilation is not affected by incremental compilation, + // all relevant files are always produced by the compiler. Because of this, + // we can always delete all previous files and copy newly created ones + // without losing anything in the process. + def deleteClientExternalBestEffortDirTask(clientClassesDir: AbsolutePath) = { + val clientExternalBestEffortDir = + clientClassesDir.underlying.resolve("META-INF/best-effort") + Task { + if (Files.exists(clientExternalBestEffortDir)) { + BloopPaths.delete(AbsolutePath(clientExternalBestEffortDir)) + } + () + }.memoize + } } diff --git a/backend/src/main/scala/sbt/internal/inc/bloop/BloopZincCompiler.scala b/backend/src/main/scala/sbt/internal/inc/bloop/BloopZincCompiler.scala index 775163ebc4..16ed24903d 100644 --- a/backend/src/main/scala/sbt/internal/inc/bloop/BloopZincCompiler.scala +++ b/backend/src/main/scala/sbt/internal/inc/bloop/BloopZincCompiler.scala @@ -58,7 +58,8 @@ object BloopZincCompiler { manager: ClassFileManager, cancelPromise: Promise[Unit], tracer: BraveTracer, - classpathOptions: ClasspathOptions + classpathOptions: ClasspathOptions, + withPreviousResult: Boolean ): Task[CompileResult] = { val config = in.options() val setup = in.setup() @@ -81,8 +82,8 @@ object BloopZincCompiler { scalacOptions, javacOptions, classpathOptions, - in.previousResult.analysis.toOption, - in.previousResult.setup.toOption, + if (withPreviousResult) in.previousResult.analysis.toOption else None, + if (withPreviousResult) in.previousResult.setup.toOption else None, perClasspathEntryLookup, reporter, order, diff --git a/docs/contributing-guide.md b/docs/contributing-guide.md index 807ce423c3..64d3349b30 100644 --- a/docs/contributing-guide.md +++ b/docs/contributing-guide.md @@ -242,3 +242,28 @@ indices. - Push the new tag and wait for the release - Announce the release after the release notes are published in the most recent release. + + +# Best effort compilation pipeline + +As of writing this part of the doc this is an experimental set of settings implemented +in the Scala 3 compiler (starting with 3.5.x). They allow the compiler to return artifacts +even when the compilation fails (returning `.betasty` files instead of `.class` and `.tasty`). +It also at this point does not support incremental compilation. This all requires special +handling from the build tool, mostly located in `Compiler.scala`, `CompileTask.scala` +and `CompileGraph.scala`: +- We save best effort artifacts separately, and allow dependent projects to compile using +that, even when the compilation has failed. If the project compiles we discard the best effort +artifacts. +- First, we try compiling partially (only the modified files), expecting regular successful compilation +- If that at some point fails, we discard the immediate results and recompile the whole module +expecting .betasty files. We do not ever move them to a readOnly directory. That readOnly directory +is also not used in dependent compilations. +- We do not try to recompile if we know we are compiling the whole module to begin with (e.g. because we +are depending on .betasty from different project, or because this is the first compilation and we +do not have any previous incremental compilation analysis). +- If we try to recompile a module that we previously compiled for .betasty, we once again, try to +recompile it 2 times - once incrementally expecting success (recompiling all files changed since +the last successful compilation, as dictated by the incremental compilation analysis) and then +recompile all - this works out to be faster than discarding the last successful result and jumping +between full successful recompilation and full best effort recompilation. diff --git a/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala b/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala index e773751230..a5b72d5eeb 100644 --- a/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala +++ b/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala @@ -176,47 +176,6 @@ object CompileTask { // Block on the task associated with this result that sets up the read-only classes dir waitOnReadClassesDir.flatMap { _ => - def getAllSourceInputs(project: Project): List[AbsolutePath] = { - import java.nio.file.Files - import scala.collection.JavaConverters._ - - val uniqueSourceDirs = project.sources - - val sourceExts = Seq(".scala", ".java") - val unmanagedSources: mutable.Set[AbsolutePath] = mutable.Set() - - uniqueSourceDirs.map(_.underlying).foreach { file => - if (!Files.exists(file)) () - else if ( - Files.isRegularFile(file) && sourceExts.exists(ext => file.toString.endsWith(ext)) - ) { - unmanagedSources.add(AbsolutePath(file)) - } else if (Files.isDirectory(file)) { - Files.walk(file).iterator().asScala.foreach { file => - if ( - Files.isRegularFile(file) && sourceExts - .exists(ext => file.toString.endsWith(ext)) - ) { - unmanagedSources.add(AbsolutePath(file)) - } - } - } - } - - project.sourcesGlobs.foreach { glob => - Files.walk(glob.directory.underlying).iterator().asScala.foreach { file => - if ( - Files.isRegularFile(file) && sourceExts - .exists(ext => file.toString.endsWith(ext)) && glob.matches(file) - ) { - unmanagedSources.add(AbsolutePath(file)) - } - } - } - - unmanagedSources.toList - } - // Only when the task is finished, we kickstart the compilation def compile(inputs: CompileInputs) = { val firstResult = Compiler.compile(inputs, isBestEffort, isBestEffortDep, true) @@ -230,11 +189,10 @@ object CompileTask { ) if recompile => // we restart the compilation, starting from scratch (without any previous artifacts) inputs.reporter.reset() - val foundSrcs = getAllSourceInputs(project) val emptyResult = PreviousResult.of(Optional.empty[CompileAnalysis], Optional.empty[MiniSetup]) val newInputs = inputs.copy( - sources = foundSrcs.toArray, + sources = inputs.sources, previousCompilerResult = result, previousResult = emptyResult ) @@ -535,29 +493,47 @@ object CompileTask { logger.debug(s"Scheduling to delete ${previousClassesDir} superseded by $newClassesDir") Some(previousClassesDir) } - case Failed(_, _, _, _, Some(BestEffortProducts(products, _, _))) => - val newClassesDir = products.newClassesDir - previousResult match { - case Some(Failed(_, _, _, _, Some(BestEffortProducts(previousProducts, _, _)))) => - val previousClassesDir = previousProducts.newClassesDir - if (previousClassesDir != newClassesDir) { - logger.debug( - s"Scheduling to delete ${previousClassesDir} superseded by $newClassesDir" - ) - Some(AbsolutePath(previousClassesDir)) - } else None + case _ => None + } + + val previousBestEffortToDelete = previousResult match { + case Some(Failed(_, _, _, _, Some(BestEffortProducts(previousProducts, _, _)))) => + val newClassesDirOpt = compilerResult match { + case Success(_, _, products, _, _, _, _) => Some(products.newClassesDir) + case Failed(_, _, _, _, Some(BestEffortProducts(products, _, _))) => + Some(products.newClassesDir) case _ => None } + val previousClassesDir = previousProducts.newClassesDir + + newClassesDirOpt.flatMap { newClassesDir => + if (previousClassesDir != newClassesDir) { + logger.debug( + s"Scheduling to delete ${previousClassesDir} superseded by $newClassesDir" + ) + Some(AbsolutePath(previousClassesDir)) + } else None + } case _ => None } - previousReadOnlyToDelete match { - case None => Task.unit - case Some(classesDir) => - Task.eval { - logger.debug(s"Deleting contents of orphan dir $classesDir") - BloopPaths.delete(classesDir) - }.asyncBoundary - } + def deleteOrphanDir(orphanDir: Option[AbsolutePath]): Task[Unit] = + orphanDir match { + case None => Task.unit + case Some(classesDir) => + Task.eval { + logger.debug(s"Deleting contents of orphan dir $classesDir") + BloopPaths.delete(classesDir) + }.asyncBoundary + } + + Task + .gatherUnordered( + List( + deleteOrphanDir(previousReadOnlyToDelete), + deleteOrphanDir(previousBestEffortToDelete) + ) + ) + .map(_ => ()) } }