KT-45777: Remove proto-based approach to compute Java class snapshots

as the ASM-based approach is better, and we have switched to the ASM-based approach for a while now.
2022-01-11 19:30:32 +00:00
parent f819a10968
commit 1cb509d529
11 changed files with 96 additions and 596 deletions
@@ -14,12 +14,9 @@ import org.jetbrains.kotlin.incremental.classpathDiff.ImpactAnalysis.computeImpa
 import org.jetbrains.kotlin.incremental.storage.FileToCanonicalPathConverter
 import org.jetbrains.kotlin.incremental.storage.ListExternalizer
 import org.jetbrains.kotlin.incremental.storage.loadFromFile
-import org.jetbrains.kotlin.metadata.deserialization.TypeTable
-import org.jetbrains.kotlin.metadata.deserialization.supertypes
 import org.jetbrains.kotlin.name.ClassId
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.resolve.jvm.JvmClassName
-import org.jetbrains.kotlin.serialization.deserialization.getClassId
 import java.io.File
 import java.util.*

@@ -92,34 +89,38 @@ object ClasspathChangesComputer {
        previousClassSnapshots: List<ClassSnapshot>,
        metrics: BuildMetricsReporter
    ): ChangeSet {
-        val asmBasedSnapshotPredicate: (ClassSnapshot) -> Boolean = {
+        val isKotlinClass: (ClassSnapshot) -> Boolean = {
            when (it) {
-                is RegularJavaClassSnapshot -> true
-                is KotlinClassSnapshot, is ProtoBasedJavaClassSnapshot -> false
-                else -> error("Unexpected type (it should have been handled earlier): ${it.javaClass.name}")
+                is KotlinClassSnapshot -> true
+                is JavaClassSnapshot -> false
+                is InaccessibleClassSnapshot -> error("Unexpected type (it should have been handled earlier): ${it.javaClass.name}")
            }
        }
-        val (currentAsmBasedSnapshots, currentProtoBasedSnapshots) = currentClassSnapshots.partition(asmBasedSnapshotPredicate)
-        val (previousAsmBasedSnapshots, previousProtoBasedSnapshots) = previousClassSnapshots.partition(asmBasedSnapshotPredicate)
+        val (currentKotlinClassSnapshots, currentJavaClassSnapshots) = currentClassSnapshots.partition(isKotlinClass)
+        val (previousKotlinClassSnapshots, previousJavaClassSnapshots) = previousClassSnapshots.partition(isKotlinClass)

+        @Suppress("UNCHECKED_CAST")
        val kotlinClassChanges = metrics.measure(BuildTime.COMPUTE_KOTLIN_CLASS_CHANGES) {
-            computeChangesForProtoBasedSnapshots(currentProtoBasedSnapshots, previousProtoBasedSnapshots)
+            computeKotlinClassChanges(
+                currentKotlinClassSnapshots as List<KotlinClassSnapshot>,
+                previousKotlinClassSnapshots as List<KotlinClassSnapshot>
+            )
        }

        @Suppress("UNCHECKED_CAST")
        val javaClassChanges = metrics.measure(BuildTime.COMPUTE_JAVA_CLASS_CHANGES) {
            JavaClassChangesComputer.compute(
-                currentAsmBasedSnapshots as List<RegularJavaClassSnapshot>,
-                previousAsmBasedSnapshots as List<RegularJavaClassSnapshot>
+                currentJavaClassSnapshots as List<JavaClassSnapshot>,
+                previousJavaClassSnapshots as List<JavaClassSnapshot>
            )
        }

        return kotlinClassChanges + javaClassChanges
    }

-    private fun computeChangesForProtoBasedSnapshots(
-        currentClassSnapshots: List<ClassSnapshot>,
-        previousClassSnapshots: List<ClassSnapshot>
+    private fun computeKotlinClassChanges(
+        currentClassSnapshots: List<KotlinClassSnapshot>,
+        previousClassSnapshots: List<KotlinClassSnapshot>
    ): ChangeSet {
        val workingDir =
            FileUtil.createTempDirectory(this::class.java.simpleName, "_WorkingDir_${UUID.randomUUID()}", /* deleteOnExit */ true)
@@ -132,28 +133,13 @@ object ClasspathChangesComputer {
        //   - The ChangesCollector result will contain symbols in the previous classes (we actually don't need them, but it's part of the
        //     API's effects).
        val unusedChangesCollector = ChangesCollector()
-        for (previousSnapshot in previousClassSnapshots) {
-            when (previousSnapshot) {
-                is KotlinClassSnapshot -> {
-                    incrementalJvmCache.saveClassToCache(
-                        kotlinClassInfo = previousSnapshot.classInfo,
-                        sourceFiles = null,
-                        changesCollector = unusedChangesCollector
-                    )
-                    incrementalJvmCache.markDirty(previousSnapshot.classInfo.className)
-                }
-                is ProtoBasedJavaClassSnapshot -> {
-                    incrementalJvmCache.saveJavaClassProto(
-                        source = null,
-                        serializedJavaClass = previousSnapshot.serializedJavaClass,
-                        collector = unusedChangesCollector
-                    )
-                    incrementalJvmCache.markDirty(JvmClassName.byClassId(previousSnapshot.serializedJavaClass.classId))
-                }
-                is RegularJavaClassSnapshot, is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
-                    error("Unexpected type (it should have been handled earlier): ${previousSnapshot.javaClass.name}")
-                }
-            }
+        previousClassSnapshots.forEach {
+            incrementalJvmCache.saveClassToCache(
+                kotlinClassInfo = it.classInfo,
+                sourceFiles = null,
+                changesCollector = unusedChangesCollector
+            )
+            incrementalJvmCache.markDirty(it.classInfo.className)
        }

        // Step 2:
@@ -164,26 +150,12 @@ object ClasspathChangesComputer {
        //   - The intermediate ChangesCollector result will contain symbols in added classes and changed (added/modified/removed) symbols
        //     in modified classes. We will collect symbols in removed classes in step 3.
        val changesCollector = ChangesCollector()
-        for (currentSnapshot in currentClassSnapshots) {
-            when (currentSnapshot) {
-                is KotlinClassSnapshot -> {
-                    incrementalJvmCache.saveClassToCache(
-                        kotlinClassInfo = currentSnapshot.classInfo,
-                        sourceFiles = null,
-                        changesCollector = changesCollector
-                    )
-                }
-                is ProtoBasedJavaClassSnapshot -> {
-                    incrementalJvmCache.saveJavaClassProto(
-                        source = null,
-                        serializedJavaClass = currentSnapshot.serializedJavaClass,
-                        collector = changesCollector
-                    )
-                }
-                is RegularJavaClassSnapshot, is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
-                    error("Unexpected type (it should have been handled earlier): ${currentSnapshot.javaClass.name}")
-                }
-            }
+        currentClassSnapshots.forEach {
+            incrementalJvmCache.saveClassToCache(
+                kotlinClassInfo = it.classInfo,
+                sourceFiles = null,
+                changesCollector = changesCollector
+            )
        }

        // Step 3:
@@ -303,11 +275,8 @@ internal object ImpactAnalysis {
 internal fun ClassSnapshot.getClassId(): ClassId {
    return when (this) {
        is KotlinClassSnapshot -> classInfo.classId
-        is RegularJavaClassSnapshot -> classId
-        is ProtoBasedJavaClassSnapshot -> serializedJavaClass.classId
-        is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
-            error("Unexpected type (it should have been handled earlier): ${javaClass.name}")
-        }
+        is JavaClassSnapshot -> classId
+        is InaccessibleClassSnapshot -> error("Unexpected type (it should have been handled earlier): ${javaClass.name}")
    }
 }

@@ -320,13 +289,7 @@ internal fun ClassSnapshot.getClassId(): ClassId {
 internal fun ClassSnapshot.getSupertypes(classIdResolver: (JvmClassName) -> ClassId?): List<ClassId> {
    return when (this) {
        is KotlinClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) }
-        is RegularJavaClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) }
-        is ProtoBasedJavaClassSnapshot -> {
-            val (proto, nameResolver) = serializedJavaClass.toProtoData()
-            proto.supertypes(TypeTable(proto.typeTable)).map { nameResolver.getClassId(it.className) }
-        }
-        is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
-            error("Unexpected type (it should have been handled earlier): ${javaClass.name}")
-        }
+        is JavaClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) }
+        is InaccessibleClassSnapshot -> error("Unexpected type (it should have been handled earlier): ${javaClass.name}")
    }
 }
