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
The logic of detecting changes in Kotlin facets was changed from "Include selected fields" to "Include all compiler arguments and exclude selected". This will help to avoid multiple IC issues when new change-sensitive compiler arguments will be added
(#KTIJ-17137, #KT-51536, #KTIJ-17170, #KTIJ-17300, #KT-47983) Fixed
Merge-request: KT-MR-7455
Merged-by: Aleksei Cherepanov <aleksei.cherepanov@jetbrains.com>
This fix improves performance of incremental compilation on Windows
machines.
When portable JPS caches are used, DirtyFilesHolder.getRemovedFiles
could return file names which are not allowed on current platform.
Previously it resulted in IOException on File.getCanonicalFile.
#KT-53735 Fixed
This is especially needed for portable caches. Now hash counting on mac/windows is the same as logic on linux, which allows to create caches on remote servers and use them together with incremental compilation locally.
#KT-53932 Fixed
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
Review: https://jetbrains.team/p/kt/reviews/6753
Meaningful semantic change was splitted into 5 commits to simplify the
change review. Sinle commit would be too big.
Why replace source to binary: to get rid of kotlin-reflect in Kotlin
plugin artifact KTIJ-22276
Note: Kotlin Maven artifacts (./gradlew publish) changed their
dependency on kotlin-reflect
Review: https://jetbrains.team/p/kt/reviews/6753
Why: I'm going to replace source dependency on kotlin-reflect with
binary dependency. Normalize reflect dependency before global
processing.
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
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
Sorting invokes descriptor renderer, so the fewer descriptors we are
sorting, the better. This helps to save a few percent of compilation
time in a synthetic project from KT-52326.
- 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
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
Add another one reason for rebuild: if pluginClasspaths was changed or even its jar content, module will be marked for rebuild
Merge-request: KT-MR-5839
Merged-by: Aleksei Cherepanov <aleksei.cherepanov@jetbrains.com>
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
If LeverVersion is set to greater than 1 of the current compiler version, metadata became null here org.jetbrains.kotlin.load.kotlin.header.ReadKotlinClassHeaderAnnotationVisitor.createHeader, which leads to NPE during incremental analysis. Using the same incompatible data as fallback prevent it
#KTIJ-20954 Fixed
Merge-request: KT-MR-5680
Merged-by: Aleksei Cherepanov <aleksei.cherepanov@jetbrains.com>
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.
Andrey Uskov