From b7cfbf031c51cba1f883fe390463426ff03f0526 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Mon, 4 Oct 2021 18:32:22 +0200 Subject: [PATCH] Snapshot compile tasks outputs and restoring them on compilation error. It happens Gradle has low performance on passing 'Array' or 'List' via properties api. In this change, task outputs approach was changed to snapshot pre-execution state by coping outputs to the new directory and coping them back on incremental compilation error. ^KT-46406 Fixed --- .../GradleCompilerRunnerWithWorkers.kt | 55 +++++++----- .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 12 ++- .../kotlin/gradle/tasks/TasksOutputsBackup.kt | 86 ++++++++++++------- 3 files changed, 100 insertions(+), 53 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt index 618417e9b92..801715de5f8 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt @@ -6,8 +6,8 @@ package org.jetbrains.kotlin.compilerRunner import org.gradle.api.GradleException -import org.gradle.api.file.ConfigurableFileCollection -import org.gradle.api.provider.MapProperty +import org.gradle.api.file.* +import org.gradle.api.logging.Logging import org.gradle.api.provider.Property import org.gradle.workers.WorkAction import org.gradle.workers.WorkParameters @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.build.report.metrics.measure import org.jetbrains.kotlin.gradle.tasks.GradleCompileTaskProvider import org.jetbrains.kotlin.gradle.tasks.TaskOutputsBackup import java.io.File +import javax.inject.Inject /** * Uses Gradle worker api to run kotlin compilation. @@ -36,43 +37,54 @@ internal class GradleCompilerRunnerWithWorkers( ): WorkQueue { val workQueue = workerExecutor.noIsolation() - workQueue.submit(GradleKotlinCompilerWorkAction::class.java) { - it.compilerWorkArguments.set(workArgs) + workQueue.submit(GradleKotlinCompilerWorkAction::class.java) { params -> + params.compilerWorkArguments.set(workArgs) if (taskOutputsBackup != null) { - it.taskOutputs.from(taskOutputsBackup.outputs) - it.taskOutputsSnapshot.set(taskOutputsBackup.previousOutputs) - it.metricsReporter.set(buildMetrics) - } else { - // MapProperty has empty value by default: https://github.com/gradle/gradle/issues/7485 - it.taskOutputsSnapshot.set(null as Map>?) + params.taskOutputs.from(taskOutputsBackup.outputs) + params.buildDir.set(taskOutputsBackup.buildDirectory) + params.snapshotsDir.set(taskOutputsBackup.snapshotsDir) + params.metricsReporter.set(buildMetrics) } } return workQueue } - internal abstract class GradleKotlinCompilerWorkAction - : WorkAction { + internal abstract class GradleKotlinCompilerWorkAction @Inject constructor( + private val fileSystemOperations: FileSystemOperations + ) : WorkAction { + + private val logger = Logging.getLogger("kotlin-compile-worker") override fun execute() { + val taskOutputsBackup = if (parameters.snapshotsDir.isPresent) { + TaskOutputsBackup( + fileSystemOperations, + parameters.buildDir, + parameters.snapshotsDir, + parameters.taskOutputs + ) + } else { + null + } + try { GradleKotlinCompilerWork( parameters.compilerWorkArguments.get() ).run() } catch (e: GradleException) { - if (parameters.taskOutputsSnapshot.isPresent) { - val taskOutputsBackup = TaskOutputsBackup( - parameters.taskOutputs, - parameters.taskOutputsSnapshot.get() - ) - // Currently, metrics are not reported as in the worker we are getting new instance of [BuildMetricsReporter] - // [BuildDataRecorder] knows nothing about this new instance. Possibly could be fixed in the future by migrating - // [BuildMetricsReporter] to be shared Gradle service. + // Currently, metrics are not reported as in the worker we are getting new instance of [BuildMetricsReporter] + // [BuildDataRecorder] knows nothing about this new instance. Possibly could be fixed in the future by migrating + // [BuildMetricsReporter] to be shared Gradle service. + if (taskOutputsBackup != null) { parameters.metricsReporter.get().measure(BuildTime.RESTORE_OUTPUT_FROM_BACKUP) { + logger.info("Restoring task outputs to pre-compilation state") taskOutputsBackup.restoreOutputs() } } throw e + } finally { + taskOutputsBackup?.deleteSnapshot() } } } @@ -80,7 +92,8 @@ internal class GradleCompilerRunnerWithWorkers( internal interface GradleKotlinCompilerWorkParameters : WorkParameters { val compilerWorkArguments: Property val taskOutputs: ConfigurableFileCollection - val taskOutputsSnapshot: MapProperty> + val snapshotsDir: DirectoryProperty + val buildDir: DirectoryProperty val metricsReporter: Property } } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt index 5bd633122c7..b50e3268191 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt @@ -213,6 +213,9 @@ abstract class AbstractKotlinCompile : AbstractKotl private val layout = project.layout + @get:Inject + internal abstract val fileSystemOperations: FileSystemOperations + @get:Internal protected val objects: ObjectFactory = project.objects @@ -339,7 +342,14 @@ abstract class AbstractKotlinCompile : AbstractKotl val outputsBackup: TaskOutputsBackup? = if (isIncrementalCompilationEnabled() && inputChanges.isIncremental) buildMetrics.measure(BuildTime.BACKUP_OUTPUT) { - TaskOutputsBackup(allOutputFiles()) + TaskOutputsBackup( + fileSystemOperations, + layout.buildDirectory, + layout.buildDirectory.dir("snapshot/kotlin/$name"), + allOutputFiles() + ).also { + it.createSnapshot() + } } else null diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksOutputsBackup.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksOutputsBackup.kt index d213eb4a377..ceb38f30c03 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksOutputsBackup.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/TasksOutputsBackup.kt @@ -5,43 +5,67 @@ package org.jetbrains.kotlin.gradle.tasks +import org.gradle.api.file.Directory +import org.gradle.api.file.DirectoryProperty import org.gradle.api.file.FileCollection -import org.jetbrains.kotlin.utils.keysToMap +import org.gradle.api.file.FileSystemOperations +import org.gradle.api.provider.Provider import java.io.File -import java.util.HashSet internal class TaskOutputsBackup( - val outputs: FileCollection, - val previousOutputs: Map> = outputs - .flatMap { - if (it.isDirectory) { - it.walk().filter(File::isFile) - } else { - sequenceOf(it) - } - } - .filter { it.exists() } - .toSet() - .keysToMap { it.readBytes().toTypedArray() } + val fileSystemOperations: FileSystemOperations, + val buildDirectory: DirectoryProperty, + val snapshotsDir: Provider, + val outputs: FileCollection ) { - - fun restoreOutputs() { - outputs.forEach { - if (it.isDirectory) { - it.deleteRecursively() - } else if (it.isFile) { - it.delete() + fun createSnapshot() { + // Kotlin JS compilation task declares one file from 'destinationDirectory' output as task `@OutputFile' + // property. To avoid snapshot sync collisions, each snapshot output directory has also 'index' as prefix. + outputs.files.toSortedSet().forEachIndexed { index, outputPath -> + val pathInSnapshot = "$index${File.separator}${outputPath.pathRelativeToBuildDirectory}" + if (outputPath.isDirectory) { + fileSystemOperations.sync { spec -> + spec.from(outputPath) + spec.into(snapshotsDir.map { it.dir(pathInSnapshot) }) + } + } else { + fileSystemOperations.copy { spec -> + spec.from(outputPath) + spec.into(snapshotsDir.map { it.file(pathInSnapshot).asFile.parentFile }) + } } } - - val dirs = HashSet() - - for ((file, bytes) in previousOutputs) { - val dir = file.parentFile - if (dirs.add(dir)) { - dir.mkdirs() - } - file.writeBytes(bytes.toByteArray()) - } } + + fun restoreOutputs() { + fileSystemOperations.delete { + it.delete(outputs) + } + + outputs.files.toSortedSet().forEachIndexed { index, outputPath -> + val pathInSnapshot = "$index${File.separator}${outputPath.pathRelativeToBuildDirectory}" + val fileInSnapshot = snapshotsDir.get().file(pathInSnapshot).asFile + if (fileInSnapshot.isDirectory) { + fileSystemOperations.sync { spec -> + spec.from(snapshotsDir.map { it.dir(pathInSnapshot) }) + spec.into(outputPath) + } + } else { + fileSystemOperations.copy { spec -> + spec.from(snapshotsDir.map { it.file(pathInSnapshot).asFile.parentFile }) + spec.into(outputPath.parentFile) + } + } + } + } + + fun deleteSnapshot() { + fileSystemOperations.delete { it.delete(snapshotsDir) } + } + + private val File.pathRelativeToBuildDirectory: String + get() { + val buildDir = buildDirectory.get().asFile + return relativeTo(buildDir).path + } }