KT-45777: Don't compute snapshots for inaccessible classes

Also visit a class file with ASM once to extract all information we need in advance, instead of visiting the class file each time some piece of info is needed.
2021-11-17 11:48:12 +00:00
parent f52be5f471
commit 6bee7948e7
13 changed files with 395 additions and 540 deletions
@@ -0,0 +1,115 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.incremental.classpathDiff
+
+import org.jetbrains.kotlin.load.kotlin.FileBasedKotlinClass.*
+import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
+import org.jetbrains.kotlin.load.kotlin.header.ReadKotlinClassHeaderAnnotationVisitor
+import org.jetbrains.kotlin.name.ClassId
+import org.jetbrains.kotlin.resolve.jvm.JvmClassName
+import org.jetbrains.org.objectweb.asm.AnnotationVisitor
+import org.jetbrains.org.objectweb.asm.ClassReader
+import org.jetbrains.org.objectweb.asm.ClassVisitor
+import org.jetbrains.org.objectweb.asm.Opcodes
+
+/** Basic information about a class (e.g., [classId], [kotlinClassHeader] and [supertypes]). */
+class BasicClassInfo(
+    val classId: ClassId,
+    val kotlinClassHeader: KotlinClassHeader?, // null if this is not a Kotlin class
+    val supertypes: List<JvmClassName>,
+
+    private val accessFlags: Int,
+    val isAnonymous: Boolean
+) {
+    val isKotlinClass = kotlinClassHeader != null
+    val isPrivate = flagEnabled(accessFlags, Opcodes.ACC_PRIVATE)
+    val isLocal = classId.isLocal
+
+    /** Whether this is a synthetic Java class. */
+    val isJavaSynthetic = !isKotlinClass && flagEnabled(accessFlags, Opcodes.ACC_SYNTHETIC)
+
+    /**
+     * Whether this is a synthetic Kotlin class.
+     *
+     * Note that we use [KotlinClassHeader.Kind], not [accessFlags], to make this determination. For example,
+     * 'kotlin/Metadata.DefaultImpls' is synthetic according to its [KotlinClassHeader.Kind], but not synthetic according to its
+     * [accessFlags].
+     */
+    @Suppress("unused")
+    val isKotlinSynthetic = isKotlinClass && kotlinClassHeader!!.kind == KotlinClassHeader.Kind.SYNTHETIC_CLASS
+
+    private fun flagEnabled(accessFlags: Int, flagToCheck: Int) = (accessFlags and flagToCheck) != 0
+
+    companion object {
+
+        fun compute(classContents: ByteArray): BasicClassInfo {
+            val kotlinClassHeaderClassVisitor = KotlinClassHeaderClassVisitor()
+            val innerClassesClassVisitor = InnerClassesClassVisitor(kotlinClassHeaderClassVisitor)
+            val basicClassInfoVisitor = BasicClassInfoClassVisitor(innerClassesClassVisitor)
+
+            ClassReader(classContents).accept(
+                basicClassInfoVisitor,
+                ClassReader.SKIP_CODE or ClassReader.SKIP_DEBUG or ClassReader.SKIP_FRAMES
+            )
+
+            val className = basicClassInfoVisitor.getClassName()
+            val innerClassesInfo = innerClassesClassVisitor.getInnerClassesInfo()
+
+            return BasicClassInfo(
+                classId = resolveNameByInternalName(className, innerClassesInfo),
+                kotlinClassHeader = kotlinClassHeaderClassVisitor.getKotlinClassHeader(),
+                supertypes = basicClassInfoVisitor.getSupertypes(),
+                accessFlags = basicClassInfoVisitor.getAccessFlags(),
+                isAnonymous = innerClassesInfo[className]?.let { it.innerSimpleName == null } ?: false
+            )
+        }
+    }
+}
+
+private class BasicClassInfoClassVisitor(cv: ClassVisitor) : ClassVisitor(Opcodes.API_VERSION, cv) {
+    private var className: String? = null
+    private var classAccess: Int? = null
+    private val supertypeNames = mutableListOf<String>()
+
+    override fun visit(version: Int, access: Int, name: String, signature: String?, superName: String?, interfaces: Array<String>?) {
+        className = name
+        classAccess = access
+        superName?.let { supertypeNames.add(it) }
+        interfaces?.let { supertypeNames.addAll(it) }
+        super.visit(version, access, name, signature, superName, interfaces)
+    }
+
+    fun getClassName(): String = className!!
+    fun getAccessFlags(): Int = classAccess!!
+    fun getSupertypes(): List<JvmClassName> = supertypeNames.map { JvmClassName.byInternalName(it) }
+}
+
+private class InnerClassesClassVisitor(cv: ClassVisitor) : ClassVisitor(Opcodes.API_VERSION, cv) {
+
+    private val innerClassesInfo = InnerClassesInfo()
+
+    override fun visitInnerClass(name: String, outerName: String?, innerName: String?, access: Int) {
+        innerClassesInfo.add(name, outerName, innerName)
+        super.visitInnerClass(name, outerName, innerName, access)
+    }
+
+    fun getInnerClassesInfo(): InnerClassesInfo = innerClassesInfo
+}
+
+private class KotlinClassHeaderClassVisitor : ClassVisitor(Opcodes.API_VERSION) {
+
+    private val kotlinClassHeaderAnnotationVisitor = ReadKotlinClassHeaderAnnotationVisitor()
+
+    override fun visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor? {
+        return convertAnnotationVisitor(
+            kotlinClassHeaderAnnotationVisitor,
+            descriptor,
+            InnerClassesInfo() // This info is not needed to resolve KotlinClassHeader
+        )
+    }
+
+    fun getKotlinClassHeader(): KotlinClassHeader? = kotlinClassHeaderAnnotationVisitor.createHeader()
+}
@@ -153,7 +153,7 @@ object ClasspathChangesComputer {
                    )
                    incrementalJvmCache.markDirty(JvmClassName.byClassId(previousSnapshot.serializedJavaClass.classId))
                }
