Skip to content

Commit

Permalink
Allow DiffAwareness to share precomputed information about the work…
Browse files Browse the repository at this point in the history
…space and

propagate it to the `WorkspaceStatusAction`.

Each successful Bazel build issue 2 `BuildInfo` events -- one based on the
result of running the command indicated by `workspace_status_command` and a
dummy one based on a subset of available information. Receivers of those
discard all but the first one. If a build fails before execution phase, the
dummy event will be the only even issued, hence it will be used.

`DiffAwareness` operates on the workspace to figure out the diffs and in the
process probes properties of it. Allow `DiffAwareness` to share such
information with the intention to make it available for the rest of the build.

Propagate unanimous precomputed workspace information when it is available from
`DiffAwareness` through the `DiffAwarenessManager` and `SkyframeExecutor` to
`CommandEnvironment` and store it there to make it available for the rest of
the build.

Make precomputed workspace information available to the dummy `BuildInfo`
event.

PiperOrigin-RevId: 355072552
  • Loading branch information
alexjski authored and philwo committed Apr 21, 2021
1 parent 9e7e592 commit 14abe4f
Show file tree
Hide file tree
Showing 14 changed files with 277 additions and 20 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe:target_pattern_phase_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:top_down_action_cache",
"//src/main/java/com/google/devtools/build/lib/skyframe:workspace_info",
"//src/main/java/com/google/devtools/build/lib/unix",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:TestType",
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1196,13 +1196,15 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/skyframe:workspace_info",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
"//third_party:jsr305",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.WorkspaceStatus;
import com.google.devtools.build.lib.server.FailureDetails.WorkspaceStatus.Code;
import com.google.devtools.build.lib.skyframe.WorkspaceInfoFromDiff;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.OptionsUtils;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
Expand All @@ -40,6 +41,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;

/**
* An action writing the workspace status files.
Expand Down Expand Up @@ -174,6 +176,10 @@ public interface Environment {
public interface DummyEnvironment {
Path getWorkspace();

/** Returns optional precomputed workspace info to include in the build info event. */
@Nullable
WorkspaceInfoFromDiff getWorkspaceInfoFromDiff();

String getBuildRequestId();

OptionsProvider getOptions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.WorkspaceInfoFromDiff;
import com.google.devtools.build.lib.skyframe.actiongraph.v2.ActionGraphDump;
import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler;
import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler.OutputType;
Expand Down Expand Up @@ -274,6 +275,12 @@ public String getBuildRequestId() {
public OptionsProvider getOptions() {
return env.getOptions();
}

@Nullable
@Override
public WorkspaceInfoFromDiff getWorkspaceInfoFromDiff() {
return env.getWorkspaceInfoFromDiff();
}
})));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.runtime;

import static com.google.common.base.Preconditions.checkState;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.eventbus.EventBus;
Expand All @@ -40,6 +42,7 @@
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TopDownActionCache;
import com.google.devtools.build.lib.skyframe.WorkspaceInfoFromDiff;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.io.OutErr;
Expand Down Expand Up @@ -100,6 +103,7 @@ public class CommandEnvironment {
private TopDownActionCache topDownActionCache;
private String workspaceName;
private boolean hasSyncedPackageLoading = false;
@Nullable private WorkspaceInfoFromDiff workspaceInfoFromDiff;

// This AtomicReference is set to:
// - null, if neither BlazeModuleEnvironment#exit nor #precompleteCommand have been called
Expand Down Expand Up @@ -515,7 +519,7 @@ public String getWorkspaceName() {
}

public void setWorkspaceName(String workspaceName) {
Preconditions.checkState(this.workspaceName == null, "workspace name can only be set once");
checkState(this.workspaceName == null, "workspace name can only be set once");
this.workspaceName = workspaceName;
eventBus.post(new ExecRootEvent(getExecRoot()));
}
Expand Down Expand Up @@ -579,6 +583,18 @@ public void setOutputServiceForTesting(@Nullable OutputService outputService) {
this.outputService = outputService;
}

/**
* Returns precomputed workspace information or null.
*
* <p>Precomputed workspace info is an optimization allowing to share information about the
* workspace if it was derived at the time of synchronizing the workspace. This way we can make it
* available earlier during the build and avoid retrieving it again.
*/
@Nullable
public WorkspaceInfoFromDiff getWorkspaceInfoFromDiff() {
return workspaceInfoFromDiff;
}

public ActionCache getPersistentActionCache() throws IOException {
return workspace.getPersistentActionCache(reporter);
}
Expand Down Expand Up @@ -677,16 +693,17 @@ public void syncPackageLoading(OptionsProvider options)
"We should never call this method more than once over the course of a single command");
}
hasSyncedPackageLoading = true;
getSkyframeExecutor()
.sync(
reporter,
options.getOptions(PackageOptions.class),
packageLocator,
options.getOptions(BuildLanguageOptions.class),
getCommandId(),
clientEnv,
timestampGranularityMonitor,
options);
workspaceInfoFromDiff =
getSkyframeExecutor()
.sync(
reporter,
options.getOptions(PackageOptions.class),
packageLocator,
options.getOptions(BuildLanguageOptions.class),
getCommandId(),
clientEnv,
timestampGranularityMonitor,
options);
}

