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.

Sign up for GitHub

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

Jump to bottom

PERF: reduce PSI tree access in very hot item resolution #4210

Merged
merged 1 commit into from Aug 5, 2019
Merged

PERF: reduce PSI tree access in very hot item resolution #4210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/kotlin/org/rust/cargo/runconfig/test/CargoBenchRunConfigurationProducer.kt
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ import com.intellij.psi.PsiElement
import org.rust.lang.core.psi.RsFunction
import org.rust.lang.core.psi.ext.RsMod
import org.rust.lang.core.psi.ext.isBench
import org.rust.lang.core.psi.ext.processExpandedItemsExceptImpls
import org.rust.lang.core.psi.ext.processExpandedItemsExceptImplsAndUses

class CargoBenchRunConfigurationProducer : CargoTestRunConfigurationProducerBase() {
override val commandName: String = "bench"
Expand All @@ -23,6 +23,6 @@ class CargoBenchRunConfigurationProducer : CargoTestRunConfigurationProducerBase

companion object {
private fun hasBenchFunction(mod: RsMod): Boolean =
mod.processExpandedItemsExceptImpls { it is RsFunction && it.isBench || it is RsMod && hasBenchFunction(it) }
mod.processExpandedItemsExceptImplsAndUses { it is RsFunction && it.isBench || it is RsMod && hasBenchFunction(it) }
}
}
2 changes: 1 addition & 1 deletion src/main/kotlin/org/rust/cargo/runconfig/test/CargoTestRunConfigurationProducer.kt
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ class CargoTestRunConfigurationProducer : CargoTestRunConfigurationProducerBase(

companion object {
private fun hasTestFunction(mod: RsMod): Boolean =
mod.processExpandedItemsExceptImpls { it is RsFunction && it.isTest || it is RsMod && hasTestFunction(it) }
mod.processExpandedItemsExceptImplsAndUses { it is RsFunction && it.isTest || it is RsMod && hasTestFunction(it) }
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/org/rust/lang/core/psi/RsPsiImplUtil.kt
View file
Open in desktop
Expand Up @@ -118,4 +118,4 @@ object RsPsiImplUtil {
}

private fun RsMod.hasChildModules(): Boolean =
expandedItemsExceptImpls.any { it is RsModDeclItem || it is RsModItem && it.hasChildModules() }
expandedItemsExceptImplsAndUses.any { it is RsModDeclItem || it is RsModItem && it.hasChildModules() }
76 changes: 64 additions & 12 deletions src/main/kotlin/org/rust/lang/core/psi/ext/RsItemsOwner.kt
View file
Open in desktop
Expand Up @@ -12,10 +12,11 @@ import com.intellij.psi.util.CachedValue
import com.intellij.psi.util.CachedValueProvider
import com.intellij.psi.util.CachedValuesManager
import com.intellij.util.SmartList
import org.rust.lang.core.psi.RsFile
import org.rust.lang.core.psi.RsImplItem
import org.rust.lang.core.psi.RsMacroCall
import org.rust.lang.core.psi.rustStructureOrAnyPsiModificationTracker
import org.rust.lang.core.psi.*
import org.rust.lang.core.psi.ext.RsCachedItems.CachedNamedImport
import org.rust.lang.core.psi.ext.RsCachedItems.CachedStarImport
import org.rust.openapiext.testAssert
import org.rust.stdext.optimizeList

interface RsItemsOwner : RsElement

Expand All @@ -42,29 +43,73 @@ val RsItemsOwner.itemsAndMacros: Sequence<RsElement>
}.filterIsInstance<RsElement>()
}

