Make FrontierSerializer reuse the PROJECT.scl evaluated during loading phase for creating the active_directories matcher.

.. and delete the bespoke `PROJECT.scl` prefix matcher builder in `FrontierSerializer`. If `PROJECT.scl` file was not found but the matcher is needed a `LoadingFailedException` will thrown (see tests).

Also move the serialization step from `SerializationModule#afterCommand` to the end of `BuildTool#buildTargets`. The `afterCommand` step unconditionally runs even if an exception was thrown earlier, which makes error handling significantly more complicated.

`FrontierSerializer` is also refactored to avoid a cyclic dependency on `runtime.CommandEnvironment`, so it can be called from the `runtime` package.

PiperOrigin-RevId: 671606525
Change-Id: Ie1037ddf270e3f8de0c8835fea6d513a01309fe3

Author: jin

src/main/java/com/google/devtools/build/lib/BUILD

@@ -452,6 +452,8 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/config",
         "//src/main/java/com/google/devtools/build/lib/skyframe/rewinding:action_rewound_event",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
+        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:frontier_serializer",
+        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:options",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:TestType",
         "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",

src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.buildtool;
 
 import static com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.configurationId;
+import static com.google.devtools.build.lib.server.FailureDetails.RemoteAnalysisCaching.Code.PROJECT_FILE_NOT_FOUND;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
@@ -53,6 +54,7 @@
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.server.FailureDetails.BuildConfiguration.Code;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.RemoteAnalysisCaching;
 import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
 import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
 import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
@@ -61,6 +63,7 @@
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetAnalyzedEvent;
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetSkippedEvent;
 import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey;
