Improve cache-reuse by preventing invalidation during ongoing sync

salesforce · Nov 3, 2023 · 9caa0f7 · 9caa0f7
1 parent f9b0c92
commit 9caa0f7
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 6 deletions.
diff --git a/...rce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelModelManager.java b/...rce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelModelManager.java
@@ -151,6 +151,13 @@ public BazelModel getModel() {
         return model;
     }
 
+    /**
+     * @return the resource change processor
+     */
+    ResourceChangeProcessor getResourceChangeProcessor() {
+        return resourceChangeProcessor;
+    }
+
     /**
      * @return the Eclipse {@link IWorkspace}.
      */

diff --git a/...sforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelWorkspace.java b/...sforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelWorkspace.java
@@ -26,6 +26,7 @@
 
 import java.nio.file.Path;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Predicate;
@@ -464,8 +465,8 @@ public void open(Collection<BazelPackage> bazelPackages) throws CoreException {
         for (BazelPackage bazelPackage : closedPackages) {
             var targets = targetsByPackage.get(bazelPackage);
             if (targets == null) {
-                LOG.debug("No result for: '{}'", bazelPackage);
-                continue;
+                LOG.debug("Empty package: '{}'", bazelPackage);
+                targets = Collections.emptyMap();
             }
             bazelPackage.openIfNecessary(
                 new BazelPackageInfo(

diff --git a/...zel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/ResourceChangeProcessor.java b/...zel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/ResourceChangeProcessor.java
@@ -29,6 +29,8 @@
 
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IFile;
@@ -48,6 +50,8 @@ class ResourceChangeProcessor implements IResourceChangeListener {
 
     private final BazelModelManager modelManager;
 
+    private final ConcurrentMap<BazelElement<?, ?>, Integer> suspendedElement = new ConcurrentHashMap<>();
+
     public ResourceChangeProcessor(BazelModelManager modelManager) {
         this.modelManager = modelManager;
     }
@@ -161,22 +165,30 @@ private void deleting(IProject project) throws CoreException {
     private void invalidateBazelWorkspaceCache(IProject project) {
         var bazelProject = modelManager.getBazelProject(project);
         try {
-            bazelProject.getBazelWorkspace().invalidateInfo();
+            invalidateCache(bazelProject.getBazelWorkspace());
         } catch (CoreException e) {
             // ignore
         }
     }
 
+    private void invalidateCache(BazelElement<?, ?> element) {
+        if (isInvalidationSuspendedFor(element) || !element.hasInfo()) {
+            return;
+        }
+
+        element.invalidateInfo();
+    }
+
     private void invalidateCache(IProject project) {
         var bazelProject = modelManager.getBazelProject(project);
         try {
             if (bazelProject.isWorkspaceProject()) {
-                bazelProject.getBazelWorkspace().invalidateInfo();
+                invalidateCache(bazelProject.getBazelWorkspace());
             } else if (bazelProject.isPackageProject()) {
-                bazelProject.getBazelPackage().invalidateInfo();
+                invalidateCache(bazelProject.getBazelPackage());
             } else if (bazelProject.isTargetProject()) {
                 // validate the whole package
-                bazelProject.getBazelPackage().invalidateInfo();
+                invalidateCache(bazelProject.getBazelPackage());
             }
         } catch (CoreException e) {
             // ignore
@@ -226,6 +238,17 @@ boolean isAutoBuilding() {
         return ResourcesPlugin.getWorkspace().isAutoBuilding();
     }
 
+    boolean isInvalidationSuspendedFor(BazelElement<?, ?> element) {
+        while (element != null) {
+            var refCount = suspendedElement.get(element);
+            if ((refCount != null) && (refCount > 0)) {
+                return true;
+            }
+            element = element.getParent();
+        }
+        return false;
+    }
+
     @Override
     public void resourceChanged(IResourceChangeEvent event) {
         var resource = event.getResource();
@@ -250,4 +273,49 @@ public void resourceChanged(IResourceChangeEvent event) {
         }
     }
 
+    /**
+     * Instructs the processor to suspend cache invalidation for all children belonging to the element.
+     *
+     * @param element
+     */
+    void resumeInvalidationFor(BazelElement<?, ?> element) {
+        var refCount = suspendedElement.get(element);
+        if (refCount != null) {
+            var old = refCount;
+            Integer newRefCount = old - 1;
+            var removedOrReplaced = false;
+            while (!removedOrReplaced) {
+                if (newRefCount > 0) {
+                    removedOrReplaced = suspendedElement.replace(element, old, newRefCount);
+                } else {
+                    removedOrReplaced = suspendedElement.remove(element, old);
+                }
+                if (!removedOrReplaced) {
+                    old = suspendedElement.get(element);
+                    newRefCount = old != null ? old - 1 : 0;
+                }
+            }
+        }
+    }
+
+    /**
+     * Instructs the processor to suspend cache invalidation for all children belonging to the element.
+     * <p>
+     * Internally a counter is maintained, i.e. for each call to <code>suspendInvalidationFor</code>,
+     * <code>resumeInvalidationFor</code> must be called.
+     * </p>
+     *
+     * @param element
+     */
+    void suspendInvalidationFor(BazelElement<?, ?> element) {
+        var refCount = suspendedElement.putIfAbsent(element, 1);
+        if (refCount != null) {
+            var old = refCount;
+            Integer newRefCount = old + 1;
+            while (!suspendedElement.replace(element, old, newRefCount)) {
+                old = suspendedElement.get(element);
+                newRefCount = old != null ? old + 1 : 1;
+            }
+        }
+    }
 }
diff --git a/...l.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java b/...l.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java
@@ -571,6 +571,10 @@ public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException {
             // ideally we would monitor resource change events and invalidate individual targets/packages only when necessary
             workspace.getModel().getInfoCache().invalidateAll();
 
+            // during synchronization resource changes may occur; however, they are triggered by the synchronization activities
+            // therefore we suspend cache invalidation of the model due to resource changes
+            workspace.getModelManager().getResourceChangeProcessor().suspendInvalidationFor(workspace);
+
             // trigger loading of the project view
             projectView = workspace.getBazelProjectView();
             importRoots = createImportRoots(workspace);
@@ -627,6 +631,9 @@ public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException {
             initializeClasspaths(targetProjects, workspace, progress.split(1, SUPPRESS_NONE));
             return Status.OK_STATUS;
         } finally {
+            // resume cache invalidation
+            workspace.getModelManager().getResourceChangeProcessor().resumeInvalidationFor(workspace);
+
             if (monitor != null) {
                 monitor.done();
             }