Merge pull request #3914 from Hannah-Sten/command-alias-update-freeze

Improve performance of alias manager
Hannah-Sten · Feb 25, 2025 · f64d8a3 · f64d8a3
2 parents f581c86 + b9a941a
commit f64d8a3
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 ### Added
+* Improve performance of the alias manager
 * Add WSL support to the bibtex run configuration, using a new setting to choose the LaTeX distribution
 * Default to 0.15.0 behaviour if inputs field is not found in Tectonic.toml
 

diff --git a/src/nl/hannahsten/texifyidea/lang/alias/AliasManager.kt b/src/nl/hannahsten/texifyidea/lang/alias/AliasManager.kt
@@ -1,5 +1,6 @@
 package nl.hannahsten.texifyidea.lang.alias
 
+import com.intellij.openapi.application.runReadAction
 import com.intellij.openapi.project.Project
 import nl.hannahsten.texifyidea.index.LatexDefinitionIndex
 import nl.hannahsten.texifyidea.psi.LatexCommands
@@ -10,6 +11,9 @@ import java.util.concurrent.ConcurrentHashMap
  */
 abstract class AliasManager {
 
+    // This doesn't completely guarantee that only one refresh will happen at a time, but at least we limit the amount of unnecessary work
+    private var isCacheFillInProgress = false
+
     /**
      * Maps a command to a set of aliases including the command itself.
      * Similar for environments.
@@ -122,7 +126,6 @@ abstract class AliasManager {
      * If needed (based on the number of indexed \newcommand-like commands) check for new aliases of the given alias set. This alias can be any alias of its alias set.
      * If the alias set is not yet registered, it will be registered as a new alias set.
      */
-    @Synchronized
     fun updateAliases(aliasSet: Collection<String>, project: Project) {
         // Register if needed
         if (aliasSet.isEmpty()) return
@@ -136,22 +139,27 @@ abstract class AliasManager {
 
         // If the command name itself is not directly in the given set, check if it is perhaps an alias of a command in the set
         // Uses projectScope now, may be improved to filesetscope
-        val indexedCommandDefinitions = LatexDefinitionIndex.Util.getItems(project).toSet()
+        val indexedCommandDefinitions = runReadAction { LatexDefinitionIndex.Util.getItems(project).toSet() }
 
         // Check if something has changed (the number of indexed command might be the same while the content is different), and if so, update the aliases.
         // Also do this the first time something is registered, because then we have to update aliases as well
         val hasChanged = this.indexedCommandDefinitions != indexedCommandDefinitions
-        if (hasChanged || wasRegistered) {
+        // If a refresh is already in progress, no need to start another one, but no need to block this thread either
+        if (!isCacheFillInProgress && (hasChanged || wasRegistered)) {
             // Update everything, since it is difficult to know beforehand what aliases could be added or not
             // Alternatively we could save a numberOfIndexedCommandDefinitions per alias set, and only update the
             // requested alias set (otherwise only the first alias set requesting an update will get it)
             // We have to deepcopy the set of alias sets before iterating over it, because we want to modify aliases
-            synchronized(aliases) {
+            try {
+                isCacheFillInProgress = true
                 val deepCopy = aliases.values.map { it1 -> it1.map { it }.toSet() }.toSet()
                 for (copiedAliasSet in deepCopy) {
                     findAllAliases(copiedAliasSet, indexedCommandDefinitions)
                 }
             }
+            finally {
+                isCacheFillInProgress = false
+            }
 
             this.indexedCommandDefinitions = indexedCommandDefinitions.toSet()
         }

diff --git a/src/nl/hannahsten/texifyidea/lang/alias/CommandManager.kt b/src/nl/hannahsten/texifyidea/lang/alias/CommandManager.kt
@@ -1,5 +1,6 @@
 package nl.hannahsten.texifyidea.lang.alias
 
+import com.intellij.openapi.application.runReadAction
 import nl.hannahsten.texifyidea.lang.LabelingCommandInformation
 import nl.hannahsten.texifyidea.psi.LatexCommands
 import nl.hannahsten.texifyidea.util.containsAny
@@ -12,7 +13,6 @@ import java.util.*
 import java.util.concurrent.ConcurrentHashMap
 import java.util.stream.Stream
 import java.util.stream.StreamSupport
-import kotlin.collections.set
 
 /**
  * Manages all available LaTeX commands and their aliases.
@@ -149,63 +149,65 @@ object CommandManager : Iterable<String?>, Serializable, AliasManager() {
         indexedDefinitions.filter {
             // Assume the parameter definition has the command being defined in the first required parameter,
             // and the command definition itself in the second
-            it.requiredParameter(1)?.containsAny(aliasSet) == true
+            runReadAction { it.requiredParameter(1)?.containsAny(aliasSet) == true }
         }
-            .mapNotNull { it.requiredParameter(0) }
+            .mapNotNull { runReadAction { it.requiredParameter(0) } }
             .forEach { registerAlias(firstAlias, it) }
 
         // Extract label parameter positions
         // Assumes the predefined label definitions all have the label parameter in the same position
         // For example, in \newcommand{\mylabel}[2]{\section{#1}\label{sec:#2}} we want to parse out the 2 in #2
         if (aliasSet.intersect(CommandMagic.labelDefinitionsWithoutCustomCommands).isNotEmpty()) {
             indexedDefinitions.forEach { commandDefinition ->
-                val definedCommand = commandDefinition.requiredParameter(0) ?: return@forEach
-                if (definedCommand.isBlank()) return@forEach
-
-                val isFirstParameterOptional = commandDefinition.parameterList.filter { it.optionalParam != null }.size > 1
-
-                val parameterCommands = commandDefinition.requiredParameters().getOrNull(1)
-                    ?.requiredParamContentList
-                    ?.flatMap { it.childrenOfType(LatexCommands::class) }
-                    ?.asSequence()
-
-                // Positions of label parameters in the custom commands (starting from 0)
-                val positions = parameterCommands
-                    ?.filter { it.name in CommandMagic.labelDefinitionsWithoutCustomCommands }
-                    ?.mapNotNull { it.requiredParameter(0) }
-                    ?.mapNotNull {
-                        if (it.indexOf('#') != -1) {
-                            it.getOrNull(it.indexOf('#') + 1)
+                runReadAction {
+                    val definedCommand = commandDefinition.requiredParameter(0) ?: return@runReadAction
+                    if (definedCommand.isBlank()) return@runReadAction
+
+                    val isFirstParameterOptional = commandDefinition.parameterList.filter { it.optionalParam != null }.size > 1
+
+                    val parameterCommands = commandDefinition.requiredParameters().getOrNull(1)
+                        ?.requiredParamContentList
+                        ?.flatMap { it.childrenOfType(LatexCommands::class) }
+                        ?.asSequence()
+
+                    // Positions of label parameters in the custom commands (starting from 0)
+                    val positions = parameterCommands
+                        ?.filter { it.name in CommandMagic.labelDefinitionsWithoutCustomCommands }
+                        ?.mapNotNull { it.requiredParameter(0) }
+                        ?.mapNotNull {
+                            if (it.indexOf('#') != -1) {
+                                it.getOrNull(it.indexOf('#') + 1)
+                            }
+                            else null
                         }
-                        else null
-                    }
-                    ?.map(Character::getNumericValue)
-                    // LaTeX starts from 1, we from 0 (consistent with how we count required parameters)
-                    ?.map { it - 1 }
-                    // For the moment we only consider required parameters and ignore the optional one
-                    ?.map { if (isFirstParameterOptional) it - 1 else it }
-                    ?.filter { it >= 0 }
-                    ?.toList() ?: return@forEach
-                if (positions.isEmpty()) return@forEach
-
-                // Check if there is a command which increases a counter before the \label
-                // If so, the \label just labels the counter increasing command, and not whatever will appear before usages of the custom labeling command
-                val definitionContainsIncreaseCounterCommand =
-                    parameterCommands.takeWhile { it.name !in CommandMagic.labelDefinitionsWithoutCustomCommands }
-                        .any { it.name in CommandMagic.increasesCounter }
-
-                val prefix = parameterCommands.filter { it.name in CommandMagic.labelDefinitionsWithoutCustomCommands }
-                    .mapNotNull { it.requiredParameter(0) }
-                    .map {
-                        if (it.indexOf('#') != -1) {
-                            val prefix = it.substring(0, it.indexOf('#'))
-                            prefix.ifBlank { "" }
-                        }
-                        else ""
-                    }.firstOrNull() ?: ""
-
-                labelAliasesInfo[definedCommand] =
-                    LabelingCommandInformation(positions, !definitionContainsIncreaseCounterCommand, prefix)
+                        ?.map(Character::getNumericValue)
+                        // LaTeX starts from 1, we from 0 (consistent with how we count required parameters)
+                        ?.map { it - 1 }
+                        // For the moment we only consider required parameters and ignore the optional one
+                        ?.map { if (isFirstParameterOptional) it - 1 else it }
+                        ?.filter { it >= 0 }
+                        ?.toList() ?: return@runReadAction
+                    if (positions.isEmpty()) return@runReadAction
+
+                    // Check if there is a command which increases a counter before the \label
+                    // If so, the \label just labels the counter increasing command, and not whatever will appear before usages of the custom labeling command
+                    val definitionContainsIncreaseCounterCommand =
+                        parameterCommands.takeWhile { it.name !in CommandMagic.labelDefinitionsWithoutCustomCommands }
+                            .any { it.name in CommandMagic.increasesCounter }
+
+                    val prefix = parameterCommands.filter { it.name in CommandMagic.labelDefinitionsWithoutCustomCommands }
+                        .mapNotNull { it.requiredParameter(0) }
+                        .map {
+                            if (it.indexOf('#') != -1) {
+                                val prefix = it.substring(0, it.indexOf('#'))
+                                prefix.ifBlank { "" }
+                            }
+                            else ""
+                        }.firstOrNull() ?: ""
+
+                    labelAliasesInfo[definedCommand] =
+                        LabelingCommandInformation(positions, !definitionContainsIncreaseCounterCommand, prefix)
+                }
             }
         }
     }

diff --git a/src/nl/hannahsten/texifyidea/util/CommandAlias.kt b/src/nl/hannahsten/texifyidea/util/CommandAlias.kt
@@ -1,7 +1,5 @@
 package nl.hannahsten.texifyidea.util
 
-import com.intellij.openapi.application.ApplicationManager
-import com.intellij.openapi.application.runReadAction
 import com.intellij.openapi.project.Project
 import nl.hannahsten.texifyidea.lang.alias.CommandManager
 import nl.hannahsten.texifyidea.lang.commands.LatexCommand
@@ -19,13 +17,11 @@ fun updateAndGetIncludeCommands(project: Project): Set<String> {
 fun updateIncludeCommandsAliasesAsync(project: Project) {
     if (!isUpdatingIncludeAliases.getAndSet(true)) {
         // Don't run with progress indicator, because this takes a short time (a few tenths) and runs in practice on every letter typed
-        ApplicationManager.getApplication().invokeLater {
+        runInBackgroundWithoutProgress {
             try {
                 // Because every command has different parameters and behaviour (e.g. allowed file types), we keep track of them separately
                 for (command in defaultIncludeCommands) {
-                    runReadAction {
-                        CommandManager.updateAliases(setOf(command), project)
-                    }
+                    CommandManager.updateAliases(setOf(command), project)
                 }
             }
             finally {

diff --git a/src/nl/hannahsten/texifyidea/util/General.kt b/src/nl/hannahsten/texifyidea/util/General.kt
@@ -120,8 +120,9 @@ suspend fun runInBackground(project: Project, description: String, function: sus
     }
 }
 
+// https://plugins.jetbrains.com/docs/intellij/background-processes.html
 fun runInBackgroundWithoutProgress(function: () -> Unit) {
-    ApplicationManager.getApplication().invokeLater {
+    ApplicationManager.getApplication().executeOnPooledThread {
         function()
     }
 }
diff --git a/...l/hannahsten/texifyidea/inspections/latex/probablebugs/LatexFileNotFoundInspectionTest.kt b/...l/hannahsten/texifyidea/inspections/latex/probablebugs/LatexFileNotFoundInspectionTest.kt
@@ -1,13 +1,15 @@
 package nl.hannahsten.texifyidea.inspections.latex.probablebugs
 
+import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.SystemInfo
 import io.mockk.every
 import io.mockk.mockkStatic
 import nl.hannahsten.texifyidea.file.LatexFileType
 import nl.hannahsten.texifyidea.gutter.LatexNavigationGutter
 import nl.hannahsten.texifyidea.inspections.TexifyInspectionTestBase
+import nl.hannahsten.texifyidea.lang.alias.CommandManager
+import nl.hannahsten.texifyidea.util.defaultIncludeCommands
 import nl.hannahsten.texifyidea.util.runCommandWithExitCode
-import nl.hannahsten.texifyidea.util.updateIncludeCommandsAliasesAsync
 import java.io.File
 import java.nio.file.Path
 import java.nio.file.Paths
@@ -196,14 +198,20 @@ class LatexFileNotFoundInspectionTest : TexifyInspectionTestBase(LatexFileNotFou
     fun testCommandAlias() {
         myFixture.configureByText(LatexFileType, """\newcommand{\myinput}{\input} \myinput{<error descr="File 'doesnotexist.tex' not found">doesnotexist.tex</error>}""")
         // In practice, this will be triggered by the first something to ask for include commands aliases, for performance reasons
-        updateIncludeCommandsAliasesAsync(myFixture.project)
+        updateIncludeCommandsBlocking(myFixture.project)
         myFixture.checkHighlighting()
     }
 
     fun testCommandAliasMoreParameters() {
         myFixture.configureByText(LatexFileType, """\newcommand{\myinput}[2]{\input{#1}\section{#2}} \myinput{<error descr="File 'doesnotexist.tex' not found">doesnotexist.tex</error>}{My section}""")
         // In practice, this will be triggered by the first something to ask for include commands aliases, for performance reasons
-        updateIncludeCommandsAliasesAsync(myFixture.project)
+        updateIncludeCommandsBlocking(myFixture.project)
         myFixture.checkHighlighting()
     }
+
+    fun updateIncludeCommandsBlocking(project: Project) {
+        for (command in defaultIncludeCommands) {
+            CommandManager.updateAliases(setOf(command), project)
+        }
+    }
 }