optimization: do not create thousands of empty VirtualFilePointerContainers

2026-09-27 10:03:11 +07:00 · 2018-11-20 18:42:42 +03:00
parent 92a5e18746
commit 91c7656afe
6 changed files with 132 additions and 77 deletions
@@ -101,13 +101,12 @@ public class JavaModuleExternalPathsImpl extends JavaModuleExternalPaths {
  }

  @Override
-  @SuppressWarnings("deprecation")
  public void readExternal(@NotNull Element element) throws InvalidDataException {
    for (PersistentOrderRootType orderRootType : OrderRootType.getAllPersistentTypes()) {
      String paths = orderRootType.getModulePathsName();
      if (paths != null) {
        final Element pathsElement = element.getChild(paths);
-        if (pathsElement != null) {
+        if (pathsElement != null && !pathsElement.getChildren(ROOT_ELEMENT).isEmpty()) {
          VirtualFilePointerContainer container = VirtualFilePointerManager.getInstance().createContainer(this, null);
          myOrderRootPointerContainers.put(orderRootType, container);
          container.readExternal(pathsElement, ROOT_ELEMENT, false);
@@ -117,7 +116,6 @@ public class JavaModuleExternalPathsImpl extends JavaModuleExternalPaths {
  }

  @Override
-  @SuppressWarnings("deprecation")
  public void writeExternal(@NotNull Element element) throws WriteExternalException {
    for (OrderRootType orderRootType : myOrderRootPointerContainers.keySet()) {
      VirtualFilePointerContainer container = myOrderRootPointerContainers.get(orderRootType);
@@ -29,13 +29,13 @@ public class RootsAsVirtualFilePointers implements RootProvider {
  private final Map<OrderRootType, VirtualFilePointerContainer> myRoots = new THashMap<>();

  private final boolean myNoCopyJars;
+  private final VirtualFilePointerListener myListener;
+  @NotNull private final Disposable myParent;

  RootsAsVirtualFilePointers(boolean noCopyJars, VirtualFilePointerListener listener, @NotNull Disposable parent) {
    myNoCopyJars = noCopyJars;
-
-    for (OrderRootType rootType : OrderRootType.getAllTypes()) {
-      myRoots.put(rootType, VirtualFilePointerManager.getInstance().createContainer(parent, listener));
-    }
+    myListener = listener;
+    myParent = parent;
  }

  @Override
@@ -53,15 +53,18 @@ public class RootsAsVirtualFilePointers implements RootProvider {
  }

  public void addRoot(@NotNull VirtualFile virtualFile, @NotNull OrderRootType type) {
-    myRoots.get(type).add(virtualFile);
+    getOrCreateContainer(type).add(virtualFile);
  }

  public void addRoot(@NotNull String url, @NotNull OrderRootType type) {
-    myRoots.get(type).add(url);
+    getOrCreateContainer(type).add(url);
  }

  public void removeAllRoots(@NotNull OrderRootType type) {
-    myRoots.get(type).clear();
+    VirtualFilePointerContainer container = myRoots.get(type);
+    if (container != null) {
+      container.clear();
+    }
  }

  public void removeRoot(@NotNull VirtualFile root, @NotNull OrderRootType type) {
@@ -70,7 +73,7 @@ public class RootsAsVirtualFilePointers implements RootProvider {

  public void removeRoot(@NotNull String url, @NotNull OrderRootType type) {
    VirtualFilePointerContainer container = myRoots.get(type);
-    VirtualFilePointer pointer = container.findByUrl(url);
+    VirtualFilePointer pointer = container == null ? null : container.findByUrl(url);
    if (pointer != null) {
      container.remove(pointer);
    }
@@ -94,17 +97,10 @@ public class RootsAsVirtualFilePointers implements RootProvider {
        }
      }
    }));
-
-    for (OrderRootType type : OrderRootType.getAllTypes()) {
-      if (myRoots.get(type) == null) {
-        LOG.error(type + " wasn't serialized");
-      }
-    }
  }

  public void writeExternal(@NotNull Element element) {
-    List<PersistentOrderRootType> allTypes = OrderRootType.getSortedRootTypes();
-    for (PersistentOrderRootType type : allTypes) {
+    for (PersistentOrderRootType type : OrderRootType.getSortedRootTypes()) {
      write(element, type);
    }
  }
@@ -151,12 +147,10 @@ public class RootsAsVirtualFilePointers implements RootProvider {
      LOG.error(composites);
    }
    Element composite = composites.get(0);
-
-    VirtualFilePointerContainer container = myRoots.get(type);
-    if (container == null) {
-      LOG.error("unknown root type: " + type);
+    if (!composite.getChildren("root").isEmpty()) {
+      VirtualFilePointerContainer container = getOrCreateContainer(type);
+      container.readExternal(composite, "root", false);
    }
-    container.readExternal(composite, "root", false);
  }

  /**
@@ -179,7 +173,10 @@ public class RootsAsVirtualFilePointers implements RootProvider {
    Element composite = new Element("root");
    composite.setAttribute("type", "composite");
    e.addContent(composite);
-    myRoots.get(type).writeExternal(composite, "root", false);
+    VirtualFilePointerContainer container = myRoots.get(type);
+    if (container != null) {
+      container.writeExternal(composite, "root", false);
+    }
    for (Element root : composite.getChildren()) {
      root.setAttribute("type", "simple");
    }
@@ -199,4 +196,14 @@ public class RootsAsVirtualFilePointers implements RootProvider {
  public void removeRootSetChangedListener(@NotNull RootSetChangedListener listener) {
    throw new RuntimeException();
  }
+
+  @NotNull
+  private VirtualFilePointerContainer getOrCreateContainer(@NotNull OrderRootType rootType) {
+    VirtualFilePointerContainer roots = myRoots.get(rootType);
+    if (roots == null) {
+      roots = VirtualFilePointerManager.getInstance().createContainer(myParent, myListener);
+      myRoots.put(rootType, roots);
+    }
+    return roots;
+  }
 }
@@ -22,14 +22,16 @@ import com.intellij.openapi.util.Disposer;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointerContainer;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointerManager;
+import com.intellij.util.ArrayUtil;
 import com.intellij.util.ConcurrencyUtil;
+import com.intellij.util.ObjectUtils;
 import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;

 import java.util.Collection;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;

 /**
 * @author nik
@@ -53,32 +55,45 @@ class OrderRootsCache {
    }
  }

-  VirtualFilePointerContainer setCachedRoots(@NotNull OrderRootType rootType, int flags, @NotNull Collection<String> urls) {
-    final VirtualFilePointerContainer container = VirtualFilePointerManager.getInstance().createContainer(myRootsDisposable);
+  private static final VirtualFilePointerContainer EMPTY = ObjectUtils.sentinel("Empty roots container", VirtualFilePointerContainer.class);
+  private VirtualFilePointerContainer setCachedRoots(@NotNull CacheKey key, @NotNull Collection<String> urls) {
+    // optimization: avoid creating heavy container for empty list, use 'EMPTY' stub for that case
+    VirtualFilePointerContainer container = urls.isEmpty() ? EMPTY : VirtualFilePointerManager.getInstance().createContainer(myRootsDisposable);
    for (String url : urls) {
      container.add(url);
    }
    Map<CacheKey, VirtualFilePointerContainer> map = myRoots.get();
    if (map == null) map = ConcurrencyUtil.cacheOrGet(myRoots, ContainerUtil.newConcurrentMap());
-    map.put(new CacheKey(rootType, flags), container);
+    map.put(key, container);
    return container;
  }

-  @Nullable
-  public VirtualFile[] getCachedRoots(@NotNull OrderRootType rootType, int flags) {
+  private VirtualFilePointerContainer getOrComputeContainer(@NotNull OrderRootType rootType,
+                                                            int flags,
+                                                            @NotNull Supplier<? extends Collection<String>> computer) {
    Map<CacheKey, VirtualFilePointerContainer> map = myRoots.get();
-    final VirtualFilePointerContainer cached = map == null ? null : map.get(new CacheKey(rootType, flags));
-    return cached == null ? null : cached.getFiles();
+    CacheKey key = new CacheKey(rootType, flags);
+    VirtualFilePointerContainer cached = map == null ? null : map.get(key);
+    if (cached == null) {
+      Collection<String> roots = computer.get();
+      cached = setCachedRoots(key, roots);
+    }
+    return cached == EMPTY ? null : cached;
  }

-  @Nullable
-  public String[] getCachedUrls(@NotNull OrderRootType rootType, int flags) {
-    Map<CacheKey, VirtualFilePointerContainer> map = myRoots.get();
-    final VirtualFilePointerContainer cached = map == null ? null : map.get(new CacheKey(rootType, flags));
-    return cached != null ? cached.getUrls() : null;
+  @NotNull
+  VirtualFile[] getOrComputeRoots(@NotNull OrderRootType rootType, int flags, @NotNull Supplier<? extends Collection<String>> computer) {
+    VirtualFilePointerContainer container = getOrComputeContainer(rootType, flags, computer);
+    return container == null ? VirtualFile.EMPTY_ARRAY : container.getFiles();
  }

-  public void clearCache() {
+  @NotNull
+  String[] getOrComputeUrls(@NotNull OrderRootType rootType, int flags, @NotNull Supplier<? extends Collection<String>> computer) {
+    VirtualFilePointerContainer container = getOrComputeContainer(rootType, flags, computer);
+    return container == null ? ArrayUtil.EMPTY_STRING_ARRAY : container.getUrls();
+  }
+
+  void clearCache() {
    ApplicationManager.getApplication().assertIsDispatchThread();
    disposePointers();
    myRoots.set(null);
@@ -63,11 +63,7 @@ class OrderRootsEnumeratorImpl implements OrderRootsEnumerator {
      final OrderRootsCache cache = myOrderEnumerator.getCache();
      if (cache != null) {
        final int flags = myOrderEnumerator.getFlags();
-        final VirtualFile[] cached = cache.getCachedRoots(myRootType, flags);
-        if (cached == null) {
-          return cache.setCachedRoots(myRootType, flags, computeRootsUrls()).getFiles();
-        }
-        return cached;
+        return cache.getOrComputeRoots(myRootType, flags, this::computeRootsUrls);
      }
    }

@@ -82,11 +78,7 @@ class OrderRootsEnumeratorImpl implements OrderRootsEnumerator {
      final OrderRootsCache cache = myOrderEnumerator.getCache();
      if (cache != null) {
        final int flags = myOrderEnumerator.getFlags();
-        String[] cached = cache.getCachedUrls(myRootType, flags);
-        if (cached == null) {
-          return cache.setCachedRoots(myRootType, flags, computeRootsUrls()).getUrls();
-        }
-        return cached;
+        return cache.getOrComputeUrls(myRootType, flags, this::computeRootsUrls);
      }
    }
    return ArrayUtil.toStringArray(computeRootsUrls());
@@ -51,7 +51,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
  private static final String EXCLUDED_ROOTS_TAG = "excluded";
  private String myName;
  private final LibraryTable myLibraryTable;
-  private final Map<OrderRootType, VirtualFilePointerContainer> myRoots;
+  private final Map<OrderRootType, VirtualFilePointerContainer> myRoots = new HashMap<>(2);
  @Nullable private VirtualFilePointerContainer myExcludedRoots;
  private final LibraryImpl mySource;
  private PersistentLibraryKind<?> myKind;
@@ -87,9 +87,10 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
      myProperties.loadState(from.myProperties.getState());
    }
    for (OrderRootType rootType : getAllRootTypes()) {
-      final VirtualFilePointerContainer thisContainer = myRoots.get(rootType);
      final VirtualFilePointerContainer thatContainer = from.myRoots.get(rootType);
-      thisContainer.addAll(thatContainer);
+      if (thatContainer != null) {
+        getOrCreateContainer(rootType).addAll(thatContainer);
+      }
    }
    if (from.myExcludedRoots != null) {
      myExcludedRoots = from.myExcludedRoots.clone(myPointersDisposable);
@@ -106,8 +107,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    myKind = kind;
    myName = name;
    myExternalSource = externalSource;
-    //init roots depends on my myKind
-    myRoots = initRoots();
+    Disposer.register(this, myPointersDisposable);
  }

  @Nullable
@@ -203,7 +203,8 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
  public List<String> getInvalidRootUrls(@NotNull OrderRootType type) {
    if (myDisposed) return Collections.emptyList();

-    final List<VirtualFilePointer> pointers = myRoots.get(type).getList();
+    VirtualFilePointerContainer container = myRoots.get(type);
+    final List<VirtualFilePointer> pointers = container == null ? Collections.emptyList() : container.getList();
    List<String> invalidPaths = null;
    for (VirtualFilePointer pointer : pointers) {
      if (!pointer.isValid()) {
@@ -268,6 +269,16 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
  @NonNls private static final String ROOT_TYPE_ATTR = "type";
  private static final OrderRootType DEFAULT_JAR_DIRECTORY_TYPE = OrderRootType.CLASSES;

+  @NotNull
+  private VirtualFilePointerContainer getOrCreateContainer(@NotNull OrderRootType rootType) {
+    VirtualFilePointerContainer roots = myRoots.get(rootType);
+    if (roots == null) {
+      roots = VirtualFilePointerManager.getInstance().createContainer(myPointersDisposable, getListener());
+      myRoots.put(rootType, roots);
+    }
+    return roots;
+  }
+
  // just to maintain .xml compatibility
  // VirtualFilePointerContainerImpl does the same but stores its jar dirs attributes inside <root> element
  @Deprecated // todo to remove sometime later
@@ -278,7 +289,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
      if (url != null) {
        final String recursive = jarDir.getAttributeValue(VirtualFilePointerContainerImpl.RECURSIVE_ATTR);
        final OrderRootType rootType = getJarDirectoryRootType(jarDir.getAttributeValue(ROOT_TYPE_ATTR));
-        VirtualFilePointerContainer roots = myRoots.get(rootType);
+        VirtualFilePointerContainer roots = getOrCreateContainer(rootType);
        boolean recursively = Boolean.parseBoolean(recursive);
        roots.addJarDirectory(url, recursively);
      }
@@ -318,14 +329,13 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
      if (rootChild == null) {
        continue;
      }
-      VirtualFilePointerContainer roots = myRoots.get(rootType);
-      if (roots == null) {
-        LOG.error("Unknown root type: " + rootType + "; all roots: " + myRoots.keySet());
+      if (!rootChild.getChildren(ROOT_PATH_ELEMENT).isEmpty()) {
+        VirtualFilePointerContainer roots = getOrCreateContainer(rootType);
+        roots.readExternal(rootChild, ROOT_PATH_ELEMENT, false);
      }
-      roots.readExternal(rootChild, ROOT_PATH_ELEMENT, false);
    }
    Element excludedRoot = element.getChild(EXCLUDED_ROOTS_TAG);
-    if (excludedRoot != null) {
+    if (excludedRoot != null && !excludedRoot.getChildren(ROOT_PATH_ELEMENT).isEmpty()) {
      getOrCreateExcludedRoots().readExternal(excludedRoot, ROOT_PATH_ELEMENT, false);
    }
  }
@@ -387,14 +397,16 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
      storableRootTypes.addAll(Arrays.asList(myKind.getAdditionalRootTypes()));
    }
    for (OrderRootType rootType : sortRootTypes(storableRootTypes)) {
-      final VirtualFilePointerContainer roots = myRoots.get(rootType);
-      if (roots.size() == 0 && rootType.skipWriteIfEmpty()) {
+      VirtualFilePointerContainer roots = myRoots.get(rootType);
+      if ((roots == null || roots.size() == 0) && rootType.skipWriteIfEmpty()) {
        //compatibility iml/ipr
        continue;
      }

      final Element rootTypeElement = new Element(rootType.name());
-      roots.writeExternal(rootTypeElement, ROOT_PATH_ELEMENT, false);
+      if (roots != null) {
+        roots.writeExternal(rootTypeElement, ROOT_PATH_ELEMENT, false);
+      }
      element.addContent(rootTypeElement);
    }
    if (myExcludedRoots != null && myExcludedRoots.size() > 0) {
@@ -489,7 +501,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    checkDisposed();
    LOG.assertTrue(isWritable());

-    final VirtualFilePointerContainer container = myRoots.get(rootType);
+    final VirtualFilePointerContainer container = getOrCreateContainer(rootType);
    container.add(url);
  }

@@ -498,7 +510,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    checkDisposed();
    LOG.assertTrue(isWritable());

-    final VirtualFilePointerContainer container = myRoots.get(rootType);
+    final VirtualFilePointerContainer container = getOrCreateContainer(rootType);
    container.add(file);
  }

@@ -517,7 +529,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    checkDisposed();
    LOG.assertTrue(isWritable());

-    final VirtualFilePointerContainer container = myRoots.get(rootType);
+    final VirtualFilePointerContainer container = getOrCreateContainer(rootType);
    container.addJarDirectory(url, recursive);
  }

@@ -526,7 +538,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    checkDisposed();
    LOG.assertTrue(isWritable());

-    final VirtualFilePointerContainer container = myRoots.get(rootType);
+    final VirtualFilePointerContainer container = getOrCreateContainer(rootType);
    container.addJarDirectory(file.getUrl(), recursive);
  }

@@ -538,6 +550,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
  @Override
  public boolean isJarDirectory(@NotNull final String url, @NotNull final OrderRootType rootType) {
    VirtualFilePointerContainer container = myRoots.get(rootType);
+    if (container == null) return false;
    List<Pair<String, Boolean>> jarDirectories = container.getJarDirectories();
    return jarDirectories.contains(Pair.create(url, false)) || jarDirectories.contains(Pair.create(url, true));
  }
@@ -545,7 +558,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
  @Override
  public boolean isValid(@NotNull final String url, @NotNull final OrderRootType rootType) {
    final VirtualFilePointerContainer container = myRoots.get(rootType);
-    final VirtualFilePointer fp = container.findByUrl(url);
+    final VirtualFilePointer fp = container == null ? null : container.findByUrl(url);
    return fp != null && fp.isValid();
  }

@@ -555,7 +568,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    LOG.assertTrue(isWritable());

    final VirtualFilePointerContainer container = myRoots.get(rootType);
-    final VirtualFilePointer byUrl = container.findByUrl(url);
+    final VirtualFilePointer byUrl = container == null ? null : container.findByUrl(url);
    if (byUrl != null) {
      container.remove(byUrl);
      if (myExcludedRoots != null) {
@@ -589,7 +602,9 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    LOG.assertTrue(isWritable());

    final VirtualFilePointerContainer container = myRoots.get(rootType);
-    container.moveUp(url);
+    if (container != null) {
+      container.moveUp(url);
+    }
  }

  @Override
@@ -598,7 +613,9 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    LOG.assertTrue(isWritable());

    final VirtualFilePointerContainer container = myRoots.get(rootType);
-    container.moveDown(url);
+    if (container != null) {
+      container.moveDown(url);
+    }
  }

  @Override
@@ -674,6 +691,7 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    return myLibraryTable;
  }

+  @Override
  public boolean equals(final Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;
@@ -681,17 +699,16 @@ public class LibraryImpl extends TraceableDisposable implements LibraryEx.Modifi
    final LibraryImpl library = (LibraryImpl)o;

    if (myName != null ? !myName.equals(library.myName) : library.myName != null) return false;
-    if (myRoots != null ? !myRoots.equals(library.myRoots) : library.myRoots != null) return false;
+    if (!myRoots.equals(library.myRoots)) return false;
    if (myKind != null ? !myKind.equals(library.myKind) : library.myKind != null) return false;
    if (myProperties != null ? !myProperties.equals(library.myProperties) : library.myProperties != null) return false;
-    if (!Comparing.equal(myExcludedRoots, library.myExcludedRoots)) return false;
-
-    return true;
+    return Comparing.equal(myExcludedRoots, library.myExcludedRoots);
  }

+  @Override
  public int hashCode() {
    int result = myName != null ? myName.hashCode() : 0;
-    result = 31 * result + (myRoots != null ? myRoots.hashCode() : 0);
+    result = 31 * result + myRoots.hashCode();
    return result;
  }

@@ -22,6 +22,9 @@ import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.util.Comparator;
 import java.util.List;

@@ -60,6 +63,29 @@ public class ObjectUtils {
    }
  }

+  /**
+   * Creates an instance of class {@code ofInterface} with its {@link Object#toString()} method returning {@code name}.
+   * No other guarantees about return value behaviour.
+   * {@code ofInterface} must represent an interface class.
+   * Useful for stubs in generic code, e.g. for storing in {@code List<T>} to represent empty special value.
+   */
+  @NotNull
+  public static <T> T sentinel(@NotNull final String name, @NotNull Class<T> ofInterface) {
+    if (!ofInterface.isInterface()) {
+      throw new IllegalArgumentException("Expected interface but got: " + ofInterface);
+    }
+    //noinspection unchecked
+    return (T)Proxy.newProxyInstance(ObjectUtils.class.getClassLoader(), new Class[]{ofInterface}, new InvocationHandler() {
+      @Override
+      public Object invoke(Object proxy, Method method, Object[] args) {
+        if ("toString".equals(method.getName()) && args.length == 0) {
+          return name;
+        }
+        throw new AbstractMethodError();
+      }
+    });
+  }
+
  @NotNull
  public static <T> T assertNotNull(@Nullable T t) {
    return notNull(t);