@@ -6,7 +6,6 @@
 package org.jetbrains.kotlin.incremental.classpathDiff

 import org.jetbrains.kotlin.incremental.KotlinClassInfo
-import org.jetbrains.kotlin.incremental.SerializedJavaClass
 import org.jetbrains.kotlin.incremental.md5
 import org.jetbrains.kotlin.incremental.storage.toByteArray
 import org.jetbrains.kotlin.name.ClassId
@@ -72,10 +71,7 @@ class KotlinClassSnapshot(
 }

 /** [ClassSnapshot] of a Java class. */
-sealed class JavaClassSnapshot : ClassSnapshot()
-
-/** [JavaClassSnapshot] of a typical Java class. */
-class RegularJavaClassSnapshot(
+class JavaClassSnapshot(

    /** [ClassId] of the class. It is part of the class's ABI ([classAbiExcludingMembers]). */
    val classId: ClassId,
@@ -92,7 +88,7 @@ class RegularJavaClassSnapshot(
    /** [AbiSnapshot]s of the class's methods. */
    val methodsAbi: List<AbiSnapshot>

-) : JavaClassSnapshot() {
+) : ClassSnapshot() {

    val className by lazy {
        JvmClassName.byClassId(classId).also {
@@ -123,9 +119,6 @@ class AbiSnapshotForTests(

 ) : AbiSnapshot(name, abiHash)

-/** [JavaClassSnapshot] of a typical Java class which uses protos internally. */
-class ProtoBasedJavaClassSnapshot(val serializedJavaClass: SerializedJavaClass) : JavaClassSnapshot()
-
 /**
 * [ClassSnapshot] of an inaccessible class.
 *
@@ -133,10 +126,3 @@ class ProtoBasedJavaClassSnapshot(val serializedJavaClass: SerializedJavaClass)
 * will not require recompilation of other source files.
 */
 object InaccessibleClassSnapshot : ClassSnapshot()
-
-/**
- * [JavaClassSnapshot] of a Java class where a proper snapshot can't be created for some reason, so we use the hash of the class contents as
- * the snapshot instead, so that at least it's still correct when used as an input of the `KotlinCompile` task (when the class contents have
- * changed, this snapshot will also change).
- */
-class ContentHashJavaClassSnapshot(val contentHash: Long) : JavaClassSnapshot()
@@ -6,7 +6,6 @@
 package org.jetbrains.kotlin.incremental.classpathDiff

 import com.intellij.util.io.DataExternalizer
-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
@@ -166,27 +165,6 @@ object PackageMemberExternalizer : DataExternalizer<PackageMember> {
 object JavaClassSnapshotExternalizer : DataExternalizer<JavaClassSnapshot> {

    override fun save(output: DataOutput, snapshot: JavaClassSnapshot) {
-        output.writeString(snapshot.javaClass.name)
-        when (snapshot) {
-            is RegularJavaClassSnapshot -> RegularJavaClassSnapshotExternalizer.save(output, snapshot)
-            is ProtoBasedJavaClassSnapshot -> ProtoBasedJavaClassSnapshotExternalizer.save(output, snapshot)
-            is ContentHashJavaClassSnapshot -> ContentHashJavaClassSnapshotExternalizer.save(output, snapshot)
-        }
-    }
-
-    override fun read(input: DataInput): JavaClassSnapshot {
-        return when (val className = input.readString()) {
-            RegularJavaClassSnapshot::class.java.name -> RegularJavaClassSnapshotExternalizer.read(input)
-            ProtoBasedJavaClassSnapshot::class.java.name -> ProtoBasedJavaClassSnapshotExternalizer.read(input)
-            ContentHashJavaClassSnapshot::class.java.name -> ContentHashJavaClassSnapshotExternalizer.read(input)
-            else -> error("Unrecognized class name: $className")
-        }
-    }
-}
-
-object RegularJavaClassSnapshotExternalizer : DataExternalizer<RegularJavaClassSnapshot> {
-
-    override fun save(output: DataOutput, snapshot: RegularJavaClassSnapshot) {
        ClassIdExternalizer.save(output, snapshot.classId)
        ListExternalizer(JvmClassNameExternalizer).save(output, snapshot.supertypes)
        AbiSnapshotExternalizer.save(output, snapshot.classAbiExcludingMembers)
@@ -194,8 +172,8 @@ object RegularJavaClassSnapshotExternalizer : DataExternalizer<RegularJavaClassS
        ListExternalizer(AbiSnapshotExternalizer).save(output, snapshot.methodsAbi)
    }

-    override fun read(input: DataInput): RegularJavaClassSnapshot {
-        return RegularJavaClassSnapshot(
+    override fun read(input: DataInput): JavaClassSnapshot {
+        return JavaClassSnapshot(
            classId = ClassIdExternalizer.read(input),
            supertypes = ListExternalizer(JvmClassNameExternalizer).read(input),
            classAbiExcludingMembers = AbiSnapshotExternalizer.read(input),
@@ -217,17 +195,6 @@ object AbiSnapshotExternalizer : DataExternalizer<AbiSnapshot> {
    }
 }

-object ProtoBasedJavaClassSnapshotExternalizer : DataExternalizer<ProtoBasedJavaClassSnapshot> {
-
-    override fun save(output: DataOutput, snapshot: ProtoBasedJavaClassSnapshot) {
-        JavaClassProtoMapValueExternalizer.save(output, snapshot.serializedJavaClass)
-    }
-
-    override fun read(input: DataInput): ProtoBasedJavaClassSnapshot {
-        return ProtoBasedJavaClassSnapshot(serializedJavaClass = JavaClassProtoMapValueExternalizer.read(input))
-    }
-}
-
 object InaccessibleClassSnapshotExternalizer : DataExternalizer<InaccessibleClassSnapshot> {

    override fun save(output: DataOutput, snapshot: InaccessibleClassSnapshot) {
@@ -238,14 +205,3 @@ object InaccessibleClassSnapshotExternalizer : DataExternalizer<InaccessibleClas
        return InaccessibleClassSnapshot
    }
 }
-
-object ContentHashJavaClassSnapshotExternalizer : DataExternalizer<ContentHashJavaClassSnapshot> {
-
-    override fun save(output: DataOutput, snapshot: ContentHashJavaClassSnapshot) {
-        LongExternalizer.save(output, snapshot.contentHash)
-    }
-
-    override fun read(input: DataInput): ContentHashJavaClassSnapshot {
-        return ContentHashJavaClassSnapshot(contentHash = LongExternalizer.read(input))
-    }
-}
@@ -7,10 +7,8 @@ package org.jetbrains.kotlin.incremental.classpathDiff

 import org.jetbrains.kotlin.incremental.*
 import org.jetbrains.kotlin.incremental.ChangesCollector.Companion.getNonPrivateMemberNames
-import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor
 import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader.Kind.*
 import org.jetbrains.kotlin.name.ClassId
-import org.jetbrains.kotlin.utils.DFS
 import java.io.File
 import java.util.zip.ZipInputStream

@@ -20,145 +18,53 @@ object ClasspathEntrySnapshotter {
    private val DEFAULT_CLASS_FILTER = { unixStyleRelativePath: String, isDirectory: Boolean ->
        !isDirectory
                && unixStyleRelativePath.endsWith(".class", ignoreCase = true)
-                && !unixStyleRelativePath.endsWith("module-info.class", ignoreCase = true)
-                && !unixStyleRelativePath.startsWith("meta-inf", ignoreCase = true)
+                && !unixStyleRelativePath.startsWith("meta-inf/", ignoreCase = true)
+                && !unixStyleRelativePath.equals("module-info.class", ignoreCase = true)
    }

-    fun snapshot(classpathEntry: File, protoBased: Boolean? = null): ClasspathEntrySnapshot {
-        val classes =
-            DirectoryOrJarContentsReader.read(classpathEntry, DEFAULT_CLASS_FILTER)
-                .map { (unixStyleRelativePath, contents) ->
-                    ClassFileWithContents(ClassFile(classpathEntry, unixStyleRelativePath), contents)
-                }
+    fun snapshot(classpathEntry: File): ClasspathEntrySnapshot {
+        val classes = DirectoryOrJarContentsReader
+            .read(classpathEntry, DEFAULT_CLASS_FILTER)
+            .map { (unixStyleRelativePath, contents) ->
+                ClassFileWithContents(ClassFile(classpathEntry, unixStyleRelativePath), contents)
+            }

-        val snapshots = try {
-            ClassSnapshotter.snapshot(classes, protoBased).map { it.withHash }
-        } catch (e: Throwable) {
-            if ((protoBased ?: protoBasedDefaultValue) && isKnownProblematicClasspathEntry(classpathEntry)) {
-                classes.map { ContentHashJavaClassSnapshot(it.contents.md5()).withHash }
-            } else throw e
-        }
+        val snapshots = ClassSnapshotter.snapshot(classes).map { it.withHash }

-        val relativePathsToSnapshotsMap = classes.map { it.classFile.unixStyleRelativePath }.zipToMap(snapshots)
+        val relativePathsToSnapshotsMap = classes.map { it.classFile.unixStyleRelativePath }.zip(snapshots).toMap(LinkedHashMap())
        return ClasspathEntrySnapshot(relativePathsToSnapshotsMap)
    }
-
-    /** Returns `true` if it is known that the snapshot of the given classpath entry can't be created for some reason. */
-    private fun isKnownProblematicClasspathEntry(classpathEntry: File): Boolean {
-        if (classpathEntry.name.startsWith("tools-jar-api")) {
-            // [FAULTY JAR] kotlin/dependencies/tools-jar-api/build/libs/tools-jar-api-1.6.255-SNAPSHOT.jar contains class
-            // com/sun/tools/javac/comp/Infer$GraphStrategy$NodeNotFoundException, but doesn't contain its outer class
-            // com/sun/tools/javac/comp/Infer$GraphStrategy.
-            // This happens with a few other similar classes in this jar.
-            // Therefore, this is a faulty jar, and our snapshotting logic cannot process it.
-            return true
-        }
-        if (classpathEntry.name.startsWith("platform-impl")) {
-            // ~/.gradle/kotlin-build-dependencies/repo/kotlin.build/ideaIC/203.8084.24/artifacts/lib/platform-impl.jar contains class
-            // com/intellij/application/options/codeStyle/OptionTableWithPreviewPanel.IntOption. When processing that class,
-            // BinaryJavaAnnotation$Companion.computeTargetType$resolution_common_jvm in Annotations.kt requires the targetType to be
-            // JavaClassifierType, but the actual type is PlainJavaPrimitiveType.
-            // TODO: It's likely that this requirement is incorrect, but let's fix it later.
-            return true
-        }
-        return false
-    }
 }

 /** Creates [ClassSnapshot]s of classes. */
 object ClassSnapshotter {

    /** Creates [ClassSnapshot]s of the given classes. */
-    fun snapshot(
-        classes: List<ClassFileWithContents>,
-        protoBased: Boolean? = null,
-        includeDebugInfoInSnapshot: Boolean? = null
-    ): List<ClassSnapshot> {
-        // Find inaccessible classes first, their snapshots will be `InaccessibleClassSnapshot`s.
+    fun snapshot(classes: List<ClassFileWithContents>, includeDebugInfoInJavaSnapshot: Boolean? = null): List<ClassSnapshot> {
+        // Find inaccessible classes first
        val classesInfo: List<BasicClassInfo> = classes.map { it.classInfo }
        val inaccessibleClassesInfo: Set<BasicClassInfo> = getInaccessibleClasses(classesInfo).toSet()

-        // Snapshot the remaining accessible classes
-        val accessibleClasses: List<ClassFileWithContents> = classes.filter { it.classInfo !in inaccessibleClassesInfo }
-        val accessibleSnapshots: List<ClassSnapshot> = doSnapshot(accessibleClasses, protoBased, includeDebugInfoInSnapshot)
-        val accessibleClassSnapshots: Map<ClassFileWithContents, ClassSnapshot> = accessibleClasses.zipToMap(accessibleSnapshots)
-
-        return classes.map { accessibleClassSnapshots[it] ?: InaccessibleClassSnapshot }
-    }
-
-    private fun doSnapshot(
-        classes: List<ClassFileWithContents>,
-        protoBased: Boolean? = null,
-        includeDebugInfoInSnapshot: Boolean? = null
-    ): List<ClassSnapshot> {
-        // Snapshot Kotlin classes first
-        val kotlinSnapshots: List<KotlinClassSnapshot?> = classes.map { clazz ->
-            trySnapshotKotlinClass(clazz)
-        }
-        val kotlinClassSnapshots: Map<ClassFileWithContents, KotlinClassSnapshot?> = classes.zipToMap(kotlinSnapshots)
-
-        // Snapshot the remaining Java classes
-        val javaClasses: List<ClassFileWithContents> = classes.filter { kotlinClassSnapshots[it] == null }
-        val javaSnapshots: List<JavaClassSnapshot> = snapshotJavaClasses(javaClasses, protoBased, includeDebugInfoInSnapshot)
-        val javaClassSnapshots: Map<ClassFileWithContents, JavaClassSnapshot> = javaClasses.zipToMap(javaSnapshots)
-
-        return classes.map { kotlinClassSnapshots[it] ?: javaClassSnapshots[it]!! }
-    }
-
-    /** Creates [KotlinClassSnapshot] of the given class, or returns `null` if the class is not a Kotlin class. */
-    private fun trySnapshotKotlinClass(classFile: ClassFileWithContents): KotlinClassSnapshot? {
-        return if (classFile.classInfo.isKotlinClass) {
-            val kotlinClassInfo =
-                KotlinClassInfo.createFrom(classFile.classInfo.classId, classFile.classInfo.kotlinClassHeader!!, classFile.contents)
-            val packageMembers = when (kotlinClassInfo.classKind) {
-                CLASS, MULTIFILE_CLASS -> null // See `KotlinClassSnapshot.packageMembers`'s kdoc
-                else -> (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames().map {
-                    PackageMember(kotlinClassInfo.classId.packageFqName, it)
-                }
-            }
-            KotlinClassSnapshot(kotlinClassInfo, classFile.classInfo.supertypes, packageMembers)
-        } else null
-    }
-
-    /** Creates [JavaClassSnapshot]s of the given Java classes. */
-    private fun snapshotJavaClasses(
-        classes: List<ClassFileWithContents>,
-        protoBased: Boolean? = null,
-        includeDebugInfoInSnapshot: Boolean? = null
-    ): List<JavaClassSnapshot> {
-        return if (protoBased ?: protoBasedDefaultValue) {
-            snapshotJavaClassesProtoBased(classes)
-        } else {
-            classes.map { JavaClassSnapshotter.snapshot(it, includeDebugInfoInSnapshot) }
-        }
-    }
-
-    private fun snapshotJavaClassesProtoBased(classFilesWithContents: List<ClassFileWithContents>): List<JavaClassSnapshot> {
-        val classIds = classFilesWithContents.map { it.classInfo.classId }
-        val classesContents = classFilesWithContents.map { it.contents }
-        val descriptors: List<JavaClassDescriptor?> = JavaClassDescriptorCreator.create(classIds, classesContents)
-        val snapshots: List<JavaClassSnapshot> = descriptors.mapIndexed { index, descriptor ->
-            val classFileWithContents = classFilesWithContents[index]
-            if (descriptor != null) {
-                try {
-                    ProtoBasedJavaClassSnapshot(descriptor.toSerializedJavaClass())
-                } catch (e: Throwable) {
-                    if (isKnownExceptionWhenReadingDescriptor(e)) {
-                        ContentHashJavaClassSnapshot(classFileWithContents.contents.md5())
-                    } else throw e
-                }
-            } else {
-                if (isKnownProblematicClass(classFileWithContents.classFile)) {
-                    ContentHashJavaClassSnapshot(classFileWithContents.contents.md5())
-                } else {
-                    error(
-                        "Failed to create JavaClassDescriptor for class '${classFileWithContents.classFile.unixStyleRelativePath}'" +
-                                " in '${classFileWithContents.classFile.classRoot.path}'"
-                    )
-                }
+        return classes.map {
+            when {
+                it.classInfo in inaccessibleClassesInfo -> InaccessibleClassSnapshot
+                it.classInfo.isKotlinClass -> snapshotKotlinClass(it)
+                else -> JavaClassSnapshotter.snapshot(it, includeDebugInfoInJavaSnapshot)
            }
        }
-        return snapshots
+    }
+
+    /** Creates [KotlinClassSnapshot] of the given Kotlin class (the caller must ensure that the given class is a Kotlin class). */
+    private fun snapshotKotlinClass(classFile: ClassFileWithContents): KotlinClassSnapshot {
+        val kotlinClassInfo =
+            KotlinClassInfo.createFrom(classFile.classInfo.classId, classFile.classInfo.kotlinClassHeader!!, classFile.contents)
+        val packageMembers = when (kotlinClassInfo.classKind) {
+            CLASS, MULTIFILE_CLASS -> null // See `KotlinClassSnapshot.packageMembers`'s kdoc
+            else -> (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames().map {
+                PackageMember(kotlinClassInfo.classId.packageFqName, it)
+            }
+        }
+        return KotlinClassSnapshot(kotlinClassInfo, classFile.classInfo.supertypes, packageMembers)
    }

    /**
@@ -208,57 +114,6 @@ object ClassSnapshotter {

        return classesInfo.filter { it.isTransitivelyInaccessible() }
    }
-
-    /** Returns `true` if it is known that the given exception can be thrown when calling [JavaClassDescriptor.toSerializedJavaClass]. */
-    private fun isKnownExceptionWhenReadingDescriptor(throwable: Throwable): Boolean {
-        // When building the Kotlin repo with `./gradlew publish -Pbootstrap.local=true
-        // -Pbootstrap.local.path=/path/to/kotlin/build/repo -Pkotlin.incremental.useClasspathSnapshot=true`, the build can fail with:
-        //   org.gradle.api.internal.artifacts.transform.TransformException: Execution failed for ClasspathEntrySnapshotTransform: ~/.gradle/wrapper/dists/gradle-6.9-bin/2ecsmyp3bolyybemj56vfn4mt/gradle-6.9/lib/kotlin-reflect-1.4.20.jar
-        //   Caused by: java.lang.IncompatibleClassChangeError: Expected static method 'java.lang.Object org.jetbrains.kotlin.utils.DFS.dfsFromNode(java.lang.Object, org.jetbrains.kotlin.utils.DFS$Neighbors, org.jetbrains.kotlin.utils.DFS$Visited, org.jetbrains.kotlin.utils.DFS$NodeHandler)'
-        //     at org.jetbrains.kotlin.builtins.FunctionTypesKt.isTypeOrSubtypeOf(functionTypes.kt:31)
-        //     (... at JavaClassDescriptor.toSerializedJavaClass)
-        // The reason is that:
-        //   - org/jetbrains/kotlin/builtins/FunctionTypesKt.class is located in ~/.gradle/caches/jars-8/66425fb82fd14126e9aa07dcd3100b42/kotlin-compiler-embeddable-1.6.255-20210909.213620-55.jar
-        //   - org/jetbrains/kotlin/utils/DFS.class is located in ~/.gradle/caches/jars-8/c716e2e2d26b16f6f1462e59ba44cf3b/buildSrc.jar
-        // And somehow the two classes are incompatible (probably similar to the NoSuchMethodError documented at JavaClassDescriptorCreatorKt.createBinaryJavaClass).
-        // This happens to a few other jars inside gradle-6.9/lib.
-        // However, outside the Kotlin repo build, we don't have this issue (org/jetbrains/kotlin/builtins/FunctionTypesKt.class and
-        // org/jetbrains/kotlin/utils/DFS.class will be located in the same kotlin-compiler-embeddable.jar).
-        // Therefore, we special-case the Kotlin repo build below.
-        // TODO: See how we can address this issue.
-        return (throwable is IncompatibleClassChangeError &&
-                DFS::class.java.classLoader.getResource(DFS::class.java.name.replace('.', '/') + ".class")
-                    ?.path?.contains("buildSrc.jar") == true
-                )
-    }
-
-    /** Returns `true` if it is known that the snapshot of the given class can't be created for some reason. */
-    private fun isKnownProblematicClass(classFile: ClassFile): Boolean {
-        if (classFile.classRoot.name.startsWith("groovy")
-            && classFile.unixStyleRelativePath.endsWith("\$CollectorHelper.class")
-        ) {
-            // [FAULTY JAR] In groovy-all-1.3-2.5.12.jar and groovy-2.5.11.jar, the bytecode of
-            // groovy/cli/OptionField\$CollectorHelper.class indicates that its outer class is groovy/cli/OptionField, but the bytecode of
-            // groovy/cli/OptionField.class does not list any nested classes.
-            // This happens with a few other CollectorHelper classes in these jars.
-            // Therefore, these are faulty jars, and our snapshotting logic cannot process it.
-            return true
-        }
-        if (classFile.classRoot.name.startsWith("gradle-api")
-            && classFile.unixStyleRelativePath.startsWith("org/gradle/internal/impldep/META-INF/versions")
-        ) {
-            // [FAULTY JAR] gradle-api-6.9.jar has the following entries:
-            //   - org/gradle/internal/impldep/org/junit/platform/commons/util/ModuleUtils.class
-            //   - org/gradle/internal/impldep/META-INF/versions/9/org/junit/platform/commons/util/ModulesUtils.class
-            //   - org/gradle/internal/impldep/META-INF/versions/9/org/junit/platform/commons/util/ModuleUtils$ModuleReferenceScanner.class
-            // The META-INF directories are located not at the top level (which is not expected), and those directories escaped our filter
-            // which filters out top-level META-INF directories. We then failed to snapshot ModuleUtils$ModuleReferenceScanner.class as
-            // there are 2 versions of ModuleUtils.class, and the one outside the META-INF directory doesn't have any nested classes.
-            // Therefore, this is a faulty jar, and our snapshotting logic cannot process it.
-            return true
-        }
-        return false
-    }
 }

 /** Utility to read the contents of a directory or jar. */
@@ -317,20 +172,3 @@ private object DirectoryOrJarContentsReader {
        return relativePathsToContents.toSortedMap().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 <K, V> List<K>.zipToMap(other: List<V>): LinkedHashMap<K, V> {
-    check(this.size == other.size)
-    val map = LinkedHashMap<K, V>(size)
-    indices.forEach { index ->
-        map[this[index]] = other[index]
-    }
-    return map
-}
-
-private const val protoBasedDefaultValue = false
@@ -18,11 +18,11 @@ object JavaClassChangesComputer {
     * Each list must not contain duplicate classes (having the same [JvmClassName]/[ClassId]).
     */
    fun compute(
-        currentJavaClassSnapshots: List<RegularJavaClassSnapshot>,
-        previousJavaClassSnapshots: List<RegularJavaClassSnapshot>
+        currentJavaClassSnapshots: List<JavaClassSnapshot>,
+        previousJavaClassSnapshots: List<JavaClassSnapshot>
    ): ChangeSet {
-        val currentClasses: Map<ClassId, RegularJavaClassSnapshot> = currentJavaClassSnapshots.associateBy { it.classId }
-        val previousClasses: Map<ClassId, RegularJavaClassSnapshot> = previousJavaClassSnapshots.associateBy { it.classId }
+        val currentClasses: Map<ClassId, JavaClassSnapshot> = currentJavaClassSnapshots.associateBy { it.classId }
+        val previousClasses: Map<ClassId, JavaClassSnapshot> = previousJavaClassSnapshots.associateBy { it.classId }

        // Note: Added classes can also impact recompilation.
        // For example, suppose a source file uses `SomeClass` through `*` imports:
@@ -50,8 +50,8 @@ object JavaClassChangesComputer {
     * The two classes must have the same [ClassId].
     */
    private fun collectClassChanges(
-        currentClassSnapshot: RegularJavaClassSnapshot,
-        previousClassSnapshot: RegularJavaClassSnapshot,
+        currentClassSnapshot: JavaClassSnapshot,
+        previousClassSnapshot: JavaClassSnapshot,
        changes: ChangeSet.Collector
    ) {
        val classId = currentClassSnapshot.classId.also { check(it == previousClassSnapshot.classId) }
@@ -52,7 +52,7 @@ object JavaClassSnapshotter {
        abiClass.methods.clear()
        val classAbiExcludingMembers = abiClass.let { snapshotJavaElement(it, it.name, includeDebugInfoInSnapshot) }

-        return RegularJavaClassSnapshot(
+        return JavaClassSnapshot(
            classFile.classInfo.classId, classFile.classInfo.supertypes,
            classAbiExcludingMembers, fieldsAbi, methodsAbi
        )
@@ -13,8 +13,6 @@ import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotTestCommo
 import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
-import org.junit.runner.RunWith
-import org.junit.runners.Parameterized
 import java.io.File
 import kotlin.test.fail

@@ -185,18 +183,11 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {
    }
 }

-@RunWith(Parameterized::class)
-class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : ClasspathChangesComputerTest() {
-
-    companion object {
-        @Parameterized.Parameters(name = "protoBased={0}")
-        @JvmStatic
-        fun parameters() = listOf(true, false)
-    }
+class JavaOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {

    @Test
    override fun testAbiVersusNonAbiChanges() {
-        val changes = computeClasspathChanges(File(testDataDir, "testAbiVersusNonAbiChanges/src/java"), tmpDir, protoBased)
+        val changes = computeClasspathChanges(File(testDataDir, "testAbiVersusNonAbiChanges/src/java"), tmpDir)
        Changes(
            lookupSymbols = setOf(
                LookupSymbol(name = "publicFieldChangedType", scope = "com.example.SomeClass"),
@@ -209,7 +200,7 @@ class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : Cl

    @Test
    override fun testModifiedAddedRemovedElements() {
-        val changes = computeClasspathChanges(File(testDataDir, "testModifiedAddedRemovedElements/src/java"), tmpDir, protoBased)
+        val changes = computeClasspathChanges(File(testDataDir, "testModifiedAddedRemovedElements/src/java"), tmpDir)
        Changes(
            lookupSymbols = setOf(
                // ModifiedClassUnchangedMembers
@@ -241,7 +232,7 @@ class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : Cl

    @Test
    override fun testImpactAnalysis() {
-        val changes = computeClasspathChanges(File(testDataDir, "testImpactAnalysis_JavaOnly/src"), tmpDir, protoBased)
+        val changes = computeClasspathChanges(File(testDataDir, "testImpactAnalysis_JavaOnly/src"), tmpDir)
        Changes(
            lookupSymbols = setOf(
                LookupSymbol(name = "changedField", scope = "com.example.ChangedSuperClass"),
@@ -305,20 +296,20 @@ class KotlinAndJavaClasspathChangesComputerTest : ClasspathSnapshotTestCommon()
    }
 }

-private fun computeClasspathChanges(classpathSourceDir: File, tmpDir: TemporaryFolder, protoBased: Boolean = false): Changes {
-    val currentSnapshot = snapshotClasspath(File(classpathSourceDir, "current-classpath"), tmpDir, protoBased)
-    val previousSnapshot = snapshotClasspath(File(classpathSourceDir, "previous-classpath"), tmpDir, protoBased)
+private fun computeClasspathChanges(classpathSourceDir: File, tmpDir: TemporaryFolder): Changes {
+    val currentSnapshot = snapshotClasspath(File(classpathSourceDir, "current-classpath"), tmpDir)
+    val previousSnapshot = snapshotClasspath(File(classpathSourceDir, "previous-classpath"), tmpDir)
    return computeClasspathChanges(currentSnapshot, previousSnapshot)
 }

-private fun snapshotClasspath(classpathSourceDir: File, tmpDir: TemporaryFolder, protoBased: Boolean): ClasspathSnapshot {
+private fun snapshotClasspath(classpathSourceDir: File, tmpDir: TemporaryFolder): ClasspathSnapshot {
    val classpath = mutableListOf<File>()
    val classpathEntrySnapshots = classpathSourceDir.listFiles()!!.sortedBy { it.name }.map { classpathEntrySourceDir ->
        val classFiles = compileAll(classpathEntrySourceDir, classpath, tmpDir)
        classpath.addAll(listOfNotNull(classFiles.firstOrNull()?.classRoot))

        val relativePaths = classFiles.map { it.unixStyleRelativePath }
-        val classSnapshots = classFiles.snapshot(protoBased).map { it.withHash }
+        val classSnapshots = classFiles.snapshot().map { it.withHash }
        ClasspathEntrySnapshot(
            classSnapshots = relativePaths.zip(classSnapshots).toMap(LinkedHashMap())
        )
@@ -164,11 +164,11 @@ abstract class ClasspathSnapshotTestCommon {

        fun ClassFile.readBytes() = asFile().readBytes()

-        fun ClassFile.snapshot(protoBased: Boolean? = null): ClassSnapshot = listOf(this).snapshot(protoBased).single()
+        fun ClassFile.snapshot(): ClassSnapshot = listOf(this).snapshot().single()

-        fun List<ClassFile>.snapshot(protoBased: Boolean? = null): List<ClassSnapshot> {
+        fun List<ClassFile>.snapshot(): List<ClassSnapshot> {
            val classFilesWithContents = this.map { ClassFileWithContents(it, it.readBytes()) }
-            return ClassSnapshotter.snapshot(classFilesWithContents, protoBased)
+            return ClassSnapshotter.snapshot(classFilesWithContents)
        }
    }
 }
@@ -34,7 +34,7 @@ abstract class ClasspathSnapshotterTest : ClasspathSnapshotTestCommon() {
    @Test
    fun `test ClassSnapshotter's result against expected snapshot`() {
        val classSnapshot = sourceFile.compileSingle().let {
-            ClassSnapshotter.snapshot(listOf(ClassFileWithContents(it, it.readBytes())), includeDebugInfoInSnapshot = true)
+            ClassSnapshotter.snapshot(listOf(ClassFileWithContents(it, it.readBytes())), includeDebugInfoInJavaSnapshot = true)
        }.single()

        assertEquals(expectedSnapshotFile.readText(), classSnapshot.toGson())