-                is RegularJavaClassSnapshot, is EmptyJavaClassSnapshot, is ContentHashJavaClassSnapshot -> {
+                is RegularJavaClassSnapshot, is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
                    error("Unexpected type (it should have been handled earlier): ${previousSnapshot.javaClass.name}")
                }
            }
@@ -183,7 +183,7 @@ object ClasspathChangesComputer {
                        collector = changesCollector
                    )
                }
-                is RegularJavaClassSnapshot, is EmptyJavaClassSnapshot, is ContentHashJavaClassSnapshot -> {
+                is RegularJavaClassSnapshot, is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
                    error("Unexpected type (it should have been handled earlier): ${currentSnapshot.javaClass.name}")
                }
            }
@@ -331,7 +331,7 @@ internal fun ClassSnapshot.getClassId(): ClassId {
        is KotlinClassSnapshot -> classInfo.classId
        is RegularJavaClassSnapshot -> classId
        is ProtoBasedJavaClassSnapshot -> serializedJavaClass.classId
-        is EmptyJavaClassSnapshot, is ContentHashJavaClassSnapshot -> {
+        is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
            error("Unexpected type (it should have been handled earlier): ${javaClass.name}")
        }
    }
@@ -351,7 +351,7 @@ internal fun ClassSnapshot.getSupertypes(classIdResolver: (JvmClassName) -> Clas
            val (proto, nameResolver) = serializedJavaClass.toProtoData()
            proto.supertypes(TypeTable(proto.typeTable)).map { nameResolver.getClassId(it.className) }
        }
-        is EmptyJavaClassSnapshot, is ContentHashJavaClassSnapshot -> {
+        is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> {
            error("Unexpected type (it should have been handled earlier): ${javaClass.name}")
        }
    }