inline fun RsItemsOwner.processExpandedItemsExceptImpls(processor: (RsItemElement) -> Boolean): Boolean {
for (element in expandedItemsExceptImpls) {
inline fun RsItemsOwner.processExpandedItemsExceptImplsAndUses(processor: (RsItemElement) -> Boolean): Boolean {
for (element in expandedItemsExceptImplsAndUses) {
if (processor(element)) return true
}
return false
}

private val EXPANDED_ITEMS_KEY: Key<CachedValue<List<RsItemElement>>> = Key.create("EXPANDED_ITEMS_KEY")
val RsItemsOwner.expandedItemsExceptImplsAndUses: List<RsItemElement>
get() = expandedItemsCached.rest

val RsItemsOwner.expandedItemsExceptImpls: List<RsItemElement>
private val EXPANDED_ITEMS_KEY: Key<CachedValue<RsCachedItems>> = Key.create("EXPANDED_ITEMS_KEY")

val RsItemsOwner.expandedItemsCached: RsCachedItems
get() = CachedValuesManager.getCachedValue(this, EXPANDED_ITEMS_KEY) {
val items = SmartList<RsItemElement>()
val namedImports = SmartList<CachedNamedImport>()
val starImports = SmartList<CachedStarImport>()
val rest = SmartList<RsItemElement>()
processExpandedItemsInternal {
// optimization: impls are not named elements, so we don't need them for name resolution
if (it !is RsImplItem) items.add(it)
when (it) {
// Optimization: impls are not named elements, so we don't need them for name resolution
is RsImplItem -> Unit

// Optimization: prepare use items to reduce PSI tree access in hotter code
is RsUseItem -> {
val isPublic = it.isPublic
it.useSpeck?.forEachLeafSpeck { speck ->
if (speck.isStarImport) {
starImports += CachedStarImport(isPublic, speck)
} else {
testAssert { speck.useGroup == null }
val path = speck.path ?: return@forEachLeafSpeck
val nameInScope = speck.nameInScope ?: return@forEachLeafSpeck
val isAtom = speck.alias == null && path.isAtom
namedImports += CachedNamedImport(isPublic, path, nameInScope, isAtom)
}
}
}

else -> rest.add(it)
}
false
}
CachedValueProvider.Result.create(
if (items.isNotEmpty()) items else emptyList(),
RsCachedItems(namedImports.optimizeList(), starImports.optimizeList(), rest.optimizeList()),
rustStructureOrAnyPsiModificationTracker
)
}

/**
* Used for optimization purposes, to reduce access to a cache and PSI tree in one very hot
* place - [org.rust.lang.core.resolve.processItemDeclarations]
*/
class RsCachedItems(
val namedImports: List<CachedNamedImport>,
val starImports: List<CachedStarImport>,
val rest: List<RsItemElement>
) {
data class CachedNamedImport(
val isPublic: Boolean,
val path: RsPath,
val nameInScope: String,
val isAtom: Boolean
)

data class CachedStarImport(val isPublic: Boolean, val speck: RsUseSpeck)
}

private fun RsItemsOwner.processExpandedItemsInternal(processor: (RsItemElement) -> Boolean): Boolean {
return itemsAndMacros.any { it.processItem(processor) }
}
Expand All @@ -74,3 +119,10 @@ private fun RsElement.processItem(processor: (RsItemElement) -> Boolean) = when
is RsItemElement -> processor(this)
else -> false
}

private val RsPath.isAtom: Boolean
get() = when (kind) {
PathKind.IDENTIFIER -> qualifier == null
PathKind.SELF -> qualifier?.isAtom == true
else -> false
}
36 changes: 10 additions & 26 deletions src/main/kotlin/org/rust/lang/core/resolve/ItemResolution.kt
View file
Open in desktop
Expand Up @@ -53,25 +53,16 @@ fun processItemDeclarations(
): Boolean {
val withPrivateImports = ipm != ItemProcessingMode.WITHOUT_PRIVATE_IMPORTS

val starImports = mutableListOf<RsUseSpeck>()
val itemImports = mutableListOf<RsUseSpeck>()

val directlyDeclaredNames = HashSet<String>()
val processor = { e: ScopeEntry ->
directlyDeclaredNames += e.name
originalProcessor(e)
}

loop@ for (item in scope.expandedItemsExceptImpls) {
when (item) {
is RsUseItem ->
if (item.isPublic || withPrivateImports) {
val rootSpeck = item.useSpeck ?: continue@loop
rootSpeck.forEachLeafSpeck { speck ->
(if (speck.isStarImport) starImports else itemImports) += speck
}
}
val cachedItems = scope.expandedItemsCached

loop@ for (item in cachedItems.rest) {
when (item) {
// Unit like structs are both types and values
is RsStructItem -> {
if (item.namespaces.intersects(ns) && processor(item)) return true
Expand Down Expand Up @@ -100,12 +91,10 @@ fun processItemDeclarations(
}

val isEdition2018 = scope.isEdition2018
for (speck in itemImports) {
check(speck.useGroup == null)
val path = speck.path ?: continue
val name = speck.nameInScope ?: continue
for ((isPublic, path, name, isAtom) in cachedItems.namedImports) {
if (!(isPublic || withPrivateImports)) continue

if (isEdition2018 && speck.alias == null && path.isAtom) {
if (isEdition2018 && isAtom) {
// Use items like `use foo;` or `use foo::{self}` are meaningless on 2018 edition.
// We should ignore it or it breaks resolve of such `foo` in other places.
ItemResolutionTestmarks.extraAtomUse.hit()
Expand Down Expand Up @@ -141,7 +130,7 @@ fun processItemDeclarations(
val crateRoot = scope.crateRoot
if (crateRoot != null) {
val result = processWithShadowing(directlyDeclaredNames, processor) { shadowingProcessor ->
crateRoot.processExpandedItemsExceptImpls { item ->
crateRoot.processExpandedItemsExceptImplsAndUses { item ->
if (item is RsExternCrateItem) {
processExternCrateItem(item, shadowingProcessor, true)
} else {
Expand Down Expand Up @@ -170,7 +159,9 @@ fun processItemDeclarations(
if (result) return true
}

for (speck in starImports) {
for ((isPublic, speck) in cachedItems.starImports) {
if (!(isPublic || withPrivateImports)) continue

val path = speck.path
val basePath = if (path == null && speck.context is RsUseGroup) {
// `use foo::bar::{self, *}`
Expand Down Expand Up @@ -293,13 +284,6 @@ fun processItemDeclarationsWithCache(
}
}

private val RsPath.isAtom: Boolean
get() = when (kind) {
PathKind.IDENTIFIER -> qualifier == null
PathKind.SELF -> qualifier?.isAtom == true
else -> false
}

enum class ItemProcessingMode(val withExternCrates: Boolean) {
WITHOUT_PRIVATE_IMPORTS(false),
WITH_PRIVATE_IMPORTS(false),
Expand Down