[Skymeld] Turn off skymeld when --experimental_skyframe_cpu_heavy_sky…

…keys_thread_pool_size is <= 0. PiperOrigin-RevId: 529019278 Change-Id: I57bf559a01110cc4a50e28221d528cb10e3dd1f6
fweikert · May 3, 2023 · f40a244 · f40a244
1 parent 0597a07
commit f40a244
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 17 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2414,6 +2414,7 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/lib:build-request-options",
         "//src/main/java/com/google/devtools/build/lib:runtime",
+        "//src/main/java/com/google/devtools/build/lib/analysis:analysis_options",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
     ],

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkymeldModule.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkymeldModule.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.devtools.build.lib.analysis.AnalysisOptions;
 import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -31,46 +32,60 @@ boolean determineIfMergingAnalysisExecution(CommandEnvironment env) {
     PathPackageLocator packageLocator = env.getPackageLocator();
     BuildRequestOptions buildRequestOptions =
         env.getOptions().getOptions(BuildRequestOptions.class);
-    boolean plainValueFromFlag = getPlainValueFromFlag(buildRequestOptions);
+    boolean effectiveValue = getPlainValueFromFlag(buildRequestOptions);
+
     // --nobuild means no execution will be carried out, hence it doesn't make sense to interleave
     // analysis and execution in that case and --experimental_merged_skyframe_analysis_execution
     // should be ignored.
-    // Aquery and Cquery implicitly set --nobuild, so there's no need to have a warning here: it
-    // makes no different from the users' perspective.
-    if (plainValueFromFlag
-        && !buildRequestOptions.performExecutionPhase
-        && !(commandName.equals("aquery") || commandName.equals("cquery"))) {
-      env.getReporter()
-          .handle(
-              Event.warn(
-                  "--experimental_merged_skyframe_analysis_execution is incompatible with --nobuild"
-                      + " and will be ignored."));
+    if (effectiveValue && !buildRequestOptions.performExecutionPhase) {
+      // Aquery and Cquery implicitly set --nobuild, so there's no need to have a warning here: it
+      // makes no different from the users' perspective.
+      if (!(commandName.equals("aquery") || commandName.equals("cquery"))) {
+        env.getReporter()
+            .handle(
+                Event.warn(
+                    "--experimental_merged_skyframe_analysis_execution is incompatible with"
+                        + " --nobuild and will be ignored."));
+      }
+      effectiveValue = false;
     }
     // TODO(b/245922903): Make --explain compatible with Skymeld.
-    if (plainValueFromFlag && buildRequestOptions.explanationPath != null) {
+    if (effectiveValue && buildRequestOptions.explanationPath != null) {
       env.getReporter()
           .handle(
               Event.warn(
                   "--experimental_merged_skyframe_analysis_execution is incompatible with --explain"
                       + " and will be ignored."));
+      effectiveValue = false;
     }
 
     boolean havingMultiPackagePath =
         packageLocator != null && packageLocator.getPathEntries().size() > 1;
     // TODO(b/246324830): Skymeld and multi-package_path are incompatible.
-    if (plainValueFromFlag && havingMultiPackagePath) {
+    if (effectiveValue && havingMultiPackagePath) {
       env.getReporter()
           .handle(
               Event.warn(
                   "--experimental_merged_skyframe_analysis_execution is "
                       + "incompatible with multiple --package_path ("
                       + packageLocator.getPathEntries()
                       + ") and its value will be ignored."));
+      effectiveValue = false;
     }
-    return plainValueFromFlag
-        && buildRequestOptions.performExecutionPhase
-        && buildRequestOptions.explanationPath == null
-        && !havingMultiPackagePath;
+
+    if (effectiveValue
+        && env.getOptions().getOptions(AnalysisOptions.class) != null
+        && env.getOptions().getOptions(AnalysisOptions.class).cpuHeavySkyKeysThreadPoolSize <= 0) {
+      env.getReporter()
+          .handle(
+              Event.warn(
+                  "--experimental_merged_skyframe_analysis_execution is incompatible with a"
+                      + " non-positive --experimental_skyframe_cpu_heavy_skykeys_thread_pool_size"
+                      + " and its value will be ignored."));
+      effectiveValue = false;
+    }
+
+    return effectiveValue;
   }
 
   static boolean getPlainValueFromFlag(BuildRequestOptions buildRequestOptions) {