Make two Skyframe nodes with the same events and values equal.

We do this by implementing equality for TaggedEvents (and all objects
it transitively includes). Before this change, if a Skyframe node
re-evaluated to the same value as in the previous build, but had
(transitive) events, change pruning would not cut off the evaluation
of its parents. This is not a big issue in practice because most nodes
that would re-evaluate to the same value (like FileValues or
GlobValues) never emit events, and others (like ActionExecutionValues)
have secondary caches that mask this effect.

Also do a drive-by fix where we were using the hash code of a nested
set instead of the shallow hash code (didn't have any bad effects in
practice because we never hash these values).

(Minor formatting clean-ups from https://bazel-review.googlesource.com/1610 )

--
Change-Id: I751a8479627f0456993c5ec8834528aeb593d736
Reviewed-on: https://bazel-review.googlesource.com/1610
MOS_MIGRATED_REVID=98115908

2 files changed, 46 insertions, 1 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/events/Event.java b/src/main/java/com/google/devtools/build/lib/events/Event.java
index c35e769289..27c122e8fd 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Event.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Event.java
@@ -18,6 +18,8 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import com.google.common.base.Preconditions;
 
 import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Objects;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
@@ -116,6 +118,24 @@ public final class Event implements Serializable {
         + getMessage();
   }
 
+  @Override
+  public int hashCode() {
+    return Objects.hash(kind, location, message, tag, Arrays.hashCode(messageBytes));
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (other == null || !other.getClass().equals(getClass())) {
+      return false;
+    }
+    Event that = (Event) other;
+    return Objects.equals(this.kind, that.kind)
+        && Objects.equals(this.location, that.location)
+        && Objects.equals(this.tag, that.tag)
+        && Objects.equals(this.message, that.message)
+        && Arrays.equals(this.messageBytes, that.messageBytes);
+  }
+
   /**
    * Replay a sequence of events on an {@link EventHandler}.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/events/Location.java b/src/main/java/com/google/devtools/build/lib/events/Location.java
index 160d4a586d..58301b7ea2 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Location.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Location.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 /**
  * A Location is a range of characters within a file.
@@ -52,6 +53,22 @@ public abstract class Location implements Serializable {
     public LineAndColumn getStartLineAndColumn() {
       return startLineAndColumn;
     }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(path, startLineAndColumn, internalHashCode());
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      if (other == null || !other.getClass().equals(getClass())) {
+        return false;
+      }
+      LocationWithPathAndStartColumn that = (LocationWithPathAndStartColumn) other;
+      return internalEquals(that)
+          && Objects.equals(this.path, that.path)
+          && Objects.equals(this.startLineAndColumn, that.startLineAndColumn);
+    }
   }
 
   protected final int startOffset;
@@ -212,6 +229,14 @@ public abstract class Location implements Serializable {
     return print();
   }
 
+  protected int internalHashCode() {
+    return Objects.hash(startOffset, endOffset);
+  }
+
+  protected boolean internalEquals(Location that) {
+    return this.startOffset == that.startOffset && this.endOffset == that.endOffset;
+  }
+
   /**
    * A value class that describes the line and column of an offset in a file.
    */
@@ -247,7 +272,7 @@ public abstract class Location implements Serializable {
 
     @Override
     public int hashCode() {
-      return line * 81 + column;
+      return line * 41 + column;
     }
   }
 }