Always track parallelism in EvaluationContext
EvaluationContext has 2 members for different configurations it is run with
(either is set, never both):
1. Integer numThreads -- this allows delegating the decision of what kind
ExecutorService to create to the user
2. Supplier<ExecutorService> executorService -- factory of ExecutorService
objects -- makes the user tied to what it returns
Some places in the code are interested in knowing the parallelism of the
executor, which currently is achieved by assuming the kind of thread pool we
would use in case numThreads is not set.
Change the EvaluationContext to always require and store the parallelism (even
if the ExecutorService supplier is specified). Rename the getter to
'getParallelism' to avoid confusion with the old concept.
PiperOrigin-RevId: 239826228
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 1bfbc06..fb99f12 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
@@ -2152,6 +2152,7 @@
EvaluationContext evaluationContext =
EvaluationContext.newBuilder()
.setKeepGoing(keepGoing)
+ .setNumThreads(numThreads)
.setExecutorServiceSupplier(
() -> NamedForkJoinPool.newNamedPool("skyframe-evaluator", numThreads))
.setEventHander(eventHandler)
diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluationContext.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationContext.java
index e457b59..4e1b579 100644
--- a/src/main/java/com/google/devtools/build/skyframe/EvaluationContext.java
+++ b/src/main/java/com/google/devtools/build/skyframe/EvaluationContext.java
@@ -17,6 +17,7 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.skyframe.WalkableGraph.WalkableGraphFactory;
+import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.function.Supplier;
import javax.annotation.Nullable;
@@ -26,28 +27,29 @@
* BuildDriver#evaluate} and {@link WalkableGraphFactory#prepareAndGet}
*/
public class EvaluationContext {
- @Nullable private final Integer numThreads;
- @Nullable private final Supplier<ExecutorService> executorService;
+ private final int numThreads;
+ @Nullable private final Supplier<ExecutorService> executorServiceSupplier;
private final boolean keepGoing;
private final ExtendedEventHandler eventHandler;
protected EvaluationContext(
- @Nullable Integer numThread,
- @Nullable Supplier<ExecutorService> executorService,
+ int numThreads,
+ @Nullable Supplier<ExecutorService> executorServiceSupplier,
boolean keepGoing,
ExtendedEventHandler eventHandler) {
- this.numThreads = numThread;
- this.executorService = executorService;
+ Preconditions.checkArgument(0 < numThreads, "numThreads must be positive");
+ this.numThreads = numThreads;
+ this.executorServiceSupplier = executorServiceSupplier;
this.keepGoing = keepGoing;
- this.eventHandler = eventHandler;
+ this.eventHandler = Preconditions.checkNotNull(eventHandler);
}
- public Integer getNumThreads() {
+ public int getParallelism() {
return numThreads;
}
- public Supplier<ExecutorService> getExecutorService() {
- return executorService;
+ public Optional<Supplier<ExecutorService>> getExecutorServiceSupplier() {
+ return Optional.ofNullable(executorServiceSupplier);
}
public boolean getKeepGoing() {
@@ -63,7 +65,7 @@
return this;
} else {
return new EvaluationContext(
- this.numThreads, this.executorService, keepGoing, this.eventHandler);
+ this.numThreads, this.executorServiceSupplier, keepGoing, this.eventHandler);
}
}
@@ -73,30 +75,28 @@
/** Builder for {@link EvaluationContext}. */
public static class Builder {
- private Integer numThread;
- private Supplier<ExecutorService> executorService;
+ private int numThreads;
+ private Supplier<ExecutorService> executorServiceSupplier;
private boolean keepGoing;
private ExtendedEventHandler eventHandler;
private Builder() {}
public Builder copyFrom(EvaluationContext evaluationContext) {
- this.numThread = evaluationContext.numThreads;
- this.executorService = evaluationContext.executorService;
+ this.numThreads = evaluationContext.numThreads;
+ this.executorServiceSupplier = evaluationContext.executorServiceSupplier;
this.keepGoing = evaluationContext.keepGoing;
this.eventHandler = evaluationContext.eventHandler;
return this;
}
- public Builder setNumThreads(int numThread) {
- this.numThread = numThread;
- this.executorService = null;
+ public Builder setNumThreads(int numThreads) {
+ this.numThreads = numThreads;
return this;
}
- public Builder setExecutorServiceSupplier(Supplier<ExecutorService> executorService) {
- this.executorService = executorService;
- this.numThread = null;
+ public Builder setExecutorServiceSupplier(Supplier<ExecutorService> executorServiceSupplier) {
+ this.executorServiceSupplier = executorServiceSupplier;
return this;
}
@@ -111,13 +111,7 @@
}
public EvaluationContext build() {
- Preconditions.checkState(
- (numThread == null && executorService != null)
- || (numThread != null && executorService == null),
- "Exactly one of numThread and executorService must be set. %s %s",
- numThread,
- executorService);
- return new EvaluationContext(numThread, executorService, keepGoing, eventHandler);
+ return new EvaluationContext(numThreads, executorServiceSupplier, keepGoing, eventHandler);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
index 2616646..0a647bf 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -194,11 +194,12 @@
evaluationContext.getKeepGoing(),
progressReceiver,
graphInconsistencyReceiver,
- evaluationContext.getExecutorService() == null
- ? () ->
- AbstractQueueVisitor.createExecutorService(
- evaluationContext.getNumThreads(), "skyframe-evaluator")
- : evaluationContext.getExecutorService(),
+ evaluationContext
+ .getExecutorServiceSupplier()
+ .orElse(
+ () ->
+ AbstractQueueVisitor.createExecutorService(
+ evaluationContext.getParallelism(), "skyframe-evaluator")),
new SimpleCycleDetector(),
EvaluationVersionBehavior.GRAPH_VERSION);
result = evaluator.eval(roots);
diff --git a/src/main/java/com/google/devtools/build/skyframe/SequentialBuildDriver.java b/src/main/java/com/google/devtools/build/skyframe/SequentialBuildDriver.java
index 963de0c..d9c5b13 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SequentialBuildDriver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SequentialBuildDriver.java
@@ -38,15 +38,16 @@
return memoizingEvaluator.evaluate(
roots,
curVersion,
- evaluationContext.getExecutorService() == null
- ? EvaluationContext.newBuilder()
+ evaluationContext.getExecutorServiceSupplier().isPresent()
+ ? evaluationContext
+ : EvaluationContext.newBuilder()
.copyFrom(evaluationContext)
+ .setNumThreads(evaluationContext.getParallelism())
.setExecutorServiceSupplier(
() ->
AbstractQueueVisitor.createExecutorService(
- evaluationContext.getNumThreads(), "skyframe-evaluator"))
- .build()
- : evaluationContext);
+ evaluationContext.getParallelism(), "skyframe-evaluator"))
+ .build());
} finally {
curVersion = curVersion.next();
}
