'LibraryTable.getLibraryByName' optimized (IDEA-142882); now LibraryTable.ModifiableModel must be either committed or disposed like other modifiable models

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;

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);
       }
     }
   }

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<Library>(myLibrary2EditorMap.keySet())) {
       final Library existingLibrary = myTable.getLibraryByName(library.getName());
       if (existingLibrary != library) {

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);

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);
 }

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);
+  }
 }

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.<br>
-   * <b>Note:</b> returned library table does not support listeners.
+   * <b>Note:</b> returned library table does not support listeners. Also it should not be neither committed nor disposed.
    *
    * @return library table to be modified
    */

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.
+   * <strong>The returned model must be either committed {@link ModifiableModel#commit()} or disposed {@link com.intellij.openapi.util.Disposer#dispose(Disposable)}</strong>
+   *
+   * @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);

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();

platform/projectModel-impl/src/com/intellij/openapi/roots/impl/libraries/LibraryTableBase.java

@@ -168,7 +168,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
     myFirstLoad = false;
     ApplicationManager.getApplication().assertWriteAccessAllowed();
     if (!model.isChanged()) {
-      myModel = model;
+      Disposer.dispose(model);
       return;
     }
     //todo[nik] remove LibraryImpl#equals method instead of using identity sets
@@ -181,7 +181,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
       fireBeforeLibraryRemoved(library);
     }
 
-    myModel = model;
+    myModel.copyFrom(model);
     for (Library library : removedLibraries) {
       Disposer.dispose(library);
       fireAfterLibraryRemoved(library);
@@ -189,6 +189,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
     for (Library library : addedLibraries) {
       fireLibraryAdded(library);
     }
+    Disposer.dispose(model);
   }
 
   private void fireAfterLibraryRemoved(Library library) {
@@ -210,15 +211,18 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
   public interface ModifiableModelEx extends ModifiableModel {
   }
 
-  public class LibraryModel implements ModifiableModelEx, JDOMExternalizable {
+  public class LibraryModel implements ModifiableModelEx, JDOMExternalizable, Listener, Disposable {
     private final ArrayList<Library> myLibraries = new ArrayList<Library>();
+    private volatile Map<String, Library> 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<Eleme
       myWritable = false;
     }
 
+    @Override
+    public void dispose() {
+    }
+
     @Override
     @NotNull
     public Iterator<Library> getLibraryIterator() {
@@ -238,16 +246,25 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
     @Override
     @Nullable
     public Library getLibraryByName(@NotNull String name) {
-      for (Library myLibrary : myLibraries) {
-        LibraryImpl library = (LibraryImpl)myLibrary;
-        if (Comparing.equal(name, library.getName())) return library;
+      Map<String, Library> cache = myLibraryByNameCache;
+      if (cache == null) {
+        cache = new HashMap<String, Library>();
+        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<Eleme
       assertWritable();
       final LibraryImpl library = new LibraryImpl(name, kind, LibraryTableBase.this, null);
       myLibraries.add(library);
+      myLibraryByNameCache = null;
       return library;
     }
 
@@ -280,6 +298,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
     public void removeLibrary(@NotNull Library library) {
       assertWritable();
       myLibraries.remove(library);
+      myLibraryByNameCache = null;
     }
 
     @Override
@@ -305,6 +324,25 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
           Disposer.dispose(library);
         }
       }
+      myLibraryByNameCache = null;
+    }
+
+    @Override
+    public void afterLibraryAdded(Library newLibrary) {
+    }
+
+    @Override
+    public void afterLibraryRenamed(Library library) {
+      myLibraryByNameCache = null;
+    }
+
+    @Override
+    public void beforeLibraryRemoved(Library library) {
+    }
+
+    @Override
+    public void afterLibraryRemoved(Library library) {
+
     }
 
     @Override
@@ -330,5 +368,11 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
         }
       }
     }
+
+    void copyFrom(LibraryModel model) {
+      myLibraries.clear();
+      myLibraries.addAll(model.myLibraries);
+      myLibraryByNameCache = null;
+    }
   }
 }

platform/projectModel-impl/src/com/intellij/project/model/impl/library/JpsLibraryTableImpl.java

@@ -182,6 +182,10 @@ public class JpsLibraryTableImpl implements LibraryTable, Disposable {
       throw new UnsupportedOperationException();
     }
 
+    @Override
+    public void dispose() {
+    }
+
     @Override
     public boolean isChanged() {
       return false;

plugins/maven/src/main/java/org/jetbrains/idea/maven/utils/library/RepositoryLibrarySupport.java

@@ -63,6 +63,9 @@ public class RepositoryLibrarySupport {
     if (library == null) {
       library = createNewLibrary(module, modifiableModel);
     }
+    else {
+      modifiableModelsProvider.disposeLibraryTableModifiableModel(modifiableModel);
+    }
     final DependencyScope dependencyScope = LibraryDependencyScopeSuggester.getDefaultScope(library);
     final ModifiableRootModel moduleModifiableModel = modifiableModelsProvider.getModuleModifiableModel(module);
     try {

python/pluginSrc/com/jetbrains/python/facet/PythonFacet.java

@@ -66,6 +66,7 @@ public class PythonFacet extends Facet<PythonFacetConfiguration> 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);