In commit 4e89dcf, we have prepared the API for IC maps in top
interfaces and provide the implementation in abstract classes.
In this commit, we refactor IC maps so that they directly inherit/reuse
the implementation from the superclasses without having to reimplement
the APIs for a map.
Test: Existing tests (refactoring change)
^KT-63456: In progress
Authored-by: Hung Nguyen <hungnv@google.com>
Merge-request: KOTLIN-MR-801
Merged-by: Evgenii Mazhukin <evgenii.mazhukin@jetbrains.com>
`project.extraProperties` may contain values that are not `String`s. To
avoid failing on those values, we should cast the value with
`as? String` instead of `as String?`.
Test: New PropertiesBuildServiceTest
^KT-62496 Fixed
In commit 5ceebec, the MemoizedCallable instance was created inside the
Provider, which means that the MemoizedCallable is not reused the next
time the Provider is resolved. This was not the intended effect.
In this commit, we will create the MemoizedCallable instance up front so
that each time the Provider is resolved, it will reuse the same
MemoizedCallable and therefore avoid recomputing the value.
Test: Manually verified on the large project in KT-62496
^KT-62496 Fixed
Currently, the performance overhead when loading a Gradle property
consists of:
1. Creating a Provider
2. Resolving the Provider
Both steps are performed even when the same property was loaded before.
That means this overhead will multiply when there are a large number of
property read requests (e.g., in KT-62496, the tested project 400,000
read requests for only 17 properties).
To improve performance, we will now cache both steps with a
BuildService.
With this commit, configuration time for the tested project returns to
the same level before commit d6becee where the performance regression
happened (that commit can't be reverted because it introduced
`Provider`s which are required for proper Gradle usage).
Test: Manually verified on the large project in KT-62496
^KT-62496 Fixed
In commit 4e89dcf, we removed `flush()` from `PersistentStorage`
(previously called `LazyStorage`) because it is not used in Gradle
builds.
However, `flush()` is still used in JPS builds, so we need to add that
API back in this commit.
#KT-62486 Fixed
Co-authored-by: Hung Nguyen <hungnv@google.com>
Merge-request: KOTLIN-MR-796
Merged-by: Aleksei Cherepanov <aleksei.cherepanov@jetbrains.com>
To support build cache relocatability, we need to convert absolute paths
into relative paths before storing them in IC caches.
Before this commit, we computed relative paths based on the (root)
project directory and FAIL if some source files are located outside the
project directory.
With this commit, we will NOT FAIL in that case. This means relative
paths may start with "../". It's not "clean", but it can work.
Test: New test in BuildCacheRelocationIT
^KT-61852 Fixed
IC caches often contain file paths. To make them relocatable, we need
to convert these file paths into relative paths, relative to a base
directory.
- If the file paths are source files, we can use the root project
directory as base.
- If the file paths are class files, we should use the classes
directory as base (before this commit, we used the root project
directory in both cases, that's why we hit KT-58547).
The key changes in this commit include:
- RelocatableFileToPathConverter: converts paths to relative paths
- IncrementalCompilationContext: contains 2 different path converters,
one for source files and one for class files
- SourceToOutputFilesMap: maps source files to class files using the
above path converters
- IncrementalCompilerRunner: creates the path converters based on file
locations
Test: RelocatableFileToPathConverterTest unit test
SourceToOutputFilesMapTest unit test
BuildCacheRelocationIT.testCustomBuildDirectory integration test
^KT-58547 Fixed
A constant is a static final field with non-null value. In a previous
commit (0b09be7), we accidentally removed the *non-null value*
filter when looking for constants in the bytecode.
This commit re-adds that filter to make sure the detection is correct.
Test: Added KotlinOnlyClasspathChangesComputerTest.testDelegatedProperties
^KT-58986: Fixed
Both the new and old incremental compilation (IC) analysis rely on
Kotlin class metadata to detect a change.
However, Kotlin metadata currently doesn't contain info about
annotations (KT-57919), so the IC will not be able to detect a change
to them.
With this commit, we'll fix the new IC such that it can detect a change
to class annotations by not relying only on metadata.
We currently scope this fix to the new IC (cross-module analysis) first.
We'll fix this issue for within-module analysis later.
Performance: There seems to be no performance impact from this change.
Snapshotting the 400MB ideaIC-2022.1.4/app.jar takes 4.1s before and
after this change.
Test: Added ClasspathChangesComputerTest.testChangedAnnotations
^KT-58289: Fixed
Previously, when snapshotting the classpath with
`ClasspathEntrySnapshotTransform`, for each jar we load all classes
in memory at once. This was needed to detect inaccessible classes
(classes that don't impact incremental compilation and therefore
don't need to be snapshotted).
To reduce memory consumption, this commit updates the algorithm such
that we can now load one class at a time while still being able to
detect inaccessible classes.
In addition, we now read jar files with `java.util.zip.ZipFile` API
instead of `java.util.zip.ZipInputStream` to avoid current JDK bugs with
`ZipInputStream` (e.g., https://bugs.openjdk.org/browse/JDK-8298530).
^KT-57757 Fixed
^KT-57767 Fixed
IC: Compute symbols impacted by classpath changes
Incremental compilation has 4 key steps:
1. Compile changed/impacted files
2. Detect symbols that have changed after compiling
3. Detect symbols that are impacted by the changed symbols
4. Based on the changed-or-impacted symbols, identify files that need
to be recompiled. Go back to step 1.
Normally, step 2 and 3 are done together when the changed symbols
and impacted symbols are in the same module.
However, if the changed symbols and impacted symbols are in different
modules (e.g., a `Subclass` in lib1 extends a `Superclass` in lib2),
we currently do not compute symbols in the current module that are
impacted by changes in another module (step 3 above).
This is the case for both the new IC and the old IC.
In this commit, we will compute impacted symbols for the new IC. We can
fix the old IC later if necessary (they can't be fixed together easily).
Test: Added BaseIncrementalCompilationMultiProjectIT.testChangeInterfaceInLib
^KT-56197 Fixed
A LookupSymbol should only refer to either a class, a class member, or a
package member.
When a LookupSymbol refers to a file facade (e.g.,
LookupSymbol(name="FooKt", scope="com.example")), it is redundant as it
doesn't impact the IC analysis to find files to recompile.
Previously, the new IC (ClasspathChangesComputer) would fail when
detecting that IncrementalJvmCache reported these redundant
LookupSymbols. With this change, the new IC will just ignore them.
Note: A better approach would be to fix IncrementalJvmCache to not
report these LookupSymbols, but it will require some significant
cleanup/refactoring work, so we can consider it later.
Test: New KotlinOnlyClasspathChangesComputerTest.testRenameFileFacade
^KT-55021 Fixed
Make the code more readable and address a small issue where we
did not close caches when returning `ICResult.RequiresRebuild`
(it was not exactly a correctness issue because we would recompile
non-incrementally in that case).
^KT-55709 In progress (This is a follow-up on a previous change for this
bug)
Previously, we didn't close caches if incremental compilation failed.
The assumption was that we would fall back to non-incremental
compilation, where it shouldn't matter whether caches were closed or not
as non-incremental compilation should be able to recover from a
corrupted state of the caches.
However, this choice might have caused issues such as KT-55709 where
non-incremental compilation fails after fallback.
In this commit, we will now always close the caches before falling back,
just to be safe.
TODO:
1. We'll need to verify whether not closing the caches was the actual
cause of KT-55709, and that this commit fixes it.
2. Even if this commit fixes the issue, we'll need to see why
non-incremental compilation fails to recover from unclosed caches
(it would remain unsafe if that was not addressed).
^KT-55709 In progress (see TODO #1 above)
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
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
Extract KotlinClassInfo to a separate class
to reduce the size of IncrementalJvmCache and prepare for the next
change.
^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
Update unit tests for handling `@JvmName`s
In a previous commit, we made a behavior change for inline property
accessors: The existing behavior is that if the implementation of an
inline getter has changed, only usages of the getter will be impacted
but not usages of the setter (and vice versa).
After that previous commit, usages of *both* the getter and setter will
now be impacted (i.e., we might compile slightly more files). This is
because a change to either the getter or the setter will now be
considered a change to the property, which will help simplify our change
analysis.
This commit updates the relevant unit tests to reflect the new behavior.
Test: Updated Incremental*TestGenerated.PureKotlin#testInlinePropertyInClass
and Incremental*TestGenerated.PureKotlin#testInlinePropertyOnTopLevel
^KT-54144 Fixed
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
Problem: ClasspathChangesComputer (a core component of the new IC) uses
the existing inline function analysis from the old IC
(IncrementalJvmCache.InlineFunctionsMap). If an inline property accessor
has changed, the inline function analysis will report the name of the
property accessor, not the name of the property.
ClasspathChangesComputer doesn't see the property accessor's name in the
list of class/package members, so it will throw an exception.
Solution: There are 2 options:
1. If an inline property accessor has changed, the inline function
analysis can report the name of the property instead of the
property accessor.
2. ClasspathChangesComputer can include property accessors in the list
of class/package members.
In this commit, we will choose option 2 as it is simpler.
Test: New KotlinOnlyClasspathChangesComputerTest.testPropertyAccessors
Small cleanup - PLS SQUASH INTO PREVIOUS COMMIT
- Address review
- Fix failed tests
- Add some trivial changes
^KT-53871 Fixed
Make it clear that there 3 distinct cases:
1. Incremental compilation completed with an ExitCode.
2. Incremental compilation was not possible for some valid reason
(e.g., for a clean build), and we will perform non-incremental
compilation.
3. Incremental compilation failed with an exception.
In this case, we will:
- Print a warning with a stack trace
- Ask the user to file a bug
- Collect rebuild reason enum for analytics
+ TODO: Collect the stack trace too
- Fall back to non-incremental compilation
Test: Existing BaseIncrementalCompilationMultiProjectIT.testFailureHandling_UserError,
Updated BaseIncrementalCompilationMultiProjectIT.testFailureHandling_ToolError
^KT-53015: In progress
When computing impacted symbols of changed symbols, previously we
considered only the supertypes-inheritors type of impact, which is the
most common type. This commit adds the constants-in-companion-objects
type of impact to address KT-53266.
We've also cleaned up impact computation to make it easier to add new
types of impact in the future.
^KT-53266 In progress
Instead of letting `compileWithDaemon` return null, allow it to throw
exceptions so that the caller can catch those exceptions and log clearer
messages on why it failed.
^KT-31602 In progress
Revise BaseIncrementalCompilationMultiProjectIT's failure handling
tests to make it clear we have covered 2 important scenarios:
- Failures caused by user errors: testFailureHandling_UserError
- Failures caused by tool errors: testFailureHandling_ToolError
^KT-53231 In progress
If incremental compilation fails, we currently fall back to
non-incremental compilation. When that happens, it would be incorrect to
shrink classpath snapshot incrementally, so this commit makes sure we'll
shrink classpath snapshot non-incrementally in that case.
^KT-53231 In progress
When incremental compilation fails, we currently log it at the `debug`
level (and fall back to non-incremental compilation). This commit will
change it to `warning` so that we can get more user reports, which will
allow us to fix the root cause.
Also make sure the warning includes a stack trace.
Additionally, let ReportSeverity.fromCode() return a non-null value
or throw an exception otherwise as that case is not expected.
^KT-52839 In Progress
The current logic works as follows:
- Try either incremental compilation or non-incremental compilation
- If the above (or any of its surrounding work) fails, fall back to
non-incremental compilation
This means we may perform non-incremental compilation twice.
This commit will fix that logic so that we fall back to non-incremental
compilation only if *incremental compilation* fails.
A nice consequence of this change is that it also resolves the critical
bugs described at KT-52669 (which occur because the current logic is
flawed).
#KT-52669 Fixed
- Remove soft references that are too old.
- Compute memory usage based on Runtime.totalMemory() instead of
Runtime.maxMemory() as the latter is not reliable.
- Remove debug logs that haven't proved to be useful.
- Collect metrics on the number of cache hits (in addition to cache
misses).
#KT-52329 In Progress
[New IC] Reduce memory usage of classpath snapshot cache
Use object interning for commonly shared objects. These include:
- supertypes of classes
- package names of classes
One experiment showed that with the above optimization, memory usage was
reduced from 660 MB down to 280 MB (+ 4 MB for the interning pool).
More aggressive object interning didn't reduce memory usage much
further, but would increase interning overhead and code complexity, so
we didn't do this to more objects.
Note that this commit optimizes the size of classpath snapshots in
memory, not their serialized data on disk. (I attempted the latter,
but the size was only reduced from 160 MB down to 130 MB, while the code
complexity became much higher as multiple interning pools would need to
be stored to disk and later loaded from disk, each per classpath entry
snapshot or shrunk classpath snapshot.)
#KT-52329 Fixed
Originally, the classpath-snapshot output directory was:
1. <project>/build/kotlin/<task>/classpath-snapshot for KotlinCompile
2. <project>/build/tmp/kapt3/classpath-snapshot/<source-set> for
KaptGenerateStubsTask
In commit 1d1c6d5, we started using location 1 for both tasks for
consistency.
Around that time, commit 46cb490 had a merge conflict with the above
commit, but the conflict resolution was not correct yet:
- The property was set twice, causing a build failure.
- Location 2 was set for KaptGenerateStubsTask again.
This commit fixes both issues, so that:
- The property is set only once.
- Location 1 is used for both tasks.
Test: Added Kapt3IT.testSimpleWithIC_withClasspathSnapshot to prevent
regression
^KT-52187 Fixed
This line was added in commit 8c611e0 to allow the
`incremental-compilation-impl` module to use Gson.
This module is no longer using Gson now (only used by its unit tests),
so we can revert that commit.
^KT-52141 Fixed
To snapshot a Java class (+ its fields and methods), previously we used
Gson to serialize a class field/method to a string via reflection, and
hash that string.
We now use an ASM ClassWriter to write a placeholder class containing
the field/method of interest and hash the bytecode of that class.
One experiment showed that this new approach is ~10 times faster than
the previous approach (140s down to 16s when snapshotting 600 jars).
Test: Updated expectation files for JavaClassSnapshotterTest unit tests
+ Existing integration tests to prevent regression
^KT-52141 In Progress
Implement an in-memory cache that uses a combination of strong
references and `SoftReference`s so that it adapts to memory
availability.
Cache eviction is currently performed after loading a classpath snapshot
(this can be changed later if necessary).
Evicted cache entries' values will be converted from strong references
into `SoftReference`s so that they can still be used for as long as the
JVM allows them.
There are 2 types of cache eviction:
- Least recently used: Oldest entries will be evicted
- Memory usage limit: If memory is limited, all entries will be
evicted
Test: Added InMemoryCacheWithEvictionTest unit test
^KT-51978 In Progress
Incremental states for Kotlin compilation are stored inside the
`<project>/build/kotlin/<task-name>` directory. Previously, that
directory was @LocalState, so we had to keep the `classpath-snapshot`
@OutputDirectory outside it.
Now, that directory has been made a parent containing @OutputDirectory's
@LocalState's, so we can safely move the `classpath-snapshot` directory
to it.
Test: Existing tests should pass (location change has no functional impact)
^KT-45777 In Progress
to reduce the size of the snapshots.
- Track metrics for Gradle classpath snapshot artifact transform
- Format size metrics so it's more readable
^KT-45777 In Progress
1. CLASS_LEVEL: allows tracking whether a .class file has changed
without tracking what specific parts of the .class file (e.g.,
fields or methods) have changed.
2. CLASS_MEMBER_LEVEL: allows tracking not only whether a .class file
has changed but also what specific parts of the .class file (e.g.,
fields or methods) have changed.
The idea is that for better performance we will use CLASS_LEVEL for
classpath entries that are usually unchanged, and CLASS_MEMBER_LEVEL
for classpath entries that are frequently changed. We'll work out the
specifics in a following commit after some measurements.
Support running kotlinc on Windows in ClasspathSnapshotTestCommon
Also add tests for different Kotlin class kinds.
Add unit tests for CLASS_LEVEL snapshotting and diffing
Test: Updated ClasspathSnapshotterTest + ClasspathChangesComputerTest
Add ClasspathChangesComputerTest.testMixedClassSnapshotGranularities
Make sure that:
- Compilation with -Pkotlin.incremental.useClasspathSnapshot=true or
-Dkotlin.incremental.classpath.snapshot.enabled=true is incremental
after cache hit.
- Default compilation (using build history files) is non-incremental
after cache hit.
Also unify the related tests into
BuildCacheRelocationIT.testKotlinIncrementalCompilation*. We don't need
corresponding tests in BuildCacheIT because BuildCacheRelocationIT
already covers it.
- Clean up BuildCacheRelocationIT.testKotlinIncrementalCompilation
- Delete BuildCacheIT.testKotlinCompileIncrementalBuildWithoutRelocation
as it is already covered by the test in BuildCacheRelocationIT
- Add BuildCacheRelocationIT.testKotlinIncrementalCompilation_withClasspathSnapshot
- Updated BuildCacheRelocationIT.testKotlinIncrementalCompilation_withClasspathSnapshot
to ensure precision (otherwise, rounding errors to milliseconds may
add up and cause unexplainable gaps in the running time).
We can still use milliseconds in the final report after all the precise
sub-build-times have been aggregated.
Test: Existing IncrementalJvmCompilerRunnerTestGenerated.PureKotlin#testCompanionConstantChanged
and similar *.PureKotlin#testCompanionConstantChanged tests
to make it easier to add more tests in the next commits.
- Add unit tests for constants and inline functions
Also add tests for different kinds of Kotlin classes: CLASS,
FILE_FACADE, MULTIFILE_CLASS.
-Add unit test for nested classes
Also remove the existing integration test for nested classes to keep the
integration tests focused on the key scenarios while unit tests will
cover the corner cases.
Ignore inline functions that are private
Currently, we shrink classpath snapshots at 2 steps:
- Classpath diffing: Shrink the current classpath snapshot against
the previous lookup symbols
- Classpath snapshot saving: Shrink the current classpath snapshot
against the current lookup symbols
With this commit, the shrinking at the second step is now incremental.
The shrinking at the first step is still non-incremental.
for performance reasons: (1) the snapshots are too big, and (2) they are
usually updated only at the end of the task execution--in a failed task
run, they are usually unchanged and therefore don't need to be restored.
The previous commit added gson to `incremental-compilation-impl` module.
However, gson is currently not added to -libraryjars for Proguard, so
running with -Pteamcity=true would fail with
```
> Task :kotlin-compiler:proguard
Warning: ... can't find referenced class com.google.gson.Gson
Warning: ... can't find referenced class com.google.gson.GsonBuilder
You may need to add missing library jars or update their versions.
If your code works fine without the missing classes, you can suppress
the warnings with '-dontwarn' options.
(http://proguard.sourceforge.net/manual/troubleshooting.html#unresolvedclass)
```
This commits adds -dontwarn com.google.gson.** to work around this
error. The proper fix is probably to add gson (and many other libraries)
to Proguard's -libraryjars.
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.
as we need access to the lookup tracker to compute classpath changes
more efficiently and reduce the size of the saved classpath snapshot.
The previous commit only changed the files' paths, this
commit actually updates the files' contents.
Note that classpath snapshotting still happens in Gradle artifact
transforms. (However, the previous commit also moved the code for
classpath snapshotting together with the code for classpath diffing as
they are closely related.)
This is faster than the current approach which creates
`JavaClassDescriptor`s, converts them to protos, and then snapshots
these protos.
- Refactor unit tests to faciliate further changes
- moves test data to a directory that matches the tests' package name
- moves expected snapshots to a separate directory
- adds public and private fields/properties to sample class
- Compute changes between ASM-based Java class snapshots
- Don't collect members of an added Java class as changes
as it's enough to report the name of the added Java class as changed (we
also do that for added Kotlin classes and Kotlin/Java removed classes).
- Add unit tests for impact analysis in advance
- Compute impacted symbols of changed symbols
Also do not collect added classes/class members as they don't impact
recompilation.
-Use ClassId when computing Java class changes
It is more precise than JvmClassName, which can be ambiguous around the
`$` character (e.g., ClassId "com/example/A$B.C" and "com/example/A.B$C"
both have the same JvmClassName "com/example/A$B$C").
- Compute impacted set of changed symbols across Kotlin and Java
- Add unit tests for impact analysis across Kotlin and Java
- Compute supertypes of Kotlin classes during snapshotting
- Handle inner classes when computing list of changed symbols.
For the reported symbols, always check all options:
class member, inner class, top level class, top level member.
Test: IncrementalJavaChangeClasspathSnapshotIT.testAddingInnerClass
to avoid unnecessarily loading unchanged ones.
Duplicate classes will make this a bit tricky. This commit outlines the
algorithm to handle them, the full implementation will follow later.
Also handle removed classes when computing classpath changes.
Test: New tests in ClasspathChangesComputerTest
There are certain classes that we are not yet able to take snapshots of,
either because the class is faulty, or there is a bug in our code.
For these classes, we will use a special snapshot and fall back to the
existing approach to compute classpath changes.
In the short run, we will update this list of cases as they arise. In
the long run, we will:
- Fix all cases that are caused by bugs in our code.
- Decide what to with the faulty jars in general and remove the list.
Test: The following should pass
- Step 1: ./gradlew publish
- Step 2: ./gradlew publish -Pbootstrap.local=true
-Pbootstrap.local.path=/path/to/kotlin/build/repo
-Pkotlin.incremental.useClasspathSnapshot=true
Create `JavaClassDescriptor`s for Java classes
Ignore anonymous and synthetic classes
as they can't impact recompilation.
Clean up handling of local and anonymous classes
Bug: KT-45777
Test: New JavaClassDescriptorCreatorTest
We are working on a feature in the Kotlin Gradle plugin called
`kotlin.incremental.useClasspathSnapshot` to improve incremental
Kotlin compilation.
To allow reuse when writing this feature, this commit moves most of the
code in RuntimeModuleData (in the ':core:descriptors.runtime' project)
to DeserializationComponentsForJava (in the ':core:descriptors.jvm'
project), so that the API can be accessed from the
':kotlin-gradle-plugin' and ':kotlin-build-common' projects where the
feature is written.
Bug: KT-45777
Test: Existing tests should pass (this is a refactoring-only change)
We are working on a feature in the Kotlin Gradle plugin called
`kotlin.incremental.useClasspathSnapshot` to improve incremental
Kotlin compilation.
In this feature, we need to extract ABI information from a .class file.
If the .class file is a Kotlin class, this info can be found in the
class header data. But if the .class file is a Java class, this info is
not readily available.
The RuntimeModuleData class in the ':core:descriptors.runtime' project
can help with that: It uses reflection to generate `ClassDescriptor`s.
However, reflection requires a full classpath to work correctly, whereas
we want to generate a `ClassDescriptor` directly for each class file
(also, reflection is probably slow).
To address that, this commit refactors RuntimeModuleData so that it can
support a generic Kotlin/JavaClassFinder, which can be based on either
reflection or bytecode analysis. The existing code continues to use
reflection while the new feature will use bytecode analysis (e.g., using
the existing BinaryJavaClass).
Bug: KT-45777
Test: Existing tests should pass (this is a refactoring-only change)
Reuse the existing IncrementalJvmCache to take snapshots and compute
changes for Kotlin classes.
Java classes will be handled next.
Bug: KT-45777
Test: New ClasspathSnapshotterTest, ClasspathChangesComputerTest
Extract minimal required information into the new `KotlinClassInfo`
class. This info will be part of the classpath snapshot of the
`KotlinCompile` task in an upcoming change.
Bug: KT-45777
Test: Existing tests should pass (this is a refactoring-only change)
This commit prepares necessary components only. The core parts of
classpath snapshotting and diffing will be implemented next.
Bug: KT-45777
Test: Existing IncrementalCompilationClasspathSnapshotJvmMultiProjectIT
and IncrementalJavaChangeClasspathSnapshotIT
This commit wires necessary components only. The actual classpath
changes will be provided later.
Bug: KT-45777
Test: Existing IncrementalCompilationClasspathSnapshotJvmMultiProjectIT
and IncrementalJavaChangeClasspathSnapshotIT
This commit sets up the artifact transform only. The actual classpath
snapshotting and computation of classpath changes will be done later.
Bug: KT-45777
Test: Existing IncrementalCompilationClasspathSnapshotJvmMultiProjectIT
and IncrementalJavaChangeClasspathSnapshotIT
This ensures we don't regress when developing the feature.
Bug: KT-45777
Test: New IncrementalCompilationClasspathSnapshotJvmMultiProjectIT
and IncrementalJavaChangeClasspathSnapshotIT
We are planning to improve incremental Kotlin compile with a new
approach using classpath snapshot (see
https://youtrack.jetbrains.com/issue/KT-45777 for more details).
The estimated steps will be as follows:
[x] Add a flag for the new approach, disabled by default
[ ] Add tests for the new approach, and update them as we go
[ ] Add computation of classpath snapshot using artifact transforms
[ ] Add computation of classpath snapshot changes
[ ] Ensure incremental compile is fully functional with the new approach
[ ] Measure performance + optimize: Repeat until we see evidence that
the new approach is clearly better than the current approach
[ ] Advertise the flag, and later on enable it by default
This commit is the first step. Although it's simple, it allows further
changes to be made safely without impacting the current approach.
Bug: KT-45777
Test: N/A (will be added in a subsequent change)
Previously when kotlin.build.report.enable=true, the build report
contained lines such as:
org.jetbrains.kotlin.gradle.report.TaskRecord@2db49688 was skipped
This commit prints the task name instead of TaskRecord@hashCode so that
it is more readable, like this:
Task ':app:compileDebugKotlin' was skipped
Bug: N/A (Clean up)
Test: Existing tests + manually checked the build report output
Refactor common code into new abstract class
IncrementalCompilationBaseIT to make the code cleaner and easier to
evolve.
Also add a few missing key scenarios for ABI and non-ABI changes in
Kotlin files, and make it consistent with the test for changes in Java
files.
Bug: KT-45777
Test: Updating them
Previously, even if `keepKdocComments=false`, we would still create the
KDocCommentKeeper object unnecessarily.
This commit makes sure we create the object only if
`keepKdocComments=true`.
Bug: Clean-up after commit e252171 for KT-43593
Test: Existing tests
Currently, KaptGenerateStubsTask may not generate KDoc comments
correctly. See KT-43593 for more details.
This commit provides a Kapt flag called
`kapt.keep.kdoc.comments.in.stubs`
to control whether KDoc comments will be included in the generated
stubs.
This flag is currently enabled by default to keep the existing behavior
and avoid breaking existing users.
Users who don't need KDoc comments in stubs but are hitting KT-43593 can
disable the flag.
Whether this flag will be disabled by default later is to be determined.
Bug: https://youtrack.jetbrains.com/issue/KT-43593
(Note that this commit only provides a workaround, it doesn't
actually fix the bug.)
Test: (Ir)ClassFileToSourceStubConverterTestGenerated#testCommentsRemoved
Class methods and fields are currently sorted at serialization (see
DescriptorSerializer.sort) and at deserialization (see
DeserializedMemberScope.OptimizedImplementation#addMembers). Therefore,
the contents of the generated stub files are sorted in incremental
builds but not in clean builds.
The consequence is that the contents of the generated stub files may not
be consistent across a clean build and an incremental build, making the
build non-deterministic and dependent tasks run unnecessarily (see
KT-40882).
To work around that, this commit sorts class methods and fields when
outputting stub files.
Bug: KT-40882 (there are actually 2 issues in here; this commit fixes
the first one)
Test: New DeterministicBuildIT + Updated existing test expectation files
AbstractKotlinTarget#buildAdhocComponentsFromKotlinVariants passes a
non-serializable local object to a lambda, which will fail during
serialization when configuration caching is enabled.
To fix that, this commit passes only the relevant parts of the object
to the lambda.
Bug: https://youtrack.jetbrains.com/issue/KT-43054
Test: Manually verified on
https://android.googlesource.com/platform/tools/metalava/+/refs/heads/master/
where the issue was detected (I haven't been able to create a mininal
reproducible project)
Hung Nguyen