From a48bf63630f91691639bfadbab75664b664d7c1e Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Wed, 25 Aug 2021 16:23:20 +0100 Subject: [PATCH] KT-45777: Compute classpath changes for incremental Kotlin compile Test: Updated unit tests + incremental compilation integration tests --- .../kotlin/incremental/ClasspathChanges.kt | 91 ++++--- .../kotlin/incremental/JavaClassName.kt | 134 ++++++----- .../incremental/storage/externalizers.kt | 82 ++++++- .../IncrementalJvmCompilerRunner.kt | 19 +- .../IncrementalCompilationMultiProjectIT.kt | 17 ++ .../kotlin/gradle/IncrementalJavaChangeIT.kt | 25 +- .../incremental/ClasspathChangesComputer.kt | 226 ++++++------------ .../gradle/incremental/ClasspathSnapshot.kt | 4 +- .../ClasspathSnapshotSerializer.kt | 56 +---- .../incremental/ClasspathSnapshotter.kt | 116 +++++---- .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 54 +++-- .../ClasspathChangesComputerTest.kt | 69 ++++-- 12 files changed, 473 insertions(+), 420 deletions(-) diff --git a/build-common/src/org/jetbrains/kotlin/incremental/ClasspathChanges.kt b/build-common/src/org/jetbrains/kotlin/incremental/ClasspathChanges.kt index f9db44aa940..f96585f477f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/ClasspathChanges.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/ClasspathChanges.kt @@ -5,10 +5,12 @@ package org.jetbrains.kotlin.incremental +import com.intellij.util.io.DataExternalizer +import org.jetbrains.kotlin.incremental.storage.FqNameExternalizer +import org.jetbrains.kotlin.incremental.storage.LinkedHashSetExternalizer +import org.jetbrains.kotlin.incremental.storage.LookupSymbolExternalizer import org.jetbrains.kotlin.name.FqName -import java.io.ObjectInputStream -import java.io.ObjectOutputStream -import java.io.Serializable +import java.io.* /** * Changes to the classpath of the `KotlinCompile` task, used to compute the source files that need to be recompiled during an incremental @@ -18,59 +20,54 @@ sealed class ClasspathChanges : Serializable { class Available() : ClasspathChanges() { - lateinit var lookupSymbols: List - private set - - lateinit var fqNames: List - private set - - constructor(lookupSymbols: List, fqNames: List) : this() { - this.lookupSymbols = lookupSymbols - this.fqNames = fqNames - } - - private fun writeObject(out: ObjectOutputStream) { - out.writeInt(lookupSymbols.size) - lookupSymbols.forEach { - out.writeUTF(it.name) - out.writeUTF(it.scope) - } - - out.writeInt(fqNames.size) - fqNames.forEach { - out.writeUTF(it.asString()) - } - } - - private fun readObject(ois: ObjectInputStream) { - val lookupSymbolsSize = ois.readInt() - val lookupSymbols = ArrayList(lookupSymbolsSize) - repeat(lookupSymbolsSize) { - val name = ois.readUTF() - val scope = ois.readUTF() - lookupSymbols.add(LookupSymbol(name, scope)) - } - this.lookupSymbols = lookupSymbols - - val fqNamesSize = ois.readInt() - val fqNames = ArrayList(fqNamesSize) - repeat(fqNamesSize) { - val fqNameString = ois.readUTF() - fqNames.add(FqName(fqNameString)) - } - this.fqNames = fqNames - } - companion object { private const val serialVersionUID = 0L } + + lateinit var lookupSymbols: LinkedHashSet + private set + + lateinit var fqNames: LinkedHashSet + private set + + constructor(lookupSymbols: LinkedHashSet, fqNames: LinkedHashSet) : this() { + this.lookupSymbols = lookupSymbols + this.fqNames = fqNames + } + + private fun writeObject(output: ObjectOutputStream) { + // Can't close DataOutputStream below as it will also close the underlying ObjectOutputStream, which is still in use. + ClasspathChangesAvailableExternalizer.save(DataOutputStream(output), this) + } + + private fun readObject(input: ObjectInputStream) { + // Can't close DataInputStream below as it will also close the underlying ObjectInputStream, which is still in use. + ClasspathChangesAvailableExternalizer.read(DataInputStream(input)).also { + lookupSymbols = it.lookupSymbols + fqNames = it.fqNames + } + } } sealed class NotAvailable : ClasspathChanges() { - object UnableToCompute : NotAvailable() object ForNonIncrementalRun : NotAvailable() object ClasspathSnapshotIsDisabled : NotAvailable() object ReservedForTestsOnly : NotAvailable() object ForJSCompiler : NotAvailable() } } + +private object ClasspathChangesAvailableExternalizer : DataExternalizer { + + override fun save(output: DataOutput, classpathChanges: ClasspathChanges.Available) { + LinkedHashSetExternalizer(LookupSymbolExternalizer).save(output, classpathChanges.lookupSymbols) + LinkedHashSetExternalizer(FqNameExternalizer).save(output, classpathChanges.fqNames) + } + + override fun read(input: DataInput): ClasspathChanges.Available { + return ClasspathChanges.Available( + lookupSymbols = LinkedHashSetExternalizer(LookupSymbolExternalizer).read(input), + fqNames = LinkedHashSetExternalizer(FqNameExternalizer).read(input) + ) + } +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/JavaClassName.kt b/build-common/src/org/jetbrains/kotlin/incremental/JavaClassName.kt index 752b6e69aac..12c550005a6 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/JavaClassName.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/JavaClassName.kt @@ -23,24 +23,39 @@ import org.jetbrains.org.objectweb.asm.Opcodes * - [NestedNonLocalClass] * - [LocalClass] (https://docs.oracle.com/javase/tutorial/java/javaOO/localclasses.html) */ -sealed class JavaClassName( +sealed class JavaClassName { /** The full name of this class (e.g., "com/example/Foo$Bar"). */ - val name: String -) { + abstract val name: String + + /** + * Whether this class is an anonymous class. + * + * Note: Even though an anonymous class has no name in the source code, it always has a (not-null, not-empty) name in the compiled + * class (e.g., "com/example/Foo$1"). + */ + abstract val isAnonymous: Boolean + + /** Whether this class is a synthetic class. */ + abstract val isSynthetic: Boolean /** The package name of this class (e.g., "com/example"). */ val packageName: String - get() = name.substringBeforeLast('/', "") + get() = name.substringBeforeLast('/', missingDelimiterValue = "") + + /** The part of the full name of this class after [packageName] (e.g., "Foo$Bar"). */ + val relativeClassName: String + get() = name.substringAfterLast('/', missingDelimiterValue = name) companion object { + /** Computes the [JavaClassName] of a compiled Java class given its contents. */ fun compute(classContents: ByteArray): JavaClassName { val nameRef = Ref.create() + val isSyntheticRef = Ref.create() val isTopLevelRef = Ref.create() val outerNameRef = Ref.create() - val isAnonymousInnerClassRef = Ref.create() - val isSyntheticInnerClassRef = Ref.create() + val isAnonymousRef = Ref.create() ClassReader(classContents).accept(object : ClassVisitor(Opcodes.API_VERSION) { override fun visit( @@ -48,120 +63,110 @@ sealed class JavaClassName( signature: String?, superName: String?, interfaces: Array? ) { nameRef.set(name) + isSyntheticRef.set((access and Opcodes.ACC_SYNTHETIC) != 0) } override fun visitInnerClass(name: String, outerName: String?, innerName: String?, access: Int) { if (name == nameRef.get()!!) { isTopLevelRef.set(false) outerNameRef.set(outerName) - isAnonymousInnerClassRef.set(innerName == null) - isSyntheticInnerClassRef.set((access and Opcodes.ACC_SYNTHETIC) != 0) + isAnonymousRef.set(innerName == null) } } }, ClassReader.SKIP_CODE or ClassReader.SKIP_DEBUG or ClassReader.SKIP_FRAMES) val name = nameRef.get()!! + val isSynthetic = isSyntheticRef.get()!! val isTopLevel = isTopLevelRef.get() ?: true val outerName = outerNameRef.get() + val isAnonymous = isAnonymousRef.get() return when { - isTopLevel -> TopLevelClass(name) - outerName != null -> NestedNonLocalClass( - name, - outerName, - isAnonymousInnerClassRef.get()!!, - isSyntheticInnerClassRef.get()!! - ) - else -> LocalClass(name) + isTopLevel -> TopLevelClass(name, isSynthetic) + outerName != null -> NestedNonLocalClass(name, outerName, isAnonymous!!, isSynthetic) + else -> LocalClass(name, isAnonymous!!, isSynthetic) } } } } /** See [JavaClassName]. */ -class TopLevelClass(name: String) : JavaClassName(name) { - - /** - * The simple name of this class (e.g., the simple name of class "com/example/Foo" is "Foo", the simple name of class - * "com/example/ClassWith$Sign" is "ClassWith$Sign"). - */ - val simpleName: String - get() = name.substringAfterLast('/') +class TopLevelClass( + override val name: String, + override val isSynthetic: Boolean +) : JavaClassName() { + override val isAnonymous: Boolean = false // A top-level class is never anonymous } /** See [JavaClassName]. */ -sealed class NestedClass(name: String) : JavaClassName(name) +sealed class NestedClass : JavaClassName() /** See [JavaClassName]. */ class NestedNonLocalClass( - name: String, + override val name: String, /** * The full name of the outer class of this class (e.g., the outer name of "com/example/OuterClass$NestedClass" is - * "com/example/OuterClass", the outer name of class "com/example/OuterClassWith$Sign$NestedClassWith$Sign" is + * "com/example/OuterClass", the outer name of "com/example/OuterClassWith$Sign$NestedClassWith$Sign" is * "com/example/OuterClassWith$Sign"). * * The outer class can be of any type ([TopLevelClass], [NestedNonLocalClass], or [LocalClass]). */ val outerName: String, - /** - * Whether this class is an anonymous class. - * - * Note: Even though an anonymous class has no name in the source code, it always has a (not-null, not-empty) name in the compiled - * class. Therefore, [simpleName] is not `null` and not empty even for anonymous classes. - * */ - val isAnonymous: Boolean, - - /** Whether this class is a synthetic class. */ - val isSynthetic: Boolean -) : NestedClass(name) { - - init { - check(name.startsWith("$outerName\$")) - } + override val isAnonymous: Boolean, + override val isSynthetic: Boolean +) : NestedClass() { /** * The simple name of this class (e.g., the simple name of "com/example/OuterClass$NestedClass" is "NestedClass", the simple name of * class "com/example/OuterClassWith$Sign$NestedClassWith$Sign" is "NestedClassWith$Sign"). * - * Note: [simpleName] is not `null` and not empty even for anonymous classes (see [isAnonymous]). + * Note: [simpleName] is not `null` and not empty even for anonymous classes (see [JavaClassName.isAnonymous]). */ val simpleName: String - get() = name.substring("$outerName\$".length) + get() = run { + check(name.startsWith("$outerName\$")) + name.substring("$outerName\$".length).also { check(it.isNotEmpty()) } + } } /** See [JavaClassName]. */ -class LocalClass(name: String) : NestedClass(name) +class LocalClass( + override val name: String, + override val isAnonymous: Boolean, + override val isSynthetic: Boolean +) : NestedClass() /** * Computes [ClassId]s of the given Java classes. * * Note that creating a [ClassId] for a nested class will require accessing the outer class for 2 reasons: - * - To disambiguate any '$' characters in the class name (e.g., "com/example/OuterClassWith$Sign$NestedClassWith$Sign"). + * - To disambiguate any '$' characters in the (outer) class name (e.g., "com/example/OuterClassWith$Sign$NestedClassWith$Sign"). * - To determine whether a class is a local class (a nested class of a local class is also considered local, see [ClassId]'s kdoc). * * Therefore, outer classes and nested classes must be passed together in one invocation of this method. */ -fun computeJavaClassIds(javaClassNames: List): List { - val nameToJavaClassName: Map = javaClassNames.associateBy { it.name } - val nameToClassId: MutableMap = nameToJavaClassName.mapValues { null }.toMutableMap() +fun computeJavaClassIds(classNames: List): List { + val classNameToClassId: MutableMap = HashMap(classNames.size) + val nameToClassName: Map = classNames.associateBy { it.name } - fun getOrCreateClassId(className: String): ClassId { - val classInfo = nameToJavaClassName[className] ?: error("Class name not found: $className") - val computedClassId = nameToClassId[className] - if (computedClassId != null) { - return computedClassId - } + fun JavaClassName.getClassId(): ClassId { + classNameToClassId[this]?.let { return it } - val packageName = FqName(classInfo.packageName.replace('/', '.')) - val classId = when (classInfo) { + val packageName = FqName(packageName.replace('/', '.')) + val classId = when (this) { is TopLevelClass -> { - ClassId(packageName, FqName(classInfo.simpleName), /* local */ false) + ClassId(packageName, FqName(relativeClassName), /* local */ false) } is NestedNonLocalClass -> { - val outerClassId = getOrCreateClassId(classInfo.outerName) - val relativeClassName = FqName(outerClassId.relativeClassName.asString() + "." + classInfo.simpleName) + // JavaClassName.relativeClassName can contain '$' but not '.', whereas ClassId.relativeClassName can contain both '$' and + // '.' (e.g., "com/example/OuterClassWith$Sign$NestedClassWith$Sign" has JavaClassName.relativeClassName + // "OuterClassWith$Sign$NestedClassWith$Sign", but its ClassId.relativeClassName will be + // "OuterClassWith$Sign.NestedClassWith$Sign". To disambiguate '$' in the (outer) class name, we need to get the ClassId of + // the outer class first. + val outerClassId = nameToClassName[outerName]?.getClassId() ?: error("Class name not found: $outerName") + val relativeClassName = FqName(outerClassId.relativeClassName.asString() + "." + simpleName) // For ClassId, a nested non-local class of a local class is also considered local (see ClassId's kdoc). val isLocal = outerClassId.isLocal @@ -180,7 +185,7 @@ fun computeJavaClassIds(javaClassNames: List): List { // } // } // } - // The above class will compile into class "com/example/Foo" and "com/example/Foo$1Bar" (or + // The above source will compile into class "com/example/Foo" and "com/example/Foo$1Bar" (or // "com/example/Foo$SomeOtherArbitraryUniqueName which need not contain the string "Bar"). // // Given that class, the difference between the computed ClassId and the expected ClassId is as follows: @@ -196,15 +201,14 @@ fun computeJavaClassIds(javaClassNames: List): List { // // Alternatively, check if we can safely adjust the definition of ClassId for a local class and update any related code // accordingly. - val relativeClassName = FqName(classInfo.name.substringAfterLast('/')) - ClassId(packageName, relativeClassName, /* local */ true) + ClassId(packageName, FqName(relativeClassName), /* local */ true) } } return classId.also { - nameToClassId[className] = it + classNameToClassId[this] = it } } - return javaClassNames.map { getOrCreateClassId(it.name) } + return classNames.map { it.getClassId() } } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt index c6eaf7aaa7d..097a936ef5f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt @@ -24,6 +24,9 @@ import com.intellij.util.io.IOUtil import com.intellij.util.io.KeyDescriptor import org.jetbrains.kotlin.cli.common.CompilerSystemProperties import org.jetbrains.kotlin.cli.common.toBooleanLenient +import org.jetbrains.kotlin.incremental.LookupSymbol +import org.jetbrains.kotlin.name.ClassId +import org.jetbrains.kotlin.name.FqName import java.io.DataInput import java.io.DataInputStream import java.io.DataOutput @@ -71,6 +74,46 @@ object LookupSymbolKeyDescriptor : KeyDescriptor { override fun isEqual(val1: LookupSymbolKey, val2: LookupSymbolKey): Boolean = val1 == val2 } +object LookupSymbolExternalizer : DataExternalizer { + + override fun save(output: DataOutput, lookupSymbol: LookupSymbol) { + output.writeString(lookupSymbol.name) + output.writeString(lookupSymbol.scope) + } + + override fun read(input: DataInput): LookupSymbol { + return LookupSymbol(name = input.readString(), scope = input.readString()) + } +} + +object FqNameExternalizer : DataExternalizer { + + override fun save(output: DataOutput, fqName: FqName) { + output.writeString(fqName.asString()) + } + + override fun read(input: DataInput): FqName { + return FqName(input.readString()) + } +} + +object ClassIdExternalizer : DataExternalizer { + + override fun save(output: DataOutput, classId: ClassId) { + FqNameExternalizer.save(output, classId.packageFqName) + FqNameExternalizer.save(output, classId.relativeClassName) + output.writeBoolean(classId.isLocal) + } + + override fun read(input: DataInput): ClassId { + return ClassId( + /* packageFqName */ FqNameExternalizer.read(input), + /* relativeClassName */ FqNameExternalizer.read(input), + /* isLocal */ input.readBoolean() + ) + } +} + object ProtoMapValueExternalizer : DataExternalizer { override fun save(output: DataOutput, value: ProtoMapValue) { output.writeBoolean(value.isPackageFacade) @@ -210,6 +253,17 @@ object PathStringDescriptor : EnumeratorStringDescriptor() { } } +/** + * [DataExternalizer] for a [Collection]. + * + * If you need a [DataExternalizer] for a more specific instance of [Collection] (e.g., [List]), use [ListExternalizer] or create another + * instance of [GenericCollectionExternalizer]. + * + * Note: The implementations of this class and [GenericCollectionExternalizer] are similar but not exactly the same: the latter reads and + * writes the size of the collection to avoid resizing the collection when reading. Therefore, if we make this class extend + * [GenericCollectionExternalizer] to share code, we will need to update some expected files in tests as the serialized data will change + * slightly. + */ open class CollectionExternalizer( private val elementExternalizer: DataExternalizer, private val newCollection: () -> MutableCollection @@ -269,27 +323,35 @@ object ByteArrayExternalizer : DataExternalizer { } } -class ListExternalizer( - private val elementExternalizer: DataExternalizer -) : DataExternalizer> { +open class GenericCollectionExternalizer>( + private val elementExternalizer: DataExternalizer, + private val newCollection: (size: Int) -> MutableCollection +) : DataExternalizer { - override fun save(output: DataOutput, value: List) { - output.writeInt(value.size) - value.forEach { + override fun save(output: DataOutput, collection: C) { + output.writeInt(collection.size) + collection.forEach { elementExternalizer.save(output, it) } } - override fun read(input: DataInput): List { + override fun read(input: DataInput): C { val size = input.readInt() - val list = ArrayList(size) + val collection = newCollection(size) repeat(size) { - list.add(elementExternalizer.read(input)) + collection.add(elementExternalizer.read(input)) } - return list + @Suppress("UNCHECKED_CAST") + return collection as C } } +class ListExternalizer(elementExternalizer: DataExternalizer) : + GenericCollectionExternalizer>(elementExternalizer, { size -> ArrayList(size) }) + +class LinkedHashSetExternalizer(elementExternalizer: DataExternalizer) : + GenericCollectionExternalizer>(elementExternalizer, { size -> LinkedHashSet(size) }) + class LinkedHashMapExternalizer( private val keyExternalizer: DataExternalizer, private val valueExternalizer: DataExternalizer 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 7bf4987d5e5..2e9d5890bf3 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 @@ -47,7 +47,6 @@ import org.jetbrains.kotlin.incremental.multiproject.EmptyModulesApiHistory import org.jetbrains.kotlin.incremental.multiproject.ModulesApiHistory import org.jetbrains.kotlin.incremental.util.BufferingMessageCollector import org.jetbrains.kotlin.incremental.util.Either -import org.jetbrains.kotlin.incremental.ClasspathChanges.NotAvailable.UnableToCompute import org.jetbrains.kotlin.incremental.ClasspathChanges.NotAvailable.ForJSCompiler import org.jetbrains.kotlin.incremental.ClasspathChanges.NotAvailable.ReservedForTestsOnly import org.jetbrains.kotlin.incremental.ClasspathChanges.NotAvailable.ForNonIncrementalRun @@ -96,14 +95,12 @@ fun makeIncrementally( } object EmptyICReporter : ICReporterBase() { - override fun reportCompileIteration(incremental: Boolean, sourceFiles: Collection, exitCode: ExitCode) { - } - - override fun report(message: () -> String) { - } - - override fun reportVerbose(message: () -> String) { - } + override fun report(message: () -> String) {} + override fun reportVerbose(message: () -> String) {} + override fun reportCompileIteration(incremental: Boolean, sourceFiles: Collection, exitCode: ExitCode) {} + override fun reportMarkDirtyClass(affectedFiles: Iterable, classFqName: String) {} + override fun reportMarkDirtyMember(affectedFiles: Iterable, scope: String, name: String) {} + override fun reportMarkDirty(affectedFiles: Iterable, reason: String) {} } inline fun withIC(enabled: Boolean = true, fn: () -> R): R { @@ -218,10 +215,10 @@ class IncrementalJvmCompilerRunner( reporter.reportVerbose { "Last Kotlin Build info -- $lastBuildInfo" } val classpathChanges = when (classpathChanges) { - // Note: classpathChanges is deserialized so they are no longer singleton objects and need to be compared using `is` (not `==`). + // Note: classpathChanges is deserialized, so they are no longer singleton objects and need to be compared using `is` (not `==`) is ClasspathChanges.Available -> ChangesEither.Known(classpathChanges.lookupSymbols, classpathChanges.fqNames) is ClasspathChanges.NotAvailable -> when (classpathChanges) { - is UnableToCompute, is ClasspathSnapshotIsDisabled, is ReservedForTestsOnly -> { + is ClasspathSnapshotIsDisabled, is ReservedForTestsOnly -> { reporter.measure(BuildTime.IC_ANALYZE_CHANGES_IN_DEPENDENCIES) { val scopes = caches.lookupCache.lookupMap.keys.map { if (it.scope.isBlank()) it.name else it.scope }.distinct() getClasspathChanges( diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt index ea8db1ac95b..25490e10c57 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt @@ -148,7 +148,24 @@ class IncrementalCompilationFirJvmMultiProjectIT : IncrementalCompilationJvmMult } class IncrementalCompilationClasspathSnapshotJvmMultiProjectIT : IncrementalCompilationJvmMultiProjectIT() { + override fun defaultBuildOptions() = super.defaultBuildOptions().copy(useClasspathSnapshot = true) + + override fun testAddDependencyInLib_expectedFiles(project: Project): Iterable { + // With classpath snapshot, no files are recompiled + return emptyList() + } + + override fun testAbiChangeInLib_afterLibClean_expectedFiles(project: Project): Iterable { + // With classpath snapshot, app compilation is incremental + return File(project.projectDir, "app").getFilesByNames("AA.kt", "AAA.kt", "BB.kt", "fooUseA.kt") + + File(project.projectDir, "lib").allKotlinFiles() + } + + override fun testCompileLibWithGroovy_expectedFiles(project: Project): Iterable { + // With classpath snapshot, no files in app are recompiled + return listOf(File(project.projectDir, "lib").getFileByName("A.kt")) + } } abstract class BaseIncrementalCompilationMultiProjectIT : IncrementalCompilationBaseIT() { diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt index a7a8d6b0a30..758f48534a4 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt @@ -20,7 +20,26 @@ open class IncrementalJavaChangeDefaultIT : IncrementalCompilationJavaChangesBas } class IncrementalJavaChangeClasspathSnapshotIT : IncrementalJavaChangeDefaultIT() { + override fun defaultBuildOptions() = super.defaultBuildOptions().copy(useClasspathSnapshot = true) + + @Test + override fun testAbiChangeInLib_changeMethodSignature() { + // With classpath snapshot, fewer Kotlin files are recompiled + doTest( + javaClass, changeSignature, + expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt") + ) + } + + @Test + override fun testNonAbiChangeInLib_changeMethodBody() { + // With classpath snapshot, no Kotlin files are recompiled + doTest( + javaClass, changeBody, + expectedCompiledFileNames = emptyList() + ) + } } class IncrementalJavaChangePreciseIT : IncrementalCompilationJavaChangesBase(usePreciseJavaTracking = true) { @@ -70,12 +89,12 @@ abstract class IncrementalCompilationJavaChangesBase(val usePreciseJavaTracking: override fun defaultBuildOptions() = super.defaultBuildOptions().copy(usePreciseJavaTracking = usePreciseJavaTracking) protected val trackedJavaClass = "TrackedJavaClass.java" - private val javaClass = "JavaClass.java" + protected val javaClass = "JavaClass.java" protected val changeBody: (String) -> String = { it.replace("Hello, World!", "Hello, World!!!!") } protected val changeSignature: (String) -> String = { it.replace("String getString", "Object getString") } @Test - fun testAbiChangeInLib_changeMethodSignature() { + open fun testAbiChangeInLib_changeMethodSignature() { doTest( javaClass, changeSignature, expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt", "useJavaClassFooMethodUsage.kt") @@ -83,7 +102,7 @@ abstract class IncrementalCompilationJavaChangesBase(val usePreciseJavaTracking: } @Test - fun testNonAbiChangeInLib_changeMethodBody() { + open fun testNonAbiChangeInLib_changeMethodBody() { doTest( javaClass, changeBody, expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt", "useJavaClassFooMethodUsage.kt") diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputer.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputer.kt index a132a10aae6..31d82d0d23b 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputer.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputer.kt @@ -5,184 +5,94 @@ package org.jetbrains.kotlin.gradle.incremental -import com.google.common.annotations.VisibleForTesting import com.intellij.openapi.util.io.FileUtil -import org.jetbrains.kotlin.build.report.BuildReporter -import org.jetbrains.kotlin.build.report.ICReporter -import org.jetbrains.kotlin.build.report.metrics.* -import org.jetbrains.kotlin.cli.common.ExitCode import org.jetbrains.kotlin.incremental.* -import java.io.File -import org.jetbrains.kotlin.gradle.incremental.ChangesCollectorResult.Success -import org.jetbrains.kotlin.gradle.incremental.ChangesCollectorResult.Failure import org.jetbrains.kotlin.incremental.storage.FileToCanonicalPathConverter import java.util.* +import kotlin.collections.LinkedHashMap /** Computes [ClasspathChanges] between two [ClasspathSnapshot]s .*/ object ClasspathChangesComputer { - fun getChanges(current: ClasspathSnapshot, previous: ClasspathSnapshot): ClasspathChanges { - val changesCollector = ChangesCollector() - return when (collectClasspathChanges(current, previous, changesCollector)) { - Success -> { - val (lookupSymbols, fqNames, _) = changesCollector.getDirtyData(emptyList(), NoOpBuildReporter) - ClasspathChanges.Available(lookupSymbols.toList(), fqNames.toList()) - } - is Failure -> ClasspathChanges.NotAvailable.UnableToCompute - } - } + fun compute(currentClasspathSnapshot: ClasspathSnapshot, previousClasspathSnapshot: ClasspathSnapshot): ClasspathChanges { + val currentClassSnapshots = currentClasspathSnapshot.getClassSnapshots() + val previousClassSnapshots = previousClasspathSnapshot.getClassSnapshots() - private fun collectClasspathChanges( - current: ClasspathSnapshot, - previous: ClasspathSnapshot, - changesCollector: ChangesCollector - ): ChangesCollectorResult { - if (current.classpathEntrySnapshots.size != previous.classpathEntrySnapshots.size) { - return Failure.AddedRemovedClasspathEntries - } - - for (index in current.classpathEntrySnapshots.indices) { - val result = collectClasspathEntryChanges( - current.classpathEntrySnapshots[index], - previous.classpathEntrySnapshots[index], - changesCollector - ) - if (result is Failure) { - return result - } - } - return Success - } - - private fun collectClasspathEntryChanges( - current: ClasspathEntrySnapshot, - previous: ClasspathEntrySnapshot, - changesCollector: ChangesCollector - ): ChangesCollectorResult { - if (current.classSnapshots.size != previous.classSnapshots.size) { - return Failure.AddedRemovedClasses - } - - for (key in current.classSnapshots.keys) { - val currentSnapshot = current.classSnapshots[key]!! - val previousSnapshot = previous.classSnapshots[key] ?: return Failure.AddedRemovedClasses - val result = collectClassChanges(currentSnapshot, previousSnapshot, changesCollector) - if (result !is Success) { - return result - } - } - return Success - } - - private fun collectClassChanges( - current: ClassSnapshot, - previous: ClassSnapshot, - @Suppress("UNUSED_PARAMETER") changesCollector: ChangesCollector - ): ChangesCollectorResult { - if (current is JavaClassSnapshot && previous is JavaClassSnapshot && - (current !is RegularJavaClassSnapshot || previous !is RegularJavaClassSnapshot) - ) { - return Failure.NotYetImplemented - } - // TODO: Store results in changesCollector and return SUCCESS here - computeClassChanges(current, previous) - return Failure.NotYetImplemented - } - - @VisibleForTesting - internal fun computeClassChanges(current: ClassSnapshot, previous: ClassSnapshot): DirtyData { - // TODO Create IncrementalJvmCache early once and reuse it here val workingDir = FileUtil.createTempDirectory(this::class.java.simpleName, "_WorkingDir_${UUID.randomUUID()}", /* deleteOnExit */ true) val incrementalJvmCache = IncrementalJvmCache(workingDir, /* targetOutputDir */ null, FileToCanonicalPathConverter) - val changesCollector = ChangesCollector() - when { - current is KotlinClassSnapshot && previous is KotlinClassSnapshot -> - collectKotlinClassChanges(current, previous, incrementalJvmCache, changesCollector) - current is JavaClassSnapshot && previous is JavaClassSnapshot -> - collectJavaClassChanges(current, previous, incrementalJvmCache, changesCollector) - else -> { - // TODO: Handle current is KotlinClassSnapshot && previous is JavaClassSnapshot, and vice versa - error("Incompatible types: ${current.javaClass.name} vs. ${previous.javaClass.name}") + // Store previous class snapshots in incrementalJvmCache, the returned ChangesCollector result is not used. + val unusedChangesCollector = ChangesCollector() + for (previousSnapshot in previousClassSnapshots) { + when (previousSnapshot) { + is KotlinClassSnapshot -> incrementalJvmCache.saveClassToCache( + kotlinClassInfo = previousSnapshot.classInfo, + sourceFiles = null, + changesCollector = unusedChangesCollector + ) + is RegularJavaClassSnapshot -> incrementalJvmCache.saveJavaClassProto( + source = null, + serializedJavaClass = previousSnapshot.serializedJavaClass, + collector = unusedChangesCollector + ) + is EmptyJavaClassSnapshot -> { + // Nothing to process + } } } + // Call the following method even though there are no removed classes, just in case the method updates the state of + // incrementalJvmCache. + incrementalJvmCache.clearCacheForRemovedClasses(unusedChangesCollector) + // Compute changes between the current class snapshots and the previously stored snapshots, and save the result in changesCollector. + val changesCollector = ChangesCollector() + for (currentSnapshot in currentClassSnapshots) { + when (currentSnapshot) { + is KotlinClassSnapshot -> incrementalJvmCache.saveClassToCache( + kotlinClassInfo = currentSnapshot.classInfo, + sourceFiles = null, + changesCollector = changesCollector + ) + is RegularJavaClassSnapshot -> incrementalJvmCache.saveJavaClassProto( + source = null, + serializedJavaClass = currentSnapshot.serializedJavaClass, + collector = changesCollector + ) + is EmptyJavaClassSnapshot -> { + // Nothing to process + } + } + } + incrementalJvmCache.clearCacheForRemovedClasses(changesCollector) + + val dirtyData = changesCollector.getDirtyData(listOf(incrementalJvmCache), EmptyICReporter) workingDir.deleteRecursively() - return changesCollector.getDirtyData(listOf(incrementalJvmCache), NoOpBuildReporter.NoOpICReporter) - } - private fun collectKotlinClassChanges( - current: KotlinClassSnapshot, - previous: KotlinClassSnapshot, - incrementalJvmCache: IncrementalJvmCache, - changesCollector: ChangesCollector - ) { - // Store previous snapshot in incrementalJvmCache, the returned ChangesCollector result is not used. - incrementalJvmCache.saveClassToCache( - kotlinClassInfo = previous.classInfo, - sourceFiles = null, - changesCollector = ChangesCollector() + return ClasspathChanges.Available( + lookupSymbols = LinkedHashSet(dirtyData.dirtyLookupSymbols), + fqNames = LinkedHashSet(dirtyData.dirtyClassesFqNames) ) - incrementalJvmCache.clearCacheForRemovedClasses(changesCollector) - - // Compute changes between the current snapshot and the previously stored snapshot, and store the result in changesCollector. - incrementalJvmCache.saveClassToCache( - kotlinClassInfo = current.classInfo, - sourceFiles = null, - changesCollector = changesCollector - ) - incrementalJvmCache.clearCacheForRemovedClasses(changesCollector) } - private fun collectJavaClassChanges( - current: JavaClassSnapshot, - previous: JavaClassSnapshot, - incrementalJvmCache: IncrementalJvmCache, - changesCollector: ChangesCollector - ) { - // Store previous snapshot in incrementalJvmCache, the returned ChangesCollector result is not used. - val previousSnapshot = (previous as RegularJavaClassSnapshot).serializedJavaClass // TODO Handle unsafe cast - incrementalJvmCache.saveJavaClassProto(/* source */ null, previousSnapshot, ChangesCollector()) - incrementalJvmCache.clearCacheForRemovedClasses(changesCollector) - - // Compute changes between the current snapshot and the previously stored snapshot, and store the result in changesCollector. - val currentSnapshot = (current as RegularJavaClassSnapshot).serializedJavaClass - incrementalJvmCache.saveJavaClassProto(/* source */ null, currentSnapshot, changesCollector) - incrementalJvmCache.clearCacheForRemovedClasses(changesCollector) - } -} - -private sealed class ChangesCollectorResult { - - object Success : ChangesCollectorResult() - - sealed class Failure : ChangesCollectorResult() { - // TODO: Handle these cases - object AddedRemovedClasspathEntries : Failure() - object AddedRemovedClasses : Failure() - object NotYetImplemented : Failure() - } -} - -private object NoOpBuildReporter : BuildReporter(NoOpICReporter, NoOpBuildMetricsReporter) { - - object NoOpICReporter : ICReporter { - override fun report(message: () -> String) {} - override fun reportVerbose(message: () -> String) {} - override fun reportCompileIteration(incremental: Boolean, sourceFiles: Collection, exitCode: ExitCode) {} - override fun reportMarkDirtyClass(affectedFiles: Iterable, classFqName: String) {} - override fun reportMarkDirtyMember(affectedFiles: Iterable, scope: String, name: String) {} - override fun reportMarkDirty(affectedFiles: Iterable, reason: String) {} - } - - object NoOpBuildMetricsReporter : BuildMetricsReporter { - override fun startMeasure(time: BuildTime, startNs: Long) {} - override fun endMeasure(time: BuildTime, endNs: Long) {} - override fun addTimeMetric(metric: BuildTime, durationMs: Long) {} - override fun addMetric(metric: BuildPerformanceMetric, value: Long) {} - override fun addAttribute(attribute: BuildAttribute) {} - override fun getMetrics(): BuildMetrics = BuildMetrics() - override fun addMetrics(metrics: BuildMetrics?) {} + private fun ClasspathSnapshot.getClassSnapshots(): List { + // If there are duplicate classes on the classpath, retain only the first one to match the compiler's behavior. + // We still need to consider whether to remove duplicate classes based on the class file name or the `ClassId`, as two different + // `ClassId`s can have the same class file name (e.g., nested class `B$C` of top-level class `A` in `first.jar` and nested class `C` + // of top-level class `A$B` in `second.jar` both have the same class file name `A$B$C`). + // - If we use class file name, only `A$B$C` in `first.jar` will be retained. This matches the compiler's behavior because when + // resolving either of those classes, the compiler will only look for `A$B$C` in the first jar, even when the actual class is + // located in the second jar. + // - If we use `ClassId`, both `A$B$C` in `first.jar` and `A$B$C` in `second.jar` will be retained. That means that the snapshot + // of the class in `second.jar` will be considered when computing classpath changes, which is not expected as it doesn't match + // the compiler's behavior. + // Therefore, we will remove duplicate classes based on the class file name, not the `ClassId`. + val classSnapshots = LinkedHashMap(classpathEntrySnapshots.sumOf { it.classSnapshots.size }) + for (classpathEntrySnapshot in classpathEntrySnapshots) { + for ((unixStyleRelativePath, classSnapshot) in classpathEntrySnapshot.classSnapshots) { + classSnapshots.putIfAbsent(unixStyleRelativePath, classSnapshot) + } + } + return classSnapshots.values.toList() } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshot.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshot.kt index e09da86c3dd..faa6593744e 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshot.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshot.kt @@ -43,9 +43,7 @@ class KotlinClassSnapshot(val classInfo: KotlinClassInfo) : ClassSnapshot() sealed class JavaClassSnapshot : ClassSnapshot() /** [JavaClassSnapshot] of a typical Java class. */ -class RegularJavaClassSnapshot( - val serializedJavaClass: SerializedJavaClass -) : JavaClassSnapshot() +class RegularJavaClassSnapshot(val serializedJavaClass: SerializedJavaClass) : JavaClassSnapshot() /** * [JavaClassSnapshot] of a Java class where there is nothing to capture. diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotSerializer.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotSerializer.kt index 4c68908c212..88db5150470 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotSerializer.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotSerializer.kt @@ -10,8 +10,6 @@ import org.jetbrains.kotlin.incremental.JavaClassProtoMapValueExternalizer import org.jetbrains.kotlin.incremental.KotlinClassInfo import org.jetbrains.kotlin.incremental.storage.* import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader -import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName import java.io.* /** Utility to serialize a [ClasspathSnapshot]. */ @@ -93,34 +91,6 @@ object KotlinClassInfoExternalizer : DataExternalizer { } } -object ClassIdExternalizer : DataExternalizer { - - override fun save(output: DataOutput, classId: ClassId) { - FqNameExternalizer.save(output, classId.packageFqName) - FqNameExternalizer.save(output, classId.relativeClassName) - output.writeBoolean(classId.isLocal) - } - - override fun read(input: DataInput): ClassId { - return ClassId( - /* packageFqName */ FqNameExternalizer.read(input), - /* relativeClassName */ FqNameExternalizer.read(input), - /* isLocal */ input.readBoolean() - ) - } -} - -object FqNameExternalizer : DataExternalizer { - - override fun save(output: DataOutput, fqName: FqName) { - output.writeString(fqName.asString()) - } - - override fun read(input: DataInput): FqName { - return FqName(input.readString()) - } -} - object JavaClassSnapshotExternalizer : DataExternalizer { override fun save(output: DataOutput, snapshot: JavaClassSnapshot) { @@ -166,39 +136,27 @@ object EmptyJavaClassSnapshotExternalizer : DataExternalizer : DataExternalizer { fun save(file: File, value: T) { - return FileOutputStream(file).buffered().use { - it.writeValue(value) + return DataOutputStream(FileOutputStream(file).buffered()).use { + save(it, value) } } fun load(file: File): T { - return FileInputStream(file).buffered().use { - it.readValue() + return DataInputStream(FileInputStream(file).buffered()).use { + read(it) } } fun toByteArray(value: T): ByteArray { val byteArrayOutputStream = ByteArrayOutputStream() - byteArrayOutputStream.buffered().use { - it.writeValue(value) + DataOutputStream(byteArrayOutputStream.buffered()).use { + save(it, value) } return byteArrayOutputStream.toByteArray() } fun fromByteArray(byteArray: ByteArray): T { - return ByteArrayInputStream(byteArray).buffered().use { - it.readValue() - } - } - - private fun OutputStream.writeValue(value: T) { - DataOutputStream(this).use { - save(it, value) - } - } - - private fun InputStream.readValue(): T { - return DataInputStream(this).use { + return DataInputStream(ByteArrayInputStream(byteArray).buffered()).use { read(it) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotter.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotter.kt index b8bf33a4d67..9d53c906e37 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotter.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathSnapshotter.kt @@ -30,8 +30,7 @@ object ClasspathEntrySnapshotter { val snapshots = ClassSnapshotter.snapshot(classes) - val relativePathsToSnapshotsMap = - classes.map { it.classFile.unixStyleRelativePath }.zip(snapshots).toMap(LinkedHashMap()) + val relativePathsToSnapshotsMap = classes.map { it.classFile.unixStyleRelativePath }.zipToMap(snapshots) return ClasspathEntrySnapshot(relativePathsToSnapshotsMap) } } @@ -48,17 +47,16 @@ object ClassSnapshotter { */ fun snapshot(classes: List): List { // Snapshot Kotlin classes first - val kotlinClassSnapshots: Map = classes.associate { - it.classFile to trySnapshotKotlinClass(it) + val kotlinClassSnapshots: Map = classes.associateWith { + trySnapshotKotlinClass(it) } - // Snapshot Java classes in one invocation - val javaClasses: List = classes.filter { kotlinClassSnapshots[it.classFile] == null } + // Snapshot the remaining Java classes in one invocation + val javaClasses: List = classes.filter { kotlinClassSnapshots[it] == null } val snapshots: List = snapshotJavaClasses(javaClasses) - val javaClassSnapshots: Map = javaClasses.map { it.classFile }.zip(snapshots).toMap() + val javaClassSnapshots: Map = javaClasses.zipToMap(snapshots) - // Return a snapshot for each class - return classes.map { kotlinClassSnapshots[it.classFile] ?: javaClassSnapshots[it.classFile]!! } + return classes.map { kotlinClassSnapshots[it] ?: javaClassSnapshots[it]!! } } /** Creates [KotlinClassSnapshot] of the given class, or returns `null` if the class is not a Kotlin class. */ @@ -75,44 +73,56 @@ object ClassSnapshotter { * Therefore, outer classes and nested classes must be passed together in one invocation of this method. */ private fun snapshotJavaClasses(classes: List): List { - val classFiles = classes.map { it.classFile } - val classesContents = classes.map { it.contents } - val classNames = classesContents.map { JavaClassName.compute(it) } - val classIds = computeJavaClassIds(classNames) + val classNames: List = classes.map { JavaClassName.compute(it.contents) } + val classNameToClassFile: LinkedHashMap = classNames.zipToMap(classes) - // Snapshot special cases first - // Map a class index to its snapshot, or `null` if it will be created later - val specialCaseSnapshots: Map = classFiles.indices.associateWith { index -> - val className = classNames[index] - val classId = classIds[index] - if (classId.isLocal) { - // A local class can't be referenced from other source files, so any changes in a local class will not cause recompilation - // of other source files. Therefore, the snapshot of a local class is empty. - // In that regard, a nested class of a local class is also considered local (which matches the definition of - // ClassId.isLocal, see ClassId's kdoc). Therefore, we checked `classId.isLocal`, which is a super set of `className is - // LocalClass`. + // We divide classes into 2 categories: + // - Special classes, which includes local, anonymous, or synthetic classes, and their nested classes. These classes can't be + // referenced from other source files, so any changes in these classes will not cause recompilation of other source files. + // Therefore, the snapshots of these classes are empty. + // - Regular classes: Any classes that do not belong to the above category. + val specialClasses = getSpecialClasses(classNames).toSet() + + // Snapshot special classes first + val specialClassSnapshots: Map = classNames.associateWith { + if (it in specialClasses) { EmptyJavaClassSnapshot - } else if (className is NestedNonLocalClass && (className.isAnonymous || className.isSynthetic)) { - // An anonymous or synthetic class also can't be referenced from other source files, so its snapshot is also empty. - EmptyJavaClassSnapshot - } else { - null + } else null + } + + // Snapshot the remaining regular classes in one invocation + val regularClasses: List = classNames.filter { specialClassSnapshots[it] == null } + val regularClassIds: List = computeJavaClassIds(regularClasses) + val regularClassesContents: List = regularClasses.map { classNameToClassFile[it]!!.contents } + + val snapshots: List = JavaClassDescriptorCreator.create(regularClassIds, regularClassesContents).map { + RegularJavaClassSnapshot(it.toSerializedJavaClass()) + } + val regularClassSnapshots: LinkedHashMap = regularClasses.zipToMap(snapshots) + + return classNames.map { specialClassSnapshots[it] ?: regularClassSnapshots[it]!! } + } + + /** Returns local, anonymous, or synthetic classes, and their nested classes. */ + private fun getSpecialClasses(classNames: List): List { + val specialClasses: MutableMap = HashMap(classNames.size) + val nameToClassName: Map = classNames.associateBy { it.name } + + fun JavaClassName.isSpecial(): Boolean { + specialClasses[this]?.let { return it } + + return if (isAnonymous || isSynthetic) { + true + } else when (this) { + is TopLevelClass -> false + is NestedNonLocalClass -> nameToClassName[outerName]?.isSpecial() ?: error("Class name not found: $outerName") + is LocalClass -> true + }.also { + specialClasses[this] = it } } - // Snapshot the remaining classes in one invocation - val remainingClassesIndices: List = classFiles.indices.filter { specialCaseSnapshots[it] == null } - val remainingClassIds: List = remainingClassesIndices.map { classIds[it] } - val remainingClassesContents: List = remainingClassesIndices.map { classes[it].contents } - - val snapshots: List = JavaClassDescriptorCreator.create(remainingClassIds, remainingClassesContents).map { - RegularJavaClassSnapshot(it.toSerializedJavaClass()) - } - val remainingSnapshots: Map /* maps a class index to its snapshot */ = - remainingClassesIndices.zip(snapshots).toMap() - - // Return a snapshot for each class - return classFiles.indices.map { specialCaseSnapshots[it] ?: remainingSnapshots[it]!! } + return classNames.filter { it.isSpecial() } } } @@ -120,14 +130,15 @@ object ClassSnapshotter { private object DirectoryOrJarContentsReader { /** - * Returns a map from Unix-style relative paths of entries to their contents. The paths are relative to the container (directory or - * jar). + * Returns a map from Unix-style relative paths of entries to their contents. The paths are relative to the given container (directory + * or jar). * * The map entries need to satisfy the given filter. * * The map entries are sorted based on their Unix-style relative paths (to ensure deterministic results across filesystems). * - * Note: If a jar has duplicate entries, only one of them will be used (there is no guarantee which one will be used). + * Note: If a jar has duplicate entries, only one of them will be used (there is no guarantee which one will be used, but the selection + * will be deterministic). */ fun read( directoryOrJar: File, @@ -172,3 +183,18 @@ private object DirectoryOrJarContentsReader { return relativePathsToContents.sortedBy { it.first }.toMap(LinkedHashMap()) } } + +/** + * Combines two lists of the same size into a map. + * + * This method is more efficient than calling `[Iterable.zip].toMap()` as it doesn't create short-lived intermediate [Pair]s as done by + * [Iterable.zip]. + */ +private fun List.zipToMap(other: List): LinkedHashMap { + check(this.size == other.size) + val map = LinkedHashMap(size) + indices.forEach { index -> + map[this[index]] = other[index] + } + return map +} 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 a427865c672..e62493eef74 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 @@ -556,7 +556,9 @@ abstract class KotlinCompile @Inject constructor( it.attributes.attribute(ARTIFACT_TYPE_ATTRIBUTE, CLASSPATH_ENTRY_SNAPSHOT_ARTIFACT_TYPE) }.files ) - task.classpathSnapshotProperties.classpathSnapshotDir.value(getClasspathSnapshotDir(task)).disallowChanges() + val classpathSnapshotDir = getClasspathSnapshotDir(task) + task.classpathSnapshotProperties.classpathSnapshotDir.value(classpathSnapshotDir).disallowChanges() + task.classpathSnapshotProperties.classpathSnapshotDirFileCollection.from(classpathSnapshotDir) } } } @@ -604,6 +606,14 @@ abstract class KotlinCompile @Inject constructor( @get:OutputDirectory @get:Optional // Set if useClasspathSnapshot == true abstract val classpathSnapshotDir: DirectoryProperty + + /** + * [FileCollection] containing a single file which is [classpathSnapshotDir], used when a [FileCollection] is required instead of a + * [DirectoryProperty]. + */ + // Set if useClasspathSnapshot == true + @get:Internal + abstract val classpathSnapshotDirFileCollection: ConfigurableFileCollection } @get:Internal @@ -682,7 +692,7 @@ abstract class KotlinCompile @Inject constructor( val classpathChanges = when { !classpathSnapshotProperties.useClasspathSnapshot.get() -> ClasspathChanges.NotAvailable.ClasspathSnapshotIsDisabled else -> when (changedFiles) { - is ChangedFiles.Known -> getClasspathChanges() + is ChangedFiles.Known -> getClasspathChanges(changedFiles) is ChangedFiles.Unknown -> ClasspathChanges.NotAvailable.ForNonIncrementalRun is ChangedFiles.Dependencies -> error("Unexpected type: ${changedFiles.javaClass.name}") } @@ -698,9 +708,16 @@ abstract class KotlinCompile @Inject constructor( ) } else null + with(classpathSnapshotProperties) { + if (isIncrementalCompilationEnabled() && useClasspathSnapshot.get()) { + copyClasspathSnapshotFilesToDir(classpathSnapshot.files.toList(), classpathSnapshotDir.get().asFile) + } + } + val environment = GradleCompilerEnvironment( defaultCompilerClasspath, messageCollector, outputItemCollector, - outputFiles = allOutputFiles(), + // The compiler runner should not manage (read, modify, or delete) classpathSnapshotDir + outputFiles = allOutputFiles().minus(classpathSnapshotProperties.classpathSnapshotDirFileCollection), reportingSettings = reportingSettings, incrementalCompilationEnvironment = icEnv, kotlinScriptExtensions = sourceFilesExtensions.get().toTypedArray() @@ -714,12 +731,6 @@ abstract class KotlinCompile @Inject constructor( environment, defaultKotlinJavaToolchain.get().providedJvm.get().javaHome ) - - with(classpathSnapshotProperties) { - if (isIncrementalCompilationEnabled() && useClasspathSnapshot.get()) { - copyClasspathSnapshotFilesToDir(classpathSnapshot.files.toList(), classpathSnapshotDir.get().asFile) - } - } } private fun validateKotlinAndJavaHasSameTargetCompatibility(args: K2JVMCompilerArguments) { @@ -786,14 +797,29 @@ abstract class KotlinCompile @Inject constructor( return super.source(*sources) } - private fun getClasspathChanges(): ClasspathChanges { - val currentSnapshotFiles = classpathSnapshotProperties.classpathSnapshot.files.toList() + private fun getClasspathChanges(knownChangedFiles: ChangedFiles.Known): ClasspathChanges { + // Find current snapshot files that have been changed (added or modified) + val currentSnapshotFiles = classpathSnapshotProperties.classpathSnapshot.files + val addedOrModifiedFiles = knownChangedFiles.modified.toSet() + val (changedCurrentSnapshotFiles, unchangedCurrentSnapshotFiles) = currentSnapshotFiles.partition { it in addedOrModifiedFiles } + + // Find previous snapshot files that have been changed (modified or removed) val previousSnapshotFiles = getClasspathSnapshotFilesInDir(classpathSnapshotProperties.classpathSnapshotDir.get().asFile) + var unchangedSnapshotIndex = 0 + var unchangedSnapshot: ByteArray? = unchangedCurrentSnapshotFiles.getOrNull(unchangedSnapshotIndex)?.readBytes() + val changedPreviousSnapshotFiles = previousSnapshotFiles.filter { + if (unchangedSnapshot != null && it.readBytes().contentEquals(unchangedSnapshot)) { + unchangedSnapshotIndex++ + unchangedSnapshot = unchangedCurrentSnapshotFiles.getOrNull(unchangedSnapshotIndex)?.readBytes() + false + } else true + } - val currentSnapshot = ClasspathSnapshotSerializer.load(currentSnapshotFiles) - val previousSnapshot = ClasspathSnapshotSerializer.load(previousSnapshotFiles) + // Compute changes for the changed snapshot files only, ignoring unchanged ones + val changedCurrentSnapshot = ClasspathSnapshotSerializer.load(changedCurrentSnapshotFiles) + val changedPreviousSnapshot = ClasspathSnapshotSerializer.load(changedPreviousSnapshotFiles) - return ClasspathChangesComputer.getChanges(currentSnapshot, previousSnapshot) + return ClasspathChangesComputer.compute(changedCurrentSnapshot, changedPreviousSnapshot) } /** diff --git a/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputerTest.kt b/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputerTest.kt index e2b57356815..f13ff4a94d5 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputerTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/test/kotlin/org/jetbrains/kotlin/gradle/incremental/ClasspathChangesComputerTest.kt @@ -5,8 +5,9 @@ package org.jetbrains.kotlin.gradle.incremental -import org.jetbrains.kotlin.incremental.DirtyData +import org.jetbrains.kotlin.incremental.ClasspathChanges import org.jetbrains.kotlin.incremental.LookupSymbol +import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME import org.junit.Assert.assertEquals @@ -24,6 +25,45 @@ abstract class ClasspathChangesComputerTest : ClasspathSnapshotTestCommon() { originalSnapshot = testSourceFile.compileAndSnapshot() } + /** Adapted version of [ClasspathChanges.Available] for readability in this test. */ + private data class Changes(private val lookupSymbols: Set, private val fqNames: Set) + + private fun computeClassChanges(current: ClassSnapshot, previous: ClassSnapshot): Changes { + val classChanges = + ClasspathChangesComputer.compute(current.toClasspathSnapshot(), previous.toClasspathSnapshot()) as ClasspathChanges.Available + return Changes(HashSet(classChanges.lookupSymbols), HashSet(classChanges.fqNames)) + } + + private fun ClassSnapshot.toClasspathSnapshot(): ClasspathSnapshot { + return ClasspathSnapshot( + classpathEntrySnapshots = listOf( + ClasspathEntrySnapshot( + LinkedHashMap(1).also { + it[getClassId()!!.getUnixStyleRelativePath()] = this + }) + ) + ) + } + + private fun ClassSnapshot.getClassId(): ClassId? { + return when (this) { + is KotlinClassSnapshot -> classInfo.classId + is RegularJavaClassSnapshot -> serializedJavaClass.classId + is EmptyJavaClassSnapshot -> null + } + } + + private fun ClassId.getUnixStyleRelativePath() = asString().replace('.', '$') + ".class" + + /** + * Returns the [FqName] of the class in this source file (e.g., "com/example/Foo$Bar.kt" or + * "com/example/Foo$Bar.java" has [FqName] "com.example.Foo.Bar"). + * + * This source file must contain only 1 class. + */ + private fun SourceFile.getClassFqName() = + FqName(unixStyleRelativePath.substringBeforeLast('.').replace('/', '.').replace('$', '.')) + // TODO Add more test cases: // - private/non-private fields // - inline functions @@ -31,31 +71,30 @@ abstract class ClasspathChangesComputerTest : ClasspathSnapshotTestCommon() { // - adding an annotation @Test - fun testCollectClassChanges_changedPublicMethodSignature() { + fun testComputeClassChanges_changedPublicMethodSignature() { val updatedSnapshot = testSourceFile.changePublicMethodSignature().compileAndSnapshot() - val dirtyData = ClasspathChangesComputer.computeClassChanges(updatedSnapshot, originalSnapshot) + val classChanges = computeClassChanges(updatedSnapshot, originalSnapshot) - val testClass = testSourceFile.sourceFile.unixStyleRelativePath.substringBeforeLast('.').replace('/', '.') + val testClassFqName = testSourceFile.sourceFile.getClassFqName() assertEquals( - DirtyData( - dirtyLookupSymbols = setOf( - LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = testClass), - LookupSymbol(name = "publicMethod", scope = testClass), - LookupSymbol(name = "changedPublicMethod", scope = testClass) + Changes( + lookupSymbols = setOf( + LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = testClassFqName.asString()), + LookupSymbol(name = "changedPublicMethod", scope = testClassFqName.asString()), + LookupSymbol(name = "publicMethod", scope = testClassFqName.asString()) ), - dirtyClassesFqNames = setOf(FqName(testClass)), - dirtyClassesFqNamesForceRecompile = emptySet() + fqNames = setOf(testClassFqName), ), - dirtyData + classChanges ) } @Test - fun testCollectClassChanges_changedMethodImplementation() { + fun testComputeClassChanges_changedMethodImplementation() { val updatedSnapshot = testSourceFile.changeMethodImplementation().compileAndSnapshot() - val dirtyData = ClasspathChangesComputer.computeClassChanges(updatedSnapshot, originalSnapshot) + val classChanges = computeClassChanges(updatedSnapshot, originalSnapshot) - assertEquals(DirtyData(emptySet(), emptySet(), emptySet()), dirtyData) + assertEquals(Changes(emptySet(), emptySet()), classChanges) } }