diff --git a/platform/core-impl/src/com/intellij/ide/plugins/PluginManagerCore.java b/platform/core-impl/src/com/intellij/ide/plugins/PluginManagerCore.java index 2d4982d8bcf1..51643598fa18 100644 --- a/platform/core-impl/src/com/intellij/ide/plugins/PluginManagerCore.java +++ b/platform/core-impl/src/com/intellij/ide/plugins/PluginManagerCore.java @@ -1651,11 +1651,6 @@ public class PluginManagerCore { for (IdeaPluginDescriptorImpl descriptor : loadedPlugins) { descriptor.registerExtensions(extensionPoints, container, notifyListeners); } - - // to avoid clearing cache for each plugin on registration, cache is cleared only now - // in general, on init extension point should be not initialized yet, but who knows - // (later maybe revisited, for now preserve old behaviour to be sure) - area.extensionsRegistered(extensionPoints); } /** diff --git a/platform/extensions/src/com/intellij/openapi/extensions/ExtensionPoint.java b/platform/extensions/src/com/intellij/openapi/extensions/ExtensionPoint.java index 460ca4e8e1dd..0590a184cd11 100644 --- a/platform/extensions/src/com/intellij/openapi/extensions/ExtensionPoint.java +++ b/platform/extensions/src/com/intellij/openapi/extensions/ExtensionPoint.java @@ -3,6 +3,7 @@ package com.intellij.openapi.extensions; import com.intellij.openapi.Disposable; import com.intellij.openapi.extensions.impl.ExtensionComponentAdapter; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -61,14 +62,24 @@ public interface ExtensionPoint { boolean hasAnyExtensions(); + /** + * @deprecated Use another solution, because this method instantiates all extensions. + */ @Nullable - T getExtension(); + @Deprecated + default T getExtension() { + // method is deprecated and not used, ignore not efficient implementation + return ContainerUtil.getFirstItem(getExtensionList()); + } /** * @deprecated Use another solution, because this method instantiates all extensions. */ @Deprecated - boolean hasExtension(@NotNull T extension); + default boolean hasExtension(@NotNull T extension) { + // method is deprecated and used only by one external plugin, ignore not efficient implementation + return ContainerUtil.containsIdentity(getExtensionList(), extension); + } /** * @deprecated Use another solution to unregister not applicable extension, because this method instantiates all extensions. diff --git a/platform/extensions/src/com/intellij/openapi/extensions/impl/ExtensionPointImpl.java b/platform/extensions/src/com/intellij/openapi/extensions/impl/ExtensionPointImpl.java index 6e095849a251..d9c64b7b53d6 100644 --- a/platform/extensions/src/com/intellij/openapi/extensions/impl/ExtensionPointImpl.java +++ b/platform/extensions/src/com/intellij/openapi/extensions/impl/ExtensionPointImpl.java @@ -33,7 +33,7 @@ import java.util.stream.Stream; @SuppressWarnings({"SynchronizeOnThis", "NonPrivateFieldAccessedInSynchronizedContext"}) public abstract class ExtensionPointImpl implements ExtensionPoint, Iterable { - private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.extensions.impl.ExtensionPointImpl"); + protected static final Logger LOG = Logger.getInstance("#com.intellij.openapi.extensions.impl.ExtensionPointImpl"); // test-only private static Set> POINTS_IN_READONLY_MODE; @@ -43,7 +43,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab private final String myName; private final String myClassName; - private volatile List myExtensionsCache; + private volatile List myExtensionsCache; // Since JDK 9 Arrays.ArrayList.toArray() doesn't return T[] array (https://bugs.openjdk.java.net/browse/JDK-6260652), // but instead returns Object[], so, we cannot use toArray() anymore. // Only array.clone should be used because of performance reasons (https://youtrack.jetbrains.com/issue/IDEA-198172). @@ -58,6 +58,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab // guarded by this @NotNull List myAdapters = Collections.emptyList(); + boolean myAdaptersIsSorted = true; @SuppressWarnings("unchecked") @NotNull @@ -69,6 +70,9 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab private final boolean myUnloadSafe; + // guarded by this + private boolean processingAdaptersNow; + ExtensionPointImpl(@NotNull String name, @NotNull String className, @NotNull MutablePicoContainer picoContainer, @@ -188,6 +192,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab if (myAdapters == Collections.emptyList()) { myAdapters = new ArrayList<>(); + myAdaptersIsSorted = false; } int index = firstIndex; @@ -248,7 +253,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab @NotNull @Override public List getExtensionList() { - List result = myExtensionsCache; + List result = myExtensionsCache; if (result == null) { synchronized (this) { result = myExtensionsCache; @@ -260,8 +265,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab } } } - //noinspection unchecked - return (List)result; + return result; } @Override @@ -309,23 +313,11 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab @ApiStatus.Experimental @NotNull public final Iterator iterator() { - List result = myExtensionsCache; - if (result == null) { - synchronized (this) { - result = myExtensionsCache; - if (result == null) { - return createIterator(); - } - } - } - //noinspection unchecked - return (Iterator)result.iterator(); + List result = myExtensionsCache; + return result == null ? createIterator() : result.iterator(); } public void processWithPluginDescriptor(@NotNull BiConsumer consumer) { - synchronized (this) { - assertBeforeProcessing(); - } CHECK_CANCELED.run(); if (isInReadOnlyMode()) { @@ -335,21 +327,8 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab return; } - List adapters; - synchronized (this) { - adapters = myAdapters; - int size = adapters.size(); - if (size == 0) { - return; - } - - LoadingOrder.sort(adapters); - adapters = new ArrayList<>(adapters); // for safe iteration outside lock - LOG.assertTrue(myListeners.length == 0); - } - - for (ExtensionComponentAdapter adapter : adapters) { - T extension = processAdapter(adapter, null /* don't even pass it */, null, null, null, adapters); + for (ExtensionComponentAdapter adapter : getThreadSafeAdapterList()) { + T extension = processAdapter(adapter, null /* don't even pass it */, null, null, null, null); if (extension == null) { break; } @@ -358,20 +337,28 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab } @NotNull - private synchronized Iterator createIterator() { - assertBeforeProcessing(); - CHECK_CANCELED.run(); - - List adapters = myAdapters; - int size = adapters.size(); - if (size == 0) { - return Collections.emptyIterator(); - } - - LoadingOrder.sort(adapters); - - // see method comment about listeners - to ensure that every client of this method doesn't introduce flaky tests/hard to reproduce bugs + private synchronized List getThreadSafeAdapterList() { LOG.assertTrue(myListeners.length == 0); + // copy for safe iteration outside lock + return ContainerUtil.copyList(getSortedAdapters()); + } + + @NotNull + private Iterator createIterator() { + int size; + List adapters; + synchronized (this) { + CHECK_CANCELED.run(); + + adapters = getSortedAdapters(); + size = adapters.size(); + if (size == 0) { + return Collections.emptyIterator(); + } + + // see method comment about listeners - to ensure that every client of this method doesn't introduce flaky tests/hard to reproduce bugs + LOG.assertTrue(myListeners.length == 0); + } return new Iterator() { private int currentIndex; @@ -385,7 +372,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab @Nullable public T next() { do { - T extension = processAdapter(adapters.get(currentIndex++), null /* don't even pass it */, null, null, null, adapters); + T extension = processAdapter(adapters.get(currentIndex++), null /* don't even pass it */, null, null, null, null); if (extension != null) { return extension; } @@ -413,29 +400,38 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab } } - private boolean processingAdaptersNow; // guarded by this + @NotNull + private synchronized List getSortedAdapters() { + assertBeforeProcessing(); + + List adapters = myAdapters; + if (!myAdaptersIsSorted) { + LoadingOrder.sort(adapters); + myAdaptersIsSorted = true; + } + return adapters; + } + @NotNull private synchronized T[] processAdapters() { - assertBeforeProcessing(); assertNotReadOnlyMode(); + // check before to avoid any "restore" work if already cancelled + CHECK_CANCELED.run(); + long startTime = StartUpMeasurer.getCurrentTime(); - int totalSize = myAdapters.size(); + List adapters = getSortedAdapters(); + int totalSize = adapters.size(); Class extensionClass = getExtensionClass(); T[] result = ArrayUtil.newArray(extensionClass, totalSize); if (totalSize == 0) { return result; } - // check before to avoid any "restore" work if already cancelled - CHECK_CANCELED.run(); processingAdaptersNow = true; try { - List adapters = myAdapters; - LoadingOrder.sort(adapters); - - OpenTHashSet duplicates = this instanceof BeanExtensionPoint ? null : new OpenTHashSet<>(adapters.size()); + OpenTHashSet duplicates = this instanceof BeanExtensionPoint ? null : new OpenTHashSet<>(totalSize); ExtensionPointListener[] listeners = myListeners; int extensionIndex = 0; @@ -466,7 +462,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab @Nullable T[] result, @Nullable OpenTHashSet duplicates, @Nullable Class extensionClassForCheck, - List adapters) { + @Nullable List adapters) { try { boolean isNotifyThatAdded = listeners != null && listeners.length != 0 && !adapter.isInstanceCreated(); // do not call CHECK_CANCELED here in loop because it is called by createInstance() @@ -534,19 +530,6 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab } } - @Override - @Nullable - public T getExtension() { - List extensions = getExtensionList(); - return extensions.isEmpty() ? null : extensions.get(0); - } - - @Override - public synchronized boolean hasExtension(@NotNull T extension) { - // method is deprecated and used only by one external plugin, ignore not efficient implementation - return ContainerUtil.containsIdentity(getExtensionList(), extension); - } - /** * Put extension point in read-only mode and replace existing extensions by supplied. * For tests this method is more preferable than {@link #registerExtension)} because makes registration more isolated and strict @@ -555,7 +538,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab * Please use {@link com.intellij.testFramework.PlatformTestUtil#maskExtensions(ExtensionPointName, List, Disposable)} instead of direct usage. */ @TestOnly - public synchronized void maskAll(@NotNull List list, @NotNull Disposable parentDisposable) { + public synchronized void maskAll(@NotNull List list, @NotNull Disposable parentDisposable) { if (POINTS_IN_READONLY_MODE == null) { //noinspection AssignmentToStaticFieldFromInstanceMethod POINTS_IN_READONLY_MODE = ContainerUtil.newIdentityTroveSet(); @@ -564,7 +547,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab assertNotReadOnlyMode(); } - List oldList = myExtensionsCache; + List oldList = myExtensionsCache; T[] oldArray = myExtensionsCacheAsArray; // any read access will use supplied list, any write access can lead to unpredictable results - asserted in clearCache myExtensionsCache = list; @@ -715,10 +698,13 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab } } - private static final ArrayFactory> LISTENER_ARRAY_FACTORY = - n -> n == 0 ? ExtensionPointListener.EMPTY_ARRAY : new ExtensionPointListener[n]; + private static final ArrayFactory> LISTENER_ARRAY_FACTORY = n -> { + return n == 0 ? ExtensionPointListener.EMPTY_ARRAY : new ExtensionPointListener[n]; + }; + + @NotNull private static ArrayFactory> listenerArrayFactory() { - //noinspection unchecked + //noinspection unchecked,rawtypes return (ArrayFactory)LISTENER_ARRAY_FACTORY; } @@ -840,6 +826,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab synchronized void clearCache() { myExtensionsCache = null; myExtensionsCacheAsArray = null; + myAdaptersIsSorted = false; // asserted here because clearCache is called on any write action assertNotReadOnlyMode(); @@ -899,6 +886,7 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab if (adapters == Collections.emptyList()) { adapters = new ArrayList<>(extensionElements.size()); myAdapters = adapters; + myAdaptersIsSorted = false; } else { ((ArrayList)adapters).ensureCapacity(adapters.size() + extensionElements.size()); @@ -932,17 +920,25 @@ public abstract class ExtensionPointImpl implements ExtensionPoint, Iterab } @Nullable - public synchronized T findExtension(@NotNull Class instanceOf, boolean isRequired) { - Iterator iterator = myListeners.length == 0 ? iterator() : getExtensionList().iterator(); - while (iterator.hasNext()) { - T object = iterator.next(); - if (instanceOf.isInstance(object)) { - return object; + public final T findExtension(@NotNull Class aClass, boolean isRequired) { + List extensionsCache = myExtensionsCache; + if (extensionsCache == null) { + for (ExtensionComponentAdapter adapter : getThreadSafeAdapterList()) { + if (aClass.isAssignableFrom(adapter.getImplementationClass())) { + return processAdapter(adapter, null /* don't even pass it */, null, null, null, null); + } + } + } + else { + for (T extension : extensionsCache) { + if (aClass.isInstance(extension)) { + return extension; + } } } if (isRequired) { - String message = "could not find extension implementation " + instanceOf; + String message = "could not find extension implementation " + aClass; if (isInReadOnlyMode()) { message += " (point in read-only mode)"; } diff --git a/platform/extensions/src/com/intellij/openapi/extensions/impl/XmlExtensionAdapter.java b/platform/extensions/src/com/intellij/openapi/extensions/impl/XmlExtensionAdapter.java index f0773ce0c7b9..dd7624dd04f7 100644 --- a/platform/extensions/src/com/intellij/openapi/extensions/impl/XmlExtensionAdapter.java +++ b/platform/extensions/src/com/intellij/openapi/extensions/impl/XmlExtensionAdapter.java @@ -1,7 +1,6 @@ // Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.openapi.extensions.impl; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.extensions.ExtensionNotApplicableException; import com.intellij.openapi.extensions.LoadingOrder; import com.intellij.openapi.extensions.PluginDescriptor; @@ -116,8 +115,6 @@ class XmlExtensionAdapter extends ExtensionComponentAdapter { } static final class SimpleConstructorInjectionAdapter extends XmlExtensionAdapter { - private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.extensions.impl.ExtensionPointImpl"); - SimpleConstructorInjectionAdapter(@NotNull String implementationClassName, @NotNull PluginDescriptor pluginDescriptor, @Nullable String orderId, @@ -147,10 +144,10 @@ class XmlExtensionAdapter extends ExtensionComponentAdapter { "), please remove constructor parameters"; PluginDescriptor pluginDescriptor = getPluginDescriptor(); if (pluginDescriptor.isBundled() && !pluginDescriptor.getPluginId().getIdString().equals("org.jetbrains.kotlin")) { - LOG.error(message, e); + ExtensionPointImpl.LOG.error(message, e); } else { - LOG.warn(message, e); + ExtensionPointImpl.LOG.warn(message, e); } } } diff --git a/platform/lang-api/src/com/intellij/ide/scratch/RootType.java b/platform/lang-api/src/com/intellij/ide/scratch/RootType.java index 860bc548aef0..fec32b31f402 100644 --- a/platform/lang-api/src/com/intellij/ide/scratch/RootType.java +++ b/platform/lang-api/src/com/intellij/ide/scratch/RootType.java @@ -41,7 +41,7 @@ public abstract class RootType { } @NotNull - public static T findByClass(Class aClass) { + public static T findByClass(@NotNull Class aClass) { return ROOT_EP.findExtensionOrFail(aClass); } diff --git a/platform/testFramework/src/com/intellij/testFramework/PlatformTestUtil.java b/platform/testFramework/src/com/intellij/testFramework/PlatformTestUtil.java index d3f17e1b5675..d1bcecab12a2 100644 --- a/platform/testFramework/src/com/intellij/testFramework/PlatformTestUtil.java +++ b/platform/testFramework/src/com/intellij/testFramework/PlatformTestUtil.java @@ -135,7 +135,7 @@ public class PlatformTestUtil { * @see ExtensionPointImpl#maskAll(List, Disposable) */ public static void maskExtensions(@NotNull ExtensionPointName pointName, - @NotNull List newExtensions, + @NotNull List newExtensions, @NotNull Disposable parentDisposable) { ((ExtensionPointImpl)pointName.getPoint(null)).maskAll(newExtensions, parentDisposable); } @@ -145,7 +145,7 @@ public class PlatformTestUtil { */ public static void maskExtensions(@NotNull ProjectExtensionPointName pointName, @NotNull Project project, - @NotNull List newExtensions, + @NotNull List newExtensions, @NotNull Disposable parentDisposable) { ((ExtensionPointImpl)pointName.getPoint(project)).maskAll(newExtensions, parentDisposable); } @@ -169,12 +169,11 @@ public class PlatformTestUtil { public static String toString(@Nullable Object node, @Nullable Queryable.PrintInfo printInfo) { if (node instanceof AbstractTreeNode) { if (printInfo != null) { - return ((AbstractTreeNode)node).toTestString(printInfo); + return ((AbstractTreeNode)node).toTestString(printInfo); } else { - @SuppressWarnings({"deprecation", "UnnecessaryLocalVariable"}) - final String presentation = ((AbstractTreeNode)node).getTestPresentation(); - return presentation; + //noinspection deprecation + return ((AbstractTreeNode)node).getTestPresentation(); } } return String.valueOf(node); @@ -480,7 +479,7 @@ public class PlatformTestUtil { return event1; } - public static StringBuilder print(AbstractTreeStructure structure, Object node, int currentLevel, @Nullable Comparator comparator, + public static StringBuilder print(AbstractTreeStructure structure, Object node, int currentLevel, @Nullable Comparator comparator, int maxRowCount, char paddingChar, @Nullable Queryable.PrintInfo printInfo) { return print(structure, node, currentLevel, comparator, maxRowCount, paddingChar, o -> toString(o, printInfo)); } @@ -489,7 +488,7 @@ public class PlatformTestUtil { return print(structure, node, 0, Comparator.comparing(nodePresenter), -1, ' ', nodePresenter).toString(); } - private static StringBuilder print(AbstractTreeStructure structure, Object node, int currentLevel, @Nullable Comparator comparator, + private static StringBuilder print(AbstractTreeStructure structure, Object node, int currentLevel, @Nullable Comparator comparator, int maxRowCount, char paddingChar, Function nodePresenter) { StringBuilder buffer = new StringBuilder(); doPrint(buffer, currentLevel, node, structure, comparator, maxRowCount, 0, paddingChar, nodePresenter); @@ -500,7 +499,7 @@ public class PlatformTestUtil { int currentLevel, Object node, AbstractTreeStructure structure, - @Nullable Comparator comparator, + @Nullable Comparator comparator, int maxRowCount, int currentLine, char paddingChar, @@ -513,8 +512,9 @@ public class PlatformTestUtil { Object[] children = structure.getChildElements(node); if (comparator != null) { - ArrayList list = new ArrayList<>(Arrays.asList(children)); - @SuppressWarnings({"UnnecessaryLocalVariable", "unchecked"}) Comparator c = comparator; + List list = new ArrayList<>(Arrays.asList(children)); + @SuppressWarnings({"unchecked"}) + Comparator c = (Comparator)comparator; Collections.sort(list, c); children = ArrayUtil.toObjectArray(list); } @@ -533,7 +533,7 @@ public class PlatformTestUtil { return c.stream().map(each -> toString(each, null)).collect(Collectors.joining("\n")); } - public static String print(ListModel model) { + public static String print(@NotNull ListModel model) { StringBuilder result = new StringBuilder(); for (int i = 0; i < model.getSize(); i++) { result.append(toString(model.getElementAt(i), null)); @@ -591,7 +591,7 @@ public class PlatformTestUtil { * An example: {@code startPerformanceTest("calculating pi",100, testRunnable).assertTiming();} */ @Contract(pure = true) // to warn about not calling .assertTiming() in the end - public static PerformanceTestInfo startPerformanceTest(@NonNls @NotNull String what, int expectedMs, @NotNull ThrowableRunnable test) { + public static PerformanceTestInfo startPerformanceTest(@NonNls @NotNull String what, int expectedMs, @NotNull ThrowableRunnable test) { return new PerformanceTestInfo(test, expectedMs, what); } @@ -792,7 +792,7 @@ public class PlatformTestUtil { @NotNull @Contract(pure = true) - public static Comparator createComparator(final Queryable.PrintInfo printInfo) { + public static Comparator> createComparator(final Queryable.PrintInfo printInfo) { return (o1, o2) -> { String displayText1 = o1.toTestString(printInfo); String displayText2 = o2.toTestString(printInfo); @@ -811,7 +811,7 @@ public class PlatformTestUtil { return StringUtil.convertLineSeparators(FileUtil.loadFile(new File(fileName))); } - public static void withEncoding(@NotNull String encoding, @NotNull ThrowableRunnable r) { + public static void withEncoding(@NotNull String encoding, @NotNull ThrowableRunnable r) { Charset.forName(encoding); // check the encoding exists try { Charset oldCharset = Charset.defaultCharset();