commit a6d7dc78891a1bc8d8da19bbd29a5bb320034661
author Googler <noreply@google.com> Mon Aug 27 11:18:15 2018 -0700
committer Copybara-Service <copybara-piper@google.com> Mon Aug 27 11:19:59 2018 -0700
tree 033177504d68d1620c18b5a17742b283f8f3b921
parent 38673e893fad7e9b20735d49390bbad26a25b7bb

Use a ForkJoinPool instead of a PriorityBlockingQueue-based ThreadPoolExecutor
for the analysis phase. During the analysis phase, lots of very short actions
are executed (File, FileState, ...) and contention in the executor has a
substantial performance impact.

Contention has always been a problem here, but the move towards rightfully
using PriorityBlockingQueue can cause even more work to be executed under a
lock. While I couldn't measure substantial wall-time differences between
PriorityBlockingQueue and the previously used LinkedBlockingQueue, this change
improves performance by about 10% on a variety of experiments I have
conducted.

RELNOTES: None.
PiperOrigin-RevId: 210398382

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

diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
index e36e547..7ec802e 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -14,13 +14,13 @@
 package com.google.devtools.build.skyframe;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.skyframe.MemoizingEvaluator.EmittedEventState;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
 /**
@@ -42,37 +42,9 @@
       EventFilter storedEventFilter,
       ErrorInfoManager errorInfoManager,
       boolean keepGoing,
-      int threadCount,
-      DirtyTrackingProgressReceiver progressReceiver,
-      GraphInconsistencyReceiver graphInconsistencyReceiver) {
-    super(
-        graph,
-        graphVersion,
-        skyFunctions,
-        reporter,
-        emittedEventState,
-        storedEventFilter,
-        errorInfoManager,
-        keepGoing,
-        progressReceiver,
-        graphInconsistencyReceiver,
-        () -> AbstractQueueVisitor.createExecutorService(threadCount),
-        new SimpleCycleDetector(),
-        EvaluationVersionBehavior.MAX_CHILD_VERSIONS);
-  }
-
-  public ParallelEvaluator(
-      ProcessableGraph graph,
-      Version graphVersion,
-      ImmutableMap<SkyFunctionName, ? extends SkyFunction> skyFunctions,
-      final ExtendedEventHandler reporter,
-      EmittedEventState emittedEventState,
-      EventFilter storedEventFilter,
-      ErrorInfoManager errorInfoManager,
-      boolean keepGoing,
       DirtyTrackingProgressReceiver progressReceiver,
       GraphInconsistencyReceiver graphInconsistencyReceiver,
-      ExecutorService executorService,
+      Supplier<ExecutorService> executorService,
       CycleDetector cycleDetector,
       EvaluationVersionBehavior evaluationVersionBehavior) {
     super(
@@ -86,7 +58,7 @@
         keepGoing,
         progressReceiver,
         graphInconsistencyReceiver,
-        () -> executorService,
+        executorService,
         cycleDetector,
         evaluationVersionBehavior);
   }