Don't expose used contexts for searching, instead use a callback.
A context (or strategy) implementation is considered "used" if it is not only registered but also reachable from at least one mnemonic or set as a default. Some logic requires this information, for example to conditionally do some setup/cleanup if one of a set of strategies ended up being used.
To localize the impact of the "used information propagation" this change removes the global way to inspect all registered contexts. Instead it introduces a callback mechanism where each context gets notified if it is used and can then invoke a callback which is listened to by the same logic that previously inspected all contexts. This approach can further be simplified in the future when the explicit notification is no longer necessary (contexts/strategies will only be instantiated if used, giving an implicit callback location).
This is a precondition for disentangling context registration and context lifecycle management which will be concluded in a future change.
RELNOTES: None
PiperOrigin-RevId: 290055330
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionContext.java
index 5eb8524b..1e44f18 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionContext.java
@@ -19,6 +19,16 @@
public interface ActionContext {
/**
+ * Performs any actions conditional on this context not only being registered but triggered as
+ * used because its identifier was requested and it was not overridden.
+ *
+ * @param actionContextRegistry a complete registry containing all used contexts (including this)
+ */
+ // TODO(schmitt): Remove once contexts are only instantiated if used, the callback can then be
+ // done upon construction.
+ default void usedContext(ActionContextRegistry actionContextRegistry) {}
+
+ /**
* A registry allowing the lookup of action contexts by identifying type during the execution
* phase.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java b/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java
index 967a8b4..b81c508 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java
@@ -31,4 +31,12 @@
*/
List<SandboxedSpawnActionContext> getDynamicSpawnActionContexts(
Spawn spawn, DynamicMode dynamicMode);
+
+ /**
+ * Notifies all strategies applying to at least one mnemonic (including the empty all-catch one)
+ * in this registry that they are {@link ActionContext#usedContext used}.
+ *
+ * @param actionContextRegistry a complete registry containing all available action contexts
+ */
+ void notifyUsedDynamic(ActionContextRegistry actionContextRegistry);
}
diff --git a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
index b461926..7442c61 100644
--- a/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/dynamic/DynamicSpawnStrategy.java
@@ -347,6 +347,13 @@
return false;
}
+ @Override
+ public void usedContext(ActionContextRegistry actionExecutionContext) {
+ actionExecutionContext
+ .getContext(DynamicStrategyRegistry.class)
+ .notifyUsedDynamic(actionExecutionContext);
+ }
+
private static FileOutErr getSuffixedFileOutErr(FileOutErr fileOutErr, String suffix) {
Path outDir = checkNotNull(fileOutErr.getOutputPath().getParentDirectory());
String outBaseName = fileOutErr.getOutputPath().getBaseName();
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java
index 24c0fbe..c47a57f 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java
@@ -26,11 +26,8 @@
*/
public abstract class ActionContextProvider {
- /**
- * Called when the executor is constructed. The parameter contains all the contexts that were
- * selected for this execution phase.
- */
- public void executorCreated(Iterable<ActionContext> usedContexts) throws ExecutorInitException {}
+ /** Called when the executor is constructed. */
+ public void executorCreated() throws ExecutorInitException {}
/** Called when the execution phase is started. */
public void executionPhaseStarting(
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
index 7e9c983..af5124c 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/BlazeExecutor.java
@@ -77,8 +77,10 @@
spawnActionContextMaps.debugPrintSpawnActionContextMaps(reporter);
}
+ spawnActionContextMaps.notifyUsed();
+
for (ActionContextProvider factory : contextProviders) {
- factory.executorCreated(spawnActionContextMaps.allContexts());
+ factory.executorCreated();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
index 12e8a27..5f1b52f 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnActionContextMaps.java
@@ -183,6 +183,27 @@
}
/**
+ * Notifies all (non-dynamic) contexts stored in this context map that they are {@link
+ * ActionContext#usedContext used}.
+ */
+ public void notifyUsed() {
+ for (ActionContext context : allContexts()) {
+ context.usedContext(this);
+ }
+ }
+
+ @Override
+ public void notifyUsedDynamic(ActionContextRegistry actionContextRegistry) {
+ for (SandboxedSpawnActionContext context : mnemonicToRemoteDynamicStrategies.values()) {
+ context.usedContext(actionContextRegistry);
+ }
+
+ for (SandboxedSpawnActionContext context : mnemonicToLocalDynamicStrategies.values()) {
+ context.usedContext(actionContextRegistry);
+ }
+ }
+
+ /**
* Print a sorted list of our (Spawn)ActionContext maps.
*
* <p>Prints out debug information about the mappings.
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
index 61f9173..44127d1 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
-import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.ActionGraph;
@@ -277,7 +276,7 @@
}
@Override
- public void executorCreated(Iterable<ActionContext> usedContexts) throws ExecutorInitException {
+ public void executorCreated() throws ExecutorInitException {
IncludeScanningOptions options = buildRequest.getOptions(IncludeScanningOptions.class);
int threads = options.includeScanningParallelism;
if (threads > 0) {
