From 288606868ec7b8edcb7e596eac97c1cb9034568b Mon Sep 17 00:00:00 2001 From: Marco Pennekamp Date: Wed, 8 Mar 2023 18:11:58 +0100 Subject: [PATCH] [FIR] KT-57207 Avoid `FirJavaFacade.knownClassNamesInPackage` in the IDE - `FirJavaFacade.knownClassNamesInPackage` cannot be computed in the IDE using the current strategy because there are multiple finders and there is no `CliFinder`. However, the cache was still used, which caused it to be filled with `null` values and additionally caused worse performance in `JavaSymbolProvider` due to hash map accesses via `hasTopLevelClassOf`. - Rewriting the strategy is non-trivial as additional indices are needed on the IDE side. See KTIJ-24642. --- .../org/jetbrains/kotlin/fir/java/FirJavaFacade.kt | 11 +++++++---- .../jetbrains/kotlin/load/java/JavaClassFinderImpl.kt | 3 +++ .../kotlin/javac/components/JavacBasedClassFinder.kt | 1 + .../kotlin/resolve/jvm/KotlinJavaPsiFacade.java | 7 ++++++- .../org/jetbrains/kotlin/load/java/JavaClassFinder.kt | 6 ++++++ .../runtime/components/ReflectJavaClassFinder.kt | 1 + 6 files changed, 24 insertions(+), 5 deletions(-) diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaFacade.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaFacade.kt index e099df8b8bd..939ed225d3a 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaFacade.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaFacade.kt @@ -42,7 +42,6 @@ import org.jetbrains.kotlin.load.java.structure.* import org.jetbrains.kotlin.load.java.structure.impl.JavaElementImpl import org.jetbrains.kotlin.name.* import org.jetbrains.kotlin.types.Variance.INVARIANT -import org.jetbrains.kotlin.util.OperatorNameConventions class FirJavaFacadeForSource( session: FirSession, @@ -66,7 +65,7 @@ abstract class FirJavaFacade( } private val packageCache = session.firCachesFactory.createCache { fqName: FqName -> - val knownClassNames: Set? = knownClassNamesInPackage.getValue(fqName) + val knownClassNames: Set? = knownClassNamesInPackage(fqName) classFinder.findPackage( fqName, mayHaveAnnotations = if (knownClassNames != null) PACKAGE_INFO_CLASS_NAME in knownClassNames else true @@ -86,11 +85,15 @@ abstract class FirJavaFacade( packageCache.getValue(fqName)?.fqName fun hasTopLevelClassOf(classId: ClassId): Boolean { - val knownNames = knownClassNamesInPackage.getValue(classId.packageFqName) ?: return true + val knownNames = knownClassNamesInPackage(classId.packageFqName) ?: return true return classId.relativeClassName.topLevelName() in knownNames } - fun knownClassNamesInPackage(packageFqName: FqName): Set? = knownClassNamesInPackage.getValue(packageFqName) + fun knownClassNamesInPackage(packageFqName: FqName): Set? { + // Avoid filling the cache with `null`s and accessing the cache if `knownClassNamesInPackage` cannot be calculated anyway. + if (!classFinder.canComputeKnownClassNamesInPackage()) return null + return knownClassNamesInPackage.getValue(packageFqName) + } abstract fun getModuleDataForClass(javaClass: JavaClass): FirModuleData diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/JavaClassFinderImpl.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/JavaClassFinderImpl.kt index 8709b7b8578..785d611474c 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/JavaClassFinderImpl.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/JavaClassFinderImpl.kt @@ -52,4 +52,7 @@ class JavaClassFinderImpl : AbstractJavaClassFinder() { return javaFacade.knownClassNamesInPackage(packageFqName, javaSearchScope) } + override fun canComputeKnownClassNamesInPackage(): Boolean { + return javaFacade.canComputeKnownClassNamesInPackage() + } } diff --git a/compiler/javac-wrapper/src/org/jetbrains/kotlin/javac/components/JavacBasedClassFinder.kt b/compiler/javac-wrapper/src/org/jetbrains/kotlin/javac/components/JavacBasedClassFinder.kt index aeacb9c86de..6bb5d010836 100644 --- a/compiler/javac-wrapper/src/org/jetbrains/kotlin/javac/components/JavacBasedClassFinder.kt +++ b/compiler/javac-wrapper/src/org/jetbrains/kotlin/javac/components/JavacBasedClassFinder.kt @@ -45,4 +45,5 @@ class JavacBasedClassFinder : AbstractJavaClassFinder() { override fun findPackage(fqName: FqName, mayHaveAnnotations: Boolean) = javac.findPackage(fqName, javaSearchScope) override fun knownClassNamesInPackage(packageFqName: FqName) = javac.knownClassNamesInPackage(packageFqName) + override fun canComputeKnownClassNamesInPackage(): Boolean = true } diff --git a/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/resolve/jvm/KotlinJavaPsiFacade.java b/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/resolve/jvm/KotlinJavaPsiFacade.java index f55f8385300..5f781b5cc99 100644 --- a/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/resolve/jvm/KotlinJavaPsiFacade.java +++ b/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/resolve/jvm/KotlinJavaPsiFacade.java @@ -220,13 +220,18 @@ public class KotlinJavaPsiFacade implements Disposable { KotlinPsiElementFinderWrapper[] finders = finders(); - if (finders.length == 1 && finders[0] instanceof CliFinder) { + if (canComputeKnownClassNamesInPackage()) { return ((CliFinder) finders[0]).knownClassNamesInPackage(packageFqName); } return null; } + public Boolean canComputeKnownClassNamesInPackage() { + KotlinPsiElementFinderWrapper[] finders = finders(); + return finders.length == 1 && finders[0] instanceof CliFinder; + } + @NotNull private PsiClass[] findClassesInDumbMode(@NotNull String qualifiedName, @NotNull GlobalSearchScope scope) { String packageName = StringUtil.getPackageName(qualifiedName); diff --git a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaClassFinder.kt b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaClassFinder.kt index 6c7a402ea84..5ae2f5eb78b 100644 --- a/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaClassFinder.kt +++ b/core/compiler.common.jvm/src/org/jetbrains/kotlin/load/java/JavaClassFinder.kt @@ -24,4 +24,10 @@ interface JavaClassFinder { fun findPackage(fqName: FqName, mayHaveAnnotations: Boolean = true): JavaPackage? fun knownClassNamesInPackage(packageFqName: FqName): Set? + + /** + * Whether [knownClassNamesInPackage] can be computed. When [canComputeKnownClassNamesInPackage] is `false`, [knownClassNamesInPackage] + * will always return `null`. + */ + fun canComputeKnownClassNamesInPackage(): Boolean } diff --git a/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectJavaClassFinder.kt b/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectJavaClassFinder.kt index be88d680817..6ac6d277d95 100644 --- a/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectJavaClassFinder.kt +++ b/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectJavaClassFinder.kt @@ -42,6 +42,7 @@ class ReflectJavaClassFinder(private val classLoader: ClassLoader) : JavaClassFi } override fun knownClassNamesInPackage(packageFqName: FqName): Set? = null + override fun canComputeKnownClassNamesInPackage(): Boolean = false } fun ClassLoader.tryLoadClass(fqName: String) =