+import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingOptions;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.RegexFilter;
@@ -169,7 +172,10 @@ public static AnalysisResult execute(
 
   /**
    * Evaluates the PROJECT.scl file and set corresponding options, if required by specific feature
-   * flags (e.g. canonical configurations).
+   * flags (e.g. canonical configurations, remote analysis caching).
+   *
+   * <p>May have a side-effect from updating the fields in the {@link CommandEnvironment} {@code
+   * env} parameter.
    *
    * <p>Shared by both Skymeld and non-Skymeld analysis.
    */
@@ -179,12 +185,19 @@ static BuildOptions evaluateProjectFile(
       TargetPatternPhaseValue targetPatternPhaseValue,
       CommandEnvironment env)
       throws LoadingFailedException, InvalidConfigurationException {
+    // Remote analysis caching feature flag.
+    var analysisCachingOpts = env.getOptions().getOptions(RemoteAnalysisCachingOptions.class);
+    boolean neededForAnalysisCaching =
+        analysisCachingOpts != null
+            && !Strings.isNullOrEmpty(analysisCachingOpts.serializedFrontierProfile);
+
     // Canonical configurations feature flag.
     String sclConfig = buildOptions.get(CoreOptions.class).sclConfig;
     boolean neededForSclConfig =
         !Strings.isNullOrEmpty(sclConfig) || request.getBuildOptions().enforceProjectConfigs;
 
-    if (!neededForSclConfig) {
+    if (!(neededForSclConfig || neededForAnalysisCaching)) {
+      // All feature flags disabled.
       return buildOptions;
     }
 
@@ -194,7 +207,28 @@ static BuildOptions evaluateProjectFile(
             env.getSkyframeExecutor(),
             env.getReporter());
 
-    if (projectFile != null) {
+    if (neededForAnalysisCaching) {
+      if (projectFile == null) {
+        // TODO: b/353233779 - consider falling back on full serialization when there is no
+        // project matcher.
+        String message =
+            "Failed to find PROJECT.scl file for: " + targetPatternPhaseValue.getTargetLabels();
+        throw new LoadingFailedException(
+            message,
+            DetailedExitCode.of(
+                FailureDetail.newBuilder()
+                    .setMessage(message)
+                    .setRemoteAnalysisCaching(
+                        RemoteAnalysisCaching.newBuilder().setCode(PROJECT_FILE_NOT_FOUND))
+                    .build()));
+      }
+      env.getRuntime()
+          .setActiveDirectoriesPrefixTrie(
+              BuildTool.getWorkingSetMatcherForSkyfocus(
+                  projectFile, env.getSkyframeExecutor(), env.getReporter()));
+    }
+
+    if (neededForSclConfig && projectFile != null) {
       // Do not apply canonical configurations if the project file doesn't exist.
       return BuildTool.applySclConfigs(
           buildOptions,

src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java

@@ -21,6 +21,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
 import com.google.common.base.Stopwatch;
+import com.google.common.base.Strings;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
@@ -82,6 +83,8 @@
 import com.google.devtools.build.lib.skyframe.actiongraph.v2.AqueryOutputHandler.OutputType;
 import com.google.devtools.build.lib.skyframe.actiongraph.v2.InvalidAqueryOutputFormatException;
 import com.google.devtools.build.lib.skyframe.config.FlagSetValue;
+import com.google.devtools.build.lib.skyframe.serialization.analysis.FrontierSerializer;
+import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingOptions;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.CrashFailureDetails;
 import com.google.devtools.build.lib.util.DetailedExitCode;
@@ -97,6 +100,7 @@
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.util.Collection;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 /**
@@ -266,6 +270,10 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
               delayedFailureDetail.getMessage(), DetailedExitCode.of(delayedFailureDetail));
         }
 
+        if (env.getSkyframeExecutor() instanceof SequencedSkyframeExecutor) {
+          serializeFrontier(request.getOptions(RemoteAnalysisCachingOptions.class));
+        }
+
         // Only consider builds with SequencedSkyframeExecutor.
         if (env.getSkyframeExecutor() instanceof SequencedSkyframeExecutor
             && request.getBuildOptions().aqueryDumpAfterBuildFormat != null) {
@@ -756,6 +764,29 @@ public BuildResult processRequest(
     return result;
   }
 
+  private void serializeFrontier(@Nullable RemoteAnalysisCachingOptions options)
+      throws InterruptedException, AbruptExitException {
+    if (options == null || Strings.isNullOrEmpty(options.serializedFrontierProfile)) {
+      return;
+    }
+
+    Optional<FailureDetail> maybeFailureDetail =
+        FrontierSerializer.dumpFrontierSerializationProfile(
+            env.getRuntime().getAnalysisCodecRegistry(),
+            env.getSkyframeBuildView().getArtifactFactory(),
+            env.getRuntime().getRuleClassProvider(),
+            env.getSkyframeExecutor(),
+            // TODO: b/353233779 - consider falling back on full serialization when there is
+            // no project matcher.
+            env.getRuntime().getActiveDirectoriesPrefixTrie(),
+            env.getReporter(),
+            options.serializedFrontierProfile);
+
+    if (maybeFailureDetail.isPresent()) {
+      throw new AbruptExitException(DetailedExitCode.of(maybeFailureDetail.get()));
+    }
+  }
+
   private static void maybeSetStopOnFirstFailure(BuildRequest request, BuildResult result) {
     if (shouldStopOnFailure(request)) {
       result.setStopOnFirstFailure(true);

src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java

@@ -49,6 +49,7 @@
 import com.google.devtools.build.lib.buildtool.buildevent.ProfilerStartedEvent;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.clock.Clock;
+import com.google.devtools.build.lib.collect.PathFragmentPrefixTrie;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.OutputFilter;
@@ -196,6 +197,8 @@ public final class BlazeRuntime implements BugReport.BlazeRuntimeInterface {
 
   @Nullable // not all environments provide this
   private Supplier<ObjectCodecRegistry> analysisCodecRegistrySupplier;
+  @Nullable // only present with PROJECT.scl
+  private PathFragmentPrefixTrie activeDirectoriesPrefixTrie;
   private final InstrumentationOutputFactory instrumentationOutputFactory;
 
   private BlazeRuntime(
@@ -518,6 +521,15 @@ public ImmutableMap<String, InfoItem> getInfoItems() {
     return infoItems;
   }
 
+  public void setActiveDirectoriesPrefixTrie(PathFragmentPrefixTrie trie) {
+    this.activeDirectoriesPrefixTrie = trie;
+  }
+
+  @Nullable
+  public PathFragmentPrefixTrie getActiveDirectoriesPrefixTrie() {
+    return activeDirectoriesPrefixTrie;
+  }
+
   public Iterable<BlazeModule> getBlazeModules() {
     return blazeModules;
   }

src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD

@@ -53,13 +53,6 @@ java_library(
         ":serialization_registry_setup_helpers",
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
-        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:frontier_serializer",
-        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:options",
-        "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
-        "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
-        "//src/main/protobuf:failure_details_java_proto",
-        "//third_party:guava",
-        "//third_party:jsr305",
     ],
 )
 

src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationModule.java

@@ -13,31 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe.serialization;
 
-import static com.google.common.base.Strings.isNullOrEmpty;
-
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
-import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.WorkspaceBuilder;
-import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
-import com.google.devtools.build.lib.server.FailureDetails.RemoteAnalysisCaching.Code;
-import com.google.devtools.build.lib.skyframe.serialization.analysis.FrontierSerializer;
-import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingOptions;
-import com.google.devtools.build.lib.util.AbruptExitException;
-import com.google.devtools.build.lib.util.DetailedExitCode;
-import java.util.Optional;
-import javax.annotation.Nullable;
 
 /** A {@link BlazeModule} to store Skyframe serialization lifecycle hooks. */
 public class SerializationModule extends BlazeModule {
 
-  @Nullable private RemoteAnalysisCachingOptions options;
-
-  // Retained only for analysis caching operations in afterCommand. Must be garbage collected once
-  // it's no longer needed. See the javadoc for CommandEnvironment for more information.
-  @Nullable private CommandEnvironment env;
-
   @Override
   public void workspaceInit(
       BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) {
@@ -58,47 +41,4 @@ public void workspaceInit(
                 directories.getWorkspace().getBaseName())));
   }
 
-  @Override
-  public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
-    RemoteAnalysisCachingOptions options =
-        env.getOptions().getOptions(RemoteAnalysisCachingOptions.class);
-    if (options == null) {
-      // not a supported command
-      return;
-    }
-
-    if (!isNullOrEmpty(options.serializedFrontierProfile)) {
-      this.options = options;
-      this.env = env;
-    }
-  }
-
-  @Override
-  public void afterCommand() throws AbruptExitException {
-    if (options == null) {
-      return;
-    }
-
-    try {
-      if (!isNullOrEmpty(options.serializedFrontierProfile)) {
-        Optional<FailureDetail> failureDetail =
-            FrontierSerializer.dumpFrontierSerializationProfile(
-                env, options.serializedFrontierProfile);
-        if (failureDetail.isPresent()) {
-          throw new AbruptExitException(DetailedExitCode.of(failureDetail.get()));
-        }
-      }
-    } catch (InterruptedException e) {
-      throw new AbruptExitException(
-          DetailedExitCode.of(
-              FrontierSerializer.createFailureDetail(
-                  "frontier serializer was interrupted: " + e.getMessage(),
-                  Code.SERIALIZED_FRONTIER_PROFILE_FAILED)));
-    } finally {
-      // Do not retain these objects between invocations.
-      this.options = null;
-      this.env = null;
-      System.gc();
-    }
-  }
 }

src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD

@@ -15,28 +15,22 @@ java_library(
     name = "frontier_serializer",
     srcs = ["FrontierSerializer.java"],
     deps = [
-        "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
-        "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
-        "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
         "//src/main/java/com/google/devtools/build/lib/skyframe:prerequisite_package_function",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:project_value",
         "//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/serialization",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/protobuf:failure_details_java_proto",
         "//third_party:guava",
-        "//third_party:jsr305",
         "//third_party/protobuf:protobuf_java",
     ],
 )