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

Restore fix #1599 #3143

Merged
merged 1 commit into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions subprojects/analysis-kotlin-descriptors/compiler/api/compiler.api
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,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 <init> (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 <init> (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
}

Original file line number Diff line number Diff line change
Expand Up @@ -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<JavaRoot>) : JvmDependenciesIndex {
class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : JvmDependenciesIndex {
private val lock = ReentrantLock()
Copy link
Member

Choose a reason for hiding this comment

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

As a side note as it wasn't obvious for me until I asked Vadim about it: these classes were copy-pasted from the compiler (or, rather, the copy-paste was updated). So this change with adding ReentrantLock is coming from there:

https://github.com/JetBrains/kotlin/blob/979d1b085a757c3a99629d33d7046717512a1750/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/index/JvmDependenciesIndexImpl.kt#L23

I hope they tested it well and it won't introduce more performance regressions or other types of concurrency issues 🤞


//these fields are computed based on _roots passed to constructor which are filled in later
private val roots: List<JavaRoot> by lazy { _roots.toList() }

Expand All @@ -46,7 +58,8 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : 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
Expand All @@ -55,7 +68,7 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : JvmDependencie
Cache().apply {
roots.indices.forEach(rootIndices::add)
rootIndices.add(maxIndex)
rootIndices.trimToSize(0)
rootIndices.trimToSize()
}
}

Expand All @@ -74,8 +87,10 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : JvmDependencie
acceptedRootTypes: Set<JavaRoot.RootType>,
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
}
}
}

Expand All @@ -85,34 +100,37 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : JvmDependencie
acceptedRootTypes: Set<JavaRoot.RootType>,
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)
}
}
}
}
}

private fun <T : Any> 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)

Expand All @@ -122,12 +140,11 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : 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)
Expand All @@ -139,12 +156,7 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : 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) {
Expand All @@ -166,15 +178,13 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : 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)
}
}

Expand Down Expand Up @@ -236,12 +246,7 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : JvmDependencie
return caches
}

private fun <E> MutableList<E>.trimToSize(newSize: Int) {
subList(newSize, size).clear()
}

private data class FindClassRequest(val classId: ClassId, override val acceptedRootTypes: Set<JavaRoot.RootType>) :
SearchRequest {
private data class FindClassRequest(val classId: ClassId, override val acceptedRootTypes: Set<JavaRoot.RootType>) : SearchRequest {
override val packageFqName: FqName
get() = classId.packageFqName
}
Expand All @@ -261,4 +266,4 @@ internal class JvmDependenciesIndexImpl(_roots: List<JavaRoot>) : JvmDependencie

object NotFound : SearchResult()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down