From 85a314072c04b8d847ab06ff147de89e89be6432 Mon Sep 17 00:00:00 2001 From: nik Date: Fri, 29 Aug 2014 09:31:22 +0400 Subject: [PATCH] optimization: use synchronization instead of copy-on-write for library table and jdk listeners (IDEA-129033) --- .../intellij/roots/libraries/LibraryTest.java | 47 ++++++- .../roots/impl/ProjectRootManagerImpl.java | 120 ++++++++++-------- 2 files changed, 110 insertions(+), 57 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/roots/libraries/LibraryTest.java b/java/java-tests/testSrc/com/intellij/roots/libraries/LibraryTest.java index d1943d6144ce..1ee498cd5d2c 100644 --- a/java/java-tests/testSrc/com/intellij/roots/libraries/LibraryTest.java +++ b/java/java-tests/testSrc/com/intellij/roots/libraries/LibraryTest.java @@ -1,9 +1,9 @@ package com.intellij.roots.libraries; import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.roots.NativeLibraryOrderRootType; -import com.intellij.openapi.roots.OrderRootType; -import com.intellij.openapi.roots.RootProvider; +import com.intellij.openapi.application.Result; +import com.intellij.openapi.application.WriteAction; +import com.intellij.openapi.roots.*; import com.intellij.openapi.roots.impl.libraries.LibraryEx; import com.intellij.openapi.roots.libraries.Library; import com.intellij.openapi.roots.libraries.LibraryTable; @@ -13,8 +13,11 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.roots.ModuleRootManagerTestCase; import com.intellij.testFramework.PlatformTestUtil; import com.intellij.testFramework.PsiTestUtil; +import com.intellij.util.CommonProcessors; import org.jdom.Element; +import org.jetbrains.annotations.NotNull; +import java.util.Collection; import java.util.Collections; /** @@ -65,6 +68,44 @@ public class LibraryTest extends ModuleRootManagerTestCase { element); } + public void testResolveDependencyToAddedLibrary() { + final ModifiableRootModel model = ModuleRootManager.getInstance(myModule).getModifiableModel(); + model.addInvalidLibrary("jdom", LibraryTablesRegistrar.PROJECT_LEVEL); + commit(model); + assertEmpty(getLibraries()); + + Library library = createLibrary("jdom", getJDomJar(), null); + assertSameElements(getLibraries(), library); + } + + public void testResolveDependencyToRenamedLibrary() { + Library library = createLibrary("jdom2", getJDomJar(), null); + + final ModifiableRootModel model = ModuleRootManager.getInstance(myModule).getModifiableModel(); + model.addInvalidLibrary("jdom", LibraryTablesRegistrar.PROJECT_LEVEL); + commit(model); + assertEmpty(getLibraries()); + + Library.ModifiableModel libModel = library.getModifiableModel(); + libModel.setName("jdom"); + commit(libModel); + assertSameElements(getLibraries(), library); + } + + private Collection getLibraries() { + CommonProcessors.CollectProcessor processor = new CommonProcessors.CollectProcessor(); + ModuleRootManager.getInstance(myModule).orderEntries().forEachLibrary(processor); + return processor.getResults(); + } + + private static void commit(final ModifiableRootModel model) { + new WriteAction() { + protected void run(@NotNull final Result result) { + model.commit(); + } + }.execute(); + } + public void testNativePathSerialization() { LibraryTable table = LibraryTablesRegistrar.getInstance().getLibraryTable(myProject); Library library = table.createLibrary("native"); diff --git a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ProjectRootManagerImpl.java b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ProjectRootManagerImpl.java index 8e80670b1f76..c439a35d0a36 100644 --- a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ProjectRootManagerImpl.java +++ b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ProjectRootManagerImpl.java @@ -134,6 +134,7 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj public ProjectRootManagerImpl(Project project) { myProject = project; myRootsCache = new OrderRootsCache(project); + myJdkTableMultiListener = new JdkTableMultiListener(project); } @Override @@ -281,7 +282,6 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj @Override public void disposeComponent() { - myJdkTableMultiListener = null; } @Override @@ -478,45 +478,52 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj void addListenerForTable(LibraryTable.Listener libraryListener, final LibraryTable libraryTable) { - LibraryTableMultilistener multilistener = myLibraryTableMultilisteners.get(libraryTable); - if (multilistener == null) { - multilistener = new LibraryTableMultilistener(libraryTable); + synchronized (myLibraryTableListenersLock) { + LibraryTableMultiListener multiListener = myLibraryTableMultiListeners.get(libraryTable); + if (multiListener == null) { + multiListener = new LibraryTableMultiListener(libraryTable); + libraryTable.addListener(multiListener); + myLibraryTableMultiListeners.put(libraryTable, multiListener); + } + multiListener.addListener(libraryListener); } - multilistener.addListener(libraryListener); } void removeListenerForTable(LibraryTable.Listener libraryListener, final LibraryTable libraryTable) { - LibraryTableMultilistener multilistener = myLibraryTableMultilisteners.get(libraryTable); - if (multilistener == null) { - multilistener = new LibraryTableMultilistener(libraryTable); + synchronized (myLibraryTableListenersLock) { + LibraryTableMultiListener multiListener = myLibraryTableMultiListeners.get(libraryTable); + if (multiListener != null) { + boolean last = multiListener.removeListener(libraryListener); + if (last) { + libraryTable.removeListener(multiListener); + myLibraryTableMultiListeners.remove(libraryTable); + } + } } - multilistener.removeListener(libraryListener); } - private final Map myLibraryTableMultilisteners - = new HashMap(); + private final Object myLibraryTableListenersLock = new Object(); + private final Map myLibraryTableMultiListeners = new HashMap(); - private class LibraryTableMultilistener implements LibraryTable.Listener { - final List myListeners = ContainerUtil.createLockFreeCopyOnWriteList(); + private class LibraryTableMultiListener implements LibraryTable.Listener { + private final Set myListeners = new LinkedHashSet(); private final LibraryTable myLibraryTable; + private LibraryTable.Listener[] myListenersArray; - private LibraryTableMultilistener(LibraryTable libraryTable) { + private LibraryTableMultiListener(LibraryTable libraryTable) { myLibraryTable = libraryTable; - myLibraryTable.addListener(this); - myLibraryTableMultilisteners.put(myLibraryTable, this); } - private void addListener(LibraryTable.Listener listener) { + private synchronized void addListener(LibraryTable.Listener listener) { myListeners.add(listener); + myListenersArray = null; } - private void removeListener(LibraryTable.Listener listener) { + private synchronized boolean removeListener(LibraryTable.Listener listener) { myListeners.remove(listener); - if (myListeners.isEmpty()) { - myLibraryTable.removeListener(this); - myLibraryTableMultilisteners.remove(myLibraryTable); - } + myListenersArray = null; + return myListeners.isEmpty(); } @Override @@ -525,20 +532,27 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj mergeRootsChangesDuring(new Runnable() { @Override public void run() { - for (LibraryTable.Listener listener : myListeners) { + for (LibraryTable.Listener listener : getListeners()) { listener.afterLibraryAdded(newLibrary); } } }); } + private synchronized LibraryTable.Listener[] getListeners() { + if (myListenersArray == null) { + myListenersArray = myListeners.toArray(new LibraryTable.Listener[myListeners.size()]); + } + return myListenersArray; + } + @Override public void afterLibraryRenamed(final Library library) { incModificationCount(); mergeRootsChangesDuring(new Runnable() { @Override public void run() { - for (LibraryTable.Listener listener : myListeners) { + for (LibraryTable.Listener listener : getListeners()) { listener.afterLibraryRenamed(library); } } @@ -551,7 +565,7 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj mergeRootsChangesDuring(new Runnable() { @Override public void run() { - for (LibraryTable.Listener listener : myListeners) { + for (LibraryTable.Listener listener : getListeners()) { listener.beforeLibraryRemoved(library); } } @@ -564,7 +578,7 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj mergeRootsChangesDuring(new Runnable() { @Override public void run() { - for (LibraryTable.Listener listener : myListeners) { + for (LibraryTable.Listener listener : getListeners()) { listener.afterLibraryRemoved(library); } } @@ -572,24 +586,33 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj } } - private JdkTableMultiListener myJdkTableMultiListener = null; + private final JdkTableMultiListener myJdkTableMultiListener; private class JdkTableMultiListener implements ProjectJdkTable.Listener { - final EventDispatcher myDispatcher = EventDispatcher.create(ProjectJdkTable.Listener.class); + private final Set myListeners = new LinkedHashSet(); private MessageBusConnection listenerConnection; + private ProjectJdkTable.Listener[] myListenersArray; private JdkTableMultiListener(Project project) { listenerConnection = project.getMessageBus().connect(); listenerConnection.subscribe(ProjectJdkTable.JDK_TABLE_TOPIC, this); } - private void addListener(ProjectJdkTable.Listener listener) { - myDispatcher.addListener(listener); + private synchronized void addListener(ProjectJdkTable.Listener listener) { + myListeners.add(listener); + myListenersArray = null; } - private void removeListener(ProjectJdkTable.Listener listener) { - myDispatcher.removeListener(listener); - uninstallListener(true); + private synchronized void removeListener(ProjectJdkTable.Listener listener) { + myListeners.remove(listener); + myListenersArray = null; + } + + private synchronized ProjectJdkTable.Listener[] getListeners() { + if (myListenersArray == null) { + myListenersArray = myListeners.toArray(new ProjectJdkTable.Listener[myListeners.size()]); + } + return myListenersArray; } @Override @@ -597,7 +620,9 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj mergeRootsChangesDuring(new Runnable() { @Override public void run() { - myDispatcher.getMulticaster().jdkAdded(jdk); + for (ProjectJdkTable.Listener listener : getListeners()) { + listener.jdkAdded(jdk); + } } }); } @@ -607,7 +632,9 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj mergeRootsChangesDuring(new Runnable() { @Override public void run() { - myDispatcher.getMulticaster().jdkRemoved(jdk); + for (ProjectJdkTable.Listener listener : getListeners()) { + listener.jdkRemoved(jdk); + } } }); } @@ -617,7 +644,9 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj mergeRootsChangesDuring(new Runnable() { @Override public void run() { - myDispatcher.getMulticaster().jdkNameChanged(jdk, previousName); + for (ProjectJdkTable.Listener listener : getListeners()) { + listener.jdkNameChanged(jdk, previousName); + } } }); String currentName = getProjectSdkName(); @@ -627,32 +656,15 @@ public class ProjectRootManagerImpl extends ProjectRootManagerEx implements Proj myProjectSdkType = jdk.getSdkType().getName(); } } - - public void uninstallListener(boolean soft) { - if (!soft || !myDispatcher.hasListeners()) { - if (listenerConnection != null) { - listenerConnection.disconnect(); - listenerConnection = null; - } - } - } } private final Map> myRegisteredRootProviders = new HashMap>(); void addJdkTableListener(ProjectJdkTable.Listener jdkTableListener) { - getJdkTableMultiListener().addListener(jdkTableListener); - } - - private JdkTableMultiListener getJdkTableMultiListener() { - if (myJdkTableMultiListener == null) { - myJdkTableMultiListener = new JdkTableMultiListener(myProject); - } - return myJdkTableMultiListener; + myJdkTableMultiListener.addListener(jdkTableListener); } void removeJdkTableListener(ProjectJdkTable.Listener jdkTableListener) { - if (myJdkTableMultiListener == null) return; myJdkTableMultiListener.removeListener(jdkTableListener); }