From 20907236ed1bc1e36447f1ed6ebd37d641cd64ed Mon Sep 17 00:00:00 2001 From: Vadim Mishenev Date: Tue, 5 Sep 2023 15:48:00 +0300 Subject: [PATCH] Fix reemerged compiler concurrency issues (#3143) #3034 broke the fix for #1599 by changing packages --- .../compiler/api/compiler.api | 24 ++++ .../configuration/JvmDependenciesIndexImpl.kt | 107 +++++++++--------- .../KotlinCliJavaFileManagerImpl.kt | 15 ++- 3 files changed, 92 insertions(+), 54 deletions(-) diff --git a/subprojects/analysis-kotlin-descriptors/compiler/api/compiler.api b/subprojects/analysis-kotlin-descriptors/compiler/api/compiler.api index bb98a7ad7a..373ec268df 100644 --- a/subprojects/analysis-kotlin-descriptors/compiler/api/compiler.api +++ b/subprojects/analysis-kotlin-descriptors/compiler/api/compiler.api @@ -100,3 +100,27 @@ public final class org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/impl public final fun getContext ()Lorg/jetbrains/dokka/plugability/DokkaContext; } +public final class org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl : com/intellij/core/CoreJavaFileManager, org/jetbrains/kotlin/resolve/jvm/KotlinCliJavaFileManager { + public static final field Companion Lorg/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl$Companion; + public fun (Lcom/intellij/psi/PsiManager;)V + public fun findClass (Ljava/lang/String;Lcom/intellij/psi/search/GlobalSearchScope;)Lcom/intellij/psi/PsiClass; + public fun findClass (Lorg/jetbrains/kotlin/load/java/JavaClassFinder$Request;Lcom/intellij/psi/search/GlobalSearchScope;)Lorg/jetbrains/kotlin/load/java/structure/JavaClass; + public final fun findClass (Lorg/jetbrains/kotlin/name/ClassId;Lcom/intellij/psi/search/GlobalSearchScope;)Lorg/jetbrains/kotlin/load/java/structure/JavaClass; + public fun findClasses (Ljava/lang/String;Lcom/intellij/psi/search/GlobalSearchScope;)[Lcom/intellij/psi/PsiClass; + public fun findModules (Ljava/lang/String;Lcom/intellij/psi/search/GlobalSearchScope;)Ljava/util/Collection; + public fun findPackage (Ljava/lang/String;)Lcom/intellij/psi/PsiPackage; + public fun getNonTrivialPackagePrefixes ()Ljava/util/Collection; + public final fun initialize (Lorg/jetbrains/kotlin/cli/jvm/index/JvmDependenciesIndex;Ljava/util/List;Lorg/jetbrains/kotlin/cli/jvm/index/SingleJavaFileRootsIndex;Z)V + public fun knownClassNamesInPackage (Lorg/jetbrains/kotlin/name/FqName;)Ljava/util/Set; +} + +public final class org/jetbrains/kotlin/cli/jvm/compiler/KotlinCliJavaFileManagerImpl$Companion { +} + +public final class org/jetbrains/kotlin/cli/jvm/index/JvmDependenciesIndexImpl : org/jetbrains/kotlin/cli/jvm/index/JvmDependenciesIndex { + public fun (Ljava/util/List;)V + public fun findClass (Lorg/jetbrains/kotlin/name/ClassId;Ljava/util/Set;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; + public fun getIndexedRoots ()Lkotlin/sequences/Sequence; + public fun traverseDirectoriesInPackage (Lorg/jetbrains/kotlin/name/FqName;Ljava/util/Set;Lkotlin/jvm/functions/Function2;)V +} + diff --git a/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/JvmDependenciesIndexImpl.kt b/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/JvmDependenciesIndexImpl.kt index 42fda61513..7cc41600bd 100644 --- a/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/JvmDependenciesIndexImpl.kt +++ b/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/JvmDependenciesIndexImpl.kt @@ -14,23 +14,35 @@ * limitations under the License. */ -package org.jetbrains.dokka.analysis.kotlin.descriptors.compiler.configuration +/** + * DO NOT MOVE IT + * This is a hack for https://github.com/Kotlin/dokka/issues/1599 + * + * Copy-pasted from 1.9.20-Beta-1 + * Can be removed for Kotlin compiler 1.9.20 and later + * + * It makes this class threadsafe for Dokka + */ +@file:Suppress("PackageDirectoryMismatch") +package org.jetbrains.kotlin.cli.jvm.index import com.intellij.ide.highlighter.JavaClassFileType import com.intellij.ide.highlighter.JavaFileType import com.intellij.openapi.vfs.VfsUtilCore import com.intellij.openapi.vfs.VirtualFile import gnu.trove.THashMap -import it.unimi.dsi.fastutil.ints.IntArrayList -import org.jetbrains.kotlin.cli.jvm.index.JavaRoot -import org.jetbrains.kotlin.cli.jvm.index.JvmDependenciesIndex import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName +import java.util.* +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock // speeds up finding files/classes in classpath/java source roots -// NOT THREADSAFE, needs to be adapted/removed if we want compiler to be multithreaded +// TODO: KT-58327 needs to be adapted/removed if we want compiler to be multithreaded // the main idea of this class is for each package to store roots which contains it to avoid excessive file system traversal -internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependenciesIndex { +class JvmDependenciesIndexImpl(_roots: List) : JvmDependenciesIndex { + private val lock = ReentrantLock() + //these fields are computed based on _roots passed to constructor which are filled in later private val roots: List by lazy { _roots.toList() } @@ -46,7 +58,8 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie // indices of roots that are known to contain this package // if this list contains [1, 3, 5] then roots with indices 1, 3 and 5 are known to contain this package, 2 and 4 are known not to (no information about roots 6 or higher) // if this list contains maxIndex that means that all roots containing this package are known - val rootIndices = IntArrayList(2) + @Suppress("DEPRECATION") // TODO: fix deprecation + val rootIndices = com.intellij.util.containers.IntArrayList(2) } // root "Cache" object corresponds to DefaultPackage which exists in every root. Roots with non-default fqname are also listed here but @@ -55,7 +68,7 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie Cache().apply { roots.indices.forEach(rootIndices::add) rootIndices.add(maxIndex) - rootIndices.trimToSize(0) + rootIndices.trimToSize() } } @@ -74,8 +87,10 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie acceptedRootTypes: Set, continueSearch: (VirtualFile, JavaRoot.RootType) -> Boolean ) { - search(TraverseRequest(packageFqName, acceptedRootTypes)) { dir, rootType -> - if (continueSearch(dir, rootType)) null else Unit + lock.withLock { + search(TraverseRequest(packageFqName, acceptedRootTypes)) { dir, rootType -> + if (continueSearch(dir, rootType)) null else Unit + } } } @@ -85,26 +100,29 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie acceptedRootTypes: Set, findClassGivenDirectory: (VirtualFile, JavaRoot.RootType) -> T? ): T? { - // make a decision based on information saved from last class search - if (lastClassSearch?.first?.classId != classId) { - return search(FindClassRequest(classId, acceptedRootTypes), findClassGivenDirectory) - } + lock.withLock { + // TODO: KT-58327 probably should be changed to thread local to fix fast-path + // make a decision based on information saved from last class search + if (lastClassSearch?.first?.classId != classId) { + return search(FindClassRequest(classId, acceptedRootTypes), findClassGivenDirectory) + } - val (cachedRequest, cachedResult) = lastClassSearch!! - return when (cachedResult) { - is SearchResult.NotFound -> { - val limitedRootTypes = acceptedRootTypes - cachedRequest.acceptedRootTypes - if (limitedRootTypes.isEmpty()) { - null - } else { - search(FindClassRequest(classId, limitedRootTypes), findClassGivenDirectory) + val (cachedRequest, cachedResult) = lastClassSearch!! + return when (cachedResult) { + is SearchResult.NotFound -> { + val limitedRootTypes = acceptedRootTypes - cachedRequest.acceptedRootTypes + if (limitedRootTypes.isEmpty()) { + null + } else { + search(FindClassRequest(classId, limitedRootTypes), findClassGivenDirectory) + } } - } - is SearchResult.Found -> { - if (cachedRequest.acceptedRootTypes == acceptedRootTypes) { - findClassGivenDirectory(cachedResult.packageDirectory, cachedResult.root.type) - } else { - search(FindClassRequest(classId, acceptedRootTypes), findClassGivenDirectory) + is SearchResult.Found -> { + if (cachedRequest.acceptedRootTypes == acceptedRootTypes) { + findClassGivenDirectory(cachedResult.packageDirectory, cachedResult.root.type) + } else { + search(FindClassRequest(classId, acceptedRootTypes), findClassGivenDirectory) + } } } } @@ -112,7 +130,7 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie private fun search(request: SearchRequest, handler: (VirtualFile, JavaRoot.RootType) -> T?): T? { // a list of package sub names, ["org", "jb", "kotlin"] - val packagesPath = request.packageFqName.pathSegments().map { it.identifier } + val packagesPath = request.packageFqName.pathSegments().map { it.identifierOrNullIfSpecial ?: return null } // a list of caches corresponding to packages, [default, "org", "org.jb", "org.jb.kotlin"] val caches = cachesPath(packagesPath) @@ -122,12 +140,11 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie // NOTE: indices manipulation instead of using caches.reversed() is here for performance reasons for (cacheIndex in caches.lastIndex downTo 0) { val cacheRootIndices = caches[cacheIndex].rootIndices - for (i in 0 until cacheRootIndices.size) { - val rootIndex = cacheRootIndices.getInt(i) + for (i in 0 until cacheRootIndices.size()) { + val rootIndex = cacheRootIndices[i] if (rootIndex <= processedRootsUpTo) continue // roots with those indices have been processed by now - val directoryInRoot = - travelPath(rootIndex, request.packageFqName, packagesPath, cacheIndex, caches) ?: continue + val directoryInRoot = travelPath(rootIndex, request.packageFqName, packagesPath, cacheIndex, caches) ?: continue val root = roots[rootIndex] if (root.type in request.acceptedRootTypes) { val result = handler(directoryInRoot, root.type) @@ -139,12 +156,7 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie } } } - processedRootsUpTo = - if (cacheRootIndices.isEmpty()) { - processedRootsUpTo - } else { - cacheRootIndices.getInt(cacheRootIndices.size - 1) - } + processedRootsUpTo = if (cacheRootIndices.isEmpty) processedRootsUpTo else cacheRootIndices[cacheRootIndices.size() - 1] } if (request is FindClassRequest) { @@ -166,15 +178,13 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie for (i in (fillCachesAfter + 1) until cachesPath.size) { // we all know roots that contain this package by now cachesPath[i].rootIndices.add(maxIndex) - cachesPath[i].rootIndices.trimToSize(0) + cachesPath[i].rootIndices.trimToSize() } return null } - return synchronized(packageCache) { - packageCache[rootIndex].getOrPut(packageFqName.asString()) { - doTravelPath(rootIndex, packagesPath, fillCachesAfter, cachesPath) - } + return packageCache[rootIndex].getOrPut(packageFqName.asString()) { + doTravelPath(rootIndex, packagesPath, fillCachesAfter, cachesPath) } } @@ -236,12 +246,7 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie return caches } - private fun MutableList.trimToSize(newSize: Int) { - subList(newSize, size).clear() - } - - private data class FindClassRequest(val classId: ClassId, override val acceptedRootTypes: Set) : - SearchRequest { + private data class FindClassRequest(val classId: ClassId, override val acceptedRootTypes: Set) : SearchRequest { override val packageFqName: FqName get() = classId.packageFqName } @@ -261,4 +266,4 @@ internal class JvmDependenciesIndexImpl(_roots: List) : JvmDependencie object NotFound : SearchResult() } -} +} \ No newline at end of file diff --git a/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/KotlinCliJavaFileManagerImpl.kt b/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/KotlinCliJavaFileManagerImpl.kt index ac120b62fe..deb092744e 100644 --- a/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/KotlinCliJavaFileManagerImpl.kt +++ b/subprojects/analysis-kotlin-descriptors/compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/configuration/KotlinCliJavaFileManagerImpl.kt @@ -14,7 +14,17 @@ * limitations under the License. */ -package org.jetbrains.dokka.analysis.kotlin.descriptors.compiler.configuration +/** + * DO NOT MOVE IT + * This is a hack for https://github.com/Kotlin/dokka/issues/1599 + * + * Copy-pasted from Kotlin compiler + * + * It makes this class threadsafe (`topLevelClassesCache` and `binaryCache`) for Dokka + * + */ +@file:Suppress("PackageDirectoryMismatch") +package org.jetbrains.kotlin.cli.jvm.compiler import com.intellij.core.CoreJavaFileManager import com.intellij.openapi.diagnostic.Logger @@ -25,7 +35,6 @@ import com.intellij.psi.impl.file.PsiPackageImpl import com.intellij.psi.search.GlobalSearchScope import gnu.trove.THashMap import gnu.trove.THashSet -import org.jetbrains.kotlin.cli.jvm.compiler.JvmPackagePartProvider import org.jetbrains.kotlin.cli.jvm.index.JavaRoot import org.jetbrains.kotlin.cli.jvm.index.JvmDependenciesIndex import org.jetbrains.kotlin.cli.jvm.index.SingleJavaFileRootsIndex @@ -44,7 +53,7 @@ import org.jetbrains.kotlin.util.PerformanceCounter // TODO: do not inherit from CoreJavaFileManager to avoid accidental usage of its methods which do not use caches/indices // Currently, the only relevant usage of this class as CoreJavaFileManager is at CoreJavaDirectoryService.getPackage, // which is indirectly invoked from PsiPackage.getSubPackages -internal class KotlinCliJavaFileManagerImpl(private val myPsiManager: PsiManager) : CoreJavaFileManager(myPsiManager), KotlinCliJavaFileManager { +class KotlinCliJavaFileManagerImpl(private val myPsiManager: PsiManager) : CoreJavaFileManager(myPsiManager), KotlinCliJavaFileManager { private val perfCounter = PerformanceCounter.create("Find Java class") private lateinit var index: JvmDependenciesIndex private lateinit var singleJavaFileRootsIndex: SingleJavaFileRootsIndex