Stop requiring the creation and storage of FileStateValues within ResolvedFile objects.

While this does not eliminate the need for stat operation yet, it gets rid of one usage of the stat result (and is also a self contained change)

RELNOTES: None
PiperOrigin-RevId: 204417559

1 files changed, 52 insertions, 121 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java
index 0b91f51eea..000daa639a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java
@@ -18,7 +18,6 @@ import com.google.common.base.Objects;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Interner;
-import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FileStateValue;
 import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot;
 import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode;
@@ -262,20 +261,17 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
 
   private static final class RegularFile implements ResolvedFile {
     private final RootedPath path;
-    @Nullable private final FileStateValue metadata;
-    @Nullable private final FileArtifactValue artifactValue;
+    private final Object metadata;
 
     /** C'tor for {@link #stripMetadataForTesting()}. */
     private RegularFile(RootedPath path) {
       this.path = Preconditions.checkNotNull(path);
-      this.metadata = null;
-      this.artifactValue = null;
+      this.metadata = EMPTY_METADATA;
     }
 
-    RegularFile(RootedPath path, FileStateValue metadata, FileArtifactValue artifactValue) {
+    RegularFile(RootedPath path, Object metadata) {
       this.path = Preconditions.checkNotNull(path);
-      this.metadata = Preconditions.checkNotNull(metadata);
-      this.artifactValue = artifactValue;
+      this.metadata = metadata;
     }
 
     @Override
@@ -290,8 +286,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
 
     @Override
     @Nullable
-    public int getMetadataHash() {
-      return metadata == null ? 0 : metadata.hashCode();
+    public Object getMetadata() {
+      return metadata;
     }
 
     @Override
@@ -303,7 +299,7 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
         return false;
       }
       return this.path.equals(((RegularFile) obj).path)
-          && Objects.equal(this.metadata, ((RegularFile) obj).metadata);
+          && this.metadata.equals(((RegularFile) obj).metadata);
     }
 
     @Override
@@ -327,11 +323,6 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    public FileArtifactValue getValueForDerivedArtifacts() {
-      return artifactValue;
-    }
-
-    @Override
     public PathFragment getTargetInSymlinkTree(boolean followSymlinks) {
       return path.asPath().asFragment();
     }
@@ -355,8 +346,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    public int getMetadataHash() {
-      return FileStateValue.DIRECTORY_FILE_STATE_NODE.hashCode();
+    public Object getMetadata() {
+      return FileStateValue.DIRECTORY_FILE_STATE_NODE;
     }
 
     @Override
@@ -391,11 +382,6 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    public FileArtifactValue getValueForDerivedArtifacts() {
-      return null;
-    }
-
-    @Override
     public PathFragment getTargetInSymlinkTree(boolean followSymlinks) {
       return path.asPath().asFragment();
     }
@@ -403,29 +389,21 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
 
   private static final class DanglingSymlink implements ResolvedFile {
     private final Symlink symlink;
-    @Nullable private final FileStateValue metadata;
-    @Nullable private final FileArtifactValue artifactValue;
+    private final Object metadata;
 
     private DanglingSymlink(Symlink symlink) {
       this.symlink = symlink;
-      this.metadata = null;
-      this.artifactValue = null;
+      this.metadata = EMPTY_METADATA;
     }
 
     private DanglingSymlink(RootedPath linkNamePath, PathFragment linkTargetPath) {
       this.symlink = new Symlink(linkNamePath, linkTargetPath);
-      this.metadata = null;
-      this.artifactValue = null;
+      this.metadata = EMPTY_METADATA;
     }
 
-    DanglingSymlink(
-        RootedPath linkNamePath,
-        PathFragment linkTargetPath,
-        FileStateValue metadata,
-        @Nullable FileArtifactValue fileArtifactValue) {
+    DanglingSymlink(RootedPath linkNamePath, PathFragment linkTargetPath, Object metadata) {
       this.symlink = new Symlink(linkNamePath, linkTargetPath);
-      this.metadata = Preconditions.checkNotNull(metadata);
-      this.artifactValue = fileArtifactValue;
+      this.metadata = metadata;
     }
 
     @Override
@@ -440,9 +418,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    @Nullable
-    public int getMetadataHash() {
-      return metadata == null ? 0 : metadata.hashCode();
+    public Object getMetadata() {
+      return metadata;
     }
 
     @Override
@@ -453,7 +430,7 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
       if (!(obj instanceof DanglingSymlink)) {
         return false;
       }
-      return Objects.equal(this.metadata, ((DanglingSymlink) obj).metadata)
+      return this.metadata.equals(((DanglingSymlink) obj).metadata)
           && this.symlink.equals(((DanglingSymlink) obj).symlink);
     }
 
@@ -478,11 +455,6 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    public FileArtifactValue getValueForDerivedArtifacts() {
-      return artifactValue;
-    }
-
-    @Override
     public PathFragment getTargetInSymlinkTree(boolean followSymlinks)
         throws DanglingSymlinkException {
       if (followSymlinks) {
@@ -496,36 +468,31 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
 
   private static final class SymlinkToFile implements ResolvedFile {
     private final RootedPath path;
-    @Nullable private final FileStateValue metadata;
+    private final Object metadata;
     private final Symlink symlink;
-    @Nullable private final FileArtifactValue fileArtifactValue;
 
     /** C'tor for {@link #stripMetadataForTesting()}. */
     private SymlinkToFile(RootedPath targetPath, Symlink symlink) {
       this.path = Preconditions.checkNotNull(targetPath);
-      this.metadata = null;
+      this.metadata = EMPTY_METADATA;
       this.symlink = Preconditions.checkNotNull(symlink);
-      this.fileArtifactValue = null;
     }
 
     private SymlinkToFile(
         RootedPath targetPath, RootedPath linkNamePath, PathFragment linkTargetPath) {
       this.path = Preconditions.checkNotNull(targetPath);
-      this.metadata = null;
+      this.metadata = EMPTY_METADATA;
       this.symlink = new Symlink(linkNamePath, linkTargetPath);
-      this.fileArtifactValue = null;
     }
 
     SymlinkToFile(
         RootedPath targetPath,
         RootedPath linkNamePath,
         PathFragment linkTargetPath,
-        FileStateValue metadata,
-        @Nullable FileArtifactValue fileArtifactValue) {
+        Object metadata) {
       this.path = Preconditions.checkNotNull(targetPath);
-      this.metadata = Preconditions.checkNotNull(metadata);
+      this.metadata = metadata;
       this.symlink = new Symlink(linkNamePath, linkTargetPath);
-      this.fileArtifactValue = fileArtifactValue;
     }
 
     @Override
@@ -539,9 +506,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    @Nullable
-    public int getMetadataHash() {
-      return metadata == null ? 0 : metadata.hashCode();
+    public Object getMetadata() {
+      return metadata;
     }
 
     @Override
@@ -553,7 +519,7 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
         return false;
       }
       return this.path.equals(((SymlinkToFile) obj).path)
-          && Objects.equal(this.metadata, ((SymlinkToFile) obj).metadata)
+          && this.metadata.equals(((SymlinkToFile) obj).metadata)
           && this.symlink.equals(((SymlinkToFile) obj).symlink);
     }
 
@@ -578,11 +544,6 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    public FileArtifactValue getValueForDerivedArtifacts() {
-      return fileArtifactValue;
-    }
-
-    @Override
     public PathFragment getTargetInSymlinkTree(boolean followSymlinks) {
       return followSymlinks ? path.asPath().asFragment() : symlink.unresolvedLinkTarget;
     }
@@ -590,36 +551,28 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
 
   private static final class SymlinkToDirectory implements ResolvedFile {
     private final RootedPath path;
-    @Nullable private final int metadataHash;
+    private final Object metadata;
     private final Symlink symlink;
-    @Nullable private final FileArtifactValue fileArtifactValue;
 
     /** C'tor for {@link #stripMetadataForTesting()}. */
     private SymlinkToDirectory(RootedPath targetPath, Symlink symlink) {
       this.path = Preconditions.checkNotNull(targetPath);
-      this.metadataHash = 0;
+      this.metadata = EMPTY_METADATA;
       this.symlink = symlink;
-      this.fileArtifactValue = null;
     }
 
     private SymlinkToDirectory(
         RootedPath targetPath, RootedPath linkNamePath, PathFragment linkValue) {
       this.path = Preconditions.checkNotNull(targetPath);
-      this.metadataHash = 0;
+      this.metadata = EMPTY_METADATA;
       this.symlink = new Symlink(linkNamePath, linkValue);
-      this.fileArtifactValue = null;
     }
 
     SymlinkToDirectory(
-        RootedPath targetPath,
-        RootedPath linkNamePath,
-        PathFragment linkValue,
-        int metadataHash,
-        @Nullable FileArtifactValue fileArtifactValue) {
+        RootedPath targetPath, RootedPath linkNamePath, PathFragment linkValue, Object metadata) {
       this.path = Preconditions.checkNotNull(targetPath);
-      this.metadataHash = metadataHash;
+      this.metadata = metadata;
       this.symlink = new Symlink(linkNamePath, linkValue);
-      this.fileArtifactValue = fileArtifactValue;
     }
 
     @Override
@@ -633,9 +586,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    @Nullable
-    public int getMetadataHash() {
-      return metadataHash;
+    public Object getMetadata() {
+      return metadata;
     }
 
     @Override
@@ -647,13 +599,13 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
         return false;
       }
       return this.path.equals(((SymlinkToDirectory) obj).path)
-          && Objects.equal(this.metadataHash, ((SymlinkToDirectory) obj).metadataHash)
+          && this.metadata.equals(((SymlinkToDirectory) obj).metadata)
           && this.symlink.equals(((SymlinkToDirectory) obj).symlink);
     }
 
     @Override
     public int hashCode() {
-      return Objects.hashCode(path, metadataHash, symlink);
+      return Objects.hashCode(path, metadata, symlink);
     }
 
     @Override
@@ -672,11 +624,6 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
 
     @Override
-    public FileArtifactValue getValueForDerivedArtifacts() {
-      return fileArtifactValue;
-    }
-
-    @Override
     public PathFragment getTargetInSymlinkTree(boolean followSymlinks) {
       return followSymlinks ? path.asPath().asFragment() : symlink.unresolvedLinkTarget;
     }
@@ -685,9 +632,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
   static final class ResolvedFileFactory {
     private ResolvedFileFactory() {}
 
-    public static ResolvedFile regularFile(
-        RootedPath path, FileStateValue metadata, FileArtifactValue fileArtifactValue) {
-      return new RegularFile(path, metadata, fileArtifactValue);
+    public static ResolvedFile regularFile(RootedPath path, Object metadata) {
+      return new RegularFile(path, metadata);
     }
 
     public static ResolvedFile directory(RootedPath path) {
@@ -698,28 +644,18 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
         RootedPath targetPath,
         RootedPath linkNamePath,
         PathFragment linkTargetPath,
-        FileStateValue metadata,
-        FileArtifactValue fileArtifactValue) {
-      return new SymlinkToFile(
-          targetPath, linkNamePath, linkTargetPath, metadata, fileArtifactValue);
+        Object metadata) {
+      return new SymlinkToFile(targetPath, linkNamePath, linkTargetPath, metadata);
     }
 
     public static ResolvedFile symlinkToDirectory(
-        RootedPath targetPath,
-        RootedPath linkNamePath,
-        PathFragment linkValue,
-        int metadataHash,
-        FileArtifactValue fileArtifactValue) {
-      return new SymlinkToDirectory(
-          targetPath, linkNamePath, linkValue, metadataHash, fileArtifactValue);
+        RootedPath targetPath, RootedPath linkNamePath, PathFragment linkValue, Object metadata) {
+      return new SymlinkToDirectory(targetPath, linkNamePath, linkValue, metadata);
     }
 
     public static ResolvedFile danglingSymlink(
-        RootedPath linkNamePath,
-        PathFragment linkValue,
-        FileStateValue metadata,
-        FileArtifactValue fileArtifactValue) {
-      return new DanglingSymlink(linkNamePath, linkValue, metadata, fileArtifactValue);
+        RootedPath linkNamePath, PathFragment linkValue, Object metadata) {
+      return new DanglingSymlink(linkNamePath, linkValue, metadata);
     }
   }
 
@@ -747,6 +683,8 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     }
   }
 
+  private static final Integer EMPTY_METADATA = new Integer(0);
+
   /**
    * Path and type information about a single file or symlink.
    *
@@ -766,15 +704,11 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
     RootedPath getPath();
 
     /**
-     * Hash code of associated metadata.
-     *
-     * <p>This is usually some hash of the {@link FileStateValue} of the underlying filesystem
-     * entity.
-     *
-     * <p>If tests stripped the metadata or the {@link ResolvedFile} was created by the
-     * {@link ResolvedFileFactoryForTesting}, this method returns 0.
+     * Return the best effort metadata about the target. Currently this will be a FileStateValue for
+     * source targets. For generated targets we try to return a FileArtifactValue when possible, or
+     * else this will be a Integer hashcode of the target.
      */
-    int getMetadataHash();
+    Object getMetadata();
 
     /**
      * Returns the path of the Fileset-output symlink relative to the output directory.
@@ -792,15 +726,12 @@ public final class RecursiveFilesystemTraversalValue implements SkyValue {
      */
     PathFragment getTargetInSymlinkTree(boolean followSymlinks) throws DanglingSymlinkException;
 
-    @Nullable
-    FileArtifactValue getValueForDerivedArtifacts();
-
     /**
-     * Returns a copy of this object with the metadata stripped away.
+     * Returns a copy of this object with the metadataHash stripped away.
      *
-     * <p>This method should only be used by tests that wish to assert that this
-     * {@link ResolvedFile} refers to the expected absolute path and has the expected type, without
-     * asserting its actual contents (which the metadata is a function of).
+     * <p>This method should only be used by tests that wish to assert that this {@link
+     * ResolvedFile} refers to the expected absolute path and has the expected type, without
+     * asserting its actual contents (which the metadataHash is a function of).
      */
     @VisibleForTesting
     ResolvedFile stripMetadataForTesting();