Don't check direct deps when doing change pruning. Since dependents of a node don't have access to its dependencies ("grandparents don't know grandchildren", or vice versa, depending on your point of view), changes to a node's dependencies can't affect downstream nodes.
--
MOS_MIGRATED_REVID=114143894
diff --git a/src/main/java/com/google/devtools/build/skyframe/BuildingState.java b/src/main/java/com/google/devtools/build/skyframe/BuildingState.java
index 9de91be..2804ecd 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BuildingState.java
+++ b/src/main/java/com/google/devtools/build/skyframe/BuildingState.java
@@ -300,16 +300,18 @@
}
/**
- * Returns true if {@code newValue}.equals the value from the last time this node was built, and
- * the deps requested during this evaluation are exactly those requested the last time this node
- * was built, in the same order. Should only be used by {@link NodeEntry#setValue}.
+ * Returns true if {@code newValue}.equals the value from the last time this node was built.
+ * Should only be used by {@link NodeEntry#setValue}.
+ *
+ * <p>Changes in direct deps do <i>not</i> force this to return false. Only the value is
+ * considered.
*/
boolean unchangedFromLastBuild(SkyValue newValue) {
checkFinishedBuildingWhenAboutToSetValue();
if (newValue instanceof NotComparableSkyValue) {
return false;
}
- return getLastBuildValue().equals(newValue) && lastBuildDirectDeps.equals(directDeps);
+ return getLastBuildValue().equals(newValue);
}
/**
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
index c23b822..7b885b8 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java
@@ -54,10 +54,10 @@
private SkyValue value = null;
/**
- * The last version of the graph at which this node entry was changed. In {@link #setValue} it
- * may be determined that the data being written to the graph at a given version is the same as
- * the already-stored data. In that case, the version will remain the same. The version can be
- * thought of as the latest timestamp at which this entry was changed.
+ * The last version of the graph at which this node's value was changed. In {@link #setValue} it
+ * may be determined that the value being written to the graph at a given version is the same as
+ * the already-stored value. In that case, the version will remain the same. The version can be
+ * thought of as the latest timestamp at which this value was changed.
*/
protected Version lastChangedVersion = MinimalVersion.INSTANCE;
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
index f29537d..1298c48 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.skyframe;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.skyframe.NodeEntrySubjectFactory.assertThatNodeEntry;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
@@ -573,7 +574,7 @@
}
@Test
- public void noPruneWhenDepsChange() {
+ public void pruneWhenDepsChange() {
NodeEntry entry = new InMemoryNodeEntry();
entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
SkyKey dep = key("dep");
@@ -593,7 +594,7 @@
assertTrue(entry.signalDep(IntVersion.of(1L)));
setValue(entry, new IntegerValue(5), /*errorInfo=*/null, /*graphVersion=*/1L);
assertTrue(entry.isDone());
- assertEquals("Version increments when deps change", IntVersion.of(1L), entry.getVersion());
+ assertThatNodeEntry(entry).hasVersionThat().isEqualTo(IntVersion.of(0L));
}
@Test
diff --git a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
new file mode 100644
index 0000000..537d738
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
@@ -0,0 +1,33 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.skyframe;
+
+import com.google.common.truth.DefaultSubject;
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.Subject;
+import com.google.common.truth.Truth;
+
+/**
+ * {@link Subject} for {@link NodeEntry}. Please add to this class if you need more
+ * functionality!
+ */
+public class NodeEntrySubject extends Subject<NodeEntrySubject, NodeEntry> {
+ public NodeEntrySubject(FailureStrategy failureStrategy, NodeEntry nodeEntry) {
+ super(failureStrategy, nodeEntry);
+ }
+
+ DefaultSubject hasVersionThat() {
+ return Truth.assertThat(getSubject().getVersion()).named("Version for " + getDisplaySubject());
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubjectFactory.java b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubjectFactory.java
new file mode 100644
index 0000000..7278085
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubjectFactory.java
@@ -0,0 +1,30 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.skyframe;
+
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.SubjectFactory;
+import com.google.common.truth.Truth;
+
+/** {@link SubjectFactory} for {@link NodeEntry} objects, providing {@link NodeEntrySubject}s. */
+public class NodeEntrySubjectFactory extends SubjectFactory<NodeEntrySubject, NodeEntry> {
+ public static NodeEntrySubject assertThatNodeEntry(NodeEntry nodeEntry) {
+ return Truth.assertAbout(new NodeEntrySubjectFactory()).that(nodeEntry);
+ }
+
+ @Override
+ public NodeEntrySubject getSubject(FailureStrategy failureStrategy, NodeEntry nodeEntry) {
+ return new NodeEntrySubject(failureStrategy, nodeEntry);
+ }
+}
