Skip to content

Commit 2065438

Browse files
coeuvremeisterT
andauthored
Remove option to disable FJP. (#18791)
FJP has been enabled for a quite a while virtually everywhere. PiperOrigin-RevId: 501512884 Change-Id: I7c05252463cb53357db5328467a4ddb46ae17ea5 Co-authored-by: Googler <[email protected]>
1 parent 946777a commit 2065438

File tree

9 files changed

+16
-65
lines changed

9 files changed

+16
-65
lines changed

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,15 @@ public static final class AllCommandGraveyardOptions extends OptionsBase {
483483
effectTags = {OptionEffectTag.NO_OP},
484484
help = "No-op")
485485
public boolean incompatibleDisableThirdPartyLicenseChecking;
486+
487+
@Option(
488+
name = "experimental_use_fork_join_pool",
489+
defaultValue = "true",
490+
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
491+
metadataTags = OptionMetadataTag.DEPRECATED,
492+
effectTags = {OptionEffectTag.NO_OP},
493+
help = "No-op.")
494+
public boolean useForkJoinPool;
486495
}
487496

488497
@Override

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -463,15 +463,6 @@ public boolean useTopLevelTargetsForSymlinks() {
463463
+ "https://github.com/bazelbuild/bazel/issues/8651")
464464
public boolean incompatibleSkipGenfilesSymlink;
465465

