Avoid unnecessary persistence of activity enablements #1084

The ActivityPersistenceHelper persists all activity enablements upon
every notification about a changed activity enablement it receives.
Persistence is, however, only done and thus necessary for activities
that are not expression-controlled. So the current implementation
performs unnecessary persistence operations.

With this change, the ActivityPersistenceHelper only persists the
enablements when the enablement of a non-expression-controlled activity
has changed. To this end, the received ActivityManagerEvent is extended
by the information about whether such an activity was affected. This
information is either directly passed to the event construction in case
it has already been calculated in the calling context or is calculated
lazily when accessing the value.

Contributes to
#1084.

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/activities/ActivityManagerEvent.java

@@ -14,6 +14,9 @@
 
 package org.eclipse.ui.activities;
 
+import static java.util.Collections.emptySet;
+
+import java.util.HashSet;
 import java.util.Set;
 import org.eclipse.ui.internal.util.Util;
 
@@ -39,6 +42,14 @@ public final class ActivityManagerEvent {
 
 	private boolean enabledActivityIdsChanged;
 
+	/**
+	 * Indicates whether enabled IDs of non-expression-controlled activities have
+	 * changed. The value can be explicitly given via constructor if already known
+	 * by the caller or is set to <code>null</null> and will be calculated on demand
+	 * upon first access to the value.
+	 */
+	private Boolean enabledNonExpressionControlledActivityIdsChanged = null;
+
 	/**
 	 * The set of activity identifiers (strings) that were defined before the change
 	 * occurred. If the defined activities did not changed, then this value is
@@ -145,6 +156,83 @@ public ActivityManagerEvent(IActivityManager activityManager, boolean definedAct
 		this.enabledActivityIdsChanged = enabledActivityIdsChanged;
 	}
 
+	/**
+	 * Creates a new instance of this class.
+	 *
+	 * @param activityManager              the instance of the interface that
+	 *                                     changed.
+	 * @param definedActivityIdsChanged    <code>true</code>, iff the
+	 *                                     definedActivityIds property changed.
+	 * @param definedCategoryIdsChanged    <code>true</code>, iff the
+	 *                                     definedCategoryIds property changed.
+	 * @param enabledActivityIdsChanged    <code>true</code>, iff the
+	 *                                     enabledActivityIds property changed.
+	 * @param enabledNonExpressionControlledActivityIdsChanged
+	 *                                     <code>true>/code> iff the
+	 *                                     enabledActivityIds property changed
+	 *                                     and any of the changed IDs belongs
+	 *                                     to a non-expression-controlled activity.
+	 * @param previouslyDefinedActivityIds the set of identifiers to previously
+	 *                                     defined activities. This set may be
+	 *                                     empty. If this set is not empty, it must
+	 *                                     only contain instances of
+	 *                                     <code>String</code> . This set must be
+	 *                                     <code>null</code> if
+	 *                                     definedActivityIdsChanged is
+	 *                                     <code>false</code> and must not be null
+	 *                                     if definedActivityIdsChanged is
+	 *                                     <code>true</code>.
+	 * @param previouslyDefinedCategoryIds the set of identifiers to previously
+	 *                                     defined category. This set may be empty.
+	 *                                     If this set is not empty, it must only
+	 *                                     contain instances of <code>String</code>.
+	 *                                     This set must be <code>null</code> if
+	 *                                     definedCategoryIdsChanged is
+	 *                                     <code>false</code> and must not be null
+	 *                                     if definedCategoryIdsChanged is
+	 *                                     <code>true</code>.
+	 * @param previouslyEnabledActivityIds the set of identifiers to previously
+	 *                                     enabled activities. This set may be
+	 *                                     empty. If this set is not empty, it must
+	 *                                     only contain instances of
+	 *                                     <code>String</code>. This set must be
+	 *                                     <code>null</code> if
+	 *                                     enabledActivityIdsChanged is
+	 *                                     <code>false</code> and must not be null
+	 *                                     if enabledActivityIdsChanged is
+	 *                                     <code>true</code>.
+	 * @since 3.131
+	 */
+	public ActivityManagerEvent(IActivityManager activityManager, boolean definedActivityIdsChanged,
+			boolean definedCategoryIdsChanged, boolean enabledActivityIdsChanged,
+			boolean enabledNonExpressionControlledActivityIdsChanged, final Set<String> previouslyDefinedActivityIds,
+			final Set<String> previouslyDefinedCategoryIds, final Set<String> previouslyEnabledActivityIds) {
+		this(activityManager, definedActivityIdsChanged, definedCategoryIdsChanged, enabledActivityIdsChanged,
+				previouslyDefinedActivityIds, previouslyDefinedCategoryIds, previouslyEnabledActivityIds);
+		this.enabledNonExpressionControlledActivityIdsChanged = enabledNonExpressionControlledActivityIdsChanged;
+	}
+
+	/**
+	 * Creates a copy of this {@link ActivityManagerEvent} containing the given
+	 * {@code newActivityManager}.
+	 *
+	 * @param newActivityManager the new activity manager to be referenced by the
+	 *                           copied event
+	 *
+	 * @return a copy of this event referencing the given activity manager
+	 * @since 3.131
+	 */
+	public ActivityManagerEvent copyFor(IActivityManager newActivityManager) {
+		if (enabledNonExpressionControlledActivityIdsChanged != null) {
+			return new ActivityManagerEvent(newActivityManager, definedActivityIdsChanged, definedCategoryIdsChanged,
+					enabledActivityIdsChanged, enabledNonExpressionControlledActivityIdsChanged.booleanValue(),
+					previouslyDefinedActivityIds, previouslyDefinedCategoryIds, previouslyEnabledActivityIds);
+		}
+		return new ActivityManagerEvent(newActivityManager, definedActivityIdsChanged, definedCategoryIdsChanged,
+				enabledActivityIdsChanged, previouslyDefinedActivityIds, previouslyDefinedCategoryIds,
+				previouslyEnabledActivityIds);
+	}
+
 	/**
 	 * Returns the instance of the interface that changed.
 	 *
@@ -214,4 +302,34 @@ public boolean haveDefinedCategoryIdsChanged() {
 	public boolean haveEnabledActivityIdsChanged() {
 		return enabledActivityIdsChanged;
 	}
+
+	/**
+	 * Returns whether or not enabledActivityIds property changed and any of the
+	 * changed IDs belongs to a non-expression-controlled activity.
+	 *
+	 * @return <code>true>/code> iff the enabledActivityIds property changed and any
+	 *         of the changed IDs belongs to a non-expression-controlled activity.
+	 * @since 3.131
+	 */
+	public boolean haveEnabledNonExpressionControlledActivityIdsChanged() {
+		if (enabledNonExpressionControlledActivityIdsChanged == null) {
+			Set<String> nonNullPreviouslyEnabledActivityIds = previouslyEnabledActivityIds == null ? emptySet()
+					: previouslyEnabledActivityIds;
+			enabledNonExpressionControlledActivityIdsChanged = haveEnabledNonExpressionControlledActivityIdsChanged(
+					activityManager, nonNullPreviouslyEnabledActivityIds);
+		}
+		return enabledNonExpressionControlledActivityIdsChanged;
+	}
+
+	private static boolean haveEnabledNonExpressionControlledActivityIdsChanged(IActivityManager activityManager,
+			Set<String> previousIds) {
+		Set<String> currentIds = activityManager.getEnabledActivityIds();
+		Set<String> unchangedIds = new HashSet<>(currentIds);
+		unchangedIds.retainAll(previousIds);
+		Set<String> changedIds = new HashSet<>(previousIds);
+		changedIds.addAll(currentIds);
+		changedIds.removeAll(unchangedIds);
+		return changedIds.stream().anyMatch(id -> activityManager.getActivity(id).getExpression() == null);
+	}
+
 }

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/ActivityPersistanceHelper.java

