Extract KotlinClassInfo to a separate class

to reduce the size of IncrementalJvmCache and prepare for the next
change.

^KT-54144 In progress

Handle changes to inline functions/property accessors with `@JvmName`s

If we detect a change in an inline function `foo` with @JvmName
`fooJvmName`, we have two options:
   1. Report that function `foo` has changed
   2. Report that method `fooJvmName` has changed

Similarly, if we detect a change in an inline property accessor with
JvmName `getFoo` of property `foo`, we have two options:
   1. Report that property `foo` has changed
   2. Report that property accessor `getFoo` has changed

The compiler is guaranteed to generate `LookupSymbol`s corresponding to
option 1 when referencing inline functions/property accessors, but it is
not guaranteed to generate `LookupSymbol`s corresponding to option 2.
(Currently the compiler seems to support option 2 for *inline*
functions/property accessors, but that may change.)

Therefore, we will choose option 1 as it is cleaner and safer.

^KT-54144 In progress

Ignore inline functions that are not found in the bytecode

^KT-54144 In progress

Add unit test for handling `@JvmName`s

Test: KotlinOnlyClasspathChangesComputerTest
             #testFunctionsAndPropertyAccessorsWithJvmNames
^KT-54144 Fixed

Small cleanup in IncrementalCompilerRunner

 - Add comment for closing caches
 - Rename providedChangedFiles to changedFiles
 - Tiny clean up in `performWorkBeforeCompilation`
 - Count directories to delete in debug logs

^KT-53015 In progress

Small cleanup in IncrementalCompilerRunner

 - Add comment for closing caches
 - Rename providedChangedFiles to changedFiles
 - Tiny clean up in `performWorkBeforeCompilation`
 - Count directories to delete in debug logs

