Allow FilesystemValueChecker to operate on a WalkableGraph and add TODOs for replacing MemoizingEvaluator#getValues et al with WalkableGraph usage. I initially attempted to do this but punted once I realized how much work it would be.
Also make DirectoryListingStateValue and FileStateValue public for use in outside callers of FilesystemValueChecker.
--
MOS_MIGRATED_REVID=107447425
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateValue.java
index 15abaf4..7a422e5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateValue.java
@@ -39,7 +39,7 @@
*
* <p>This class is an implementation detail of {@link DirectoryListingValue}.
*/
-final class DirectoryListingStateValue implements SkyValue {
+public final class DirectoryListingStateValue implements SkyValue {
private final CompactSortedDirents compactSortedDirents;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
index b774c58..28d4808 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
@@ -58,7 +58,8 @@
public static final NonexistentFileStateValue NONEXISTENT_FILE_STATE_NODE =
new NonexistentFileStateValue();
- enum Type {
+ /** Type of a path. */
+ public enum Type {
REGULAR_FILE,
SPECIAL_FILE,
DIRECTORY,
@@ -69,7 +70,7 @@
protected FileStateValue() {
}
- static FileStateValue create(RootedPath rootedPath,
+ public static FileStateValue create(RootedPath rootedPath,
@Nullable TimestampGranularityMonitor tsgm) throws InconsistentFilesystemException,
IOException {
Path path = rootedPath.asPath();
@@ -105,7 +106,7 @@
return new SkyKey(SkyFunctions.FILE_STATE, rootedPath);
}
- abstract Type getType();
+ public abstract Type getType();
PathFragment getSymlinkTarget() {
throw new IllegalStateException();
@@ -193,7 +194,7 @@
}
@Override
- Type getType() {
+ public Type getType() {
return Type.REGULAR_FILE;
}
@@ -261,7 +262,7 @@
}
@Override
- Type getType() {
+ public Type getType() {
return Type.SPECIAL_FILE;
}
@@ -307,7 +308,7 @@
}
@Override
- Type getType() {
+ public Type getType() {
return Type.DIRECTORY;
}
@@ -338,7 +339,7 @@
}
@Override
- Type getType() {
+ public Type getType() {
return Type.SYMLINK;
}
@@ -374,7 +375,7 @@
}
@Override
- Type getType() {
+ public Type getType() {
return Type.NONEXISTENT;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index 4c7efc3..0e8a5cf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -15,8 +15,6 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
-import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Range;
@@ -35,15 +33,16 @@
import com.google.devtools.build.lib.vfs.BatchStat;
import com.google.devtools.build.lib.vfs.FileStatusWithDigest;
import com.google.devtools.build.skyframe.Differencer;
-import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.WalkableGraph;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -61,7 +60,7 @@
* A helper class to find dirty values by accessing the filesystem directly (contrast with
* {@link DiffAwareness}).
*/
-class FilesystemValueChecker {
+public class FilesystemValueChecker {
private static final int DIRTINESS_CHECK_THREADS = 200;
private static final Logger LOG = Logger.getLogger(FilesystemValueChecker.class.getName());
@@ -72,70 +71,102 @@
private final TimestampGranularityMonitor tsgm;
@Nullable
private final Range<Long> lastExecutionTimeRange;
- private final Supplier<Map<SkyKey, SkyValue>> valuesSupplier;
private AtomicInteger modifiedOutputFilesCounter = new AtomicInteger(0);
private AtomicInteger modifiedOutputFilesIntraBuildCounter = new AtomicInteger(0);
- FilesystemValueChecker(Supplier<Map<SkyKey, SkyValue>> valuesSupplier,
- @Nullable TimestampGranularityMonitor tsgm, @Nullable Range<Long> lastExecutionTimeRange) {
- this.valuesSupplier = valuesSupplier;
+ public FilesystemValueChecker(@Nullable TimestampGranularityMonitor tsgm,
+ @Nullable Range<Long> lastExecutionTimeRange) {
this.tsgm = tsgm;
this.lastExecutionTimeRange = lastExecutionTimeRange;
}
- FilesystemValueChecker(final MemoizingEvaluator evaluator,
- @Nullable TimestampGranularityMonitor tsgm, @Nullable Range<Long> lastExecutionTimeRange) {
- this.tsgm = tsgm;
- this.lastExecutionTimeRange = lastExecutionTimeRange;
-
- // Construct the full map view of the entire graph at most once ("memoized"), lazily. If
- // getDirtyFilesystemValues(Iterable<SkyKey>) is called on an empty Iterable, we avoid having
- // to create the Map of value keys to values. This is useful in the case where the graph
- // getValues() method could be slow.
- this.valuesSupplier = Suppliers.memoize(new Supplier<Map<SkyKey, SkyValue>>() {
- @Override
- public Map<SkyKey, SkyValue> get() {
- return evaluator.getValues();
- }
- });
- }
-
/**
- * Returns a {@link Differencer.DiffWithDelta} containing keys from the backing graph (of the
- * {@link MemoizingEvaluator} given at construction time) that are dirty according to the
- * passed-in {@code dirtinessChecker}.
+ * Returns a {@link Differencer.DiffWithDelta} containing keys from the give map that are dirty
+ * according to the passed-in {@code dirtinessChecker}.
*/
- Differencer.DiffWithDelta getDirtyKeys(SkyValueDirtinessChecker dirtinessChecker)
- throws InterruptedException {
- return getDirtyValues(valuesSupplier.get().keySet(), dirtinessChecker,
- /*checkMissingValues=*/false);
+ // TODO(bazel-team): Refactor these methods so that FilesystemValueChecker only operates on a
+ // WalkableGraph.
+ Differencer.DiffWithDelta getDirtyKeys(Map<SkyKey, SkyValue> valuesMap,
+ SkyValueDirtinessChecker dirtinessChecker) throws InterruptedException {
+ return getDirtyValues(new MapBackedValueFetcher(valuesMap), valuesMap.keySet(),
+ dirtinessChecker, /*checkMissingValues=*/false);
}
/**
* Returns a {@link Differencer.DiffWithDelta} containing keys that are dirty according to the
* passed-in {@code dirtinessChecker}.
*/
- Differencer.DiffWithDelta getNewAndOldValues(Iterable<SkyKey> keys,
- SkyValueDirtinessChecker dirtinessChecker) throws InterruptedException {
- return getDirtyValues(keys, dirtinessChecker, /*checkMissingValues=*/true);
+ public Differencer.DiffWithDelta getNewAndOldValues(Map<SkyKey, SkyValue> valuesMap,
+ Iterable<SkyKey> keys, SkyValueDirtinessChecker dirtinessChecker)
+ throws InterruptedException {
+ return getDirtyValues(new MapBackedValueFetcher(valuesMap), keys,
+ dirtinessChecker, /*checkMissingValues=*/true);
+ }
+
+ /**
+ * Returns a {@link Differencer.DiffWithDelta} containing keys that are dirty according to the
+ * passed-in {@code dirtinessChecker}.
+ */
+ public Differencer.DiffWithDelta getNewAndOldValues(WalkableGraph walkableGraph,
+ Iterable<SkyKey> keys, SkyValueDirtinessChecker dirtinessChecker)
+ throws InterruptedException {
+ return getDirtyValues(new WalkableGraphBackedValueFetcher(walkableGraph), keys,
+ dirtinessChecker, /*checkMissingValues=*/true);
+ }
+
+ private static interface ValueFetcher {
+ @Nullable
+ SkyValue get(SkyKey key);
+ }
+
+ private static class WalkableGraphBackedValueFetcher implements ValueFetcher {
+ private final WalkableGraph walkableGraph;
+
+ private WalkableGraphBackedValueFetcher(WalkableGraph walkableGraph) {
+ this.walkableGraph = walkableGraph;
+ }
+
+ @Override
+ @Nullable
+ public SkyValue get(SkyKey key) {
+ return walkableGraph.exists(key) ? walkableGraph.getValue(key) : null;
+ }
+ }
+
+ private static class MapBackedValueFetcher implements ValueFetcher {
+ private final Map<SkyKey, SkyValue> valuesMap;
+
+ private MapBackedValueFetcher(Map<SkyKey, SkyValue> valuesMap) {
+ this.valuesMap = valuesMap;
+ }
+
+ @Override
+ @Nullable
+ public SkyValue get(SkyKey key) {
+ return valuesMap.get(key);
+ }
}
/**
* Return a collection of action values which have output files that are not in-sync with
* the on-disk file value (were modified externally).
*/
- Collection<SkyKey> getDirtyActionValues(@Nullable final BatchStat batchStatter)
- throws InterruptedException {
+ Collection<SkyKey> getDirtyActionValues(Map<SkyKey, SkyValue> valuesMap,
+ @Nullable final BatchStat batchStatter) throws InterruptedException {
// CPU-bound (usually) stat() calls, plus a fudge factor.
LOG.info("Accumulating dirty actions");
final int numOutputJobs = Runtime.getRuntime().availableProcessors() * 4;
- final Set<SkyKey> actionSkyKeys =
- Sets.filter(valuesSupplier.get().keySet(), ACTION_FILTER);
+ final Set<SkyKey> actionSkyKeys = new HashSet<>();
+ for (SkyKey key : valuesMap.keySet()) {
+ if (ACTION_FILTER.apply(key)) {
+ actionSkyKeys.add(key);
+ }
+ }
final Sharder<Pair<SkyKey, ActionExecutionValue>> outputShards =
new Sharder<>(numOutputJobs, actionSkyKeys.size());
for (SkyKey key : actionSkyKeys) {
- outputShards.add(Pair.of(key, (ActionExecutionValue) valuesSupplier.get().get(key)));
+ outputShards.add(Pair.of(key, (ActionExecutionValue) valuesMap.get(key)));
}
LOG.info("Sharded action values for batching");
@@ -285,8 +316,8 @@
return isDirty;
}
- private BatchDirtyResult getDirtyValues(
- Iterable<SkyKey> values, final SkyValueDirtinessChecker checker,
+ private BatchDirtyResult getDirtyValues(ValueFetcher fetcher,
+ Iterable<SkyKey> keys, final SkyValueDirtinessChecker checker,
final boolean checkMissingValues) throws InterruptedException {
ExecutorService executor =
Executors.newFixedThreadPool(
@@ -310,23 +341,24 @@
}
};
try (AutoProfiler prof = AutoProfiler.create(elapsedTimeReceiver)) {
- for (final SkyKey key : values) {
+ for (final SkyKey key : keys) {
numKeysScanned.incrementAndGet();
if (!checker.applies(key)) {
continue;
}
- final SkyValue value = valuesSupplier.get().get(key);
+ final SkyValue value = fetcher.get(key);
+ if (!checkMissingValues && value == null) {
+ continue;
+ }
executor.execute(
wrapper.wrap(
new Runnable() {
@Override
public void run() {
- if (value != null || checkMissingValues) {
- numKeysChecked.incrementAndGet();
- DirtyResult result = checker.check(key, value, tsgm);
- if (result.isDirty()) {
- batchResult.add(key, value, result.getNewValue());
- }
+ numKeysChecked.incrementAndGet();
+ DirtyResult result = checker.check(key, value, tsgm);
+ if (result.isDirty()) {
+ batchResult.add(key, value, result.getNewValue());
}
}
}));
@@ -342,7 +374,7 @@
}
/**
- * Result of a batch call to {@link SkyValueDirtinessChecker#maybeCheck}. Partitions the dirty
+ * Result of a batch call to {@link SkyValueDirtinessChecker#check}. Partitions the dirty
* values based on whether we have a new value available for them or not.
*/
private static class BatchDirtyResult implements Differencer.DiffWithDelta {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 7a29573..c4b11d2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -341,7 +341,7 @@
buildDriver.evaluate(ImmutableList.<SkyKey>of(), false,
DEFAULT_THREAD_COUNT, eventHandler);
- FilesystemValueChecker fsvc = new FilesystemValueChecker(memoizingEvaluator, tsgm, null);
+ FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, null);
// We need to manually check for changes to known files. This entails finding all dirty file
// system values under package roots for which we don't have diff information. If at least
// one path entry doesn't have diff information, then we're going to have to iterate over
@@ -353,6 +353,7 @@
}
Differencer.Diff diff =
fsvc.getDirtyKeys(
+ memoizingEvaluator.getValues(),
new UnionDirtinessChecker(
Iterables.concat(
customDirtinessCheckers,
@@ -444,7 +445,7 @@
}
Differencer.Diff diff;
if (modifiedFileSet.treatEverythingAsModified()) {
- diff = new FilesystemValueChecker(memoizingEvaluator, tsgm, null).getDirtyKeys(
+ diff = new FilesystemValueChecker(tsgm, null).getDirtyKeys(memoizingEvaluator.getValues(),
new BasicFilesystemDirtinessChecker());
} else {
diff = getDiff(modifiedFileSet.modifiedSourceFiles(), pathEntry);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 28ac669..76ad0e7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -22,7 +22,6 @@
import com.google.common.base.Predicates;
import com.google.common.base.Stopwatch;
import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
@@ -779,6 +778,9 @@
protected Differencer.Diff getDiff(Iterable<PathFragment> modifiedSourceFiles,
final Path pathEntry) throws InterruptedException {
+ if (Iterables.isEmpty(modifiedSourceFiles)) {
+ return new ImmutableDiff(ImmutableList.<SkyKey>of(), ImmutableMap.<SkyKey, SkyValue>of());
+ }
// TODO(bazel-team): change ModifiedFileSet to work with RootedPaths instead of PathFragments.
Iterable<SkyKey> dirtyFileStateSkyKeys = Iterables.transform(modifiedSourceFiles,
new Function<PathFragment, SkyKey>() {
@@ -794,16 +796,10 @@
// information here, so we compute it ourselves.
// TODO(bazel-team): Fancy filesystems could provide it with a hypothetically modified
// DiffAwareness interface.
- Supplier<Map<SkyKey, SkyValue>> valuesSupplier = Suppliers.memoize(
- new Supplier<Map<SkyKey, SkyValue>>() {
- @Override
- public Map<SkyKey, SkyValue> get() {
- return memoizingEvaluator.getValues();
- }
- });
- FilesystemValueChecker fsvc = new FilesystemValueChecker(valuesSupplier, tsgm, null);
+ FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, null);
+ Map<SkyKey, SkyValue> valuesMap = memoizingEvaluator.getValues();
Differencer.DiffWithDelta diff =
- fsvc.getNewAndOldValues(dirtyFileStateSkyKeys, new FileDirtinessChecker());
+ fsvc.getNewAndOldValues(valuesMap, dirtyFileStateSkyKeys, new FileDirtinessChecker());
Set<SkyKey> valuesToInvalidate = new HashSet<>();
Map<SkyKey, SkyValue> valuesToInject = new HashMap<>();
@@ -830,7 +826,7 @@
changedType = !oldValue.getType().equals(newValue.getType());
} else {
DirectoryListingStateValue oldDirListingStateValue =
- (DirectoryListingStateValue) valuesSupplier.get().get(dirListingStateKey);
+ (DirectoryListingStateValue) valuesMap.get(dirListingStateKey);
if (oldDirListingStateValue != null) {
String baseName = rootedPath.getRelativePath().getBaseName();
Dirent oldDirent = oldDirListingStateValue.getDirents().maybeGetDirent(baseName);
@@ -858,14 +854,6 @@
return DirectoryListingStateValue.key(parentDirRootedPath);
}
- protected static SkyKey createFileStateKey(RootedPath rootedPath) {
- return FileStateValue.key(rootedPath);
- }
-
- protected static SkyKey createDirectoryListingStateKey(RootedPath rootedPath) {
- return DirectoryListingStateValue.key(rootedPath);
- }
-
/**
* Sets the packages that should be treated as deleted and ignored.
*/
@@ -1653,9 +1641,9 @@
if (checkOutputFiles) {
// Detect external modifications in the output tree.
- FilesystemValueChecker fsvc = new FilesystemValueChecker(memoizingEvaluator, tsgm,
- lastExecutionTimeRange);
- invalidateDirtyActions(fsvc.getDirtyActionValues(batchStatter));
+ FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, lastExecutionTimeRange);
+ invalidateDirtyActions(fsvc.getDirtyActionValues(memoizingEvaluator.getValues(),
+ batchStatter));
modifiedFiles += fsvc.getNumberOfModifiedOutputFiles();
outputDirtyFiles += fsvc.getNumberOfModifiedOutputFiles();
modifiedFilesDuringPreviousBuild += fsvc.getNumberOfModifiedOutputFilesDuringPreviousBuild();
diff --git a/src/main/java/com/google/devtools/build/skyframe/Differencer.java b/src/main/java/com/google/devtools/build/skyframe/Differencer.java
index f30558c..ddcb34f 100644
--- a/src/main/java/com/google/devtools/build/skyframe/Differencer.java
+++ b/src/main/java/com/google/devtools/build/skyframe/Differencer.java
@@ -92,5 +92,6 @@
/**
* Returns the value keys that have changed between the two Versions.
*/
- Diff getDiff(Version fromVersion, Version toVersion) throws InterruptedException;
+ Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVersion)
+ throws InterruptedException;
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
index e35cc54..a16ff87 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -153,7 +153,8 @@
// It clears the internal data structures after getDiff is called and will not return
// diffs for historical versions. This makes the following code sensitive to interrupts.
// Ideally we would simply not update lastGraphVersion if an interrupt occurs.
- Diff diff = differencer.getDiff(lastGraphVersion, version);
+ Diff diff = differencer.getDiff(new DelegatingWalkableGraph(graph), lastGraphVersion,
+ version);
valuesToInject.putAll(diff.changedKeysWithNewValues());
invalidate(diff.changedKeysWithoutNewValues());
pruneInjectedValues(valuesToInject);
diff --git a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
index e54dc8d..f00d88f 100644
--- a/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/MemoizingEvaluator.java
@@ -86,9 +86,11 @@
*
* <p>The returned map may be a live view of the graph.
*/
+ // TODO(bazel-team): Replace all usages of getValues, getDoneValues, getExistingValueForTesting,
+ // and getExistingErrorForTesting with usages of WalkableGraph. Changing the getValues usages
+ // require some care because getValues gives access to the previous value for changed/dirty nodes.
Map<SkyKey, SkyValue> getValues();
-
/**
* Returns the done (without error) values in the graph.
*
diff --git a/src/main/java/com/google/devtools/build/skyframe/RecordingDifferencer.java b/src/main/java/com/google/devtools/build/skyframe/RecordingDifferencer.java
index 1582e52..4d25e0e6 100644
--- a/src/main/java/com/google/devtools/build/skyframe/RecordingDifferencer.java
+++ b/src/main/java/com/google/devtools/build/skyframe/RecordingDifferencer.java
@@ -41,7 +41,7 @@
}
@Override
- public Diff getDiff(Version fromVersion, Version toVersion) {
+ public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVersion) {
Diff diff = new ImmutableDiff(valuesToInvalidate, valuesToInject);
clear();
return diff;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 430c5bf..2aed79a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -110,17 +110,17 @@
@Test
public void testEmpty() throws Exception {
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
}
@Test
public void testSimple() throws Exception {
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
Path path = fs.getPath("/foo");
FileSystemUtils.createEmptyFile(path);
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
SkyKey skyKey =
FileStateValue.key(RootedPath.toRootedPath(fs.getRootDirectory(), new PathFragment("foo")));
@@ -132,13 +132,13 @@
NullEventHandler.INSTANCE);
assertFalse(result.hasError());
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
FileSystemUtils.writeContentAsLatin1(path, "hello");
- assertDiffWithNewValues(getDirtyFilesystemKeys(checker), skyKey);
+ assertDiffWithNewValues(getDirtyFilesystemKeys(evaluator, checker), skyKey);
// The dirty bits are not reset until the FileValues are actually revalidated.
- assertDiffWithNewValues(getDirtyFilesystemKeys(checker), skyKey);
+ assertDiffWithNewValues(getDirtyFilesystemKeys(evaluator, checker), skyKey);
differencer.invalidate(ImmutableList.of(skyKey));
result =
@@ -148,7 +148,7 @@
SkyframeExecutor.DEFAULT_THREAD_COUNT,
NullEventHandler.INSTANCE);
assertFalse(result.hasError());
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
}
/**
@@ -160,7 +160,7 @@
*/
@Test
public void testDirtySymlink() throws Exception {
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
Path path = fs.getPath("/foo");
FileSystemUtils.writeContentAsLatin1(path, "foo contents");
@@ -194,12 +194,12 @@
assertTrue(symlinkValue.toString(), symlinkValue.isSymlink());
// Digest is not always available, so use size as a proxy for contents.
assertEquals(fooValue.getSize(), symlinkValue.getSize());
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
// Before second build, move sym1 to point to sym2.
assertTrue(sym1.delete());
FileSystemUtils.ensureSymbolicLink(sym1, sym2);
- assertDiffWithNewValues(getDirtyFilesystemKeys(checker), sym1FileStateKey);
+ assertDiffWithNewValues(getDirtyFilesystemKeys(evaluator, checker), sym1FileStateKey);
differencer.invalidate(ImmutableList.of(sym1FileStateKey));
result =
@@ -209,7 +209,7 @@
SkyframeExecutor.DEFAULT_THREAD_COUNT,
NullEventHandler.INSTANCE);
assertFalse(result.hasError());
- assertDiffWithNewValues(getDirtyFilesystemKeys(checker), sym1FileStateKey);
+ assertDiffWithNewValues(getDirtyFilesystemKeys(evaluator, checker), sym1FileStateKey);
// Before third build, move sym1 back to original (so change pruning will prevent signaling of
// its parents, but change symlink for real.
@@ -217,7 +217,7 @@
FileSystemUtils.ensureSymbolicLink(sym1, path);
assertTrue(symlink.delete());
FileSystemUtils.writeContentAsLatin1(symlink, "new symlink contents");
- assertDiffWithNewValues(getDirtyFilesystemKeys(checker), symlinkFileStateKey);
+ assertDiffWithNewValues(getDirtyFilesystemKeys(evaluator, checker), symlinkFileStateKey);
differencer.invalidate(ImmutableList.of(symlinkFileStateKey));
result =
driver.evaluate(
@@ -227,18 +227,18 @@
assertFalse(symlinkValue.toString(), symlinkValue.isSymlink());
assertEquals(fooValue, result.get(fooKey));
assertThat(symlinkValue.getSize()).isNotEqualTo(fooValue.getSize());
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
}
@Test
public void testExplicitFiles() throws Exception {
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
Path path1 = fs.getPath("/foo1");
Path path2 = fs.getPath("/foo2");
FileSystemUtils.createEmptyFile(path1);
FileSystemUtils.createEmptyFile(path2);
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
SkyKey key1 =
FileStateValue.key(
@@ -252,20 +252,20 @@
skyKeys, false, SkyframeExecutor.DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
assertFalse(result.hasError());
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
FileSystemUtils.writeContentAsLatin1(path1, "hello1");
FileSystemUtils.writeContentAsLatin1(path1, "hello2");
path1.setLastModifiedTime(27);
path2.setLastModifiedTime(42);
- assertDiffWithNewValues(getDirtyFilesystemKeys(checker), key1, key2);
+ assertDiffWithNewValues(getDirtyFilesystemKeys(evaluator, checker), key1, key2);
differencer.invalidate(skyKeys);
result =
driver.evaluate(
skyKeys, false, SkyframeExecutor.DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
assertFalse(result.hasError());
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
}
@Test
@@ -285,8 +285,8 @@
assertTrue(result.hasError());
fs.readlinkThrowsIoException = false;
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
- Diff diff = getDirtyFilesystemKeys(checker);
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
+ Diff diff = getDirtyFilesystemKeys(evaluator, checker);
assertThat(diff.changedKeysWithoutNewValues()).isEmpty();
assertThat(diff.changedKeysWithNewValues()).isEmpty();
}
@@ -309,8 +309,8 @@
NullEventHandler.INSTANCE);
assertTrue(result.hasError());
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
- Diff diff = getDirtyFilesystemKeys(checker);
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
+ Diff diff = getDirtyFilesystemKeys(evaluator, checker);
assertThat(diff.changedKeysWithoutNewValues()).isEmpty();
assertThat(diff.changedKeysWithNewValues()).isEmpty();
}
@@ -336,14 +336,15 @@
driver
.evaluate(ImmutableList.<SkyKey>of(), false, 1, NullEventHandler.INSTANCE)
.hasError());
- assertThat(new FilesystemValueChecker(evaluator, tsgm, null).getDirtyActionValues(batchStatter))
- .isEmpty();
+ assertThat(new FilesystemValueChecker(tsgm, null).getDirtyActionValues(evaluator.getValues(),
+ batchStatter)).isEmpty();
FileSystemUtils.writeContentAsLatin1(out1.getPath(), "goodbye");
assertEquals(
ActionExecutionValue.key(action1),
Iterables.getOnlyElement(
- new FilesystemValueChecker(evaluator, tsgm, null).getDirtyActionValues(batchStatter)));
+ new FilesystemValueChecker(tsgm, null).getDirtyActionValues(evaluator.getValues(),
+ batchStatter)));
}
private Artifact createDerivedArtifact(String relPath) throws IOException {
@@ -432,13 +433,13 @@
ImmutableList.of(FileValue.key(RootedPath.toRootedPath(pkgRoot, new PathFragment("foo"))));
driver.evaluate(
values, false, SkyframeExecutor.DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
- FilesystemValueChecker checker = new FilesystemValueChecker(evaluator, tsgm, null);
+ FilesystemValueChecker checker = new FilesystemValueChecker(tsgm, null);
- assertEmptyDiff(getDirtyFilesystemKeys(checker));
+ assertEmptyDiff(getDirtyFilesystemKeys(evaluator, checker));
fs.statThrowsRuntimeException = true;
try {
- getDirtyFilesystemKeys(checker);
+ getDirtyFilesystemKeys(evaluator, checker);
fail();
} catch (RuntimeException e) {
assertThat(e).hasMessage("bork");
@@ -531,8 +532,8 @@
};
}
- private static Diff getDirtyFilesystemKeys(FilesystemValueChecker checker)
- throws InterruptedException {
- return checker.getDirtyKeys(new BasicFilesystemDirtinessChecker());
+ private static Diff getDirtyFilesystemKeys(MemoizingEvaluator evaluator,
+ FilesystemValueChecker checker) throws InterruptedException {
+ return checker.getDirtyKeys(evaluator.getValues(), new BasicFilesystemDirtinessChecker());
}
}
