Update Analysis API to 2.1.20-dev-4370 #3802

vmishenev · 2024-09-16T12:16:48Z

The goal of PR is to update Analysis API to the latest version - 2.1.20-dev-4370.

After some changes in AA, it is necessary to dispose of global resources (including threads of workers) at the end of the analysis to avoid memory leaks when rerunning the Dokka task (see https://youtrack.jetbrains.com/issue/KT-71862/Analysis-API-Standalone-Memory-leak-LowMemoryWatcherManager-does-not-release-resources ).

Also, Dokka encountered exhausted metaspace memory during one of the stress tests. On JVM 9 and later, the test passed successfully, but it failed on JVM 8, even with doubled or tripled memory. We resolved the issue using the flag -XX:SoftRefLRUPolicyMSPerMB=10 (collect soft references more aggressively) from KT-55831 on JVM 8.

vmishenev · 2024-09-30T12:21:03Z

This is blocked by https://youtrack.jetbrains.com/issue/KT-71862/Analysis-API-Standalone-Memory-leak-LowMemoryWatcherManager-does-not-release-resources

gradle/libs.versions.toml

…ev/update-analysis-api-to-2.1.0-dev-7103

.../src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/plugin/SymbolsAnalysisPlugin.kt

whyoleg · 2024-11-20T11:14:54Z

@vmishenev could you rebase on latest master? I see test failure on CI which is fixed there

whyoleg

The main question at my side is, how it will affect end-users?
I mean, okay, we "patched" (via scary flag SoftRefLRUPolicyMSPerMB) our test, but what will be on user-projects without it?

In any case, it would be nice to update PR description with what and why we are doing here

Also, I think we should check, that it will work fine with DGPv2, at least in IT before merging.
And also we should test combination DGPv2 + K2 with bigger external projects (kotlinx, ktor) before 2.0.0 release, so that we are sure that it works fine keeping in mind the difference in how Dokka is executed

whyoleg · 2024-11-20T10:42:39Z

...ts/analysis-java-psi/src/main/kotlin/org/jetbrains/dokka/analysis/java/JavaAnalysisPlugin.kt

+     * Note: Disposing of resources, including threads, allows unloading Dokka's class loader.
+     */
+    @InternalDokkaApi
+    public fun disposeGlobalStandaloneApplicationServices() {


why do we add this function inside analysis-java-psi module and not call directly inside K2 analysis?

Only from analysis-java-psi, you can call the methods of the IJ Platform. The K2 module has the truncated version of IJ from the compiler dependencies.
On the other hand, the calling of disposeGlobalStandaloneApplicationServices might affect the PSI analysis somehow.

Anyway, Marco will merge the endpoint into AA so we will be able to call it from K2. I left TODO

Probably it will be nice to also create an issue on our side, to keep track of it

dokka-subprojects/plugin-base/src/test/kotlin/model/ClassesTest.kt

whyoleg · 2024-11-20T10:50:10Z

...dle/src/testTemplateProjectTasksExecutionStress/kotlin/SequentialTasksExecutionStressTest.kt

-            jvmArgs = listOf("-Xmx1G", "-XX:MaxMetaspaceSize=500m"),
+            jvmArgs = listOf(
+                "-Xmx1G", "-XX:MaxMetaspaceSize=500m",
+                "-XX:SoftRefLRUPolicyMSPerMB=10" // to free up the metaspace on JVM 8, see https://youtrack.jetbrains.com/issue/KT-55831/


I don't really like that we need to add this flag here, only in tests... But not sure, that there are a lot of users which will be affected by issue with JDK 8 (note: Gradle migrating to JDK 17 by default for execution)

Overall, as far as I understand, this means, that K2 analysis could cause OOM on user projects with JDK 8? I think it would be nice to mention this in release notes if so, WDYT?

Overall, as far as I understand, this means, that K2 analysis could cause OOM on user projects with JDK 8?

Yes, but as you said it is unknown how many real user projects it affects.
This test is quite artificial with almost no code.
On real projects with the same number of modules, the behavior of JVM 8 might be different.

.../src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/plugin/SymbolsAnalysisPlugin.kt

...ts/analysis-java-psi/src/main/kotlin/org/jetbrains/dokka/analysis/java/JavaAnalysisPlugin.kt

vmishenev · 2024-11-20T15:02:45Z

The main question at my side is, how it will affect end-users?

I do not know) This test is quite artificial with almost no code.
On real projects with the same number of modules, the behavior of JVM 8 might be different.

I mean, okay, we "patched" (via scary flag SoftRefLRUPolicyMSPerMB) our test, but what will be on user-projects without it?

Another "popular" solution is increasing memory. It does not work for the stress test since its goal is to check the consumption of memory.

There are some observations:

It seems there is no memory leak in AA. It works well on JDK 9+. On Jvm 8, the flag SoftRefLRUPolicyMSPerMB is only about collection soft references more aggressively.
We can do nothing with JVM 8 right now. We cold write some optimizations/heuristics in the code to avoid SoftRefLRUPolicyMSPerMB, but it requires research.
I do know what types of projects it affects.

Also, I think we should check, that it will work fine with DGPv2, at least in IT before merging.

Does it really block merging? Haven't we checked DGPv2+K2 in IT at all yet?

And also we should test combination DGPv2 + K2 with bigger external projects (kotlinx, ktor) before 2.0.0 release

Sure

…ev/update-analysis-api-to-2.1.0-dev-7103

whyoleg · 2024-11-21T16:57:30Z

Does it really block merging? Haven't we checked DGPv2+K2 in IT at all yet?

Okay, let's merge at test it manually and add DGPv2 + K2 testing separately.
The main reason for this to work is that we will have both DGPv2 and K2 as opt-in and I think that most of the users who will try those flags will be the same, so DGPv2 + K2 combination will be rather popular

We can do nothing with JVM 8 right now

Yeah, let's see how it will go.

whyoleg · 2024-11-21T14:27:04Z

...ts/analysis-java-psi/src/main/kotlin/org/jetbrains/dokka/analysis/java/JavaAnalysisPlugin.kt

+     * Note: Disposing of resources, including threads, allows unloading Dokka's class loader.
+     */
+    @InternalDokkaApi
+    public fun disposeGlobalStandaloneApplicationServices() {


Probably it will be nice to also create an issue on our side, to keep track of it

dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/configuration.kt

vmishenev · 2024-11-26T11:22:25Z

JIC I have tested K2 with DGPv2 on a project (50 submodules) from the stress test (where a special VM flag is used). It works well on JVM 8 without any additional options or flags.

Update: I also tried with org.gradle.workers.max=1 and 100 submodules on JVM 8.

Mute test

1297659

vmishenev marked this pull request as draft September 19, 2024 10:10

Dispose Global Standalone Application Services

3a3ffbd

vmishenev force-pushed the vmishenev/update-analysis-api-to-2.1.0-dev-7103 branch from 8decdca to 3a3ffbd Compare October 4, 2024 09:12

vmishenev added 2 commits October 31, 2024 12:05

Trigger

43bee3c

Double metaspace

62d243f

whyoleg reviewed Nov 19, 2024

View reviewed changes

gradle/libs.versions.toml Outdated Show resolved Hide resolved

vmishenev added 3 commits November 19, 2024 12:03

Update the AA to the latest

b1fc404

Decrease metaspace

70a4bcc

Add KDoc and a condition for disposing of AA

e0f78b4

vmishenev force-pushed the vmishenev/update-analysis-api-to-2.1.0-dev-7103 branch from 83e35b1 to e0f78b4 Compare November 19, 2024 12:12

Merge branch 'master' of https://github.com/Kotlin/dokka into vmishen…

977340a

…ev/update-analysis-api-to-2.1.0-dev-7103

vmishenev changed the title ~~Update Analysis API to 2.1.0-dev-7103~~ Update Analysis API to 2.1.20-dev-4370 Nov 19, 2024

vmishenev added 2 commits November 19, 2024 14:46

Fix tests

fa90692

Add the flag SoftRefLRUPolicyMSPerMB

9e26139

vmishenev force-pushed the vmishenev/update-analysis-api-to-2.1.0-dev-7103 branch from 86156e2 to 9e26139 Compare November 19, 2024 13:15

vmishenev commented Nov 19, 2024

View reviewed changes

.../src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/plugin/SymbolsAnalysisPlugin.kt Show resolved Hide resolved

vmishenev marked this pull request as ready for review November 19, 2024 15:19

vmishenev requested a review from whyoleg November 19, 2024 15:19

whyoleg requested changes Nov 20, 2024

View reviewed changes

whyoleg reviewed Nov 20, 2024

View reviewed changes

...ts/analysis-java-psi/src/main/kotlin/org/jetbrains/dokka/analysis/java/JavaAnalysisPlugin.kt Show resolved Hide resolved

vmishenev added 3 commits November 20, 2024 17:11

Unmute test

9d707c2

Merge branch 'master' of https://github.com/Kotlin/dokka into vmishen…

a58d9b2

…ev/update-analysis-api-to-2.1.0-dev-7103

Add TODO

09f60db

vmishenev requested a review from whyoleg November 20, 2024 16:17

vmishenev linked an issue Nov 20, 2024 that may be closed by this pull request

Regression: a check in Analysis API is failed #3787

Closed

Edit KDoc of finalizeCoroutines

4fa533f

whyoleg approved these changes Nov 21, 2024

View reviewed changes

Add link to #3936

54038cf

vmishenev merged commit 897be0e into master Nov 26, 2024
14 checks passed

vmishenev deleted the vmishenev/update-analysis-api-to-2.1.0-dev-7103 branch November 26, 2024 11:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Analysis API to 2.1.20-dev-4370 #3802

Update Analysis API to 2.1.20-dev-4370 #3802

vmishenev commented Sep 16, 2024 •

edited

Loading

vmishenev commented Sep 30, 2024

whyoleg commented Nov 20, 2024

whyoleg left a comment •

edited

Loading

whyoleg Nov 20, 2024

vmishenev Nov 20, 2024 •

edited

Loading

vmishenev Nov 20, 2024 •

edited

Loading

whyoleg Nov 21, 2024

whyoleg Nov 20, 2024

vmishenev Nov 20, 2024

vmishenev commented Nov 20, 2024 •

edited

Loading

whyoleg commented Nov 21, 2024

whyoleg Nov 21, 2024

vmishenev commented Nov 26, 2024 •

edited

Loading

Update Analysis API to 2.1.20-dev-4370 #3802

Update Analysis API to 2.1.20-dev-4370 #3802

Conversation

vmishenev commented Sep 16, 2024 • edited Loading

vmishenev commented Sep 30, 2024

whyoleg commented Nov 20, 2024

whyoleg left a comment • edited Loading

Choose a reason for hiding this comment

whyoleg Nov 20, 2024

Choose a reason for hiding this comment

vmishenev Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

vmishenev Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

whyoleg Nov 21, 2024

Choose a reason for hiding this comment

whyoleg Nov 20, 2024

Choose a reason for hiding this comment

vmishenev Nov 20, 2024

Choose a reason for hiding this comment

vmishenev commented Nov 20, 2024 • edited Loading

whyoleg commented Nov 21, 2024

whyoleg Nov 21, 2024

Choose a reason for hiding this comment

vmishenev commented Nov 26, 2024 • edited Loading

vmishenev commented Sep 16, 2024 •

edited

Loading

whyoleg left a comment •

edited

Loading

vmishenev Nov 20, 2024 •

edited

Loading

vmishenev Nov 20, 2024 •

edited

Loading

vmishenev commented Nov 20, 2024 •

edited

Loading

vmishenev commented Nov 26, 2024 •

edited

Loading