Have ExecutionFinishedEvent contain the number of source files manually checked for changes because of a lack of precise diff information.
PiperOrigin-RevId: 291789856
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExecutionFinishedEvent.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExecutionFinishedEvent.java
index 2853756..4c0b79a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExecutionFinishedEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExecutionFinishedEvent.java
@@ -13,47 +13,52 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.base.Preconditions;
+import com.google.auto.value.AutoValue;
import java.time.Duration;
/**
- * Event signaling the end of the execution phase. Contains statistics about the action cache,
- * the metadata cache and about last file save times.
+ * Event signaling the end of the execution phase. Contains statistics about the action cache, the
+ * metadata cache and about last file save times.
*/
-public class ExecutionFinishedEvent {
+@AutoValue
+public abstract class ExecutionFinishedEvent {
public static final ExecutionFinishedEvent EMPTY =
- new ExecutionFinishedEvent(0, 0, Duration.ZERO, Duration.ZERO);
+ builder()
+ .setOutputDirtyFiles(0)
+ .setOutputModifiedFilesDuringPreviousBuild(0)
+ .setSourceDiffCheckingDuration(Duration.ZERO)
+ .setNumSourceFilesCheckedBecauseOfMissingDiffs(0)
+ .setOutputTreeDiffCheckingDuration(Duration.ZERO)
+ .build();
- private final int outputDirtyFiles;
- private final int outputModifiedFilesDuringPreviousBuild;
- private final Duration sourceDiffCheckingDuration;
- private final Duration outputTreeDiffCheckingDuration;
+ public abstract int outputDirtyFiles();
- ExecutionFinishedEvent(
- int outputDirtyFiles,
- int outputModifiedFilesDuringPreviousBuild,
- Duration sourceDiffCheckingDuration,
- Duration outputTreeDiffCheckingDuration) {
- this.outputDirtyFiles = outputDirtyFiles;
- this.outputModifiedFilesDuringPreviousBuild = outputModifiedFilesDuringPreviousBuild;
- this.sourceDiffCheckingDuration = Preconditions.checkNotNull(sourceDiffCheckingDuration);
- this.outputTreeDiffCheckingDuration =
- Preconditions.checkNotNull(outputTreeDiffCheckingDuration);
+ public abstract int outputModifiedFilesDuringPreviousBuild();
+
+ public abstract Duration sourceDiffCheckingDuration();
+
+ public abstract int numSourceFilesCheckedBecauseOfMissingDiffs();
+
+ public abstract Duration outputTreeDiffCheckingDuration();
+
+ static Builder builder() {
+ return new AutoValue_ExecutionFinishedEvent.Builder();
}
- public int getOutputDirtyFiles() {
- return outputDirtyFiles;
- }
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder setOutputDirtyFiles(int outputDirtyFiles);
- public int getOutputModifiedFilesDuringPreviousBuild() {
- return outputModifiedFilesDuringPreviousBuild;
- }
+ abstract Builder setOutputModifiedFilesDuringPreviousBuild(
+ int outputModifiedFilesDuringPreviousBuild);
- public Duration getSourceDiffCheckingDuration() {
- return sourceDiffCheckingDuration;
- }
+ abstract Builder setSourceDiffCheckingDuration(Duration sourceDiffCheckingDuration);
- public Duration getOutputTreeDiffCheckingDuration() {
- return outputTreeDiffCheckingDuration;
+ abstract Builder setNumSourceFilesCheckedBecauseOfMissingDiffs(
+ int numSourceFilesCheckedBecauseOfMissingDiffs);
+
+ abstract Builder setOutputTreeDiffCheckingDuration(Duration outputTreeDiffCheckingDuration);
+
+ abstract ExecutionFinishedEvent build();
}
}
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 6c5342b..44665ee 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
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.skyframe.Differencer;
+import com.google.devtools.build.skyframe.Differencer.DiffWithDelta.Delta;
import com.google.devtools.build.skyframe.FunctionHermeticity;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
@@ -99,8 +100,9 @@
*/
// 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 {
+ ImmutableBatchDirtyResult getDirtyKeys(
+ Map<SkyKey, SkyValue> valuesMap, SkyValueDirtinessChecker dirtinessChecker)
+ throws InterruptedException {
return getDirtyValues(new MapBackedValueFetcher(valuesMap), valuesMap.keySet(),
dirtinessChecker, /*checkMissingValues=*/false);
}
@@ -109,9 +111,11 @@
* Returns a {@link Differencer.DiffWithDelta} containing keys that are dirty according to the
* passed-in {@code dirtinessChecker}.
*/
- public Differencer.DiffWithDelta getNewAndOldValues(Map<SkyKey, SkyValue> valuesMap,
- Iterable<SkyKey> keys, SkyValueDirtinessChecker dirtinessChecker)
- throws InterruptedException {
+ public ImmutableBatchDirtyResult getNewAndOldValues(
+ Map<SkyKey, SkyValue> valuesMap,
+ Iterable<SkyKey> keys,
+ SkyValueDirtinessChecker dirtinessChecker)
+ throws InterruptedException {
return getDirtyValues(new MapBackedValueFetcher(valuesMap), keys,
dirtinessChecker, /*checkMissingValues=*/true);
}
@@ -478,19 +482,22 @@
return headPath != null && headPath.startsWith(artifactExecPath);
}
- private BatchDirtyResult getDirtyValues(ValueFetcher fetcher,
- Iterable<SkyKey> keys, final SkyValueDirtinessChecker checker,
- final boolean checkMissingValues) throws InterruptedException {
+ private ImmutableBatchDirtyResult getDirtyValues(
+ ValueFetcher fetcher,
+ Iterable<SkyKey> keys,
+ final SkyValueDirtinessChecker checker,
+ final boolean checkMissingValues)
+ throws InterruptedException {
ExecutorService executor =
Executors.newFixedThreadPool(
DIRTINESS_CHECK_THREADS,
new ThreadFactoryBuilder().setNameFormat("FileSystem Value Invalidator %d").build());
- final BatchDirtyResult batchResult = new BatchDirtyResult();
ThrowableRecordingRunnableWrapper wrapper =
new ThrowableRecordingRunnableWrapper("FilesystemValueChecker#getDirtyValues");
final AtomicInteger numKeysScanned = new AtomicInteger(0);
final AtomicInteger numKeysChecked = new AtomicInteger(0);
+ MutableBatchDirtyResult batchResult = new MutableBatchDirtyResult(numKeysChecked);
ElapsedTimeReceiver elapsedTimeReceiver =
elapsedTimeNanos -> {
if (elapsedTimeNanos > 0) {
@@ -540,19 +547,57 @@
throw new InterruptedException();
}
}
- return batchResult;
+ return batchResult.toImmutable();
+ }
+
+ static class ImmutableBatchDirtyResult implements Differencer.DiffWithDelta {
+ private final Collection<SkyKey> dirtyKeysWithoutNewValues;
+ private final Map<SkyKey, Delta> dirtyKeysWithNewAndOldValues;
+ private final int numKeysChecked;
+
+ private ImmutableBatchDirtyResult(
+ Collection<SkyKey> dirtyKeysWithoutNewValues,
+ Map<SkyKey, Delta> dirtyKeysWithNewAndOldValues,
+ int numKeysChecked) {
+ this.dirtyKeysWithoutNewValues = dirtyKeysWithoutNewValues;
+ this.dirtyKeysWithNewAndOldValues = dirtyKeysWithNewAndOldValues;
+ this.numKeysChecked = numKeysChecked;
+ }
+
+ @Override
+ public Collection<SkyKey> changedKeysWithoutNewValues() {
+ return dirtyKeysWithoutNewValues;
+ }
+
+ @Override
+ public Map<SkyKey, Delta> changedKeysWithNewAndOldValues() {
+ return dirtyKeysWithNewAndOldValues;
+ }
+
+ @Override
+ public Map<SkyKey, SkyValue> changedKeysWithNewValues() {
+ return Delta.newValues(dirtyKeysWithNewAndOldValues);
+ }
+
+ int getNumKeysChecked() {
+ return numKeysChecked;
+ }
}
/**
- * 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.
+ * 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 {
-
+ private static class MutableBatchDirtyResult {
private final Set<SkyKey> concurrentDirtyKeysWithoutNewValues =
Collections.newSetFromMap(new ConcurrentHashMap<SkyKey, Boolean>());
private final ConcurrentHashMap<SkyKey, Delta> concurrentDirtyKeysWithNewAndOldValues =
new ConcurrentHashMap<>();
+ private final AtomicInteger numChecked;
+
+ private MutableBatchDirtyResult(AtomicInteger numChecked) {
+ this.numChecked = numChecked;
+ }
private void add(SkyKey key, @Nullable SkyValue oldValue, @Nullable SkyValue newValue) {
if (newValue == null) {
@@ -566,19 +611,11 @@
}
}
- @Override
- public Collection<SkyKey> changedKeysWithoutNewValues() {
- return concurrentDirtyKeysWithoutNewValues;
- }
-
- @Override
- public Map<SkyKey, Delta> changedKeysWithNewAndOldValues() {
- return concurrentDirtyKeysWithNewAndOldValues;
- }
-
- @Override
- public Map<SkyKey, SkyValue> changedKeysWithNewValues() {
- return Delta.newValues(concurrentDirtyKeysWithNewAndOldValues);
+ private ImmutableBatchDirtyResult toImmutable() {
+ return new ImmutableBatchDirtyResult(
+ concurrentDirtyKeysWithoutNewValues,
+ concurrentDirtyKeysWithNewAndOldValues,
+ numChecked.get());
}
}
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 7f2df10..bea8cce 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
@@ -71,6 +71,7 @@
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFilesKnowledge;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType;
+import com.google.devtools.build.lib.skyframe.FilesystemValueChecker.ImmutableBatchDirtyResult;
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.actiongraph.ActionGraphDump;
@@ -86,7 +87,6 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.BuildDriver;
import com.google.devtools.build.skyframe.Differencer;
-import com.google.devtools.build.skyframe.Differencer.Diff;
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.GraphInconsistencyReceiver;
import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
@@ -147,6 +147,7 @@
private int outputDirtyFiles;
private int modifiedFilesDuringPreviousBuild;
private Duration sourceDiffCheckingDuration = Duration.ofSeconds(-1L);
+ private int numSourceFilesCheckedBecauseOfMissingDiffs;
private Duration outputTreeDiffCheckingDuration = Duration.ofSeconds(-1L);
// If this is null then workspace header pre-calculation won't happen.
@@ -331,6 +332,7 @@
throws InterruptedException, AbruptExitException {
TimestampGranularityMonitor tsgm = this.tsgm.get();
modifiedFiles = 0;
+ numSourceFilesCheckedBecauseOfMissingDiffs = 0;
boolean managedDirectoriesChanged =
managedDirectoriesKnowledge != null && refreshWorkspaceHeader(eventHandler);
@@ -432,6 +434,7 @@
handleChangedFiles(
ImmutableList.of(pathEntry),
getDiff(tsgm, modifiedFileSet.modifiedSourceFiles(), pathEntry),
+ /*numSourceFilesCheckedIfDiffWasMissing=*/ 0,
managedDirectoriesChanged);
processableModifiedFileSet.markProcessed();
}
@@ -500,9 +503,9 @@
logger.info(
"About to scan skyframe graph checking for filesystem nodes of types "
+ Iterables.toString(fileTypesToCheck));
- Differencer.Diff diff;
+ ImmutableBatchDirtyResult batchDirtyResult;
try (SilentCloseable c = Profiler.instance().profile("fsvc.getDirtyKeys")) {
- diff =
+ batchDirtyResult =
fsvc.getDirtyKeys(
memoizingEvaluator.getValues(),
new UnionDirtinessChecker(
@@ -512,7 +515,11 @@
new ExternalDirtinessChecker(tmpExternalFilesHelper, fileTypesToCheck),
new MissingDiffDirtinessChecker(diffPackageRootsUnderWhichToCheck)))));
}
- handleChangedFiles(diffPackageRootsUnderWhichToCheck, diff, managedDirectoriesChanged);
+ handleChangedFiles(
+ diffPackageRootsUnderWhichToCheck,
+ batchDirtyResult,
+ /*numSourceFilesCheckedIfDiffWasMissing=*/ batchDirtyResult.getNumKeysChecked(),
+ managedDirectoriesChanged);
for (Pair<Root, DiffAwarenessManager.ProcessableModifiedFileSet> pair :
pathEntriesWithoutDiffInformation) {
@@ -527,7 +534,8 @@
private void handleChangedFiles(
Collection<Root> diffPackageRootsUnderWhichToCheck,
- Diff diff,
+ Differencer.Diff diff,
+ int numSourceFilesCheckedIfDiffWasMissing,
boolean managedDirectoriesChanged) {
int numWithoutNewValues = diff.changedKeysWithoutNewValues().size();
Iterable<SkyKey> keysToBeChangedLaterInThisBuild = diff.changedKeysWithoutNewValues();
@@ -552,6 +560,7 @@
recordingDiffer.inject(changedKeysWithNewValues);
modifiedFiles += getNumberOfModifiedFiles(keysToBeChangedLaterInThisBuild);
modifiedFiles += getNumberOfModifiedFiles(changedKeysWithNewValues.keySet());
+ numSourceFilesCheckedBecauseOfMissingDiffs += numSourceFilesCheckedIfDiffWasMissing;
incrementalBuildMonitor.accrue(keysToBeChangedLaterInThisBuild);
incrementalBuildMonitor.accrue(changedKeysWithNewValues.keySet());
}
@@ -855,11 +864,14 @@
@Override
public ExecutionFinishedEvent createExecutionFinishedEvent() {
ExecutionFinishedEvent result =
- new ExecutionFinishedEvent(
- outputDirtyFiles,
- modifiedFilesDuringPreviousBuild,
- sourceDiffCheckingDuration,
- outputTreeDiffCheckingDuration);
+ ExecutionFinishedEvent.builder()
+ .setOutputDirtyFiles(outputDirtyFiles)
+ .setOutputModifiedFilesDuringPreviousBuild(modifiedFilesDuringPreviousBuild)
+ .setSourceDiffCheckingDuration(sourceDiffCheckingDuration)
+ .setNumSourceFilesCheckedBecauseOfMissingDiffs(
+ numSourceFilesCheckedBecauseOfMissingDiffs)
+ .setOutputTreeDiffCheckingDuration(outputTreeDiffCheckingDuration)
+ .build();
outputDirtyFiles = 0;
modifiedFilesDuringPreviousBuild = 0;
sourceDiffCheckingDuration = Duration.ZERO;
