Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Analysis API to 2.1.20-dev-4370 #3802

Merged
merged 15 commits into from
Nov 26, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ class SequentialTasksExecutionStressTest : AbstractGradleIntegrationTest() {
buildVersions,
"runTasks",
"-Ptask_number=$iterations",
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/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

),
enableBuildCache = false,
).buildRelaxed()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public final class org/jetbrains/dokka/analysis/java/ExceptionJavadocTag$Compani

public final class org/jetbrains/dokka/analysis/java/JavaAnalysisPlugin : org/jetbrains/dokka/plugability/DokkaPlugin {
public fun <init> ()V
public final fun disposeGlobalStandaloneApplicationServices ()V
public final fun getDocCommentCreators ()Lorg/jetbrains/dokka/plugability/ExtensionPoint;
public final fun getDocCommentFinder ()Lorg/jetbrains/dokka/analysis/java/doccomment/DocCommentFinder;
public final fun getDocCommentParsers ()Lorg/jetbrains/dokka/plugability/ExtensionPoint;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ public class JavaAnalysisPlugin : DokkaPlugin() {
DocCommentFinder(logger, docCommentFactory)
}

/**
* Disposes global resources which would persist after unloading Analysis API (Symbols analysis) and IJ platform classes.
vmishenev marked this conversation as resolved.
Show resolved Hide resolved
*
* **Important:** Once this function has been called, Analysis API *and* IntelliJ platform classes should not be used anymore. The classes
* should either be unloaded or the whole program should be shut down.
*
* Note: Disposing of resources, including threads, allows unloading Dokka's class loader.
*/
@InternalDokkaApi
public fun disposeGlobalStandaloneApplicationServices() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@vmishenev vmishenev Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@vmishenev vmishenev Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Suppress("UnstableApiUsage")
com.intellij.util.concurrency.AppExecutorUtil.shutdownApplicationScheduledExecutorService()
}

internal val javaDocCommentCreator by extending {
docCommentCreators providing { JavaDocCommentCreator() }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fun TestDokkaConfiguration.toDokkaConfiguration(projectDir: File): DokkaConfigur
override val suppressInheritedMembers: Boolean
get() = throw NotImplementedError("Not expected to be used by analysis modules")
override val finalizeCoroutines: Boolean
get() = throw NotImplementedError("Not expected to be used by analysis modules")
get() = false
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ import org.jetbrains.dokka.analysis.kotlin.symbols.services.*
import org.jetbrains.dokka.analysis.kotlin.symbols.services.KotlinDocumentableSourceLanguageParser
import org.jetbrains.dokka.analysis.kotlin.symbols.services.SymbolExternalDocumentablesProvider
import org.jetbrains.dokka.analysis.kotlin.symbols.translators.DefaultSymbolToDocumentableTranslator
import org.jetbrains.dokka.plugability.DokkaPlugin
import org.jetbrains.dokka.plugability.DokkaPluginApiPreview
import org.jetbrains.dokka.plugability.PluginApiPreviewAcknowledgement
import org.jetbrains.dokka.plugability.querySingle
import org.jetbrains.dokka.plugability.*
import org.jetbrains.dokka.renderers.PostAction
import org.jetbrains.kotlin.asJava.elements.KtLightAbstractAnnotation

Expand All @@ -42,7 +39,13 @@ public class SymbolsAnalysisPlugin : DokkaPlugin() {
}

internal val disposeKotlinAnalysisPostAction by extending {
CoreExtensions.postActions with PostAction { querySingle { kotlinAnalysis }.close() }
CoreExtensions.postActions providing { context ->
PostAction {
querySingle { kotlinAnalysis }.close()
if (context.configuration.finalizeCoroutines)
whyoleg marked this conversation as resolved.
Show resolved Hide resolved
javaAnalysisPlugin.disposeGlobalStandaloneApplicationServices()
}
}
}

internal val symbolToDocumentableTranslator by extending {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ class ClassesTest : AbstractModelTest("/src/main/kotlin/classes/Test.kt", "class
}

@Test
@OnlyDescriptors("#3787")
vmishenev marked this conversation as resolved.
Show resolved Hide resolved
fun nestedGenericClasses() {
inlineModelTest(
"""
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ javaDiffUtils = "4.12"

## Analysis
kotlin-compiler = "2.0.20"
kotlin-compiler-k2 = "2.1.0-dev-5441"
kotlin-compiler-k2 = "2.1.20-dev-4370"

# MUST match the version of the intellij platform used in the kotlin compiler,
# otherwise this will lead to different versions of psi API and implementations
Expand Down
Loading