Allow for a set of known modified files to be passed into the FileSystemValueChecker when checking for dirty actions.

--
MOS_MIGRATED_REVID=108046467

1 files changed, 45 insertions, 21 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index 0e8a5cf1bc..30088ef3d3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -17,6 +17,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Range;
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
@@ -32,6 +33,8 @@ import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
 import com.google.devtools.build.lib.vfs.BatchStat;
 import com.google.devtools.build.lib.vfs.FileStatusWithDigest;
+import com.google.devtools.build.lib.vfs.ModifiedFileSet;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.Differencer;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -152,8 +155,12 @@ public class FilesystemValueChecker {
    * the on-disk file value (were modified externally).
    */
   Collection<SkyKey> getDirtyActionValues(Map<SkyKey, SkyValue> valuesMap,
-      @Nullable final BatchStat batchStatter) throws InterruptedException {
-    // CPU-bound (usually) stat() calls, plus a fudge factor.
+      @Nullable final BatchStat batchStatter, ModifiedFileSet modifiedOutputFiles)
+          throws InterruptedException {
+    if (modifiedOutputFiles == ModifiedFileSet.NOTHING_MODIFIED) {
+      LOG.info("Not checking for dirty actions since nothing was modified");
+      return ImmutableList.of();
+    }
     LOG.info("Accumulating dirty actions");
     final int numOutputJobs = Runtime.getRuntime().availableProcessors() * 4;
     final Set<SkyKey> actionSkyKeys = new HashSet<>();
@@ -180,10 +187,14 @@ public class FilesystemValueChecker {
 
     modifiedOutputFilesCounter.set(0);
     modifiedOutputFilesIntraBuildCounter.set(0);
+    ImmutableSet<PathFragment> knownModifiedOutputFiles =
+            modifiedOutputFiles == ModifiedFileSet.EVERYTHING_MODIFIED
+                    ? null
+                    : modifiedOutputFiles.modifiedSourceFiles();
     for (List<Pair<SkyKey, ActionExecutionValue>> shard : outputShards) {
       Runnable job = (batchStatter == null)
-          ? outputStatJob(dirtyKeys, shard)
-          : batchStatJob(dirtyKeys, shard, batchStatter);
+          ? outputStatJob(dirtyKeys, shard, knownModifiedOutputFiles)
+          : batchStatJob(dirtyKeys, shard, batchStatter, knownModifiedOutputFiles);
       executor.submit(wrapper.wrap(job));
     }
 
@@ -197,8 +208,8 @@ public class FilesystemValueChecker {
   }
 
   private Runnable batchStatJob(final Collection<SkyKey> dirtyKeys,
-                                       final List<Pair<SkyKey, ActionExecutionValue>> shard,
-                                       final BatchStat batchStatter) {
+          final List<Pair<SkyKey, ActionExecutionValue>> shard,
+          final BatchStat batchStatter, final ImmutableSet<PathFragment> knownModifiedOutputFiles) {
     return new Runnable() {
       @Override
       public void run() {
@@ -209,7 +220,9 @@ public class FilesystemValueChecker {
             dirtyKeys.add(keyAndValue.getFirst());
           } else {
             for (Artifact artifact : actionValue.getAllOutputArtifactData().keySet()) {
-              artifactToKeyAndValue.put(artifact, keyAndValue);
+              if (shouldCheckArtifact(knownModifiedOutputFiles, artifact)) {
+                artifactToKeyAndValue.put(artifact, keyAndValue);
+              }
             }
           }
         }
@@ -222,7 +235,7 @@ public class FilesystemValueChecker {
         } catch (IOException e) {
           // Batch stat did not work. Log an exception and fall back on system calls.
           LoggingUtil.logToRemote(Level.WARNING, "Unable to process batch stat", e);
-          outputStatJob(dirtyKeys, shard).run();
+          outputStatJob(dirtyKeys, shard, knownModifiedOutputFiles).run();
           return;
         } catch (InterruptedException e) {
           // We handle interrupt in the main thread.
@@ -266,13 +279,15 @@ public class FilesystemValueChecker {
   }
 
   private Runnable outputStatJob(final Collection<SkyKey> dirtyKeys,
-                                 final List<Pair<SkyKey, ActionExecutionValue>> shard) {
+          final List<Pair<SkyKey, ActionExecutionValue>> shard,
+          final ImmutableSet<PathFragment> knownModifiedOutputFiles) {
     return new Runnable() {
       @Override
       public void run() {
         for (Pair<SkyKey, ActionExecutionValue> keyAndValue : shard) {
           ActionExecutionValue value = keyAndValue.getSecond();
-          if (value == null || actionValueIsDirtyWithDirectSystemCalls(value)) {
+          if (value == null
+              || actionValueIsDirtyWithDirectSystemCalls(value, knownModifiedOutputFiles)) {
             dirtyKeys.add(keyAndValue.getFirst());
           }
         }
@@ -292,30 +307,39 @@ public class FilesystemValueChecker {
     return modifiedOutputFilesIntraBuildCounter.get();
   }
 
-  private boolean actionValueIsDirtyWithDirectSystemCalls(ActionExecutionValue actionValue) {
+  private boolean actionValueIsDirtyWithDirectSystemCalls(ActionExecutionValue actionValue,
+      ImmutableSet<PathFragment> knownModifiedOutputFiles) {
     boolean isDirty = false;
     for (Map.Entry<Artifact, FileValue> entry :
         actionValue.getAllOutputArtifactData().entrySet()) {
       Artifact artifact = entry.getKey();
       FileValue lastKnownData = entry.getValue();
-      try {
-        FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(artifact, null, tsgm);
-        if (!fileValue.equals(lastKnownData)) {
-          updateIntraBuildModifiedCounter(fileValue.exists()
-              ? fileValue.realRootedPath().asPath().getLastModifiedTime()
-              : -1, lastKnownData.isSymlink(), fileValue.isSymlink());
+      if (shouldCheckArtifact(knownModifiedOutputFiles, artifact)) {
+        try {
+          FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(artifact, null, tsgm);
+          if (!fileValue.equals(lastKnownData)) {
+            updateIntraBuildModifiedCounter(fileValue.exists()
+                ? fileValue.realRootedPath().asPath().getLastModifiedTime()
+                : -1, lastKnownData.isSymlink(), fileValue.isSymlink());
+            modifiedOutputFilesCounter.getAndIncrement();
+            isDirty = true;
+          }
+        } catch (IOException e) {
+          // This is an unexpected failure getting a digest or symlink target.
           modifiedOutputFilesCounter.getAndIncrement();
           isDirty = true;
         }
-      } catch (IOException e) {
-        // This is an unexpected failure getting a digest or symlink target.
-        modifiedOutputFilesCounter.getAndIncrement();
-        isDirty = true;
       }
     }
     return isDirty;
   }
 
+  private static boolean shouldCheckArtifact(ImmutableSet<PathFragment> knownModifiedOutputFiles,
+      Artifact artifact) {
+    return knownModifiedOutputFiles == null
+        || knownModifiedOutputFiles.contains(artifact.getExecPath());
+  }
+
   private BatchDirtyResult getDirtyValues(ValueFetcher fetcher,
       Iterable<SkyKey> keys, final SkyValueDirtinessChecker checker,
       final boolean checkMissingValues) throws InterruptedException {