@@ -57,7 +57,7 @@ final class ActivityPersistanceHelper {
 			// state
 			loadEnabledStates(activityManagerEvent.getActivityManager().getEnabledActivityIds(), delta);
 		}
-		if (activityManagerEvent.haveEnabledActivityIdsChanged()) {
+		if (activityManagerEvent.haveEnabledNonExpressionControlledActivityIdsChanged()) {
 			saveEnabledStates();
 		}
 	};

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/MutableActivityManager.java

@@ -526,8 +526,11 @@ private void updateListeners(boolean activityManagerChanged, Map<String, Activit
 		}
 
 		if (activityManagerChanged) {
+			boolean haveNonExpressionControlledActivityIdsChanged = deltaActivityIds.stream()
+					.anyMatch(id -> getActivity(id).getExpression() == null);
 			fireActivityManagerChanged(
-					new ActivityManagerEvent(this, false, false, true, null, null, previouslyEnabledActivityIds));
+					new ActivityManagerEvent(this, false, false, true, haveNonExpressionControlledActivityIdsChanged,
+							null, null, previouslyEnabledActivityIds));
 		}
 	}
 

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ProxyActivityManager.java

@@ -32,13 +32,7 @@ public ProxyActivityManager(IActivityManager activityManager) {
 		this.activityManager = activityManager;
 
 		this.activityManager.addActivityManagerListener(activityManagerEvent -> {
-			ActivityManagerEvent proxyActivityManagerEvent = new ActivityManagerEvent(ProxyActivityManager.this,
-					activityManagerEvent.haveDefinedActivityIdsChanged(),
-					activityManagerEvent.haveDefinedCategoryIdsChanged(),
-					activityManagerEvent.haveEnabledActivityIdsChanged(),
-					activityManagerEvent.getPreviouslyDefinedActivityIds(),
-					activityManagerEvent.getPreviouslyDefinedCategoryIds(),
-					activityManagerEvent.getPreviouslyEnabledActivityIds());
+			ActivityManagerEvent proxyActivityManagerEvent = activityManagerEvent.copyFor(ProxyActivityManager.this);
 			fireActivityManagerChanged(proxyActivityManagerEvent);
 		});
 	}