1 files changed, 18 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index f91c5cde5a..a974646d55 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -45,7 +45,6 @@ import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
@@ -108,8 +107,7 @@ public class Artifact
         ActionInput,
         FileApi,
         Comparable<Object>,
-        CommandLineItem,
-        SkyKey {
+        CommandLineItem {
 
   /** Compares artifact according to their exec paths. Sorts null values first. */
   @SuppressWarnings("ReferenceEquality")  // "a == b" is an optimization
@@ -228,9 +226,6 @@ public class Artifact
       throw new IllegalArgumentException(
           "it is illegal to create an artifact with an empty execPath");
     }
-    // The ArtifactOwner is not part of this computation because it is very rare that two Artifacts
-    // have the same execPath and different owners, so a collision is fine there. If this is
-    // changed, OwnerlessArtifactWrapper must also be changed.
     this.hashCode = execPath.hashCode() + this.getClass().hashCode() * 13;
     this.root = root;
     this.execPath = execPath;
@@ -458,15 +453,6 @@ public class Artifact
     public SourceArtifact asSourceArtifact() {
       return this;
     }
-
-    /**
-     * SourceArtifacts are compared without their owners, since owners do not affect behavior,
-     * unlike with derived artifacts, whose owners determine their generating actions.
-     */
-    @Override
-    public boolean equals(Object other) {
-      return other instanceof SourceArtifact && equalsWithoutOwner((SourceArtifact) other);
-    }
   }
 
   /**
@@ -649,26 +635,36 @@ public class Artifact
   }
 
   @Override
-  public boolean equals(Object other) {
+  public final boolean equals(Object other) {
     if (!(other instanceof Artifact)) {
       return false;
     }
     if (!getClass().equals(other.getClass())) {
       return false;
     }
+    // We don't bother to check root in the equivalence relation, because we
+    // assume that no root is an ancestor of another one.
     Artifact that = (Artifact) other;
-    return equalsWithoutOwner(that) && owner.equals(that.getArtifactOwner());
+    return Objects.equals(this.execPath, that.execPath) && Objects.equals(this.root, that.root);
   }
 
-  public boolean equalsWithoutOwner(Artifact other) {
-    return Objects.equals(this.execPath, other.execPath) && Objects.equals(this.root, other.root);
+  /**
+   * Compare equality including Artifact owner equality, a notable difference compared to the
+   * {@link #equals(Object)} method of {@link Artifact}.
+   */
+  public static boolean equalWithOwner(@Nullable Artifact first, @Nullable Artifact second) {
+    if (first != null) {
+      return first.equals(second) && first.getArtifactOwner().equals(second.getArtifactOwner());
+    } else {
+      return second == null;
+    }
   }
 
   @Override
   public final int hashCode() {
-    // This is just execPath.hashCode() (along with the class). We cache a copy in the Artifact
-    // object to reduce LLC misses during operations which build a HashSet out of many Artifacts.
-    // This is a slight loss for memory but saves ~1% overall CPU in some real builds.
+    // This is just path.hashCode().  We cache a copy in the Artifact object to reduce LLC misses
+    // during operations which build a HashSet out of many Artifacts.  This is a slight loss for
+    // memory but saves ~1% overall CPU in some real builds.
     return hashCode;
   }
 
@@ -976,33 +972,4 @@ public class Artifact
       printer.append("<generated file " + rootRelativePath + ">");
     }
   }
-
-  /**
-   * A utility class that compares {@link Artifact}s without taking their owners into account.
-   * Should only be used for detecting action conflicts and merging shared action data.
-   */
-  public static class OwnerlessArtifactWrapper {
-    private final Artifact artifact;
-
-    public OwnerlessArtifactWrapper(Artifact artifact) {
-      this.artifact = artifact;
-    }
-
-    @Override
-    public int hashCode() {
-      // Depends on the fact that Artifact#hashCode does not use ArtifactOwner.
-      return artifact.hashCode();
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      return obj instanceof OwnerlessArtifactWrapper
-          && this.artifact.equalsWithoutOwner(((OwnerlessArtifactWrapper) obj).artifact);
-    }
-  }
-
-  @Override
-  public SkyFunctionName functionName() {
-    return ARTIFACT;
-  }
 }