466-
@Option(
467-
name = "experimental_use_fork_join_pool",
468-
defaultValue = "false",
469-
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
470-
metadataTags = OptionMetadataTag.EXPERIMENTAL,
471-
effectTags = {OptionEffectTag.EXECUTION},
472-
help = "If this flag is set, use a fork join pool in the abstract queue visitor.")
473-
public boolean useForkJoinPool;
474-
475466
@Option(
476467
name = "experimental_replay_action_out_err",
477468
defaultValue = "false",

src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.concurrent.CountDownLatch;
3030
import java.util.concurrent.ExecutorService;
3131
import java.util.concurrent.ForkJoinPool;
32-
import java.util.concurrent.PriorityBlockingQueue;
3332
import java.util.concurrent.RejectedExecutionException;
3433
import java.util.concurrent.ThreadPoolExecutor;
3534
import java.util.concurrent.TimeUnit;
@@ -147,9 +146,6 @@ private static ExecutorService createExecutorService(
147146
BlockingQueue<Runnable> workQueue,
148147
String poolName) {
149148

150-
if ("1".equals(System.getProperty("experimental_use_fork_join_pool"))) {
151-
return new NamedForkJoinPool(poolName, parallelism);
152-
}
153149
return new ThreadPoolExecutor(
154150
/*corePoolSize=*/ parallelism,
155151
/*maximumPoolSize=*/ parallelism,
@@ -161,21 +157,8 @@ private static ExecutorService createExecutorService(
161157
.build());
162158
}
163159

164-
public static ExecutorService createExecutorService(
165-
int parallelism, String poolName, boolean useForkJoinPool) {
166-
if (useForkJoinPool) {
167-
return new NamedForkJoinPool(poolName, parallelism);
168-
}
169-
return createExecutorService(parallelism, poolName);
170-
}
171-
172160
public static ExecutorService createExecutorService(int parallelism, String poolName) {
173-
return createExecutorService(
174-
parallelism,
175-
/*keepAliveTime=*/ 1,
176-
TimeUnit.SECONDS,
177-
new PriorityBlockingQueue<>(),
178-
poolName);
161+
return new NamedForkJoinPool(poolName, parallelism);
179162
}
180163

181164
public static AbstractQueueVisitor createWithExecutorService(

src/main/java/com/google/devtools/build/lib/query2/common/QueryTransitivePackagePreloader.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ public void preloadTransitiveTargets(
146146
.setKeepGoing(keepGoing)
147147
.setNumThreads(parallelThreads)
148148
.setEventHandler(eventHandler)
149-
.setUseForkJoinPool(true)
150149
.build();
151150
EvaluationResult<SkyValue> result =
152151
memoizingEvaluatorSupplier.get().evaluate(valueNames, evaluationContext);

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,7 +1506,6 @@ public EvaluationResult<?> buildArtifacts(
15061506
newEvaluationContextBuilder()
15071507
.setKeepGoing(options.getOptions(KeepGoingOption.class).keepGoing)
15081508
.setNumThreads(options.getOptions(BuildRequestOptions.class).jobs)
1509-
.setUseForkJoinPool(options.getOptions(BuildRequestOptions.class).useForkJoinPool)
15101509
.setEventHandler(reporter)
15111510
.setExecutionPhase()
15121511
.build();
@@ -1643,7 +1642,6 @@ EvaluationResult<SkyValue> targetPatterns(
16431642
.setKeepGoing(keepGoing)
16441643
.setNumThreads(numThreads)
16451644
.setEventHandler(eventHandler)
1646-
.setUseForkJoinPool(true)
16471645
.build();
16481646
return memoizingEvaluator.evaluate(patternSkyKeys, evaluationContext);
16491647
}

src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,8 @@ private static Supplier<QuiescingExecutor> getQuiescingExecutorSupplier(
179179
MultiExecutorQueueVisitor.createWithExecutorServices(
180180
executorService.get(),
181181
AbstractQueueVisitor.createExecutorService(
182-
/*parallelism=*/ cpuHeavySkyKeysThreadPoolSize,
183-
"skyframe-evaluator-cpu-heavy",
184-
// FJP performs much better on machines with many cores.
185-
/*useForkJoinPool=*/ true),
186-
/*failFastOnException=*/ true,
182+
/* parallelism= */ cpuHeavySkyKeysThreadPoolSize, "skyframe-evaluator-cpu-heavy"),
183+
/* failFastOnException= */ true,
187184
NodeEntryVisitor.NODE_ENTRY_VISITOR_ERROR_CLASSIFIER);
188185
}
189186
// We only consider the experimental case of merged Skyframe phases WITH a separate pool for
@@ -192,16 +189,10 @@ private static Supplier<QuiescingExecutor> getQuiescingExecutorSupplier(
192189
MultiExecutorQueueVisitor.createWithExecutorServices(
193190
executorService.get(),
194191
AbstractQueueVisitor.createExecutorService(
195-
/*parallelism=*/ cpuHeavySkyKeysThreadPoolSize,
196-
"skyframe-evaluator-cpu-heavy",
197-
// FJP performs much better on machines with many cores.
198-
/*useForkJoinPool=*/ true),
192+
/* parallelism= */ cpuHeavySkyKeysThreadPoolSize, "skyframe-evaluator-cpu-heavy"),
199193
AbstractQueueVisitor.createExecutorService(
200-
/*parallelism=*/ executionJobsThreadPoolSize,
201-
"skyframe-evaluator-execution",
202-
// FJP performs much better on machines with many cores.
203-
/*useForkJoinPool=*/ true),
204-
/*failFastOnException=*/ true,
194+
/* parallelism= */ executionJobsThreadPoolSize, "skyframe-evaluator-execution"),
195+
/* failFastOnException= */ true,
205196
NodeEntryVisitor.NODE_ENTRY_VISITOR_ERROR_CLASSIFIER);
206197
}
207198

src/main/java/com/google/devtools/build/skyframe/EvaluationContext.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ public class EvaluationContext {
3333
@Nullable private final Supplier<ExecutorService> executorServiceSupplier;
3434
private final boolean keepGoing;
3535
private final ExtendedEventHandler eventHandler;
36-
private final boolean useForkJoinPool;
3736
private final boolean isExecutionPhase;
3837
private final int cpuHeavySkyKeysThreadPoolSize;
3938
private final int executionPhaseThreadPoolSize;
@@ -44,7 +43,6 @@ protected EvaluationContext(
4443
@Nullable Supplier<ExecutorService> executorServiceSupplier,
4544
boolean keepGoing,
4645
ExtendedEventHandler eventHandler,
47-
boolean useForkJoinPool,
4846
boolean isExecutionPhase,
4947
int cpuHeavySkyKeysThreadPoolSize,
5048
int executionPhaseThreadPoolSize,
@@ -54,7 +52,6 @@ protected EvaluationContext(
5452
this.executorServiceSupplier = executorServiceSupplier;
5553
this.keepGoing = keepGoing;
5654
this.eventHandler = Preconditions.checkNotNull(eventHandler);
57-
this.useForkJoinPool = useForkJoinPool;
5855
this.isExecutionPhase = isExecutionPhase;
5956
this.cpuHeavySkyKeysThreadPoolSize = cpuHeavySkyKeysThreadPoolSize;
6057
this.executionPhaseThreadPoolSize = executionPhaseThreadPoolSize;
@@ -77,10 +74,6 @@ public ExtendedEventHandler getEventHandler() {
7774
return eventHandler;
7875
}
7976

80-
public boolean getUseForkJoinPool() {
81-
return useForkJoinPool;
82-
}
83-
8477
/**
8578
* Returns the size of the thread pool for CPU-heavy tasks set by
8679
* --experimental_skyframe_cpu_heavy_skykeys_thread_pool_size.
@@ -157,7 +150,6 @@ public static class Builder {
157150
protected Supplier<ExecutorService> executorServiceSupplier;
158151
protected boolean keepGoing;
159152
protected ExtendedEventHandler eventHandler;
160-
protected boolean useForkJoinPool;
161153
protected int cpuHeavySkyKeysThreadPoolSize;
162154
protected int executionJobsThreadPoolSize = 0;
163155
protected boolean isExecutionPhase = false;
@@ -173,7 +165,6 @@ protected Builder copyFrom(EvaluationContext evaluationContext) {
173165
this.keepGoing = evaluationContext.keepGoing;
174166
this.eventHandler = evaluationContext.eventHandler;
175167
this.isExecutionPhase = evaluationContext.isExecutionPhase;
176-
this.useForkJoinPool = evaluationContext.useForkJoinPool;
177168
this.executionJobsThreadPoolSize = evaluationContext.executionPhaseThreadPoolSize;
178169
this.cpuHeavySkyKeysThreadPoolSize = evaluationContext.cpuHeavySkyKeysThreadPoolSize;
179170
this.unnecessaryTemporaryStateDropperReceiver =
@@ -205,12 +196,6 @@ public Builder setEventHandler(ExtendedEventHandler eventHandler) {
205196
return this;
206197
}
207198

208-
@CanIgnoreReturnValue
209-
public Builder setUseForkJoinPool(boolean useForkJoinPool) {
210-
this.useForkJoinPool = useForkJoinPool;
211-
return this;
212-
}
213-
214199
@CanIgnoreReturnValue
215200
public Builder setCPUHeavySkyKeysThreadPoolSize(int cpuHeavySkyKeysThreadPoolSize) {
216201
this.cpuHeavySkyKeysThreadPoolSize = cpuHeavySkyKeysThreadPoolSize;
@@ -242,7 +227,6 @@ public EvaluationContext build() {
242227
executorServiceSupplier,
243228
keepGoing,
244229
eventHandler,
245-
useForkJoinPool,
246230
isExecutionPhase,
247231
cpuHeavySkyKeysThreadPoolSize,
248232
executionJobsThreadPoolSize,

src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,7 @@ private static EvaluationContext ensureExecutorService(EvaluationContext evaluat
221221
.setExecutorServiceSupplier(
222222
() ->
223223
AbstractQueueVisitor.createExecutorService(
224-
evaluationContext.getParallelism(),
225-
"skyframe-evaluator",
226-
evaluationContext.getUseForkJoinPool()))
224+
evaluationContext.getParallelism(), "skyframe-evaluator"))
227225
.build();
228226
}
229227

src/test/java/com/google/devtools/build/lib/query2/common/QueryTransitivePackagePreloaderTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ public void setUpMocks() {
9292
when(contextBuilder.setKeepGoing(ArgumentMatchers.anyBoolean())).thenReturn(contextBuilder);
9393
when(contextBuilder.setNumThreads(ArgumentMatchers.anyInt())).thenReturn(contextBuilder);
9494
when(contextBuilder.setEventHandler(ArgumentMatchers.any())).thenReturn(contextBuilder);
95-
when(contextBuilder.setUseForkJoinPool(ArgumentMatchers.anyBoolean()))
96-
.thenReturn(contextBuilder);
9795
when(contextBuilder.build()).thenReturn(context);
9896
}
9997

0 commit comments

Comments
 (0)