From fa4f4b806e6e17f92129a551bdf74b226434cfae Mon Sep 17 00:00:00 2001 From: nik Date: Wed, 16 Sep 2015 11:16:20 +0300 Subject: [PATCH] 'LibraryTable.getLibraryByName' optimized (IDEA-142882); now LibraryTable.ModifiableModel must be either committed or disposed like other modifiable models --- .../roots/IdeaModifiableModelsProvider.java | 10 +++ .../BaseLibrariesConfigurable.java | 3 +- .../projectRoot/LibrariesModifiableModel.java | 11 +++- .../AbstractIdeModifiableModelsProvider.java | 2 +- .../roots/ModifiableModelsProvider.java | 1 + .../PlatformModifiableModelsProvider.java | 6 ++ .../openapi/roots/ModifiableRootModel.java | 2 +- .../openapi/roots/libraries/LibraryTable.java | 8 ++- .../roots/impl/ModuleLibraryTable.java | 4 ++ .../impl/libraries/LibraryTableBase.java | 62 ++++++++++++++++--- .../impl/library/JpsLibraryTableImpl.java | 4 ++ .../library/RepositoryLibrarySupport.java | 3 + .../jetbrains/python/facet/PythonFacet.java | 1 + 13 files changed, 103 insertions(+), 14 deletions(-) diff --git a/java/idea-ui/src/com/intellij/openapi/roots/IdeaModifiableModelsProvider.java b/java/idea-ui/src/com/intellij/openapi/roots/IdeaModifiableModelsProvider.java index b7b73ff1da0b..30e9989f8562 100644 --- a/java/idea-ui/src/com/intellij/openapi/roots/IdeaModifiableModelsProvider.java +++ b/java/idea-ui/src/com/intellij/openapi/roots/IdeaModifiableModelsProvider.java @@ -12,7 +12,9 @@ import com.intellij.openapi.roots.ui.configuration.LibraryTableModifiableModelPr import com.intellij.openapi.roots.ui.configuration.ModuleEditor; import com.intellij.openapi.roots.ui.configuration.ModulesConfigurator; import com.intellij.openapi.roots.ui.configuration.ProjectStructureConfigurable; +import com.intellij.openapi.roots.ui.configuration.projectRoot.LibrariesModifiableModel; import com.intellij.openapi.roots.ui.configuration.projectRoot.StructureConfigurableContext; +import com.intellij.openapi.util.Disposer; import org.jetbrains.annotations.Nullable; import java.lang.reflect.Proxy; @@ -103,6 +105,14 @@ public class IdeaModifiableModelsProvider implements ModifiableModelsProvider { return LibraryTablesRegistrar.getInstance().getLibraryTable(project).getModifiableModel(); } + @Override + public void disposeLibraryTableModifiableModel(LibraryTable.ModifiableModel model) { + //IDEA should dispose this model instead of us, because it is was given from StructureConfigurableContext + if (!(model instanceof LibrariesModifiableModel)) { + Disposer.dispose(model); + } + } + @Nullable private static StructureConfigurableContext getProjectStructureContext(Project project) { if (ApplicationManager.getApplication().isHeadlessEnvironment()) return null; diff --git a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/BaseLibrariesConfigurable.java b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/BaseLibrariesConfigurable.java index 163394ca6b14..7ae03a0fee48 100644 --- a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/BaseLibrariesConfigurable.java +++ b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/BaseLibrariesConfigurable.java @@ -37,6 +37,7 @@ import com.intellij.openapi.ui.Messages; import com.intellij.openapi.ui.NamedConfigurable; import com.intellij.openapi.ui.NonEmptyInputValidator; import com.intellij.openapi.util.Comparing; +import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.text.StringUtil; import com.intellij.util.containers.MultiMap; import com.intellij.util.ui.tree.TreeUtil; @@ -198,7 +199,7 @@ public abstract class BaseLibrariesConfigurable extends BaseStructureConfigurabl public void dispose() { if (myContext != null) { for (final LibrariesModifiableModel provider : myContext.myLevel2Providers.values()) { - provider.disposeUncommittedLibraries(); + Disposer.dispose(provider); } } } diff --git a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/LibrariesModifiableModel.java b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/LibrariesModifiableModel.java index 3eee989e8185..00669c2cdcc7 100644 --- a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/LibrariesModifiableModel.java +++ b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/projectRoot/LibrariesModifiableModel.java @@ -173,7 +173,16 @@ public class LibrariesModifiableModel implements LibraryTableBase.ModifiableMode return myLibrariesModifiableModel; } - public void disposeUncommittedLibraries() { + @Override + public void dispose() { + if (myLibrariesModifiableModel != null) { + Disposer.dispose(myLibrariesModifiableModel); + myLibrariesModifiableModel = null; + } + disposeUncommittedLibraries(); + } + + private void disposeUncommittedLibraries() { for (final Library library : new ArrayList(myLibrary2EditorMap.keySet())) { final Library existingLibrary = myTable.getLibraryByName(library.getName()); if (existingLibrary != library) { diff --git a/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/AbstractIdeModifiableModelsProvider.java b/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/AbstractIdeModifiableModelsProvider.java index 221be477a335..b1196198a1cf 100644 --- a/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/AbstractIdeModifiableModelsProvider.java +++ b/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/AbstractIdeModifiableModelsProvider.java @@ -28,7 +28,6 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.*; import com.intellij.openapi.roots.ex.ProjectRootManagerEx; import com.intellij.openapi.roots.impl.ModifiableModelCommitter; -import com.intellij.openapi.roots.impl.ModuleRootManagerImpl; import com.intellij.openapi.roots.libraries.Library; import com.intellij.openapi.roots.libraries.LibraryTable; import com.intellij.openapi.roots.libraries.LibraryTablesRegistrar; @@ -411,6 +410,7 @@ public abstract class AbstractIdeModifiableModelsProvider implements IdeModifiab if (each.isDisposed()) continue; each.dispose(); } + Disposer.dispose(getModifiableProjectLibrariesModel()); for (Library.ModifiableModel each : myModifiableLibraryModels.values()) { Disposer.dispose(each); diff --git a/platform/lang-api/src/com/intellij/openapi/roots/ModifiableModelsProvider.java b/platform/lang-api/src/com/intellij/openapi/roots/ModifiableModelsProvider.java index 5032d21b470e..3b6c5d99c216 100644 --- a/platform/lang-api/src/com/intellij/openapi/roots/ModifiableModelsProvider.java +++ b/platform/lang-api/src/com/intellij/openapi/roots/ModifiableModelsProvider.java @@ -30,4 +30,5 @@ public interface ModifiableModelsProvider { LibraryTable.ModifiableModel getLibraryTableModifiableModel(); LibraryTable.ModifiableModel getLibraryTableModifiableModel(Project project); + void disposeLibraryTableModifiableModel(LibraryTable.ModifiableModel model); } diff --git a/platform/lang-impl/src/com/intellij/openapi/roots/PlatformModifiableModelsProvider.java b/platform/lang-impl/src/com/intellij/openapi/roots/PlatformModifiableModelsProvider.java index 2619ad08b638..9a32a7fb2817 100644 --- a/platform/lang-impl/src/com/intellij/openapi/roots/PlatformModifiableModelsProvider.java +++ b/platform/lang-impl/src/com/intellij/openapi/roots/PlatformModifiableModelsProvider.java @@ -6,6 +6,7 @@ import com.intellij.openapi.module.Module; import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.libraries.LibraryTable; import com.intellij.openapi.roots.libraries.LibraryTablesRegistrar; +import com.intellij.openapi.util.Disposer; import org.jetbrains.annotations.NotNull; /** @@ -46,4 +47,9 @@ public class PlatformModifiableModelsProvider implements ModifiableModelsProvide public LibraryTable.ModifiableModel getLibraryTableModifiableModel(Project project) { return LibraryTablesRegistrar.getInstance().getLibraryTable(project).getModifiableModel(); } + + @Override + public void disposeLibraryTableModifiableModel(LibraryTable.ModifiableModel model) { + Disposer.dispose(model); + } } diff --git a/platform/projectModel-api/src/com/intellij/openapi/roots/ModifiableRootModel.java b/platform/projectModel-api/src/com/intellij/openapi/roots/ModifiableRootModel.java index 5c1640d9f3e2..54b76bc25054 100644 --- a/platform/projectModel-api/src/com/intellij/openapi/roots/ModifiableRootModel.java +++ b/platform/projectModel-api/src/com/intellij/openapi/roots/ModifiableRootModel.java @@ -127,7 +127,7 @@ public interface ModifiableRootModel extends ModuleRootModel { /** * Returns library table with module libraries.
- * Note: returned library table does not support listeners. + * Note: returned library table does not support listeners. Also it should not be neither committed nor disposed. * * @return library table to be modified */ diff --git a/platform/projectModel-api/src/com/intellij/openapi/roots/libraries/LibraryTable.java b/platform/projectModel-api/src/com/intellij/openapi/roots/libraries/LibraryTable.java index 929fe4f556f2..4ee102195792 100644 --- a/platform/projectModel-api/src/com/intellij/openapi/roots/libraries/LibraryTable.java +++ b/platform/projectModel-api/src/com/intellij/openapi/roots/libraries/LibraryTable.java @@ -49,6 +49,12 @@ public interface LibraryTable { boolean isEditable(); + /** + * Returns the interface which allows to create or removed libraries from the table. + * The returned model must be either committed {@link ModifiableModel#commit()} or disposed {@link com.intellij.openapi.util.Disposer#dispose(Disposable)} + * + * @return the modifiable library table model. + */ @NotNull ModifiableModel getModifiableModel(); @@ -58,7 +64,7 @@ public interface LibraryTable { void removeListener(@NotNull Listener listener); - interface ModifiableModel { + interface ModifiableModel extends Disposable { Library createLibrary(String name); Library createLibrary(String name, @Nullable PersistentLibraryKind type); diff --git a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java index 6cc3b11daa88..cb46c6f2216c 100644 --- a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java +++ b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleLibraryTable.java @@ -186,6 +186,10 @@ public class ModuleLibraryTable implements LibraryTable, LibraryTableBase.Modifi public void commit() { } + @Override + public void dispose() { + } + @Override public boolean isChanged() { return myRootModel.isChanged(); diff --git a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/libraries/LibraryTableBase.java b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/libraries/LibraryTableBase.java index 76458997f2c7..ee5443ec7c86 100644 --- a/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/libraries/LibraryTableBase.java +++ b/platform/projectModel-impl/src/com/intellij/openapi/roots/impl/libraries/LibraryTableBase.java @@ -168,7 +168,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent myLibraries = new ArrayList(); + private volatile Map myLibraryByNameCache; private boolean myWritable; private LibraryModel() { + myDispatcher.addListener(this); myWritable = false; } private LibraryModel(LibraryModel that) { + myDispatcher.addListener(this); myWritable = true; myLibraries.addAll(that.myLibraries); } @@ -229,6 +233,10 @@ public abstract class LibraryTableBase implements PersistentStateComponent getLibraryIterator() { @@ -238,16 +246,25 @@ public abstract class LibraryTableBase implements PersistentStateComponent cache = myLibraryByNameCache; + if (cache == null) { + cache = new HashMap(); + for (Library library : myLibraries) { + cache.put(library.getName(), library); + } + myLibraryByNameCache = cache; } + Library library = cache.get(name); + if (library != null) { + return library; + } + @NonNls final String libraryPrefix = "library."; final String libPath = System.getProperty(libraryPrefix + name); if (libPath != null) { - final LibraryImpl library = new LibraryImpl(name, null, LibraryTableBase.this, null); - library.addRoot(libPath, OrderRootType.CLASSES); - return library; + final LibraryImpl libraryFromProperty = new LibraryImpl(name, null, LibraryTableBase.this, null); + libraryFromProperty.addRoot(libPath, OrderRootType.CLASSES); + return libraryFromProperty; } return null; } @@ -273,6 +290,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent implements Libr final ModifiableModelsProvider provider = ModifiableModelsProvider.SERVICE.getInstance(); final LibraryTable.ModifiableModel libraryTableModifiableModel = provider.getLibraryTableModifiableModel(); Library library = libraryTableModifiableModel.getLibraryByName(name); + provider.disposeLibraryTableModifiableModel(libraryTableModifiableModel); if (library == null) { // we just create new project library library = PythonSdkTableListener.addLibrary(sdk);