@@ -107,12 +107,12 @@ class AbiSnapshotForTests(
 class ProtoBasedJavaClassSnapshot(val serializedJavaClass: SerializedJavaClass) : JavaClassSnapshot()

 /**
- * [JavaClassSnapshot] of a Java class where there is nothing to capture.
+ * [ClassSnapshot] of an inaccessible class.
 *
- * For example, the snapshot of a local class is empty as a local class can't be referenced from other source files and therefore any
- * changes in a local class will not cause recompilation of other source files.
+ * For example, a local class is inaccessible as it can't be referenced from other source files and therefore any changes in a local class
+ * will not require recompilation of other source files.
 */
-object EmptyJavaClassSnapshot : JavaClassSnapshot()
+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
@@ -65,19 +65,28 @@ object ClasspathEntrySnapshotExternalizer : DataExternalizer<ClasspathEntrySnaps
 object ClassSnapshotExternalizer : DataExternalizer<ClassSnapshot> {

    override fun save(output: DataOutput, snapshot: ClassSnapshot) {
-        output.writeBoolean(snapshot is KotlinClassSnapshot)
        when (snapshot) {
-            is KotlinClassSnapshot -> KotlinClassSnapshotExternalizer.save(output, snapshot)
-            is JavaClassSnapshot -> JavaClassSnapshotExternalizer.save(output, snapshot)
+            is KotlinClassSnapshot -> {
+                output.writeString(KotlinClassSnapshot::class.java.name)
+                KotlinClassSnapshotExternalizer.save(output, snapshot)
+            }
+            is JavaClassSnapshot -> {
+                output.writeString(JavaClassSnapshot::class.java.name)
+                JavaClassSnapshotExternalizer.save(output, snapshot)
+            }
+            is InaccessibleClassSnapshot -> {
+                output.writeString(InaccessibleClassSnapshot::class.java.name)
+                InaccessibleClassSnapshotExternalizer.save(output, snapshot)
+            }
        }
    }

    override fun read(input: DataInput): ClassSnapshot {
-        val isKotlinClassSnapshot = input.readBoolean()
-        return if (isKotlinClassSnapshot) {
-            KotlinClassSnapshotExternalizer.read(input)
-        } else {
-            JavaClassSnapshotExternalizer.read(input)
+        return when (val className = input.readString()) {
+            KotlinClassSnapshot::class.java.name -> KotlinClassSnapshotExternalizer.read(input)
+            JavaClassSnapshot::class.java.name -> JavaClassSnapshotExternalizer.read(input)
+            InaccessibleClassSnapshot::class.java.name -> InaccessibleClassSnapshotExternalizer.read(input)
+            else -> error("Unrecognized class name: $className")
        }
    }
 }
@@ -144,7 +153,6 @@ object JavaClassSnapshotExternalizer : DataExternalizer<JavaClassSnapshot> {
        when (snapshot) {
            is RegularJavaClassSnapshot -> RegularJavaClassSnapshotExternalizer.save(output, snapshot)
            is ProtoBasedJavaClassSnapshot -> ProtoBasedJavaClassSnapshotExternalizer.save(output, snapshot)
-            is EmptyJavaClassSnapshot -> EmptyJavaClassSnapshotExternalizer.save(output, snapshot)
            is ContentHashJavaClassSnapshot -> ContentHashJavaClassSnapshotExternalizer.save(output, snapshot)
        }
    }
@@ -153,7 +161,6 @@ object JavaClassSnapshotExternalizer : DataExternalizer<JavaClassSnapshot> {
        return when (val className = input.readString()) {
            RegularJavaClassSnapshot::class.java.name -> RegularJavaClassSnapshotExternalizer.read(input)
            ProtoBasedJavaClassSnapshot::class.java.name -> ProtoBasedJavaClassSnapshotExternalizer.read(input)
-            EmptyJavaClassSnapshot::class.java.name -> EmptyJavaClassSnapshotExternalizer.read(input)
            ContentHashJavaClassSnapshot::class.java.name -> ContentHashJavaClassSnapshotExternalizer.read(input)
            else -> error("Unrecognized class name: $className")
        }
@@ -204,14 +211,14 @@ object ProtoBasedJavaClassSnapshotExternalizer : DataExternalizer<ProtoBasedJava
    }
 }

-object EmptyJavaClassSnapshotExternalizer : DataExternalizer<EmptyJavaClassSnapshot> {
+object InaccessibleClassSnapshotExternalizer : DataExternalizer<InaccessibleClassSnapshot> {

-    override fun save(output: DataOutput, snapshot: EmptyJavaClassSnapshot) {
+    override fun save(output: DataOutput, snapshot: InaccessibleClassSnapshot) {
        // Nothing to save
    }

-    override fun read(input: DataInput): EmptyJavaClassSnapshot {
-        return EmptyJavaClassSnapshot
+    override fun read(input: DataInput): InaccessibleClassSnapshot {
+        return InaccessibleClassSnapshot
    }
 }

@@ -20,7 +20,7 @@ object ClasspathSnapshotShrinker {
    /**
     * Shrinks the given [ClasspathSnapshot] by retaining only classes that are referenced. Referencing info is stored in [LookupStorage].
     *
-     * This method also removes duplicate classes and [EmptyJavaClassSnapshot]s first.
+     * This method also removes duplicate classes and [InaccessibleClassSnapshot]s first.
     */
    fun shrink(
        classpathSnapshot: ClasspathSnapshot,
@@ -28,10 +28,10 @@ object ClasspathSnapshotShrinker {
        metrics: BuildMetricsReporter
    ): List<ClassSnapshotWithHash> {
        val allClasses = metrics.measure(BuildTime.GET_NON_DUPLICATE_CLASSES) {
-            // It's important to remove duplicate classes first before removing `EmptyJavaClassSnapshot`s.
+            // It's important to remove duplicate classes first before removing `InaccessibleClassSnapshot`s.
            // For example, if jar1!/com/example/A.class is empty and jar2!/com/example/A.class is non-empty, incorrect order of the actions
            // will lead to incorrect results.
-            classpathSnapshot.getNonDuplicateClassSnapshots().filter { it.classSnapshot !is EmptyJavaClassSnapshot }
+            classpathSnapshot.getNonDuplicateClassSnapshots().filter { it.classSnapshot !is InaccessibleClassSnapshot }
        }
        val lookupSymbols = metrics.measure(BuildTime.GET_LOOKUP_SYMBOLS) {
            lookupStorage.lookupMap.keys
@@ -5,18 +5,14 @@

 package org.jetbrains.kotlin.incremental.classpathDiff

-import org.jetbrains.kotlin.incremental.*
+import org.jetbrains.kotlin.incremental.JavaClassDescriptorCreator
+import org.jetbrains.kotlin.incremental.KotlinClassInfo
+import org.jetbrains.kotlin.incremental.md5
+import org.jetbrains.kotlin.incremental.toSerializedJavaClass
 import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor
-import org.jetbrains.kotlin.metadata.deserialization.TypeTable
-import org.jetbrains.kotlin.metadata.deserialization.supertypes
-import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil
+import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
 import org.jetbrains.kotlin.name.ClassId
-import org.jetbrains.kotlin.resolve.jvm.JvmClassName
-import org.jetbrains.kotlin.serialization.deserialization.getClassId
 import org.jetbrains.kotlin.utils.DFS
-import org.jetbrains.org.objectweb.asm.ClassReader
-import org.jetbrains.org.objectweb.asm.ClassVisitor
-import org.jetbrains.org.objectweb.asm.Opcodes
 import java.io.File
 import java.util.zip.ZipInputStream

@@ -40,7 +36,7 @@ object ClasspathEntrySnapshotter {
        val snapshots = try {
            ClassSnapshotter.snapshot(classes, protoBased).map { it.addHash() }
        } catch (e: Throwable) {
-            if (isKnownProblematicClasspathEntry(classpathEntry)) {
+            if ((protoBased ?: protoBasedDefaultValue) && isKnownProblematicClasspathEntry(classpathEntry)) {
                classes.map { ContentHashJavaClassSnapshot(it.contents.md5()).addHash() }
            } else throw e
        }
@@ -74,112 +70,83 @@ object ClasspathEntrySnapshotter {
 /** Creates [ClassSnapshot]s of classes. */
 object ClassSnapshotter {

-    /**
-     * Creates [ClassSnapshot]s of the given classes.
-     *
-     * Note that for Java (non-Kotlin) classes, creating a [ClassSnapshot] for a nested class will require accessing the outer class (and
-     * possibly vice versa). Therefore, outer classes and nested classes must be passed together in one invocation of this method.
-     */
+    /** Creates [ClassSnapshot]s of the given classes. */
    fun snapshot(
        classes: List<ClassFileWithContents>,
        protoBased: Boolean? = null,
        includeDebugInfoInSnapshot: Boolean? = null
    ): List<ClassSnapshot> {
-        // Snapshot Kotlin classes first
-        val kotlinClassSnapshots: Map<ClassFileWithContents, KotlinClassSnapshot?> = classes.associateWith {
-            trySnapshotKotlinClass(it)
-        }
+        val classesInfo: List<BasicClassInfo> = classes.map { BasicClassInfo.compute(it.contents) }

-        // Snapshot the remaining Java classes in one invocation
+        // Find inaccessible classes first, their snapshots will be `InaccessibleClassSnapshot`s.
+        val inaccessibleClasses: Set<BasicClassInfo> = getInaccessibleClasses(classesInfo).toSet()
+
+        // Snapshot the remaining accessible classes
+        val accessibleClasses: List<ClassFileWithContents> = classes.mapIndexedNotNull { index, clazz ->
+            if (classesInfo[index] in inaccessibleClasses) null else clazz
+        }
+        val accessibleClassesInfo: List<BasicClassInfo> = classesInfo.filterNot { it in inaccessibleClasses }
+        val accessibleSnapshots: List<ClassSnapshot> =
+            doSnapshot(accessibleClasses, accessibleClassesInfo, protoBased, includeDebugInfoInSnapshot)
+        val accessibleClassSnapshots: Map<ClassFileWithContents, ClassSnapshot> = accessibleClasses.zipToMap(accessibleSnapshots)
+
+        return classes.map { accessibleClassSnapshots[it] ?: InaccessibleClassSnapshot }
+    }
+
+    private fun doSnapshot(
+        classes: List<ClassFileWithContents>,
+        classesInfo: List<BasicClassInfo>,
+        protoBased: Boolean? = null,
+        includeDebugInfoInSnapshot: Boolean? = null
+    ): List<ClassSnapshot> {
+        // Snapshot Kotlin classes first
+        val kotlinSnapshots: List<KotlinClassSnapshot?> = classes.mapIndexed { index, clazz ->
+            trySnapshotKotlinClass(clazz, classesInfo[index])
+        }
+        val kotlinClassSnapshots: Map<ClassFileWithContents, KotlinClassSnapshot?> = classes.zipToMap(kotlinSnapshots)
+
+        // Snapshot the remaining Java classes
        val javaClasses: List<ClassFileWithContents> = classes.filter { kotlinClassSnapshots[it] == null }
-        val snapshots: List<JavaClassSnapshot> = snapshotJavaClasses(javaClasses, protoBased, includeDebugInfoInSnapshot)
-        val javaClassSnapshots: Map<ClassFileWithContents, JavaClassSnapshot> = javaClasses.zipToMap(snapshots)
+        val javaClassesInfo: List<BasicClassInfo> = classesInfo.mapIndexedNotNull { index, classInfo ->
+            val javaClass = classes[index]
+            if (kotlinClassSnapshots[javaClass] == null) classInfo else null
+        }
+        val javaSnapshots: List<JavaClassSnapshot> =
+            snapshotJavaClasses(javaClasses, javaClassesInfo, 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(clazz: ClassFileWithContents): KotlinClassSnapshot? {
-        return KotlinClassInfo.tryCreateFrom(clazz.contents)?.let {
-            KotlinClassSnapshot(it, computeSupertypes(it, clazz.contents))
-        }
+    private fun trySnapshotKotlinClass(classFile: ClassFileWithContents, classInfo: BasicClassInfo): KotlinClassSnapshot? {
+        return if (classInfo.isKotlinClass) {
+            val kotlinClassInfo = KotlinClassInfo.createFrom(classInfo.classId, classInfo.kotlinClassHeader!!, classFile.contents)
+            KotlinClassSnapshot(kotlinClassInfo, classInfo.supertypes)
+        } else null
    }

-    // TODO: Find a faster way to get supertypes without loading protos (e.g., attach to an existing ASM visitor)
-    private fun computeSupertypes(classInfo: KotlinClassInfo, classContents: ByteArray): List<JvmClassName> {
-        return try {
-            val (nameResolver, proto) = JvmProtoBufUtil.readClassDataFrom(classInfo.classHeaderData, classInfo.classHeaderStrings)
-            val supertypeClassIds = proto.supertypes(TypeTable(proto.typeTable)).map { nameResolver.getClassId(it.className) }
-            supertypeClassIds.map { JvmClassName.byClassId(it) }
-        } catch (e: Exception) {
-            // The above method call currently fails on a few classes for some reason:
-            //   - org.jetbrains.kotlin.protobuf.InvalidProtocolBufferException: Message was missing required fields.
-            //     (Lite runtime could not determine which fields were missing) for SomeClassKt.class
-            //   - java.lang.NullPointerException: parseDelimitedFrom(this, EXTENSION_REGISTRY) must not be null for
-            //     kotlin-stdlib-1.6.255-SNAPSHOT.jar
-            // Fall back to ASM visitor to get the supertypes.
-            val supertypeClassNames = mutableListOf<String>()
-            ClassReader(classContents).accept(object : ClassVisitor(Opcodes.API_VERSION) {
-                override fun visit(
-                    version: Int, access: Int, name: String, signature: String?, superName: String?, interfaces: Array<String>?
-                ) {
-                    superName?.let { supertypeClassNames.add(it) }
-                    interfaces?.let { supertypeClassNames.addAll(it) }
-                }
-            }, ClassReader.SKIP_CODE or ClassReader.SKIP_DEBUG)
-            supertypeClassNames.map { JvmClassName.byInternalName(it) }
-        }
-    }
-
-    /**
-     * Creates [JavaClassSnapshot]s of the given Java classes.
-     *
-     * Note that creating a [JavaClassSnapshot] for a nested class will require accessing the outer class (and possibly vice versa).
-     * Therefore, outer classes and nested classes must be passed together in one invocation of this method.
-     */
+    /** Creates [JavaClassSnapshot]s of the given Java classes. */
    private fun snapshotJavaClasses(
        classes: List<ClassFileWithContents>,
+        classesInfo: List<BasicClassInfo>,
        protoBased: Boolean? = null,
        includeDebugInfoInSnapshot: Boolean? = null
    ): List<JavaClassSnapshot> {
-        val classNames: List<JavaClassName> = classes.map { JavaClassName.compute(it.contents) }
-        val classNameToClassFile: Map<JavaClassName, ClassFileWithContents> = classNames.zipToMap(classes)
-
-        // 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<JavaClassName, JavaClassSnapshot?> = classNames.associateWith {
-            if (it in specialClasses) {
-                EmptyJavaClassSnapshot
-            } else null
-        }
-
-        // Snapshot the remaining regular classes
-        val regularClasses: List<JavaClassName> = classNames.filter { specialClassSnapshots[it] == null }
-        val regularClassIds = computeJavaClassIds(regularClasses)
-        val regularClassFiles: List<ClassFileWithContents> = regularClasses.map { classNameToClassFile[it]!! }
-
-        val snapshots: List<JavaClassSnapshot> = if (protoBased ?: protoBasedDefaultValue) {
-            snapshotJavaClassesProtoBased(regularClassIds, regularClassFiles)
+        return if (protoBased ?: protoBasedDefaultValue) {
+            snapshotJavaClassesProtoBased(classes, classesInfo)
        } else {
-            regularClassIds.mapIndexed { index, classId ->
-                JavaClassSnapshotter.snapshot(classId, regularClassFiles[index].contents, includeDebugInfoInSnapshot)
+            classes.mapIndexed { index, clazz ->
+                JavaClassSnapshotter.snapshot(clazz.contents, classesInfo[index], includeDebugInfoInSnapshot)
            }
        }
-        val regularClassSnapshots: Map<JavaClassName, JavaClassSnapshot> = regularClasses.zipToMap(snapshots)
-
-        return classNames.map { specialClassSnapshots[it] ?: regularClassSnapshots[it]!! }
    }

    private fun snapshotJavaClassesProtoBased(
-        classIds: List<ClassId>,
-        classFilesWithContents: List<ClassFileWithContents>
+        classFilesWithContents: List<ClassFileWithContents>,
+        classesInfo: List<BasicClassInfo>
    ): List<JavaClassSnapshot> {
+        val classIds = classesInfo.map { it.classId }
        val classesContents = classFilesWithContents.map { it.contents }
        val descriptors: List<JavaClassDescriptor?> = JavaClassDescriptorCreator.create(classIds, classesContents)
        val snapshots: List<JavaClassSnapshot> = descriptors.mapIndexed { index, descriptor ->
@@ -206,28 +173,52 @@ object ClassSnapshotter {
        return snapshots
    }

-    /** Returns local, anonymous, or synthetic classes, and their nested classes. */
-    private fun getSpecialClasses(classNames: List<JavaClassName>): List<JavaClassName> {
-        val specialClasses: MutableMap<JavaClassName, Boolean> = HashMap(classNames.size)
-        val nameToClassName: Map<String, JavaClassName> = 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("Can't find outer class '$outerName' of class '$name'")
-                }
-                is LocalClass -> true
-            }.also {
-                specialClasses[this] = it
+    /**
+     * Returns inaccessible classes, i.e. classes that can't be referenced from other source files (and therefore any changes in these
+     * classes will not require recompilation of other source files).
+     *
+     * Examples include private, local, anonymous, and synthetic classes.
+     *
+     * If a class is inaccessible, its nested classes (if any) are also inaccessible.
+     *
+     * NOTE: If we do not have enough info to determine whether a class is inaccessible, we will assume that the class is accessible to be
+     * safe.
+     */
+    private fun getInaccessibleClasses(classesInfo: List<BasicClassInfo>): List<BasicClassInfo> {
+        fun BasicClassInfo.isInaccessible(): Boolean {
+            if (this.isKotlinClass && this.kotlinClassHeader!!.kind != KotlinClassHeader.Kind.CLASS) {
+                // We're not sure about these kinds of Kotlin classes, so we assume it's accessible (see this method's kdoc)
+                return false
+            }
+            return if (isKotlinClass) {
+                // TODO: Is it safe to add isKotlinSynthetic to this lists?
+                isPrivate || isLocal || isAnonymous
+            } else {
+                isPrivate || isLocal || isAnonymous || isJavaSynthetic
            }
        }

-        return classNames.filter { it.isSpecial() }
+        val classIsInaccessible: MutableMap<BasicClassInfo, Boolean> = HashMap(classesInfo.size)
+        val classIdToClassInfo: Map<ClassId, BasicClassInfo> = classesInfo.associateBy { it.classId }
+
+        fun BasicClassInfo.isTransitivelyInaccessible(): Boolean {
+            classIsInaccessible[this]?.let { return it }
+
+            val inaccessible = if (isInaccessible()) {
+                true
+            } else classId.outerClassId?.let { outerClassId ->
+                classIdToClassInfo[outerClassId]?.isTransitivelyInaccessible()
+                // If we can't find the outer class from the given list of classes (which could happen with faulty jars), we assume that
+                // the class is accessible (see this method's kdoc).
+                    ?: false
+            } ?: false
+
+            return inaccessible.also {
+                classIsInaccessible[this] = inaccessible
+            }
+        }
+
+        return classesInfo.filter { it.isTransitivelyInaccessible() }
    }

    /** Returns `true` if it is known that the given exception can be thrown when calling [JavaClassDescriptor.toSerializedJavaClass]. */
@@ -7,8 +7,6 @@ package org.jetbrains.kotlin.incremental.classpathDiff

 import com.google.gson.GsonBuilder
 import org.jetbrains.kotlin.incremental.md5
-import org.jetbrains.kotlin.name.ClassId
-import org.jetbrains.kotlin.resolve.jvm.JvmClassName
 import org.jetbrains.org.objectweb.asm.ClassReader
 import org.jetbrains.org.objectweb.asm.Opcodes
 import org.jetbrains.org.objectweb.asm.tree.ClassNode
@@ -16,7 +14,7 @@ import org.jetbrains.org.objectweb.asm.tree.ClassNode
 /** Computes a [JavaClassSnapshot] of a Java class. */
 object JavaClassSnapshotter {

-    fun snapshot(classId: ClassId, classContents: ByteArray, includeDebugInfoInSnapshot: Boolean? = null): JavaClassSnapshot {
+    fun snapshot(classContents: ByteArray, classInfo: BasicClassInfo, includeDebugInfoInSnapshot: Boolean? = null): JavaClassSnapshot {
        // We will extract ABI information from the given class and store it into the `abiClass` variable.
        // It is acceptable to collect more info than required, but it is incorrect to collect less info than required.
        // There are 2 approaches:
@@ -28,28 +26,25 @@ object JavaClassSnapshotter {

        // First, collect all info.
        // Note the parsing options passed to ClassReader:
-        //   - SKIP_CODE is set as method bodies will not be part of the ABI of the class.
+        //   - SKIP_CODE and SKIP_FRAMES are set as method bodies will not be part of the ABI of the class.
        //   - SKIP_DEBUG is not set as it would skip method parameters, which may be used by annotation processors like Room.
-        //   - SKIP_FRAMES and EXPAND_FRAMES are not relevant when SKIP_CODE is set.
+        //   - EXPAND_FRAMES is not needed (and not relevant when SKIP_CODE is set).
        val classReader = ClassReader(classContents)
-        classReader.accept(abiClass, ClassReader.SKIP_CODE)
+        classReader.accept(abiClass, ClassReader.SKIP_CODE or ClassReader.SKIP_FRAMES)

-        // Then, remove non-ABI info:
-        //   - Method bodies have already been removed (see SKIP_CODE above).
-        //   - If the class is private, its snapshot will be empty. Otherwise, remove its private fields and methods.
-        if (abiClass.access.isPrivate()) {
-            return EmptyJavaClassSnapshot
-        }
+        // Then, remove non-ABI info, which includes:
+        //   - Method bodies: Should have already been removed (see SKIP_CODE above)
+        //   - Private fields and methods
+        fun Int.isPrivate() = (this and Opcodes.ACC_PRIVATE) != 0
        abiClass.fields.removeIf { it.access.isPrivate() }
        abiClass.methods.removeIf { it.access.isPrivate() }

-        // Sort fields and methods as their order is not important (we still use List instead of Set as we want the serialized snapshot to
-        // be deterministic).
+        // Normalize the class: Sort fields and methods as their order is not important (we still use List instead of Set as we want the
+        // serialized snapshot to be deterministic).
        abiClass.fields.sortWith(compareBy({ it.name }, { it.desc }))
        abiClass.methods.sortWith(compareBy({ it.name }, { it.desc }))

-        val supertypes = (listOf(abiClass.superName) + abiClass.interfaces.toList()).map { JvmClassName.byInternalName(it) }
-
+        // Snapshot the class
        val fieldsAbi = abiClass.fields.map { snapshotJavaElement(it, it.name, includeDebugInfoInSnapshot) }
        val methodsAbi = abiClass.methods.map { snapshotJavaElement(it, it.name, includeDebugInfoInSnapshot) }

@@ -57,30 +52,32 @@ object JavaClassSnapshotter {
        abiClass.methods.clear()
        val classAbiExcludingMembers = abiClass.let { snapshotJavaElement(it, it.name, includeDebugInfoInSnapshot) }

-        return RegularJavaClassSnapshot(classId, supertypes, classAbiExcludingMembers, fieldsAbi, methodsAbi)
+        return RegularJavaClassSnapshot(classInfo.classId, classInfo.supertypes, classAbiExcludingMembers, fieldsAbi, methodsAbi)
    }

-    private fun Int.isPrivate() = (this and Opcodes.ACC_PRIVATE) != 0
-
    private val gson by lazy {
-        // Use serializeSpecialFloatingPointValues() to avoid
+        // Use serializeSpecialFloatingPointValues() to avoid this error
        //    "java.lang.IllegalArgumentException: NaN is not a valid double value as per JSON specification. To override this behavior, use
        //    GsonBuilder.serializeSpecialFloatingPointValues() method."
        // on jars such as ~/.gradle/kotlin-build-dependencies/repo/kotlin.build/ideaIC/203.8084.24/artifacts/lib/rhino-1.7.12.jar.
-        GsonBuilder()
-            .serializeSpecialFloatingPointValues()
+        GsonBuilder().serializeSpecialFloatingPointValues().create()
+    }
+
+    // Same as above but with `setPrettyPrinting()`
+    private val gsonForDebug by lazy {
+        GsonBuilder().serializeSpecialFloatingPointValues()
            .setPrettyPrinting()
            .create()
    }

    private fun snapshotJavaElement(javaElement: Any, javaElementName: String, includeDebugInfoInSnapshot: Boolean? = null): AbiSnapshot {
-        // TODO: Optimize this method later if necessary. Currently we focus on correctness first.
-        val abiValue = gson.toJson(javaElement)
-        val abiHash = abiValue.toByteArray().md5()
-
        return if (includeDebugInfoInSnapshot == true) {
+            val abiValue = gsonForDebug.toJson(javaElement)
+            val abiHash = abiValue.toByteArray().md5()
            AbiSnapshotForTests(javaElementName, abiHash, abiValue)
        } else {
+            val abiValue = gson.toJson(javaElement)
+            val abiHash = abiValue.toByteArray().md5()
            AbiSnapshot(javaElementName, abiHash)
        }
    }
@@ -0,0 +1,111 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.incremental.classpathDiff
+
+import org.jetbrains.kotlin.name.ClassId
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.test.KotlinTestUtils
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import java.io.File
+import kotlin.test.assertEquals
+
+class BasicClassInfoTest {
+
+    @get:Rule
+    val tmpDir = TemporaryFolder()
+
+    @Test
+    fun `compute BasicClassInfo`() {
+        val compiledClasses = compileJava(className, sourceCode)
+        val classIds = compiledClasses.map { BasicClassInfo.compute(it).classId }
+
+        assertEquals(
+            listOf(
+                classId("com.example", "TopLevelClass", local = false),
+                classId("com.example", "TopLevelClass$1", local = true),
+                classId("com.example", "TopLevelClass$1LocalClass", local = true),
+                classId("com.example", "TopLevelClass$1LocalClass$1LocalClassWithinLocalClass", local = true),
+                classId("com.example", "TopLevelClass$1LocalClass.InnerClassWithinLocalClass", local = true),
+                classId("com.example", "TopLevelClass$2", local = true),
+                classId("com.example", "TopLevelClass.InnerClass", local = false),
+                classId("com.example", "TopLevelClass\$InnerClass\$1LocalClassWithinInnerClass", local = true),
+                classId("com.example", "TopLevelClass.InnerClass.InnerClassWithinInnerClass", local = false),
+                classId("com.example", "TopLevelClass.InnerClassWith\$Sign", local = false),
+                classId("com.example", "TopLevelClass.InnerClassWith\$Sign.InnerClassWith\$SignLevel2", local = false),
+                classId("com.example", "TopLevelClass.StaticNestedClass", local = false)
+            ),
+            classIds
+        )
+    }
+
+    @Suppress("SameParameterValue")
+    private fun compileJava(className: String, sourceCode: String): List<ByteArray> {
+        val sourceFile = File(tmpDir.newFolder(), "$className.java").apply {
+            parentFile.mkdirs()
+            writeText(sourceCode)
+        }
+        val classesDir = tmpDir.newFolder()
+
+        KotlinTestUtils.compileJavaFiles(listOf(sourceFile), listOf("-d", classesDir.path))
+
+        return classesDir.walk().toList()
+            .filter { it.isFile }
+            .sortedBy { it.path.substringBefore(".class") }
+            .map { it.readBytes() }
+    }
+
+    private fun classId(@Suppress("SameParameterValue") packageFqName: String, relativeClassName: String, local: Boolean) =
+        ClassId(FqName(packageFqName), FqName(relativeClassName), local)
+}
+
+private const val className = "com/example/TopLevelClass"
+private val sourceCode = """
+package com.example;
+
+public class TopLevelClass {
+
+    public class InnerClass {
+        public class InnerClassWithinInnerClass {
+        }
+        public void someMethod() {
+            class LocalClassWithinInnerClass {
+            }
+        }
+    }
+
+    public static class StaticNestedClass {
+    }
+
+    public void methodWithinTopLevelClass() {
+        class LocalClass {
+            class InnerClassWithinLocalClass {
+            }
+            public void methodWithinLocalClass() {
+                class LocalClassWithinLocalClass {
+                }
+            }
+        }
+        Runnable anonymousLocalClassInstance = new Runnable() {
+            @Override
+            public void run() {
+            }
+        };
+    }
+
+    private Runnable anonymousNonLocalClassInstance = new Runnable() {
+        @Override
+        public void run() {
+        }
+    };
+
+    public class InnerClassWith${'$'}Sign {
+        public class InnerClassWith${'$'}SignLevel2 {
+        }
+    }
+}
+""".trimIndent()
@@ -39,7 +39,7 @@
  },
  "supertypes": [
    {
-      "internalName": "kotlin/Any"
+      "internalName": "java/lang/Object"
    }
  ]
 }