From bd8f49c5d603ec4f8ca85b7c6d53d1dadb8b85aa Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Thu, 24 Mar 2022 20:32:07 +0000 Subject: [PATCH] KT-45777: Count cache misses when loading classpath snapshot to get more insights into the build performance of that step. ^KT-45777 In Progress --- .../report/metrics/BuildPerformanceMetric.kt | 2 ++ .../IncrementalJvmCompilerRunner.kt | 14 +++++------- .../classpathDiff/ClasspathChangesComputer.kt | 3 ++- .../ClasspathSnapshotSerializer.kt | 10 ++++++++- .../ClasspathSnapshotShrinker.kt | 22 +++++++++---------- .../ClasspathEntrySnapshotTransform.kt | 16 +++++++------- .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 4 ++-- 7 files changed, 40 insertions(+), 31 deletions(-) diff --git a/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildPerformanceMetric.kt b/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildPerformanceMetric.kt index 9d1dd225e54..3ca71285a3c 100644 --- a/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildPerformanceMetric.kt +++ b/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildPerformanceMetric.kt @@ -20,10 +20,12 @@ enum class BuildPerformanceMetric(val parent: BuildPerformanceMetric? = null, va JAR_CLASSPATH_ENTRY_SIZE(parent = CLASSPATH_ENTRY_SNAPSHOT_TRANSFORM_EXECUTION_COUNT, "Size of jar classpath entry", type = SizeMetricType.BYTES), JAR_CLASSPATH_ENTRY_SNAPSHOT_SIZE(parent = CLASSPATH_ENTRY_SNAPSHOT_TRANSFORM_EXECUTION_COUNT, "Size of jar classpath entry's snapshot", type = SizeMetricType.BYTES), DIRECTORY_CLASSPATH_ENTRY_SNAPSHOT_SIZE(parent = CLASSPATH_ENTRY_SNAPSHOT_TRANSFORM_EXECUTION_COUNT, "Size of directory classpath entry's snapshot", type = SizeMetricType.BYTES), + COMPUTE_CLASSPATH_CHANGES_EXECUTION_COUNT(parent = null, "Number of times classpath changes are computed", type = SizeMetricType.NUMBER), SHRINK_AND_SAVE_CLASSPATH_SNAPSHOT_EXECUTION_COUNT(parent = null, "Number of times classpath snapshot is shrunk and saved after compilation", type = SizeMetricType.NUMBER), CLASSPATH_ENTRY_COUNT(parent = SHRINK_AND_SAVE_CLASSPATH_SNAPSHOT_EXECUTION_COUNT, "Number of classpath entries", type = SizeMetricType.NUMBER), CLASSPATH_SNAPSHOT_SIZE(parent = SHRINK_AND_SAVE_CLASSPATH_SNAPSHOT_EXECUTION_COUNT, "Size of classpath snapshot", type = SizeMetricType.BYTES), SHRUNK_CLASSPATH_SNAPSHOT_SIZE(parent = SHRINK_AND_SAVE_CLASSPATH_SNAPSHOT_EXECUTION_COUNT, "Size of shrunk classpath snapshot", type = SizeMetricType.BYTES), + LOAD_CLASSPATH_SNAPSHOT_CACHE_MISSES(parent = null, "Number of cache misses when loading classpath snapshot", type = SizeMetricType.NUMBER), ; companion object { diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt index 8c49b920126..01e2c251c62 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt @@ -29,10 +29,7 @@ import org.jetbrains.kotlin.build.GeneratedJvmClass import org.jetbrains.kotlin.build.report.BuildReporter import org.jetbrains.kotlin.build.report.DoNothingICReporter import org.jetbrains.kotlin.build.report.ICReporter -import org.jetbrains.kotlin.build.report.metrics.BuildAttribute -import org.jetbrains.kotlin.build.report.metrics.BuildTime -import org.jetbrains.kotlin.build.report.metrics.DoNothingBuildMetricsReporter -import org.jetbrains.kotlin.build.report.metrics.measure +import org.jetbrains.kotlin.build.report.metrics.* import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys import org.jetbrains.kotlin.cli.common.ExitCode import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments @@ -213,8 +210,10 @@ class IncrementalJvmCompilerRunner( return abiSnapshots } - // Used by `calculateSourcesToCompileImpl` and `performWorkAfterSuccessfulCompilation` methods below. - // Thread safety: There is no concurrent access to these variables. + // There are 2 steps where we need to load the current classpath snapshot and shrink it: + // - Before classpath diffing when `classpathChanges` is ToBeComputedByIncrementalCompiler (see `calculateSourcesToCompileImpl`) + // - After compilation (see `performWorkAfterSuccessfulCompilation`) + // To avoid duplicated work, we store the snapshots after the first step for reuse (if the first step is executed). private var currentClasspathSnapshot: List? = null private var shrunkCurrentClasspathAgainstPreviousLookups: List? = null @@ -233,11 +232,10 @@ class IncrementalJvmCompilerRunner( // Note: classpathChanges is deserialized, so they are no longer singleton objects and need to be compared using `is` (not `==`) is NoChanges -> ChangesEither.Known(emptySet(), emptySet()) is ToBeComputedByIncrementalCompiler -> reporter.measure(BuildTime.COMPUTE_CLASSPATH_CHANGES) { + reporter.addMetric(BuildPerformanceMetric.COMPUTE_CLASSPATH_CHANGES_EXECUTION_COUNT, 1) val storeCurrentClasspathSnapshotForReuse = { currentClasspathSnapshotArg: List, shrunkCurrentClasspathAgainstPreviousLookupsArg: List -> - check(currentClasspathSnapshot == null) - check(shrunkCurrentClasspathAgainstPreviousLookups == null) currentClasspathSnapshot = currentClasspathSnapshotArg shrunkCurrentClasspathAgainstPreviousLookups = shrunkCurrentClasspathAgainstPreviousLookupsArg } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt index bee66d7d204..b7bbfde003f 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt @@ -37,7 +37,8 @@ object ClasspathChangesComputer { reporter: ClasspathSnapshotBuildReporter ): ProgramSymbolSet { val currentClasspathSnapshot = reporter.measure(BuildTime.LOAD_CURRENT_CLASSPATH_SNAPSHOT) { - val classpathSnapshot = CachedClasspathSnapshotSerializer.load(classpathSnapshotFiles.currentClasspathEntrySnapshotFiles) + val classpathSnapshot = + CachedClasspathSnapshotSerializer.load(classpathSnapshotFiles.currentClasspathEntrySnapshotFiles, reporter) reporter.measure(BuildTime.REMOVE_DUPLICATE_CLASSES) { classpathSnapshot.removeDuplicateAndInaccessibleClasses() } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt index a138e974bba..abc721312ef 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.incremental.classpathDiff import com.intellij.util.io.DataExternalizer +import org.jetbrains.kotlin.build.report.metrics.BuildPerformanceMetric import org.jetbrains.kotlin.incremental.KotlinClassInfo import org.jetbrains.kotlin.incremental.storage.* import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader @@ -19,9 +20,16 @@ object CachedClasspathSnapshotSerializer { private val cache = ConcurrentHashMap() private const val RECOMMENDED_MAX_CACHE_SIZE = 100 - fun load(classpathEntrySnapshotFiles: List): ClasspathSnapshot { + fun load(classpathEntrySnapshotFiles: List, reporter: ClasspathSnapshotBuildReporter): ClasspathSnapshot { return ClasspathSnapshot(classpathEntrySnapshotFiles.map { snapshotFile -> + // Note: This cache is shared across builds, so we need to be careful if the snapshot file's path hasn't changed but its + // contents have changed. Luckily, each snapshot file is the output of a Gradle transform and Gradle has the hash of the file's + // contents encoded in the file's path. Therefore, if the file's path hasn't changed, then its contents must also not have + // changed, and we can reuse the cache entry. + // TODO: Make this code safer (not relying on how the snapshot files are produced and whether Gradle maintains the above + // guarantee). For example, maybe we can write the file's content hash in the file's name or to another file next to it. cache.computeIfAbsent(snapshotFile) { + reporter.addMetric(BuildPerformanceMetric.LOAD_CLASSPATH_SNAPSHOT_CACHE_MISSES, 1) ClasspathEntrySnapshotExternalizer.loadFromFile(it) } }).also { diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt index 28cee575956..2567c1063a9 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt @@ -201,8 +201,8 @@ private sealed class ShrinkMode { internal fun shrinkAndSaveClasspathSnapshot( classpathChanges: ClasspathChanges.ClasspathSnapshotEnabled, lookupStorage: LookupStorage, - precomputedCurrentClasspathSnapshot: List?, // Not null iff classpathChanges is ToBeComputedByIncrementalCompiler - precomputedShrunkCurrentClasspathAgainstPreviousLookups: List?, // Not null iff classpathChanges is ToBeComputedByIncrementalCompiler + currentClasspathSnapshot: List?, // Not null iff classpathChanges is ToBeComputedByIncrementalCompiler + shrunkCurrentClasspathAgainstPreviousLookups: List?, // Not null iff classpathChanges is ToBeComputedByIncrementalCompiler reporter: ClasspathSnapshotBuildReporter ) { // In the following, we'll try to shrink the classpath snapshot incrementally when possible. @@ -222,14 +222,14 @@ internal fun shrinkAndSaveClasspathSnapshot( val addedLookupSymbols = lookupStorage.addedLookupSymbols if (addedLookupSymbols.isEmpty()) { ShrinkMode.UnchangedLookupsChangedClasspath( - precomputedCurrentClasspathSnapshot!!, - precomputedShrunkCurrentClasspathAgainstPreviousLookups!! + currentClasspathSnapshot!!, + shrunkCurrentClasspathAgainstPreviousLookups!! ) } else { ShrinkMode.ChangedLookupsChangedClasspath( addedLookupSymbols, - precomputedCurrentClasspathSnapshot!!, - precomputedShrunkCurrentClasspathAgainstPreviousLookups!! + currentClasspathSnapshot!!, + shrunkCurrentClasspathAgainstPreviousLookups!! ) } } @@ -254,12 +254,12 @@ internal fun shrinkAndSaveClasspathSnapshot( when (shrinkMode) { is ShrinkMode.ChangedLookupsUnchangedClasspath -> CachedClasspathSnapshotSerializer - .load(classpathChanges.classpathSnapshotFiles.currentClasspathEntrySnapshotFiles) + .load(classpathChanges.classpathSnapshotFiles.currentClasspathEntrySnapshotFiles, reporter) .removeDuplicateAndInaccessibleClasses() is ShrinkMode.ChangedLookupsChangedClasspath -> shrinkMode.currentClasspathSnapshot } } - val shrunkCurrentClasspathAgainstPreviousLookups = + val shrunkCurrentClasspathAgainstPrevLookups = reporter.measure(BuildTime.INCREMENTAL_LOAD_SHRUNK_CURRENT_CLASSPATH_SNAPSHOT_AGAINST_PREVIOUS_LOOKUPS) { when (shrinkMode) { is ShrinkMode.ChangedLookupsUnchangedClasspath -> { @@ -272,11 +272,11 @@ internal fun shrinkAndSaveClasspathSnapshot( } } - val shrunkClasses = shrunkCurrentClasspathAgainstPreviousLookups.mapTo(mutableSetOf()) { it.classId } + val shrunkClasses = shrunkCurrentClasspathAgainstPrevLookups.mapTo(mutableSetOf()) { it.classId } val notYetShrunkClasses = currentClasspath.filter { it.classId !in shrunkClasses } val shrunkRemainingClassesAgainstNewLookups = shrinkClasses(notYetShrunkClasses, shrinkMode.addedLookupSymbols) - val shrunkCurrentClasspath = shrunkCurrentClasspathAgainstPreviousLookups + shrunkRemainingClassesAgainstNewLookups + val shrunkCurrentClasspath = shrunkCurrentClasspathAgainstPrevLookups + shrunkRemainingClassesAgainstNewLookups currentClasspath to shrunkCurrentClasspath } is ShrinkMode.NonIncremental -> { @@ -284,7 +284,7 @@ internal fun shrinkAndSaveClasspathSnapshot( reporter.measure(BuildTime.NON_INCREMENTAL_SHRINK_CURRENT_CLASSPATH_SNAPSHOT) { val currentClasspath = reporter.measure(BuildTime.NON_INCREMENTAL_LOAD_CURRENT_CLASSPATH_SNAPSHOT) { CachedClasspathSnapshotSerializer - .load(classpathChanges.classpathSnapshotFiles.currentClasspathEntrySnapshotFiles) + .load(classpathChanges.classpathSnapshotFiles.currentClasspathEntrySnapshotFiles, reporter) .removeDuplicateAndInaccessibleClasses() } val shrunkCurrentClasspath = shrinkClasspath(currentClasspath, lookupStorage) diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/transforms/ClasspathEntrySnapshotTransform.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/transforms/ClasspathEntrySnapshotTransform.kt index f9727e79606..2df5bea4f31 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/transforms/ClasspathEntrySnapshotTransform.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/transforms/ClasspathEntrySnapshotTransform.kt @@ -91,15 +91,15 @@ abstract class ClasspathEntrySnapshotTransform : TransformAction @Inject constr } ).disallowChanges() task.taskBuildCacheableOutputDirectory.value(getKotlinBuildDir(task).map { it.dir("cacheable") }).disallowChanges() - task.taskBuildLocalStateDirectory.value(getKotlinBuildDir(task).map { it.dir("localstate") }).disallowChanges() + task.taskBuildLocalStateDirectory.value(getKotlinBuildDir(task).map { it.dir("local-state") }).disallowChanges() task.localStateDirectories.from(task.taskBuildLocalStateDirectory).disallowChanges() @@ -292,7 +292,7 @@ abstract class AbstractKotlinCompile @Inject constr private fun getKotlinBuildDir(task: T): Provider = task.project.layout.buildDirectory.dir("$KOTLIN_BUILD_DIR_NAME/${task.name}") - protected open fun getClasspathSnapshotDir(task: T): Provider = + protected fun getClasspathSnapshotDir(task: T): Provider = getKotlinBuildDir(task).map { it.dir("classpath-snapshot") } }