From 7a5bb2da48d69f7047f24e85148e97ab24c77bf9 Mon Sep 17 00:00:00 2001 From: Thomas Schouten Date: Mon, 24 Feb 2025 20:51:02 +0100 Subject: [PATCH 1/3] Push down read action in commandmanager, and make updateAliases non-blocking, see #3905 --- .../texifyidea/lang/alias/AliasManager.kt | 16 ++- .../texifyidea/lang/alias/CommandManager.kt | 100 +++++++++--------- .../texifyidea/util/CommandAlias.kt | 8 +- src/nl/hannahsten/texifyidea/util/General.kt | 3 +- 4 files changed, 67 insertions(+), 60 deletions(-) diff --git a/src/nl/hannahsten/texifyidea/lang/alias/AliasManager.kt b/src/nl/hannahsten/texifyidea/lang/alias/AliasManager.kt index e925b4b93..a9cecd182 100644 --- 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, 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 index 4b6e300d5..97d27fbe7 100644 --- 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,9 +149,9 @@ object CommandManager : Iterable, 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 @@ -159,53 +159,55 @@ object CommandManager : Iterable, Serializable, AliasManager() { // 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 index f96a42904..5f94860b1 100644 --- 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 { 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 index 4f5705dce..61eeba996 100644 --- 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() } } From c8e0ffa11d32e1751e9685d6e798358bc1a21b2c Mon Sep 17 00:00:00 2001 From: Thomas Schouten Date: Mon, 24 Feb 2025 20:57:42 +0100 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a22ee0ff5..9de1e1406 100644 --- 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 From b9a941a984cb5c2262cc2ecb83dba536cfdfc66d Mon Sep 17 00:00:00 2001 From: Thomas Schouten Date: Tue, 25 Feb 2025 07:56:52 +0100 Subject: [PATCH 3/3] Try with blocking update in tests --- .../LatexFileNotFoundInspectionTest.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/nl/hannahsten/texifyidea/inspections/latex/probablebugs/LatexFileNotFoundInspectionTest.kt b/test/nl/hannahsten/texifyidea/inspections/latex/probablebugs/LatexFileNotFoundInspectionTest.kt index 47fb8d16c..72e8c1804 100644 --- a/test/nl/hannahsten/texifyidea/inspections/latex/probablebugs/LatexFileNotFoundInspectionTest.kt +++ b/test/nl/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{doesnotexist.tex}""") // 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{doesnotexist.tex}{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) + } + } } \ No newline at end of file