public void recordLastExecutionTime() {
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ java_library(
":tree_artifact_value",
":unloaded_toolchain_context",
":unloaded_toolchain_context_impl",
":workspace_info",
":workspace_name_function",
":workspace_name_value",
":workspace_status_function",
Expand Down Expand Up @@ -1278,6 +1279,7 @@ java_library(
deps = [
":broken_diff_awareness_exception",
":incompatible_view_exception",
":workspace_info",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/common/options",
"//third_party:jsr305",
Expand All @@ -1291,6 +1293,7 @@ java_library(
":broken_diff_awareness_exception",
":diff_awareness",
":incompatible_view_exception",
":workspace_info",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/common/options",
Expand Down Expand Up @@ -2767,6 +2770,11 @@ java_library(
],
)

java_library(
name = "workspace_info",
srcs = ["WorkspaceInfoFromDiff.java"],
)

java_library(
name = "workspace_name_function",
srcs = ["WorkspaceNameFunction.java"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;

import com.google.common.base.Preconditions;
import static com.google.common.base.Preconditions.checkNotNull;

/**
* Thrown on {@link DiffAwareness#getDiff} to indicate that something is wrong with the
Expand All @@ -22,6 +22,10 @@
public class BrokenDiffAwarenessException extends Exception {

public BrokenDiffAwarenessException(String msg) {
super(Preconditions.checkNotNull(msg));
super(checkNotNull(msg));
}

public BrokenDiffAwarenessException(String msg, Throwable cause) {
super(checkNotNull(msg), cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public interface Factory {

/** Opaque view of the filesystem under a package path entry at a specific point in time. */
interface View {
/** Returns workspace info unanimously associated with the package path or null. */
@Nullable
default WorkspaceInfoFromDiff getWorkspaceInfo() {
return null;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ public void reset() {
public interface ProcessableModifiedFileSet {
ModifiedFileSet getModifiedFileSet();

@Nullable
WorkspaceInfoFromDiff getWorkspaceInfo();

/**
* This should be called when the changes have been noted. Otherwise, the result from the next
* call to {@link #getDiff} will be from the baseline of the old, unprocessed, diff.
Expand Down Expand Up @@ -99,7 +102,7 @@ public ProcessableModifiedFileSet getDiff(
if (baselineView == null) {
logger.atInfo().log("Initial baseline view for %s is %s", pathEntry, newView);
diffAwarenessState.baselineView = newView;
return BrokenProcessableModifiedFileSet.INSTANCE;
return new InitialModifiedFileSet(newView.getWorkspaceInfo());
}

ModifiedFileSet diff;
Expand Down Expand Up @@ -172,6 +175,12 @@ public ModifiedFileSet getModifiedFileSet() {
return modifiedFileSet;
}

@Nullable
@Override
public WorkspaceInfoFromDiff getWorkspaceInfo() {
return nextView.getWorkspaceInfo();
}

@Override
public void markProcessed() {
DiffAwarenessState diffAwarenessState = currentDiffAwarenessStates.get(pathEntry);
Expand All @@ -191,6 +200,36 @@ public ModifiedFileSet getModifiedFileSet() {
return ModifiedFileSet.EVERYTHING_MODIFIED;
}

@Nullable
@Override
public WorkspaceInfoFromDiff getWorkspaceInfo() {
return null;
}

@Override
public void markProcessed() {}
}

/** Modified file set for a clean build. */
private static class InitialModifiedFileSet implements ProcessableModifiedFileSet {

@Nullable private final WorkspaceInfoFromDiff workspaceInfo;

InitialModifiedFileSet(@Nullable WorkspaceInfoFromDiff workspaceInfo) {
this.workspaceInfo = workspaceInfo;
}

@Override
public ModifiedFileSet getModifiedFileSet() {
return ModifiedFileSet.EVERYTHING_MODIFIED;
}

@Nullable
@Override
public WorkspaceInfoFromDiff getWorkspaceInfo() {
return workspaceInfo;
}

@Override
public void markProcessed() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,9 @@ public RecordingDifferencer getDifferencerForTesting() {
return recordingDiffer;
}

@Nullable
@Override
public void sync(
public WorkspaceInfoFromDiff sync(
ExtendedEventHandler eventHandler,
PackageOptions packageOptions,
PathPackageLocator packageLocator,
Expand Down Expand Up @@ -263,11 +264,13 @@ public void sync(
tsgm,
options);
long startTime = System.nanoTime();
handleDiffs(eventHandler, packageOptions.checkOutputFiles, options);
WorkspaceInfoFromDiff workspaceInfo =
handleDiffs(eventHandler, packageOptions.checkOutputFiles, options);
long stopTime = System.nanoTime();
Profiler.instance().logSimpleTask(startTime, stopTime, ProfilerTask.INFO, "handleDiffs");
long duration = stopTime - startTime;
sourceDiffCheckingDuration = duration > 0 ? Duration.ofNanos(duration) : Duration.ZERO;
return workspaceInfo;
}

/**
Expand Down Expand Up @@ -342,7 +345,8 @@ public void handleDiffsForTesting(ExtendedEventHandler eventHandler)
handleDiffs(eventHandler, /*checkOutputFiles=*/false, OptionsProvider.EMPTY);
}

private void handleDiffs(
@Nullable
private WorkspaceInfoFromDiff handleDiffs(
ExtendedEventHandler eventHandler, boolean checkOutputFiles, OptionsProvider options)
throws InterruptedException, AbruptExitException {
TimestampGranularityMonitor tsgm = this.tsgm.get();
Expand All @@ -355,13 +359,18 @@ private void handleDiffs(
invalidateCachedWorkspacePathsStates();
}

WorkspaceInfoFromDiff workspaceInfo = null;
Map<Root, DiffAwarenessManager.ProcessableModifiedFileSet> modifiedFilesByPathEntry =
Maps.newHashMap();
Set<Pair<Root, DiffAwarenessManager.ProcessableModifiedFileSet>>
pathEntriesWithoutDiffInformation = Sets.newHashSet();
for (Root pathEntry : pkgLocator.get().getPathEntries()) {
ImmutableList<Root> pkgRoots = pkgLocator.get().getPathEntries();
for (Root pathEntry : pkgRoots) {
DiffAwarenessManager.ProcessableModifiedFileSet modifiedFileSet =
diffAwarenessManager.getDiff(eventHandler, pathEntry, options);
if (pkgRoots.size() == 1) {
workspaceInfo = modifiedFileSet.getWorkspaceInfo();
}
if (modifiedFileSet.getModifiedFileSet().treatEverythingAsModified()) {
pathEntriesWithoutDiffInformation.add(Pair.of(pathEntry, modifiedFileSet));
} else {
Expand All @@ -382,6 +391,7 @@ private void handleDiffs(
managedDirectoriesChanged,
fsvcThreads);
handleClientEnvironmentChanges();
return workspaceInfo;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2630,8 +2630,13 @@ ActionExecutionStatusReporter getActionExecutionStatusReporterForTesting() {

/**
* Initializes and syncs the graph with the given options, readying it for the next evaluation.
*
* <p>Returns precomputed information about the workspace if it is available at this stage. This
* is an optimization allowing implementations which have such information to make it available
* early in the build.
*/
public void sync(
@Nullable
public WorkspaceInfoFromDiff sync(
ExtendedEventHandler eventHandler,
PackageOptions packageOptions,
PathPackageLocator pathPackageLocator,
Expand All @@ -2657,6 +2662,7 @@ public void sync(
dropConfiguredTargetsNow(eventHandler);
lastAnalysisDiscarded = false;
}
return null;
}

protected void syncPackageLoading(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2021 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.lib.skyframe;

/** Information for a workspace computed at the time of collecting diff. */
public interface WorkspaceInfoFromDiff {}
Loading

0 comments on commit 14abe4f

Please sign in to comment.