Run the analysis phase with as many threads as the user wants. In order to avoid memory blow-up intra-configured-target analysis, use a semaphore to ensure that CPU-bound work only occurs on #CPU-many threads.

RELNOTES: Use --loading_phase_threads to control the number of threads used during the loading/analysis phase.

--
MOS_MIGRATED_REVID=139477645

3 files changed, 32 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 71a938ab8d..38db025e0b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -87,6 +87,7 @@ import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.Semaphore;
 import javax.annotation.Nullable;
 
 /**
@@ -131,11 +132,15 @@ final class ConfiguredTargetFunction implements SkyFunction {
 
   private final BuildViewProvider buildViewProvider;
   private final RuleClassProvider ruleClassProvider;
+  private final Semaphore cpuBoundSemaphore;
 
-  ConfiguredTargetFunction(BuildViewProvider buildViewProvider,
-      RuleClassProvider ruleClassProvider) {
+  ConfiguredTargetFunction(
+      BuildViewProvider buildViewProvider,
+      RuleClassProvider ruleClassProvider,
+      Semaphore cpuBoundSemaphore) {
     this.buildViewProvider = buildViewProvider;
     this.ruleClassProvider = ruleClassProvider;
+    this.cpuBoundSemaphore = cpuBoundSemaphore;
   }
 
   private static boolean useDynamicConfigurations(BuildConfiguration config) {
@@ -195,6 +200,12 @@ final class ConfiguredTargetFunction implements SkyFunction {
     TargetAndConfiguration ctgValue = new TargetAndConfiguration(target, configuration);
 
     SkyframeDependencyResolver resolver = view.createDependencyResolver(env);
+
+    // TODO(janakr): this acquire() call may tie up this thread indefinitely, reducing the
+    // parallelism of Skyframe. This is a strict improvement over the prior state of the code, in
+    // which we ran with #processors threads, but ideally we would call #tryAcquire here, and if we
+    // failed, would exit this SkyFunction and restart it when permits were available.
+    cpuBoundSemaphore.acquire();
     try {
       // Get the configuration targets that trigger this rule's configurable attributes.
       ImmutableMap<Label, ConfigMatchingProvider> configConditions = getConfigConditions(
@@ -256,6 +267,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
       }
       throw new ConfiguredTargetFunctionException(
           new ConfiguredValueCreationException(e.getMessage(), analysisRootCause));
+    } finally {
+      cpuBoundSemaphore.release();
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 21d6f7b12c..a905a9f68a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -203,12 +203,15 @@ public final class SkyframeBuildView {
       List<ConfiguredTargetKey> values,
       List<AspectValueKey> aspectKeys,
       EventBus eventBus,
-      boolean keepGoing)
+      boolean keepGoing,
+      int numThreads)
       throws InterruptedException, ViewCreationFailedException {
     enableAnalysis(true);
     EvaluationResult<ActionLookupValue> result;
     try {
-      result = skyframeExecutor.configureTargets(eventHandler, values, aspectKeys, keepGoing);
+      result =
+          skyframeExecutor.configureTargets(
+              eventHandler, values, aspectKeys, keepGoing, numThreads);
     } finally {
       enableAnalysis(false);
     }
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 39090f75a9..4406bd8561 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
@@ -152,6 +152,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.Callable;
+import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -339,6 +340,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       Predicate<PathFragment> allowedMissingInputs) {
     ConfiguredRuleClassProvider ruleClassProvider =
         (ConfiguredRuleClassProvider) pkgFactory.getRuleClassProvider();
+    // TODO(janakr): use this semaphore to bound memory usage for SkyFunctions besides
+    // ConfiguredTargetFunction that may have a large temporary memory blow-up.
+    Semaphore cpuBoundSemaphore = new Semaphore(ResourceUsage.getAvailableProcessors());
     // We use an immutable map builder for the nice side effect that it throws if a duplicate key
     // is inserted.
     ImmutableMap.Builder<SkyFunctionName, SkyFunction> map = ImmutableMap.builder();
@@ -393,8 +397,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     map.put(SkyFunctions.TARGET_MARKER, new TargetMarkerFunction());
     map.put(SkyFunctions.TRANSITIVE_TARGET, new TransitiveTargetFunction(ruleClassProvider));
     map.put(SkyFunctions.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction());
-    map.put(SkyFunctions.CONFIGURED_TARGET,
-        new ConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
+    map.put(
+        SkyFunctions.CONFIGURED_TARGET,
+        new ConfiguredTargetFunction(
+            new BuildViewProvider(), ruleClassProvider, cpuBoundSemaphore));
     map.put(SkyFunctions.ASPECT, new AspectFunction(new BuildViewProvider(), ruleClassProvider));
     map.put(SkyFunctions.LOAD_SKYLARK_ASPECT, new ToplevelSkylarkAspectFunction());
     map.put(SkyFunctions.POST_CONFIGURED_TARGET,
@@ -1461,11 +1467,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   public abstract void invalidateTransientErrors();
 
   /** Configures a given set of configured targets. */
-  public EvaluationResult<ActionLookupValue> configureTargets(
+  EvaluationResult<ActionLookupValue> configureTargets(
       EventHandler eventHandler,
       List<ConfiguredTargetKey> values,
       List<AspectValueKey> aspectKeys,
-      boolean keepGoing)
+      boolean keepGoing,
+      int numThreads)
       throws InterruptedException {
     checkActive();
 
@@ -1473,9 +1480,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     for (AspectValueKey aspectKey : aspectKeys) {
       keys.add(aspectKey.getSkyKey());
     }
-    // Make sure to not run too many analysis threads. This can cause memory thrashing.
-    return buildDriver.evaluate(keys, keepGoing, ResourceUsage.getAvailableProcessors(),
-        eventHandler);
+    return buildDriver.evaluate(keys, keepGoing, numThreads, eventHandler);
   }
 
   /**