add baseline functionality for not saving unused artifacts

We define unused artifacts as those that aren't consumed by any
action. This can be because an action produced more outputs than
a dependent action needed, or because it's a top level artifact
and we don't care about its contents, just that it was built
without issue. Actions may prevent outputs from being discarded
by declaring them as mandatory. This is particularly useful for
test outputs. The motivation behind this change is to reduce
storage overhead for things we can do without.

It is worth noting this change doesn't cover all cases. In particular
it has difficulty identifying *_binary artifacts as orphaned. This
is due to the insertion of a virtual runfiles artifact which depends
upon the rule's outputs.

--
MOS_MIGRATED_REVID=88467504

8 files changed, 86 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 82d9248bfc..09457474bd 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -335,6 +335,11 @@ public abstract class AbstractAction implements Action {
         .setMnemonic(getMnemonic());
   }
 
+  @Override
+  public ImmutableSet<Artifact> getMandatoryOutputs() {
+    return ImmutableSet.of();
+  }
+
   /**
    * Returns input files that need to be present to allow extra_action rules to shadow this action
    * correctly when run remotely. This is at least the normal inputs of the action, but may include
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
index 07b997785f..f40003b1a5 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -216,10 +216,12 @@ public class ActionCacheChecker {
       if (!key.equals(execPath)) {
         actionCache.remove(key);
       }
-      // Output files *must* exist and be accessible after successful action execution.
-      Metadata metadata = metadataHandler.getMetadata(output);
-      Preconditions.checkState(metadata != null);
-      entry.addFile(output.getExecPath(), metadata);
+      if (!metadataHandler.artifactOmitted(output)) {
+        // Output files *must* exist and be accessible after successful action execution.
+        Metadata metadata = metadataHandler.getMetadata(output);
+        Preconditions.checkState(metadata != null);
+        entry.addFile(output.getExecPath(), metadata);
+      }
     }
     for (Artifact input : action.getInputs()) {
       entry.addFile(input.getExecPath(), metadataHandler.getMetadataMaybe(input));
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java
index 3569f07a8b..64813aed07 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java
@@ -124,6 +124,14 @@ public interface ActionMetadata {
   ImmutableSet<Artifact> getOutputs();
 
   /**
+   * Returns the set of output Artifacts that are required to be saved. This is
+   * used to identify items that would otherwise be potentially identified as
+   * orphaned (not consumed by any downstream {@link Action}s and potentially
+   * discarded during the build process.
+   */
+  public ImmutableSet<Artifact> getMandatoryOutputs();
+
+  /**
    * Returns the "primary" input of this action, if applicable.
    *
    * <p>For example, a C++ compile action would return the .cc file which is being compiled,
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
index 9d288dbbff..61a5d6d211 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
@@ -48,11 +48,27 @@ public interface MetadataHandler {
    */
   void injectDigest(ActionInput output, FileStatus statNoFollow, byte[] digest);
 
-  /** Returns true iff artifact exists. */
+  /**
+   * Marks an artifact as intentionally omitted. Acknowledges that this Artifact could have
+   * existed, but was intentionally not saved, most likely as an optimization.
+   */
+  void markOmitted(ActionInput output);
+
+  /**
+   * Returns true iff artifact exists.
+   *
+   * <p>It is important to note that implementations may cache non-existence as a side effect
+   * of this method. If there is a possibility an artifact was intentionally omitted then
+   * {@link #artifactOmitted(Artifact)} should be checked first to avoid the side effect.</p>
+   */
   boolean artifactExists(Artifact artifact);
+
   /** Returns true iff artifact is a regular file. */
   boolean isRegularFile(Artifact artifact);
 
