From aa2d1edc999ed42bf2b7e92717cc27daf6776ac7 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Thu, 5 Sep 2024 21:04:12 +0200 Subject: [PATCH] optimization: cache inspections with inconsistent shortName <-> EP.shortName to avoid wasting time enumerating all of them on each highlighting restart (and move away `new LocalInspectionWrapper()` calls from class initializer to avoid fatal ClassDefNotFoundException when is throws) GitOrigin-RevId: 7e2117288d8f475e5aae7bac830684c67858d080 --- .../ex/InspectionToolRegistrar.kt | 59 +++++++++++++++---- .../ex/LocalInspectionToolWrapper.java | 12 +--- .../ui/SpellCheckingEditorCustomization.java | 55 ++++++++++------- 3 files changed, 82 insertions(+), 44 deletions(-) diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/ex/InspectionToolRegistrar.kt b/platform/analysis-impl/src/com/intellij/codeInspection/ex/InspectionToolRegistrar.kt index 36d69a8cb337..72d87b133c85 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/ex/InspectionToolRegistrar.kt +++ b/platform/analysis-impl/src/com/intellij/codeInspection/ex/InspectionToolRegistrar.kt @@ -15,8 +15,12 @@ import com.intellij.openapi.extensions.ExtensionPointName import com.intellij.openapi.extensions.PluginDescriptor import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.progress.ProgressManager +import com.intellij.util.ConcurrencyUtil import com.intellij.util.SmartList import com.intellij.util.containers.CollectionFactory +import org.jetbrains.annotations.ApiStatus +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ConcurrentMap private val LOG = logger() private val EP_NAME = ExtensionPointName("com.intellij.inspectionToolProvider") @@ -49,10 +53,12 @@ class InspectionToolRegistrar : InspectionToolsSupplier() { } } - private fun registerInspections(factories: MutableMap>, - app: Application, - shortNames: MutableMap, - extensionPointName: ExtensionPointName) { + private fun registerInspections( + factories: MutableMap>, + app: Application, + shortNames: MutableMap, + extensionPointName: ExtensionPointName, + ) { val isInternal = app.isInternal for (extension in extensionPointName.extensionList) { registerInspection(extension, shortNames, isInternal, factories) @@ -94,6 +100,7 @@ class InspectionToolRegistrar : InspectionToolsSupplier() { for (listener in listeners) { listener.toolAdded(inspectionToolWrapper) } + inconsistentInspectionNameCache.clear() } private fun fireToolRemoved(factory: InspectionFactory) { @@ -101,6 +108,7 @@ class InspectionToolRegistrar : InspectionToolsSupplier() { for (listener in listeners) { listener.toolRemoved(inspectionToolWrapper) } + inconsistentInspectionNameCache.remove(inspectionToolWrapper.myTool?.javaClass ?: Int::class.java) } override fun createTools(): List> { @@ -117,12 +125,37 @@ class InspectionToolRegistrar : InspectionToolsSupplier() { } return tools } + + /** + * cache of inspection tool class -> inspection EP for which LocalInspectionEP.LOCAL_INSPECTION.getByKey does not work, + * because sometimes tool.getShortName() is inconsistent with `shortName="xxx"` in plugin.xml. For example: CheckDtdReferencesInspection + * + * @see com.intellij.codeInspection.ex.LocalInspectionToolWrapper.findInspectionEP + */ + private val inconsistentInspectionNameCache: ConcurrentMap, LocalInspectionEP> = ConcurrentHashMap() + private val NULL_EP: LocalInspectionEP = LocalInspectionEP() // means find was unsuccessful, null result is cached + @ApiStatus.Internal + fun findInspectionEP(tool: LocalInspectionTool): LocalInspectionEP? { + val byKey = LocalInspectionEP.LOCAL_INSPECTION.getByKey(tool.shortName, LocalInspectionToolWrapper::class.java) { it.getShortName() } + if (byKey != null) { + return byKey + } + val toolClass: Class<*> = tool.javaClass + var ep = inconsistentInspectionNameCache[toolClass] + if (ep == null) { + ep = LocalInspectionEP.LOCAL_INSPECTION.findFirstSafe { toolClass.name == it.implementationClass } + ep = ConcurrencyUtil.cacheOrGet(inconsistentInspectionNameCache, toolClass, ep?:NULL_EP) + } + return if (ep === NULL_EP) null else ep + } } -private fun registerInspection(inspection: T, - shortNames: MutableMap, - isInternal: Boolean, - factories: MutableMap>): (InspectionFactory)? { +private fun registerInspection( + inspection: InspectionEP, + shortNames: MutableMap, + isInternal: Boolean, + factories: MutableMap>, +): (InspectionFactory)? { checkForDuplicateShortName(inspection, shortNames) if (!isInternal && inspection.isInternal) { return null @@ -135,10 +168,12 @@ private fun registerInspection(inspection: T, return factory } -private fun registerToolProvider(provider: InspectionToolProvider, - pluginDescriptor: PluginDescriptor, - keyToFactories: MutableMap>, - added: MutableList?) { +private fun registerToolProvider( + provider: InspectionToolProvider, + pluginDescriptor: PluginDescriptor, + keyToFactories: MutableMap>, + added: MutableList?, +) { val factories = keyToFactories.computeIfAbsent(provider) { ArrayList() } for (aClass in provider.inspectionClasses) { val supplier = { diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/ex/LocalInspectionToolWrapper.java b/platform/analysis-impl/src/com/intellij/codeInspection/ex/LocalInspectionToolWrapper.java index e27ff800f00b..ab9a7278cc14 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/ex/LocalInspectionToolWrapper.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/ex/LocalInspectionToolWrapper.java @@ -11,7 +11,7 @@ import org.jetbrains.annotations.Nullable; public class LocalInspectionToolWrapper extends InspectionToolWrapper { /** This should be used in tests primarily */ public LocalInspectionToolWrapper(@NotNull LocalInspectionTool tool) { - super(tool, findInspectionEP(tool)); + super(tool, InspectionToolRegistrar.getInstance().findInspectionEP(tool)); } public LocalInspectionToolWrapper(@NotNull LocalInspectionEP ep) { @@ -82,14 +82,4 @@ public class LocalInspectionToolWrapper extends InspectionToolWrapper tool.getClass().getName().equals(ep.implementationClass)); - } } diff --git a/spellchecker/src/com/intellij/spellchecker/ui/SpellCheckingEditorCustomization.java b/spellchecker/src/com/intellij/spellchecker/ui/SpellCheckingEditorCustomization.java index 2910eb9da1d5..c4574ace8ff0 100644 --- a/spellchecker/src/com/intellij/spellchecker/ui/SpellCheckingEditorCustomization.java +++ b/spellchecker/src/com/intellij/spellchecker/ui/SpellCheckingEditorCustomization.java @@ -34,36 +34,49 @@ import java.util.function.Function; * Thread-safe. */ public class SpellCheckingEditorCustomization extends SimpleEditorCustomization { - private static final Map SPELL_CHECK_TOOLS = new HashMap<>(); - private static final boolean READY = init(); + private static volatile Map SPELL_CHECK_TOOLS; + private static volatile boolean READY; SpellCheckingEditorCustomization(boolean enabled) { super(enabled); } - @SuppressWarnings("unchecked") - private static boolean init() { - // It's assumed that default spell checking inspection settings are just fine for processing all types of data. - // Please perform corresponding settings tuning if that assumption is broken in the future. - - Class[] inspectionClasses = (Class[])new Class[]{SpellCheckingInspection.class}; - for (Class inspectionClass : inspectionClasses) { - try { - LocalInspectionTool tool = inspectionClass.newInstance(); - SPELL_CHECK_TOOLS.put(tool.getShortName(), new LocalInspectionToolWrapper(tool)); - } - catch (Throwable e) { - return false; + @NotNull + private static Map getSpellCheckTools() { + Map tools = SPELL_CHECK_TOOLS; + if (tools == null) { + tools = new HashMap<>(); + // It's assumed that default spell checking inspection settings are just fine for processing all types of data. + // Please perform corresponding settings tuning if that assumption is broken in the future. + Class[] inspectionClasses = (Class[])new Class[]{SpellCheckingInspection.class}; + for (Class inspectionClass : inspectionClasses) { + try { + LocalInspectionTool tool = inspectionClass.newInstance(); + tools.put(tool.getShortName(), new LocalInspectionToolWrapper(tool)); + } + catch (Throwable e) { + READY = false; + return Map.of(); + } } + SPELL_CHECK_TOOLS = tools; + READY = true; } - return true; + return tools; + } + + private static boolean isReady() { + if (SPELL_CHECK_TOOLS == null) { + getSpellCheckTools(); + } + return READY; } @Override public void customize(@NotNull EditorEx editor) { boolean apply = isEnabled(); - if (!READY) { + if (!isReady()) { return; } @@ -106,8 +119,8 @@ public class SpellCheckingEditorCustomization extends SimpleEditorCustomization return strategy instanceof MyInspectionProfileStrategy && !((MyInspectionProfileStrategy)strategy).myUseSpellCheck; } - public static Set getSpellCheckingToolNames() { - return Collections.unmodifiableSet(SPELL_CHECK_TOOLS.keySet()); + static Set getSpellCheckingToolNames() { + return Collections.unmodifiableSet(getSpellCheckTools().keySet()); } private static class MyInspectionProfileStrategy implements Function { @@ -117,7 +130,7 @@ public class SpellCheckingEditorCustomization extends SimpleEditorCustomization @Override public @NotNull InspectionProfileWrapper apply(@NotNull InspectionProfile profile) { - if (!READY) { + if (!isReady()) { return new InspectionProfileWrapper((InspectionProfileImpl)profile); } MyInspectionProfileWrapper wrapper = myWrappers.get(profile); @@ -141,7 +154,7 @@ public class SpellCheckingEditorCustomization extends SimpleEditorCustomization @Override public boolean isToolEnabled(HighlightDisplayKey key, PsiElement element) { - return (key != null && SPELL_CHECK_TOOLS.containsKey(key.getShortName()) ? myUseSpellCheck : super.isToolEnabled(key, element)); + return (key != null && getSpellCheckTools().containsKey(key.getShortName()) ? myUseSpellCheck : super.isToolEnabled(key, element)); } } }