^KT-53015 In progress
This commit is contained in:
Hung Nguyen
2022-10-13 19:32:14 +01:00
committed by nataliya.valtman
parent fce8b877c8
commit ec3da62672
19 changed files with 501 additions and 365 deletions
@@ -5,6 +5,7 @@
package org.jetbrains.kotlin.incremental
import org.jetbrains.kotlin.incremental.DifferenceCalculatorForClass.Companion.getNonPrivateMembers
import org.jetbrains.kotlin.metadata.ProtoBuf.Visibility.PRIVATE
import org.jetbrains.kotlin.metadata.deserialization.Flags
import org.jetbrains.kotlin.name.FqName
@@ -97,8 +98,7 @@ class AbiSnapshotDiffService() {
when (protoData) {
is ClassProtoData -> {
fqNames.add(fqName)
symbols.addAll(
protoData.getNonPrivateMemberNames(includeInlineAccessors = true).map { LookupSymbol(it, fqName.asString()) })
symbols.addAll(protoData.getNonPrivateMembers().map { LookupSymbol(it, fqName.asString()) })
}
is PackagePartProtoData -> {
symbols.addAll(
@@ -18,12 +18,15 @@ package org.jetbrains.kotlin.incremental
import org.jetbrains.kotlin.build.DEFAULT_KOTLIN_SOURCE_FILES_EXTENSIONS
import org.jetbrains.kotlin.build.GeneratedFile
import org.jetbrains.kotlin.build.report.*
import org.jetbrains.kotlin.build.report.BuildReporter
import org.jetbrains.kotlin.build.report.debug
import org.jetbrains.kotlin.build.report.info
import org.jetbrains.kotlin.build.report.metrics.BuildAttribute
import org.jetbrains.kotlin.build.report.metrics.BuildAttribute.*
import org.jetbrains.kotlin.build.report.metrics.BuildPerformanceMetric
import org.jetbrains.kotlin.build.report.metrics.BuildTime
import org.jetbrains.kotlin.build.report.metrics.measure
import org.jetbrains.kotlin.build.report.warn
import org.jetbrains.kotlin.cli.common.*
import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments
import org.jetbrains.kotlin.cli.common.messages.MessageCollector
@@ -80,12 +83,12 @@ abstract class IncrementalCompilerRunner<
allSourceFiles: List<File>,
args: Args,
messageCollector: MessageCollector,
// when [providedChangedFiles] is not null, changes are provided by external system (e.g. Gradle)
// when `changedFiles` is not null, changes are provided by external system (e.g. Gradle)
// otherwise we track source files changes ourselves.
providedChangedFiles: ChangedFiles?,
changedFiles: ChangedFiles?,
projectDir: File? = null
): ExitCode = reporter.measure(BuildTime.INCREMENTAL_COMPILATION_DAEMON) {
return when (val result = tryCompileIncrementally(allSourceFiles, providedChangedFiles, args, projectDir, messageCollector)) {
return when (val result = tryCompileIncrementally(allSourceFiles, changedFiles, args, projectDir, messageCollector)) {
is ICResult.Completed -> {
reporter.debug { "Incremental compilation completed" }
result.exitCode
@@ -95,7 +98,7 @@ abstract class IncrementalCompilerRunner<
reporter.addAttribute(result.reason)
compileNonIncrementally(
result.reason, allSourceFiles, args, projectDir, trackChangedFiles = providedChangedFiles == null, messageCollector
result.reason, allSourceFiles, args, projectDir, trackChangedFiles = changedFiles == null, messageCollector
)
}
is ICResult.Failed -> {
@@ -114,7 +117,7 @@ abstract class IncrementalCompilerRunner<
reporter.addAttribute(result.reason)
compileNonIncrementally(
result.reason, allSourceFiles, args, projectDir, trackChangedFiles = providedChangedFiles == null, messageCollector
result.reason, allSourceFiles, args, projectDir, trackChangedFiles = changedFiles == null, messageCollector
)
}
}
@@ -141,23 +144,24 @@ abstract class IncrementalCompilerRunner<
*/
private fun tryCompileIncrementally(
allSourceFiles: List<File>,
providedChangedFiles: ChangedFiles?,
changedFiles: ChangedFiles?,
args: Args,
projectDir: File?,
messageCollector: MessageCollector
): ICResult {
if (providedChangedFiles is ChangedFiles.Unknown) {
if (changedFiles is ChangedFiles.Unknown) {
return ICResult.RequiresRebuild(UNKNOWN_CHANGES_IN_GRADLE_INPUTS)
}
providedChangedFiles as ChangedFiles.Known?
changedFiles as ChangedFiles.Known?
val caches = createCacheManager(args, projectDir)
val exitCode: ExitCode
try {
// Step 1: Get changed files
val changedFiles: ChangedFiles.Known = try {
getChangedFiles(providedChangedFiles, allSourceFiles, caches)
val knownChangedFiles: ChangedFiles.Known = try {
getChangedFiles(changedFiles, allSourceFiles, caches)
} catch (e: Throwable) {
// Don't need to close caches in cases where we return `ICResult.Failed` because we will compile non-incrementally anyway
return ICResult.Failed(IC_FAILED_TO_GET_CHANGED_FILES, e)
}
@@ -166,7 +170,7 @@ abstract class IncrementalCompilerRunner<
// Step 2: Compute files to recompile
val compilationMode = try {
reporter.measure(BuildTime.IC_CALCULATE_INITIAL_DIRTY_SET) {
calculateSourcesToCompile(caches, changedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
calculateSourcesToCompile(caches, knownChangedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
}
} catch (e: Throwable) {
return ICResult.Failed(IC_FAILED_TO_COMPUTE_FILES_TO_RECOMPILE, e)
@@ -227,7 +231,7 @@ abstract class IncrementalCompilerRunner<
check(it.containsAll(mainOutputDirs)) { "outputDirs is missing classesDir and workingDir: $it" }
} ?: mainOutputDirs
reporter.debug { "Cleaning output directories" }
reporter.debug { "Cleaning ${outputDirsToClean.size} output directories" }
cleanOrCreateDirectories(outputDirsToClean)
}
return createCacheManager(args, projectDir).use { caches ->
@@ -266,20 +270,20 @@ abstract class IncrementalCompilerRunner<
}
private fun getChangedFiles(
providedChangedFiles: ChangedFiles.Known?,
changedFiles: ChangedFiles.Known?,
allSourceFiles: List<File>,
caches: CacheManager
): ChangedFiles.Known {
return when {
providedChangedFiles == null -> caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
providedChangedFiles.forDependencies -> {
changedFiles == null -> caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
changedFiles.forDependencies -> {
val moreChangedFiles = caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
ChangedFiles.Known(
modified = providedChangedFiles.modified + moreChangedFiles.modified,
removed = providedChangedFiles.removed + moreChangedFiles.removed
modified = changedFiles.modified + moreChangedFiles.modified,
removed = changedFiles.removed + moreChangedFiles.removed
)
}
else -> providedChangedFiles
else -> changedFiles
}
}
@@ -109,7 +109,7 @@ fun makeIncrementally(
classpathChanges = ClasspathSnapshotDisabled
)
//TODO set properly
compiler.compile(sourceFiles, args, messageCollector, providedChangedFiles = null)
compiler.compile(sourceFiles, args, messageCollector, changedFiles = null)
}
}
@@ -388,9 +388,7 @@ open class IncrementalJvmCompilerRunner(
super.performWorkBeforeCompilation(compilationMode, args)
if (compilationMode is CompilationMode.Incremental) {
val destinationDir = args.destinationAsFile
destinationDir.mkdirs()
args.classpathAsList = listOf(destinationDir) + args.classpathAsList
args.classpathAsList = listOf(args.destinationAsFile) + args.classpathAsList
}
}
@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.incremental.classpathDiff
import com.intellij.util.containers.Interner
import com.intellij.util.io.DataExternalizer
import org.jetbrains.kotlin.build.report.metrics.BuildPerformanceMetric
import org.jetbrains.kotlin.incremental.ConstantValueExternalizer
import org.jetbrains.kotlin.incremental.KotlinClassInfo
import org.jetbrains.kotlin.incremental.storage.*
import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
@@ -53,27 +54,6 @@ object CachedClasspathSnapshotSerializer {
}
}
internal open class DataExternalizerForSealedClass<T>(
val baseClass: Class<T>,
val inheritorClasses: List<Class<out T>>,
val inheritorExternalizers: List<DataExternalizer<*>>
) : DataExternalizer<T> {
override fun save(output: DataOutput, objectToExternalize: T) {
val inheritorClassIndex =
inheritorClasses.indexOfFirst { it.isAssignableFrom(objectToExternalize!!::class.java) }.also { check(it != -1) }
output.writeByte(inheritorClassIndex.also { check(it <= Byte.MAX_VALUE) }) // Write byte so the data is smaller
@Suppress("UNCHECKED_CAST")
(inheritorExternalizers[inheritorClassIndex] as DataExternalizer<T>).save(output, objectToExternalize)
}
override fun read(input: DataInput): T {
val inheritorClassIndex = input.readByte().toInt()
@Suppress("UNCHECKED_CAST")
return inheritorExternalizers[inheritorClassIndex].read(input) as T
}
}
object ClasspathEntrySnapshotExternalizer : DataExternalizer<ClasspathEntrySnapshot> {
override fun save(output: DataOutput, snapshot: ClasspathEntrySnapshot) {
@@ -87,26 +67,23 @@ object ClasspathEntrySnapshotExternalizer : DataExternalizer<ClasspathEntrySnaps
}
}
internal object ClassSnapshotExternalizer : DataExternalizerForSealedClass<ClassSnapshot>(
baseClass = ClassSnapshot::class.java,
inheritorClasses = listOf(AccessibleClassSnapshot::class.java, InaccessibleClassSnapshot::class.java),
inheritorExternalizers = listOf(AccessibleClassSnapshotExternalizer, InaccessibleClassSnapshotExternalizer)
internal object ClassSnapshotExternalizer : DataExternalizer<ClassSnapshot> by DelegateDataExternalizer(
types = listOf(AccessibleClassSnapshot::class.java, InaccessibleClassSnapshot::class.java),
typesExternalizers = listOf(AccessibleClassSnapshotExternalizer, InaccessibleClassSnapshotExternalizer)
)
internal object AccessibleClassSnapshotExternalizer : DataExternalizerForSealedClass<AccessibleClassSnapshot>(
baseClass = AccessibleClassSnapshot::class.java,
inheritorClasses = listOf(KotlinClassSnapshot::class.java, JavaClassSnapshot::class.java),
inheritorExternalizers = listOf(KotlinClassSnapshotExternalizer, JavaClassSnapshotExternalizer)
internal object AccessibleClassSnapshotExternalizer : DataExternalizer<AccessibleClassSnapshot> by DelegateDataExternalizer(
types = listOf(KotlinClassSnapshot::class.java, JavaClassSnapshot::class.java),
typesExternalizers = listOf(KotlinClassSnapshotExternalizer, JavaClassSnapshotExternalizer)
)
private object KotlinClassSnapshotExternalizer : DataExternalizerForSealedClass<KotlinClassSnapshot>(
baseClass = KotlinClassSnapshot::class.java,
inheritorClasses = listOf(
private object KotlinClassSnapshotExternalizer : DataExternalizer<KotlinClassSnapshot> by DelegateDataExternalizer(
types = listOf(
RegularKotlinClassSnapshot::class.java,
PackageFacadeKotlinClassSnapshot::class.java,
MultifileClassKotlinClassSnapshot::class.java
),
inheritorExternalizers = listOf(
typesExternalizers = listOf(
RegularKotlinClassSnapshotExternalizer,
PackageFacadeKotlinClassSnapshotExternalizer,
MultifileClassKotlinClassSnapshotExternalizer
@@ -185,8 +162,8 @@ internal object KotlinClassInfoExternalizer : DataExternalizer<KotlinClassInfo>
ListExternalizer(StringExternalizer).save(output, info.classHeaderData.toList())
ListExternalizer(StringExternalizer).save(output, info.classHeaderStrings.toList())
NullableValueExternalizer(StringExternalizer).save(output, info.multifileClassName)
LinkedHashMapExternalizer(StringExternalizer, ConstantExternalizer).save(output, info.constantsMap)
LinkedHashMapExternalizer(StringExternalizer, LongExternalizer).save(output, info.inlineFunctionsAndAccessorsMap)
MapExternalizer(StringExternalizer, ConstantValueExternalizer).save(output, info.constantsMap)
MapExternalizer(InlineFunctionOrAccessorExternalizer, LongExternalizer).save(output, info.inlineFunctionsAndAccessorsMap)
}
override fun read(input: DataInput): KotlinClassInfo {
@@ -197,8 +174,8 @@ internal object KotlinClassInfoExternalizer : DataExternalizer<KotlinClassInfo>
classHeaderData = ListExternalizer(StringExternalizer).read(input).toTypedArray(),
classHeaderStrings = ListExternalizer(StringExternalizer).read(input).toTypedArray(),
multifileClassName = NullableValueExternalizer(StringExternalizer).read(input),
constantsMap = LinkedHashMapExternalizer(StringExternalizer, ConstantExternalizer).read(input),
inlineFunctionsAndAccessorsMap = LinkedHashMapExternalizer(StringExternalizer, LongExternalizer).read(input)
constantsMap = MapExternalizer(StringExternalizer, ConstantValueExternalizer).read(input),
inlineFunctionsAndAccessorsMap = MapExternalizer(InlineFunctionOrAccessorExternalizer, LongExternalizer).read(input)
)
}
}
@@ -9,9 +9,10 @@ import org.jetbrains.kotlin.build.report.metrics.BuildMetricsReporter
import org.jetbrains.kotlin.build.report.metrics.BuildTime
import org.jetbrains.kotlin.build.report.metrics.DoNothingBuildMetricsReporter
import org.jetbrains.kotlin.build.report.metrics.measure
import org.jetbrains.kotlin.incremental.DifferenceCalculatorForPackageFacade.Companion.getNonPrivateMembers
import org.jetbrains.kotlin.incremental.KotlinClassInfo
import org.jetbrains.kotlin.incremental.PackagePartProtoData
import org.jetbrains.kotlin.incremental.classpathDiff.ClassSnapshotGranularity.CLASS_MEMBER_LEVEL
import org.jetbrains.kotlin.incremental.getNonPrivateMemberNames
import org.jetbrains.kotlin.incremental.md5
import org.jetbrains.kotlin.incremental.storage.toByteArray
import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader.Kind.*
@@ -96,7 +97,7 @@ object ClassSnapshotter {
)
FILE_FACADE, MULTIFILE_CLASS_PART -> PackageFacadeKotlinClassSnapshot(
classId, classAbiHash, classMemberLevelSnapshot,
packageMemberNames = kotlinClassInfo.protoData.getNonPrivateMemberNames(includeInlineAccessors = true).toSet()
packageMemberNames = (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMembers().toSet()
)
MULTIFILE_CLASS -> MultifileClassKotlinClassSnapshot(
classId, classAbiHash, classMemberLevelSnapshot,
@@ -223,11 +223,9 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {
LookupSymbol(name = "inlineProperty_ChangedType", scope = "com.example"),
LookupSymbol(name = "inlineProperty_ChangedType_BackingField", scope = "com.example"),
LookupSymbol(name = "getInlineProperty_ChangedType", scope = "com.example"),
LookupSymbol(name = "setInlineProperty_ChangedType", scope = "com.example"),
LookupSymbol(name = "getInlineProperty_ChangedGetterImpl", scope = "com.example"),
LookupSymbol(name = "setInlineProperty_ChangedSetterImpl", scope = "com.example"),
LookupSymbol(name = "inlineProperty_ChangedGetterImpl", scope = "com.example"),
LookupSymbol(name = "inlineProperty_ChangedSetterImpl", scope = "com.example"),
LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass")
),
@@ -238,6 +236,26 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {
).assertEquals(changes)
}
@Test
fun testFunctionsAndPropertyAccessorsWithJvmNames() {
val changes = computeClasspathChanges(File(testDataDir, "KotlinOnly/testFunctionsAndPropertyAccessorsWithJvmNames/src"), tmpDir)
Changes(
lookupSymbols = setOf(
LookupSymbol(name = "changedFunction", scope = "com.example.SomeClass"),
LookupSymbol(name = "changedPropertyAccessor", scope = "com.example.SomeClass"),
LookupSymbol(name = "changedInlineFunction", scope = "com.example"),
LookupSymbol(name = "changedInlinePropertyAccessor", scope = "com.example"),
LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass"),
),
fqNames = setOf(
"com.example",
"com.example.SomeClass"
)
).assertEquals(changes)
}
/** Tests [SupertypesInheritorsImpact]. */
@Test
override fun testImpactComputation_SupertypesInheritors() {
@@ -0,0 +1,20 @@
@file:Suppress("NOTHING_TO_INLINE")
package com.example
class SomeClass {
@JvmName("changedFunctionJvmName")
fun changedFunction(): Long = 0
val changedPropertyAccessor: Long
@JvmName("changedPropertyAccessorJvmName")
get() = 0
}
@JvmName("changedInlineFunctionJvmName")
inline fun changedInlineFunction(): Long = 0
inline val changedInlinePropertyAccessor: Long
@JvmName("changedInlinePropertyAccessorJvmName")
get() = 0
@@ -0,0 +1,20 @@
@file:Suppress("NOTHING_TO_INLINE")
package com.example
class SomeClass {
@JvmName("changedFunctionJvmName")
fun changedFunction(): Int = 0
val changedPropertyAccessor: Int
@JvmName("changedPropertyAccessorJvmName")
get() = 0
}
@JvmName("changedInlineFunctionJvmName")
inline fun changedInlineFunction(): Int = 0
inline val changedInlinePropertyAccessor: Int
@JvmName("changedInlinePropertyAccessorJvmName")
get() = 0