+  /** Returns true iff artifact was intentionally omitted (not saved). */
+  boolean artifactOmitted(Artifact artifact);
+
   /**
    * @return Whether the artifact's data was injected.
    * @throws IOException if implementation tried to stat artifact which threw an exception.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java
index 13ec62cedc..9e9c180360 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.test;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -535,6 +536,11 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa
     return "TestRunner";
   }
 
+  @Override
+  public ImmutableSet<Artifact> getMandatoryOutputs() {
+    return getOutputs();
+  }
+
   /**
    * The same set of paths as the parent test action, resolved against a given exec root.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
index 2a0de78064..5014c0f5f4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileAndMetadataCache.java
@@ -76,6 +76,7 @@ public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandl
   private final Map<ByteString, Artifact> reverseMap = new ConcurrentHashMap<>();
   private final ConcurrentMap<Artifact, FileValue> outputArtifactData =
       new ConcurrentHashMap<>();
+  private final Set<Artifact> omittedOutputs = Sets.newConcurrentHashSet();
   // See #getAdditionalOutputData for documentation of this field.
   private final ConcurrentMap<Artifact, FileArtifactValue> additionalOutputData =
       new ConcurrentHashMap<>();
@@ -328,6 +329,20 @@ public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandl
   }
 
   @Override
+  public void markOmitted(ActionInput output) {
+    if (output instanceof Artifact) {
+      Artifact artifact = (Artifact) output;
+      Preconditions.checkState(omittedOutputs.add(artifact), artifact);
+      additionalOutputData.put(artifact, FileArtifactValue.OMITTED_FILE_MARKER);
+    }
+  }
+
+  @Override
+  public boolean artifactOmitted(Artifact artifact) {
+    return omittedOutputs.contains(artifact);
+  }
+
+  @Override
   public void discardMetadata(Collection<Artifact> artifactList) {
     Preconditions.checkState(injectedArtifacts.isEmpty(),
         "Artifacts cannot be injected before action execution: %s", injectedArtifacts);
@@ -337,6 +352,7 @@ public class FileAndMetadataCache implements ActionInputFileCache, MetadataHandl
 
   @Override
   public boolean artifactExists(Artifact artifact) {
+    Preconditions.checkState(!artifactOmitted(artifact), artifact);
     return getMetadataMaybe(artifact) != null;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
index 7acc38c994..f1f0b3e12a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
@@ -36,6 +36,18 @@ public class FileArtifactValue extends ArtifactValue {
   /** Data that marks that a file is not present on the filesystem. */
   static final FileArtifactValue MISSING_FILE_MARKER = new FileArtifactValue(null, 1, 0);
 
+  /**
+   * Represents an omitted file- we are aware of it but it doesn't exist. All access methods
+   * are unsupported.
+   */
+  static final FileArtifactValue OMITTED_FILE_MARKER = new FileArtifactValue(null, 2, 0) {
+    @Override public byte[] getDigest() { throw new UnsupportedOperationException(); }
+    @Override public long getSize() { throw new UnsupportedOperationException(); }
+    @Override public long getModifiedTime() { throw new UnsupportedOperationException(); }
+    @Override public boolean equals(Object o) { return this == o; }
+    @Override public String toString() { return "OMITTED_FILE_MARKER"; }
+  };
+
   @Nullable private final byte[] digest;
   private final long mtime;
   private final long size;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index ebae390f50..7b71fba451 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -268,6 +268,17 @@ public final class SkyframeActionExecutor {
       // intra-build.
       perActionHandler.discardMetadata(artifactList);
     }
+
+    @Override
+    public void markOmitted(ActionInput output) {
+      perActionHandler.markOmitted(output);
+    }
+
+    @Override
+    public boolean artifactOmitted(Artifact artifact) {
+      return perActionHandler.artifactOmitted(artifact);
+    }
+
   }
 
   /**
@@ -944,14 +955,17 @@ public final class SkyframeActionExecutor {
   }
 
   /**
-   * Validates that all action outputs were created.
+   * Validates that all action outputs were created or intentionally omitted.
    *
    * @return false if some outputs are missing, true - otherwise.
    */
   private boolean checkOutputs(Action action, MetadataHandler metadataHandler) {
     boolean success = true;
     for (Artifact output : action.getOutputs()) {
-      if (!metadataHandler.artifactExists(output)) {
+      // artifactExists has the side effect of potentially adding the artifact to the cache,
+      // therefore we only call it if we know the artifact is indeed not omitted to avoid any
+      // unintended side effects.
+      if (!(metadataHandler.artifactOmitted(output) || metadataHandler.artifactExists(output))) {
         reportMissingOutputFile(action, output, reporter, output.getPath().isSymbolicLink());
         success = false;
       }