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
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 c35e769..27c122e 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 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 @@
+ 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 160d4a5..58301b7 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.PathFragment;
import java.io.Serializable;
+import java.util.Objects;
/**
* A Location is a range of characters within a file.
@@ -52,6 +53,22 @@
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 @@
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 @@
@Override
public int hashCode() {
- return line * 81 + column;
+ return line * 41 + column;
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java b/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
index d5a612e..1f2c609 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
@@ -43,6 +43,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -197,6 +198,26 @@
public LineAndColumn getEndLineAndColumn() {
return new LineAndColumn(endLine, endColumn);
}
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ path.hashCode(), startLine, startColumn, endLine, endColumn, internalHashCode());
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other == null || !other.getClass().equals(getClass())) {
+ return false;
+ }
+ ExplicitLocation that = (ExplicitLocation) other;
+ return this.startLine == that.startLine
+ && this.startColumn == that.startColumn
+ && this.endLine == that.endLine
+ && this.endColumn == that.endColumn
+ && internalEquals(that)
+ && Objects.equals(this.path, that.path);
+ }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
index c4a58a9..90b40e3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
@@ -26,6 +26,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Stack;
/**
@@ -182,6 +183,21 @@
public LineAndColumn getEndLineAndColumn() {
return lineNumberTable.getLineAndColumn(getEndOffset());
}
+
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(lineNumberTable, internalHashCode());
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other == null || !other.getClass().equals(getClass())) {
+ return false;
+ }
+ LexerLocation that = (LexerLocation) other;
+ return internalEquals(that) && Objects.equals(this.lineNumberTable, that.lineNumberTable);
+ }
}
/** invariant: symbol positions are half-open intervals. */
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java b/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java
index 3bcf0a0..ac629c7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java
@@ -25,8 +25,10 @@
import java.io.Serializable;
import java.nio.CharBuffer;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
+import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -146,6 +148,23 @@
? linestart[line + 1]
: bufferLength);
}
+
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(Arrays.hashCode(linestart), path, bufferLength);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other == null || !other.getClass().equals(getClass())) {
+ return false;
+ }
+ Regular that = (Regular) other;
+ return this.bufferLength == that.bufferLength
+ && Arrays.equals(this.linestart, that.linestart)
+ && Objects.equals(this.path, that.path);
+ }
}
/**
@@ -197,7 +216,7 @@
}
this.table = hashOrdering.immutableSortedCopy(unorderedTable);
this.bufferLength = buffer.length;
- this.defaultPath = defaultPath;
+ this.defaultPath = Preconditions.checkNotNull(defaultPath);
}
private SingleHashLine getHashLine(int offset) {
@@ -242,5 +261,21 @@
}
return Pair.of(0, 0);
}
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(table, defaultPath, bufferLength);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other == null || !other.getClass().equals(getClass())) {
+ return false;
+ }
+ HashLine that = (HashLine) other;
+ return this.bufferLength == that.bufferLength
+ && this.defaultPath.equals(that.defaultPath)
+ && this.table.equals(that.table);
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java b/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java
index b285b62..394f186 100644
--- a/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java
+++ b/src/main/java/com/google/devtools/build/skyframe/TaggedEvents.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.events.Event;
import java.io.Serializable;
+import java.util.Objects;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
@@ -60,4 +61,18 @@
public String toString() {
return tag == null ? "<unknown>" : tag + ": " + Iterables.toString(events);
}
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(tag, events);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other == null || !other.getClass().equals(getClass())) {
+ return false;
+ }
+ TaggedEvents that = (TaggedEvents) other;
+ return Objects.equals(this.tag, that.tag) && Objects.equals(this.events, that.events);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java b/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
index e153a68..3795832 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ValueWithMetadata.java
@@ -116,7 +116,7 @@
@Override
public int hashCode() {
- return 31 * value.hashCode() + transitiveEvents.hashCode();
+ return 31 * value.hashCode() + transitiveEvents.shallowHashCode();
}
@Override
