Allow DiffAwareness to share precomputed information about the work…

…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
fweikert · Apr 21, 2021 · 14abe4f · 14abe4f
1 parent 9e7e592
commit 14abe4f
Show file tree

Hide file tree

Showing 14 changed files with 277 additions and 20 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -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",

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -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",
     ],
 )
 

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/WorkspaceStatusAction.java b/src/main/java/com/google/devtools/build/lib/analysis/WorkspaceStatusAction.java
@@ -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;
@@ -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.
@@ -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();

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -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;
@@ -274,6 +275,12 @@ public String getBuildRequestId() {
                               public OptionsProvider getOptions() {
                                 return env.getOptions();
                               }
+
+                              @Nullable
+                              @Override
+                              public WorkspaceInfoFromDiff getWorkspaceInfoFromDiff() {
+                                return env.getWorkspaceInfoFromDiff();
+                              }
                             })));
       }
     }

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
@@ -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;
@@ -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;
@@ -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
@@ -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()));
   }
@@ -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);
   }
@@ -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() {

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -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",
@@ -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",
@@ -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",
@@ -2767,6 +2770,11 @@ java_library(
     ],
 )
 
+java_library(
+    name = "workspace_info",
+    srcs = ["WorkspaceInfoFromDiff.java"],
+)
+
 java_library(
     name = "workspace_name_function",
     srcs = ["WorkspaceNameFunction.java"],

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BrokenDiffAwarenessException.java b/src/main/java/com/google/devtools/build/lib/skyframe/BrokenDiffAwarenessException.java
@@ -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
@@ -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);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwareness.java
@@ -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;
+    }
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManager.java
@@ -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.
@@ -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;
@@ -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);
@@ -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() {
     }

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
@@ -227,8 +227,9 @@ public RecordingDifferencer getDifferencerForTesting() {
     return recordingDiffer;
   }
 
+  @Nullable
   @Override
-  public void sync(
+  public WorkspaceInfoFromDiff sync(
       ExtendedEventHandler eventHandler,
       PackageOptions packageOptions,
       PathPackageLocator packageLocator,
@@ -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;
   }
 
   /**
@@ -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();
@@ -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 {
@@ -382,6 +391,7 @@ private void handleDiffs(
         managedDirectoriesChanged,
         fsvcThreads);
     handleClientEnvironmentChanges();
+    return workspaceInfo;
   }
 
   /**

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
@@ -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,
@@ -2657,6 +2662,7 @@ public void sync(
       dropConfiguredTargetsNow(eventHandler);
       lastAnalysisDiscarded = false;
     }
+    return null;
   }
 
   protected void syncPackageLoading(

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceInfoFromDiff.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceInfoFromDiff.java
@@ -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 {}