KT-45777: Compute classpath changes for incremental Kotlin compile

Test: Updated unit tests + incremental compilation integration tests
2021-08-25 16:23:20 +01:00
parent e3d7b7a30e
commit a48bf63630
12 changed files with 473 additions and 420 deletions
@@ -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<LookupSymbol>
-            private set
-
-        lateinit var fqNames: List<FqName>
-            private set
-
-        constructor(lookupSymbols: List<LookupSymbol>, fqNames: List<FqName>) : 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<LookupSymbol>(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<FqName>(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<LookupSymbol>
+            private set
+
+        lateinit var fqNames: LinkedHashSet<FqName>
+            private set
+
+        constructor(lookupSymbols: LinkedHashSet<LookupSymbol>, fqNames: LinkedHashSet<FqName>) : 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<ClasspathChanges.Available> {
+
+    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)
+        )
+    }
+}
@@ -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<String>()
+            val isSyntheticRef = Ref.create<Boolean>()
            val isTopLevelRef = Ref.create<Boolean>()
            val outerNameRef = Ref.create<String>()
-            val isAnonymousInnerClassRef = Ref.create<Boolean>()
-            val isSyntheticInnerClassRef = Ref.create<Boolean>()
+            val isAnonymousRef = Ref.create<Boolean>()

            ClassReader(classContents).accept(object : ClassVisitor(Opcodes.API_VERSION) {
                override fun visit(
@@ -48,120 +63,110 @@ sealed class JavaClassName(
                    signature: String?, superName: String?, interfaces: Array<String?>?
                ) {
                    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<JavaClassName>): List<ClassId> {
-    val nameToJavaClassName: Map<String, JavaClassName> = javaClassNames.associateBy { it.name }
-    val nameToClassId: MutableMap<String, ClassId?> = nameToJavaClassName.mapValues { null }.toMutableMap()
+fun computeJavaClassIds(classNames: List<JavaClassName>): List<ClassId> {
+    val classNameToClassId: MutableMap<JavaClassName, ClassId> = HashMap(classNames.size)
+    val nameToClassName: Map<String, JavaClassName> = 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<JavaClassName>): List<ClassId> {
                //             }
                //         }
                //     }
-                // 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<JavaClassName>): List<ClassId> {
                //
                // 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() }
 }
@@ -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<LookupSymbolKey> {
    override fun isEqual(val1: LookupSymbolKey, val2: LookupSymbolKey): Boolean = val1 == val2
 }

+object LookupSymbolExternalizer : DataExternalizer<LookupSymbol> {
+
+    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<FqName> {
+
+    override fun save(output: DataOutput, fqName: FqName) {
+        output.writeString(fqName.asString())
+    }
+
+    override fun read(input: DataInput): FqName {
+        return FqName(input.readString())
+    }
+}
+
+object ClassIdExternalizer : DataExternalizer<ClassId> {
+
+    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<ProtoMapValue> {
    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<T>(
    private val elementExternalizer: DataExternalizer<T>,
    private val newCollection: () -> MutableCollection<T>
@@ -269,27 +323,35 @@ object ByteArrayExternalizer : DataExternalizer<ByteArray> {
    }
 }

-class ListExternalizer<T>(
-    private val elementExternalizer: DataExternalizer<T>
-) : DataExternalizer<List<T>> {
+open class GenericCollectionExternalizer<T, C : Collection<T>>(
+    private val elementExternalizer: DataExternalizer<T>,
+    private val newCollection: (size: Int) -> MutableCollection<T>
+) : DataExternalizer<C> {

-    override fun save(output: DataOutput, value: List<T>) {
-        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<T> {
+    override fun read(input: DataInput): C {
        val size = input.readInt()
-        val list = ArrayList<T>(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<T>(elementExternalizer: DataExternalizer<T>) :
+    GenericCollectionExternalizer<T, List<T>>(elementExternalizer, { size -> ArrayList(size) })
+
+class LinkedHashSetExternalizer<T>(elementExternalizer: DataExternalizer<T>) :
+    GenericCollectionExternalizer<T, LinkedHashSet<T>>(elementExternalizer, { size -> LinkedHashSet(size) })
+
 class LinkedHashMapExternalizer<K, V>(
    private val keyExternalizer: DataExternalizer<K>,
    private val valueExternalizer: DataExternalizer<V>