Track the entire OutputService instead of just the BatchStatter.
--
MOS_MIGRATED_REVID=107800790
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index bc9269b..ef5a23c 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -268,6 +268,14 @@
+ "this flag to false to see the effect on incremental build times.")
public boolean checkOutputFiles;
+ @Option(name = "experimental_output_tree_tracking",
+ defaultValue = "false",
+ category = "undocumented",
+ help = "If set, communicate with objsfd to track when files in the output tree have "
+ + "been modified externally (not by Blaze). This should improve incremental build "
+ + "speed.")
+ public boolean finalizeActions;
+
@Option(
name = "aspects",
converter = Converters.CommaSeparatedOptionListConverter.class,
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 3a95cbd..26feb53 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -84,6 +84,7 @@
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -357,15 +358,17 @@
}
OutputService outputService = env.getOutputService();
+ ModifiedFileSet modifiedOutputFiles = ModifiedFileSet.EVERYTHING_MODIFIED;
if (outputService != null) {
- outputService.startBuild(buildId);
+ modifiedOutputFiles = outputService.startBuild(buildId);
} else {
startLocalOutputBuild(); // TODO(bazel-team): this could be just another OutputService
}
ActionCache actionCache = getActionCache();
SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
- Builder builder = createBuilder(request, executor, actionCache, skyframeExecutor);
+ Builder builder =
+ createBuilder(request, executor, actionCache, skyframeExecutor, modifiedOutputFiles);
//
// Execution proper. All statements below are logically nested in
@@ -662,7 +665,8 @@
private Builder createBuilder(BuildRequest request,
Executor executor,
ActionCache actionCache,
- SkyframeExecutor skyframeExecutor) {
+ SkyframeExecutor skyframeExecutor,
+ ModifiedFileSet modifiedOutputFiles) {
BuildRequest.BuildRequestOptions options = request.getBuildOptions();
boolean verboseExplanations = options.verboseExplanations;
boolean keepGoing = request.getViewOptions().keepGoing;
@@ -682,8 +686,9 @@
return new SkyframeBuilder(skyframeExecutor,
new ActionCacheChecker(actionCache, env.getView().getArtifactFactory(), executionFilter,
verboseExplanations),
- keepGoing, actualJobs, options.checkOutputFiles, fileCache,
- request.getBuildOptions().progressReportInterval);
+ keepGoing, actualJobs,
+ options.checkOutputFiles ? modifiedOutputFiles : ModifiedFileSet.NOTHING_MODIFIED,
+ options.finalizeActions, fileCache, request.getBuildOptions().progressReportInterval);
}
private void configureResourceManager(BuildRequest request) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
index c7142e3..8bd0da3 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -51,6 +51,7 @@
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.BlazeClock;
import com.google.devtools.build.lib.util.LoggingUtil;
+import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.skyframe.CycleInfo;
import com.google.devtools.build.skyframe.ErrorInfo;
import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
@@ -79,20 +80,23 @@
private final SkyframeExecutor skyframeExecutor;
private final boolean keepGoing;
private final int numJobs;
- private final boolean checkOutputFiles;
+ private final boolean finalizeActionsToOutputService;
+ private final ModifiedFileSet modifiedOutputFiles;
private final ActionInputFileCache fileCache;
private final ActionCacheChecker actionCacheChecker;
private final int progressReportInterval;
@VisibleForTesting
public SkyframeBuilder(SkyframeExecutor skyframeExecutor, ActionCacheChecker actionCacheChecker,
- boolean keepGoing, int numJobs, boolean checkOutputFiles,
- ActionInputFileCache fileCache, int progressReportInterval) {
+ boolean keepGoing, int numJobs, ModifiedFileSet modifiedOutputFiles,
+ boolean finalizeActionsToOutputService, ActionInputFileCache fileCache,
+ int progressReportInterval) {
this.skyframeExecutor = skyframeExecutor;
this.actionCacheChecker = actionCacheChecker;
this.keepGoing = keepGoing;
this.numJobs = numJobs;
- this.checkOutputFiles = checkOutputFiles;
+ this.finalizeActionsToOutputService = finalizeActionsToOutputService;
+ this.modifiedOutputFiles = modifiedOutputFiles;
this.fileCache = fileCache;
this.progressReportInterval = progressReportInterval;
}
@@ -110,7 +114,7 @@
boolean explain,
@Nullable Range<Long> lastExecutionTimeRange)
throws BuildFailedException, AbruptExitException, TestExecException, InterruptedException {
- skyframeExecutor.prepareExecution(checkOutputFiles, lastExecutionTimeRange);
+ skyframeExecutor.prepareExecution(modifiedOutputFiles, lastExecutionTimeRange);
skyframeExecutor.setFileCache(fileCache);
// Note that executionProgressReceiver accesses builtTargets concurrently (after wrapping in a
// synchronized collection), so unsynchronized access to this variable is unsafe while it runs.
@@ -147,6 +151,7 @@
/*exclusiveTesting=*/ false,
keepGoing,
explain,
+ finalizeActionsToOutputService,
numJobs,
actionCacheChecker,
executionProgressReceiver);
@@ -181,6 +186,7 @@
true,
keepGoing,
explain,
+ finalizeActionsToOutputService,
numJobs,
actionCacheChecker,
null);
diff --git a/src/main/java/com/google/devtools/build/lib/exec/OutputService.java b/src/main/java/com/google/devtools/build/lib/exec/OutputService.java
index 2e3d883..2a48b5c 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/OutputService.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/OutputService.java
@@ -14,11 +14,15 @@
package com.google.devtools.build.lib.exec;
+import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.BuildFailedException;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.vfs.BatchStat;
+import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -53,10 +57,11 @@
* Start the build.
*
* @param buildId the UUID build identifier
+ * @return a ModifiedFileSet of changed output files.
* @throws BuildFailedException if build preparation failed
* @throws InterruptedException
*/
- void startBuild(UUID buildId)
+ ModifiedFileSet startBuild(UUID buildId)
throws BuildFailedException, AbruptExitException, InterruptedException;
/**
@@ -67,6 +72,10 @@
*/
void finalizeBuild(boolean buildSuccessful) throws BuildFailedException, AbruptExitException;
+ /** Notify the output service of a completed action. */
+ void finalizeAction(Action action, MetadataHandler metadataHandler)
+ throws IOException, EnvironmentalExecException;
+
/**
* Stages the given tool from the package path, possibly copying it to local disk.
*
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index 2401940..665c1b3 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -353,9 +353,7 @@
}
SkyframeExecutor skyframeExecutor = getSkyframeExecutor();
- skyframeExecutor.setBatchStatter(outputService == null
- ? null
- : outputService.getBatchStatter());
+ skyframeExecutor.setOutputService(outputService);
this.outputFileSystem = determineOutputFileSystem();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 2415474..e6bf2f5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.actions.Artifact.MiddlemanExpander;
import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException;
import com.google.devtools.build.lib.actions.CachedActionEvent;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.MapBasedActionGraph;
import com.google.devtools.build.lib.actions.MutableActionGraph;
@@ -59,6 +60,7 @@
import com.google.devtools.build.lib.concurrent.ThrowableRecordingRunnableWrapper;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.exec.OutputService;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.util.Pair;
@@ -130,6 +132,7 @@
private ProgressSupplier progressSupplier;
private ActionCompletedReceiver completionReceiver;
private final AtomicReference<ActionExecutionStatusReporter> statusReporterRef;
+ private OutputService outputService;
SkyframeActionExecutor(ResourceManager resourceManager,
AtomicReference<EventBus> eventBus,
@@ -331,7 +334,7 @@
}
void prepareForExecution(Reporter reporter, Executor executor, boolean keepGoing,
- boolean explain, ActionCacheChecker actionCacheChecker) {
+ boolean explain, ActionCacheChecker actionCacheChecker, OutputService outputService) {
this.reporter = Preconditions.checkNotNull(reporter);
this.executorEngine = Preconditions.checkNotNull(executor);
@@ -342,6 +345,7 @@
this.actionCacheChecker = Preconditions.checkNotNull(actionCacheChecker);
// Don't cache possibly stale data from the last build.
this.explain = explain;
+ this.outputService = outputService;
}
public void setActionLogBufferPathGenerator(
@@ -354,6 +358,7 @@
// This transitively holds a bunch of heavy objects, so it's important to clear it at the
// end of a build.
this.executorEngine = null;
+ this.outputService = null;
}
boolean probeActionExecution(Action action) {
@@ -810,6 +815,15 @@
} finally {
profiler.completeTask(ProfilerTask.ACTION_COMPLETE);
}
+
+ if (outputService != null) {
+ try {
+ outputService.finalizeAction(action, metadataHandler);
+ } catch (EnvironmentalExecException | IOException e) {
+ reportError("unable to finalize action: " + e.getMessage(), e, action, fileOutErr);
+ }
+ }
+
reportActionExecution(action, null, fileOutErr);
} catch (ActionExecutionException actionException) {
// Success in execution but failure in completion.
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 b3e5b8e..8b78370 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
@@ -76,6 +76,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.exec.OutputService;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -173,7 +174,7 @@
private final WorkspaceStatusAction.Factory workspaceStatusActionFactory;
private final BlazeDirectories directories;
@Nullable
- private BatchStat batchStatter;
+ private OutputService outputService;
// TODO(bazel-team): Figure out how to handle value builders that block internally. Blocking
// operations may need to be handled in another (bigger?) thread pool. Also, we should detect
@@ -447,8 +448,8 @@
public abstract void dumpPackages(PrintStream out);
- public void setBatchStatter(@Nullable BatchStat batchStatter) {
- this.batchStatter = batchStatter;
+ public void setOutputService(OutputService outputService) {
+ this.outputService = outputService;
}
/**
@@ -1043,6 +1044,7 @@
boolean exclusiveTesting,
boolean keepGoing,
boolean explain,
+ boolean finalizeActionsToOutputService,
int numJobs,
ActionCacheChecker actionCacheChecker,
@Nullable EvaluationProgressReceiver executionProgressReceiver)
@@ -1051,7 +1053,8 @@
Preconditions.checkState(actionLogBufferPathGenerator != null);
skyframeActionExecutor.prepareForExecution(
- reporter, executor, keepGoing, explain, actionCacheChecker);
+ reporter, executor, keepGoing, explain, actionCacheChecker,
+ finalizeActionsToOutputService ? outputService : null);
resourceManager.resetResourceUsage();
try {
@@ -1077,7 +1080,8 @@
@VisibleForTesting
public void prepareBuildingForTestingOnly(Reporter reporter, Executor executor, boolean keepGoing,
boolean explain, ActionCacheChecker checker) {
- skyframeActionExecutor.prepareForExecution(reporter, executor, keepGoing, explain, checker);
+ skyframeActionExecutor.prepareForExecution(reporter, executor, keepGoing, explain, checker,
+ outputService);
}
EvaluationResult<TargetPatternValue> targetPatterns(Iterable<SkyKey> patternSkyKeys,
@@ -1637,14 +1641,15 @@
this.statusReporterRef.set(statusReporter);
}
- public void prepareExecution(boolean checkOutputFiles,
+ public void prepareExecution(ModifiedFileSet modifiedOutputFiles,
@Nullable Range<Long> lastExecutionTimeRange) throws AbruptExitException,
InterruptedException {
maybeInjectEmbeddedArtifacts();
- if (checkOutputFiles) {
+ if (modifiedOutputFiles != ModifiedFileSet.NOTHING_MODIFIED) {
// Detect external modifications in the output tree.
FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, lastExecutionTimeRange);
+ BatchStat batchStatter = outputService == null ? null : outputService.getBatchStatter();
invalidateDirtyActions(fsvc.getDirtyActionValues(memoizingEvaluator.getValues(),
batchStatter));
modifiedFiles += fsvc.getNumberOfModifiedOutputFiles();
