From 2b71efec6c233b937b089bd80f420491e2d52db9 Mon Sep 17 00:00:00 2001 From: Michajlo Matijkiw Date: Fri, 19 Jun 2015 19:23:16 +0000 Subject: Log SkyKeys detected changed across builds Similar to previous approach, except diff-awareness agnostic. Log SkyKeys as is for simplicity- this is all for human consumption anyway. -- MOS_MIGRATED_REVID=96428248 --- .../build/lib/skyframe/FilesystemValueChecker.java | 2 +- .../lib/skyframe/SequencedSkyframeExecutor.java | 85 +++++++++++++--------- 2 files changed, 50 insertions(+), 37 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/skyframe') 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 923cc30a4d..d2f4a069ae 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 @@ -371,7 +371,7 @@ class FilesystemValueChecker { } @Override - public Iterable changedKeysWithoutNewValues() { + public Collection changedKeysWithoutNewValues() { return concurrentDirtyKeysWithoutNewValues; } 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 970c120a8f..452fb86c4a 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 @@ -284,11 +284,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { DiffAwarenessManager.ProcessableModifiedFileSet modifiedFileSet = diffAwarenessManager.getDiff(pathEntry); if (modifiedFileSet.getModifiedFileSet().treatEverythingAsModified()) { - LOG.info("DiffAwareness treating all sources as modified for " + pathEntry); pathEntriesWithoutDiffInformation.add(Pair.of(pathEntry, modifiedFileSet)); } else { - LOG.info(diffInfoLogString(pathEntry, - modifiedFileSet.getModifiedFileSet().modifiedSourceFiles())); modifiedFilesByPathEntry.put(pathEntry, modifiedFileSet); } } @@ -296,28 +293,6 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { handleDiffsWithMissingDiffInformation(pathEntriesWithoutDiffInformation); } - private static String diffInfoLogString(Path pathEntry, - ImmutableSet modifiedFileSet) { - int numModified = modifiedFileSet.size(); - StringBuilder result = new StringBuilder("DiffAwareness found ") - .append(numModified) - .append(" modified source files for ") - .append(pathEntry.getPathString()); - - if (numModified > 0) { - Iterable trimmed = PathFragment.safePathStrings( - Iterables.limit(modifiedFileSet, 5)); - result.append(": ") - .append(Joiner.on(", ").join(trimmed)); - - if (numModified > 5) { - result.append(", ..."); - } - } - - return result.toString(); - } - /** * Invalidates files under path entries whose corresponding {@link DiffAwareness} gave an exact * diff. Removes entries from the given map as they are processed. All of the files need to be @@ -334,7 +309,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { Preconditions.checkState(!modifiedFileSet.treatEverythingAsModified(), pathEntry); Iterable dirtyValues = getSkyKeysPotentiallyAffected( modifiedFileSet.modifiedSourceFiles(), pathEntry); - handleChangedFiles(new ImmutableDiff(dirtyValues, ImmutableMap.of())); + handleChangedFiles(ImmutableList.of(pathEntry), + new ImmutableDiff(dirtyValues, ImmutableMap.of())); processableModifiedFileSet.markProcessed(); } } @@ -349,11 +325,13 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { if (pathEntriesWithoutDiffInformation.isEmpty()) { return; } + // Before running the FilesystemValueChecker, ensure that all values marked for invalidation // have actually been invalidated (recall that invalidation happens at the beginning of the // next evaluate() call), because checking those is a waste of time. buildDriver.evaluate(ImmutableList.of(), false, DEFAULT_THREAD_COUNT, reporter); + FilesystemValueChecker fsnc = new FilesystemValueChecker(memoizingEvaluator, 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 @@ -364,19 +342,24 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { // Partition by package path entry. Multimap skyKeysByPathEntry = partitionSkyKeysByPackagePathEntry( ImmutableSet.copyOf(pkgLocator.get().getPathEntries()), filesystemSkyKeys); + // Contains all file system values that we need to check for dirtiness. + List pathEntriesChecked = + Lists.newArrayListWithCapacity(pathEntriesWithoutDiffInformation.size()); List> valuesToCheckManually = Lists.newArrayList(); for (Pair pair : pathEntriesWithoutDiffInformation) { Path pathEntry = pair.getFirst(); valuesToCheckManually.add(skyKeysByPathEntry.get(pathEntry)); + pathEntriesChecked.add(pathEntry); } + Differencer.Diff diff = fsnc.getDirtyFilesystemValues(Iterables.concat(valuesToCheckManually)); - handleChangedFiles(diff); + handleChangedFiles(pathEntriesChecked, diff); + for (Pair pair : pathEntriesWithoutDiffInformation) { - DiffAwarenessManager.ProcessableModifiedFileSet processableModifiedFileSet = pair.getSecond(); - processableModifiedFileSet.markProcessed(); + pair.getSecond().markProcessed(); } } @@ -404,13 +387,43 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { return multimapBuilder.build(); } - private void handleChangedFiles(Differencer.Diff diff) { - recordingDiffer.invalidate(diff.changedKeysWithoutNewValues()); - recordingDiffer.inject(diff.changedKeysWithNewValues()); - modifiedFiles += getNumberOfModifiedFiles(diff.changedKeysWithoutNewValues()); - modifiedFiles += getNumberOfModifiedFiles(diff.changedKeysWithNewValues().keySet()); - incrementalBuildMonitor.accrue(diff.changedKeysWithoutNewValues()); - incrementalBuildMonitor.accrue(diff.changedKeysWithNewValues().keySet()); + private void handleChangedFiles(List pathEntries, Differencer.Diff diff) { + Collection changedKeysWithoutNewValues = diff.changedKeysWithoutNewValues(); + Map changedKeysWithNewValues = diff.changedKeysWithNewValues(); + + logDiffInfo(pathEntries, changedKeysWithoutNewValues, changedKeysWithNewValues); + + recordingDiffer.invalidate(changedKeysWithoutNewValues); + recordingDiffer.inject(changedKeysWithNewValues); + modifiedFiles += getNumberOfModifiedFiles(changedKeysWithoutNewValues); + modifiedFiles += getNumberOfModifiedFiles(changedKeysWithNewValues.keySet()); + incrementalBuildMonitor.accrue(changedKeysWithoutNewValues); + incrementalBuildMonitor.accrue(changedKeysWithNewValues.keySet()); + } + + private static void logDiffInfo(Iterable pathEntries, + Collection changedWithoutNewValue, + Map changedWithNewValue) { + int numModified = changedWithNewValue.size() + changedWithoutNewValue.size(); + StringBuilder result = new StringBuilder("DiffAwareness found ") + .append(numModified) + .append(" modified source files and directory listings for ") + .append(Joiner.on(", ").join(pathEntries)); + + if (numModified > 0) { + Iterable allModifiedKeys = Iterables.concat(changedWithoutNewValue, + changedWithNewValue.keySet()); + Iterable trimmed = Iterables.limit(allModifiedKeys, 5); + + result.append(": ") + .append(Joiner.on(", ").join(trimmed)); + + if (numModified > 5) { + result.append(", ..."); + } + } + + LOG.info(result.toString()); } private static int getNumberOfModifiedFiles(Iterable modifiedValues) { -- cgit v1.2.3