If --batch, --keep_going, --discard_analysis_cache, and the new --noexperimental_enable_critical_path_profiling flags are all specified, then Bazel will delete Actions from ActionLookupValues as they are executed in order to save memory.

Because an already-run action may output an artifact that is only requested later in the build, we need to maintain a way for the artifact to look up the action. But in most cases we don't need to keep the action itself, just its output metadata.

Some actions unfortunately are needed post-execution, and so we special-case them.

Also includes dependency change with description:
Move action out of key. This keeps action references from polluting the graph -- actions are just stored in one SkyValue, instead of being present in SkyKeys.

This does mean additional memory used: we have a separate ActionLookupData object per Action executed. That may reach ~24M for million-action builds.

PiperOrigin-RevId: 151756383

1 files changed, 14 insertions, 48 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index 4c3c6a2c93..3d71d1b8cd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -14,22 +14,17 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
+import com.google.devtools.build.lib.actions.ActionLookupData;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
-
 import java.util.Map;
-
 import javax.annotation.Nullable;
 
 /**
@@ -38,15 +33,6 @@ import javax.annotation.Nullable;
 @Immutable
 @ThreadSafe
 public class ActionExecutionValue implements SkyValue {
-
-  private static final Function<Action, SkyKey> TO_KEY =
-      new Function<Action, SkyKey>() {
-        @Override
-        public SkyKey apply(Action action) {
-          return key(action);
-        }
-      };
-
   /*
   Concerning the data in this class:
 
@@ -120,8 +106,9 @@ public class ActionExecutionValue implements SkyValue {
   }
 
   /**
-   * @return The map from {@link Artifact}s to the corresponding {@link FileValue}s that would
-   * be returned by {@link #getData}. Should only be needed by {@link FilesystemValueChecker}.
+   * @return The map from {@link Artifact}s to the corresponding {@link FileValue}s that would be
+   *     returned by {@link #getData}. Primarily needed by {@link FilesystemValueChecker}, also
+   *     called by {@link ArtifactFunction} when aggregating a {@link TreeArtifactValue}.
    */
   ImmutableMap<Artifact, FileValue> getAllFileValues() {
     return artifactData;
@@ -129,44 +116,23 @@ public class ActionExecutionValue implements SkyValue {
 
   /**
    * @return The map from {@link Artifact}s to the corresponding {@link TreeArtifactValue}s that
-   * would be returned by {@link #getTreeArtifactValue}. Should only be needed by
-   * {@link FilesystemValueChecker}.
+   *     would be returned by {@link #getTreeArtifactValue}. Should only be needed by {@link
+   *     FilesystemValueChecker}.
    */
   ImmutableMap<Artifact, TreeArtifactValue> getAllTreeArtifactValues() {
     return treeArtifactData;
   }
 
-  @ThreadSafe
-  @VisibleForTesting
-  public static SkyKey key(Action action) {
-    return SkyKey.create(SkyFunctions.ACTION_EXECUTION, action);
-  }
-
-  static Iterable<SkyKey> keys(Iterable<Action> actions) {
-    return Iterables.transform(actions, TO_KEY);
-  }
-
-  /**
-   * Returns whether the key corresponds to a ActionExecutionValue worth reporting status about.
-   *
-   * <p>If an action can do real work, it's probably worth counting and reporting status about.
-   * Actions that don't really do any work (typically middleman actions) should not be counted
-   * towards enqueued and completed actions.
-   */
-  public static boolean isReportWorthyAction(SkyKey key) {
-    return key.functionName().equals(SkyFunctions.ACTION_EXECUTION)
-        && isReportWorthyAction((Action) key.argument());
-  }
-
   /**
-   * Returns whether the action is worth reporting status about.
-   *
-   * <p>If an action can do real work, it's probably worth counting and reporting status about.
-   * Actions that don't really do any work (typically middleman actions) should not be counted
-   * towards enqueued and completed actions.
+   * @param lookupKey A {@link SkyKey} whose argument is an {@code ActionLookupKey}, whose
+   *     corresponding {@code ActionLookupValue} contains the action to be executed.
+   * @param index the index of the action to be executed in the {@code ActionLookupValue}, to be
+   *     passed to {@code ActionLookupValue#getAction}.
    */
-  public static boolean isReportWorthyAction(Action action) {
-    return action.getActionType() == MiddlemanType.NORMAL;
+  @ThreadSafe
+  @VisibleForTesting
+  public static SkyKey key(SkyKey lookupKey, int index) {
+    return SkyKey.create(SkyFunctions.ACTION_EXECUTION, new ActionLookupData(lookupKey, index));
   }
 
   @Override