From 9caa0f7e3486bb6111ea403fadc99ccba6918e88 Mon Sep 17 00:00:00 2001 From: Gunnar Wagenknecht Date: Fri, 3 Nov 2023 11:31:02 +0100 Subject: [PATCH] Improve cache-reuse by preventing invalidation during ongoing sync --- .../eclipse/core/model/BazelModelManager.java | 7 ++ .../eclipse/core/model/BazelWorkspace.java | 5 +- .../core/model/ResourceChangeProcessor.java | 76 ++++++++++++++++++- .../core/model/SynchronizeProjectViewJob.java | 7 ++ 4 files changed, 89 insertions(+), 6 deletions(-) diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelModelManager.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelModelManager.java index 25508e16..af3067af 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelModelManager.java +++ b/bundles/com.salesforce.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/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelWorkspace.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelWorkspace.java index 67c09126..7fdef3bd 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelWorkspace.java +++ b/bundles/com.salesforce.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 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/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/ResourceChangeProcessor.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/ResourceChangeProcessor.java index a77aba82..bfd9b531 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/ResourceChangeProcessor.java +++ b/bundles/com.salesforce.bazel.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, 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. + *

+ * Internally a counter is maintained, i.e. for each call to suspendInvalidationFor, + * resumeInvalidationFor must be called. + *

+ * + * @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/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java index 1ef4e3cd..83274807 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java +++ b/bundles/com.salesforce.bazel.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(); }