Allow ConfiguredTargetFunction to release its CPU-bound semaphore during dep requests if SkyframeExecutor has reason to believe that those requests may not be CPU-bound.

PiperOrigin-RevId: 190844728

4 files changed, 282 insertions, 0 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 61377b4e90..17ffe47c77 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
@@ -126,12 +126,15 @@ public final class ConfiguredTargetFunction implements SkyFunction {
    */
   private final boolean storeTransitivePackagesForPackageRootResolution;
 
+  private final boolean shouldUnblockCpuWorkWhenFetchingDeps;
+
   ConfiguredTargetFunction(
       BuildViewProvider buildViewProvider,
       RuleClassProvider ruleClassProvider,
       Semaphore cpuBoundSemaphore,
       Supplier<Boolean> removeActionsAfterEvaluation,
       boolean storeTransitivePackagesForPackageRootResolution,
+      boolean shouldUnblockCpuWorkWhenFetchingDeps,
       BuildOptions defaultBuildOptions) {
     this.buildViewProvider = buildViewProvider;
     this.ruleClassProvider = ruleClassProvider;
@@ -139,12 +142,20 @@ public final class ConfiguredTargetFunction implements SkyFunction {
     this.removeActionsAfterEvaluation = Preconditions.checkNotNull(removeActionsAfterEvaluation);
     this.storeTransitivePackagesForPackageRootResolution =
         storeTransitivePackagesForPackageRootResolution;
+    this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps;
     this.defaultBuildOptions = defaultBuildOptions;
   }
 
   @Override
   public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunctionException,
       InterruptedException {
+    if (shouldUnblockCpuWorkWhenFetchingDeps) {
+      env =
+          new StateInformingSkyFunctionEnvironment(
+              env,
+              /*preFetch=*/ cpuBoundSemaphore::release,
+              /*postFetch=*/ cpuBoundSemaphore::acquire);
+    }
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
     NestedSetBuilder<Package> transitivePackagesForPackageRootResolution =
         storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null;
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
index 3c5769597b..066fbcac7f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -165,6 +165,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         crossRepositoryLabelViolationStrategy,
         buildFilesByPriority,
         actionOnIOExceptionReadingBuildFile,
+        /*shouldUnblockCpuWorkWhenFetchingDeps=*/ false,
         defaultBuildOptions,
         new PackageProgressReceiver());
     this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories);
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 95f28737bd..06bf59f9de 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
@@ -294,6 +294,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
 
   private final ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile;
 
+  private final boolean shouldUnblockCpuWorkWhenFetchingDeps;
+
   private final BuildOptions defaultBuildOptions;
 
   private PerBuildSyscallCache perBuildSyscallCache;
@@ -316,12 +318,14 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy,
       List<BuildFileName> buildFilesByPriority,
       ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile,
+      boolean shouldUnblockCpuWorkWhenFetchingDeps,
       BuildOptions defaultBuildOptions,
       @Nullable PackageProgressReceiver packageProgress) {
     // Strictly speaking, these arguments are not required for initialization, but all current
     // callsites have them at hand, so we might as well set them during construction.
     this.evaluatorSupplier = evaluatorSupplier;
     this.pkgFactory = pkgFactory;
+    this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps;
     this.pkgFactory.setSyscalls(syscalls);
     this.workspaceStatusActionFactory = workspaceStatusActionFactory;
     this.packageManager = new SkyframePackageManager(
@@ -435,6 +439,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
             cpuBoundSemaphore,
             removeActionsAfterEvaluation,
             shouldStoreTransitivePackagesInLoadingAndAnalysis(),
+            shouldUnblockCpuWorkWhenFetchingDeps,
             defaultBuildOptions));
     map.put(
         SkyFunctions.ASPECT,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java
new file mode 100644
index 0000000000..8e4d7d5e02
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java
@@ -0,0 +1,265 @@
+// Copyright 2018 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;
+
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.util.GroupedList;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.ValueOrException;
+import com.google.devtools.build.skyframe.ValueOrException2;
+import com.google.devtools.build.skyframe.ValueOrException3;
+import com.google.devtools.build.skyframe.ValueOrException4;
+import com.google.devtools.build.skyframe.ValueOrException5;
+import java.util.Map;
+import javax.annotation.Nullable;
+
+/** An environment that wraps each call to its delegate by informing injected {@link Informee}s. */
+class StateInformingSkyFunctionEnvironment implements SkyFunction.Environment {
+  private final SkyFunction.Environment delegate;
+  private final Informee preFetch;
+  private final Informee postFetch;
+
+  StateInformingSkyFunctionEnvironment(
+      SkyFunction.Environment delegate, Informee preFetch, Informee postFetch) {
+    this.delegate = delegate;
+    this.preFetch = preFetch;
+    this.postFetch = postFetch;
+  }
+
+  @Nullable
+  @Override
+  public SkyValue getValue(SkyKey valueName) throws InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValue(valueName);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Nullable
+  @Override
+  public <E extends Exception> SkyValue getValueOrThrow(SkyKey depKey, Class<E> exceptionClass)
+      throws E, InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValueOrThrow(depKey, exceptionClass);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Nullable
+  @Override
+  public <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(
+      SkyKey depKey, Class<E1> exceptionClass1, Class<E2> exceptionClass2)
+      throws E1, E2, InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValueOrThrow(depKey, exceptionClass1, exceptionClass2);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Nullable
+  @Override
+  public <E1 extends Exception, E2 extends Exception, E3 extends Exception>
+      SkyValue getValueOrThrow(
+          SkyKey depKey,
+          Class<E1> exceptionClass1,
+          Class<E2> exceptionClass2,
+          Class<E3> exceptionClass3)
+          throws E1, E2, E3, InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValueOrThrow(depKey, exceptionClass1, exceptionClass2, exceptionClass3);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Nullable
+  @Override
+  public <E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception>
+      SkyValue getValueOrThrow(
+          SkyKey depKey,
+          Class<E1> exceptionClass1,
+          Class<E2> exceptionClass2,
+          Class<E3> exceptionClass3,
+          Class<E4> exceptionClass4)
+          throws E1, E2, E3, E4, InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValueOrThrow(
+          depKey, exceptionClass1, exceptionClass2, exceptionClass3, exceptionClass4);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Nullable
+  @Override
+  public <
+          E1 extends Exception,
+          E2 extends Exception,
+          E3 extends Exception,
+          E4 extends Exception,
+          E5 extends Exception>
+      SkyValue getValueOrThrow(
+          SkyKey depKey,
+          Class<E1> exceptionClass1,
+          Class<E2> exceptionClass2,
+          Class<E3> exceptionClass3,
+          Class<E4> exceptionClass4,
+          Class<E5> exceptionClass5)
+          throws E1, E2, E3, E4, E5, InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValueOrThrow(
+          depKey,
+          exceptionClass1,
+          exceptionClass2,
+          exceptionClass3,
+          exceptionClass4,
+          exceptionClass5);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public Map<SkyKey, SkyValue> getValues(Iterable<SkyKey> depKeys) throws InterruptedException {
+    preFetch.inform();
+    try {
+    return delegate.getValues(depKeys);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public <E extends Exception> Map<SkyKey, ValueOrException<E>> getValuesOrThrow(
+      Iterable<? extends SkyKey> depKeys, Class<E> exceptionClass) throws InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValuesOrThrow(depKeys, exceptionClass);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public <E1 extends Exception, E2 extends Exception>
+      Map<SkyKey, ValueOrException2<E1, E2>> getValuesOrThrow(
+          Iterable<? extends SkyKey> depKeys, Class<E1> exceptionClass1, Class<E2> exceptionClass2)
+          throws InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValuesOrThrow(depKeys, exceptionClass1, exceptionClass2);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public <E1 extends Exception, E2 extends Exception, E3 extends Exception>
+      Map<SkyKey, ValueOrException3<E1, E2, E3>> getValuesOrThrow(
+          Iterable<? extends SkyKey> depKeys,
+          Class<E1> exceptionClass1,
+          Class<E2> exceptionClass2,
+          Class<E3> exceptionClass3)
+          throws InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValuesOrThrow(depKeys, exceptionClass1, exceptionClass2, exceptionClass3);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public <E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception>
+      Map<SkyKey, ValueOrException4<E1, E2, E3, E4>> getValuesOrThrow(
+          Iterable<? extends SkyKey> depKeys,
+          Class<E1> exceptionClass1,
+          Class<E2> exceptionClass2,
+          Class<E3> exceptionClass3,
+          Class<E4> exceptionClass4)
+          throws InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValuesOrThrow(
+          depKeys, exceptionClass1, exceptionClass2, exceptionClass3, exceptionClass4);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public <
+          E1 extends Exception,
+          E2 extends Exception,
+          E3 extends Exception,
+          E4 extends Exception,
+          E5 extends Exception>
+      Map<SkyKey, ValueOrException5<E1, E2, E3, E4, E5>> getValuesOrThrow(
+          Iterable<? extends SkyKey> depKeys,
+          Class<E1> exceptionClass1,
+          Class<E2> exceptionClass2,
+          Class<E3> exceptionClass3,
+          Class<E4> exceptionClass4,
+          Class<E5> exceptionClass5)
+          throws InterruptedException {
+    preFetch.inform();
+    try {
+      return delegate.getValuesOrThrow(
+          depKeys,
+          exceptionClass1,
+          exceptionClass2,
+          exceptionClass3,
+          exceptionClass4,
+          exceptionClass5);
+    } finally {
+      postFetch.inform();
+    }
+  }
+
+  @Override
+  public boolean valuesMissing() {
+    return delegate.valuesMissing();
+  }
+
+  @Override
+  public ExtendedEventHandler getListener() {
+    return delegate.getListener();
+  }
+
+  @Override
+  public boolean inErrorBubblingForTesting() {
+    return delegate.inErrorBubblingForTesting();
+  }
+
+  @Nullable
+  @Override
+  public GroupedList<SkyKey> getTemporaryDirectDeps() {
+    return delegate.getTemporaryDirectDeps();
+  }
+
+  interface Informee {
+    void inform() throws InterruptedException;
+  }
+}