Cleanup: decouple the WorkerFactory from WorkerPoolConfig.

PiperOrigin-RevId: 626031767
Change-Id: I89996e782ab7daf85e1d2edbc5f2b5c806699767

src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java

@@ -48,7 +48,7 @@ public class WorkerModule extends BlazeModule {
   private static final String STALE_TRASH = "_stale_trash";
   private CommandEnvironment env;
 
-  private WorkerFactory workerFactory;
+  @VisibleForTesting WorkerFactory workerFactory;
   private AsynchronousTreeDeleter treeDeleter;
 
   WorkerPoolConfig config;
@@ -160,13 +160,12 @@ public void buildStarting(BuildStartingEvent event) {
 
     WorkerPoolConfig newConfig =
         new WorkerPoolConfig(
-            workerFactory,
             options.useNewWorkerPool,
             options.workerMaxInstances,
             options.workerMaxMultiplexInstances);
 
     // If the config changed compared to the last run, we have to create a new pool.
-    if (workerPool == null || !newConfig.equals(config)) {
+    if (!newConfig.equals(config)) {
       shutdownPool(
           "Worker pool configuration has changed, restarting worker pool...",
           /* alwaysLog= */ true,
@@ -175,9 +174,9 @@ public void buildStarting(BuildStartingEvent event) {
 
     if (workerPool == null) {
       if (options.useNewWorkerPool) {
-        workerPool = new WorkerPoolImpl(newConfig);
+        workerPool = new WorkerPoolImpl(workerFactory, newConfig);
       } else {
-        workerPool = new WorkerPoolImplLegacy(newConfig);
+        workerPool = new WorkerPoolImplLegacy(workerFactory, newConfig);
       }
       config = newConfig;
       // If workerPool is restarted then we should recreate metrics.

src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java

@@ -24,38 +24,29 @@
  * workers.
  */
 public class WorkerPoolConfig {
-  private final WorkerFactory workerFactory;
   private final boolean useNewWorkerPool;
   private final List<Entry<String, Integer>> workerMaxInstances;
   private final List<Entry<String, Integer>> workerMaxMultiplexInstances;
 
   public WorkerPoolConfig(
-      WorkerFactory workerFactory,
       boolean useNewWorkerPool,
       List<Entry<String, Integer>> workerMaxInstances,
       List<Entry<String, Integer>> workerMaxMultiplexInstances) {
-    this.workerFactory = workerFactory;
     this.useNewWorkerPool = useNewWorkerPool;
     this.workerMaxInstances = workerMaxInstances;
     this.workerMaxMultiplexInstances = workerMaxMultiplexInstances;
   }
 
   @VisibleForTesting
   public WorkerPoolConfig(
-      WorkerFactory workerFactory,
       List<Entry<String, Integer>> workerMaxInstances,
       List<Entry<String, Integer>> workerMaxMultiplexInstances) {
     this(
-        workerFactory,
         /* useNewWorkerPool= */ false,
         workerMaxInstances,
         workerMaxMultiplexInstances);
   }
 
-  public WorkerFactory getWorkerFactory() {
-    return workerFactory;
-  }
-
   public List<Entry<String, Integer>> getWorkerMaxInstances() {
     return workerMaxInstances;
   }
@@ -72,15 +63,13 @@ public boolean equals(Object o) {
     if (!(o instanceof WorkerPoolConfig that)) {
       return false;
     }
-    return workerFactory.equals(that.workerFactory)
-        && useNewWorkerPool == that.useNewWorkerPool
+    return useNewWorkerPool == that.useNewWorkerPool
         && workerMaxInstances.equals(that.workerMaxInstances)
         && workerMaxMultiplexInstances.equals(that.workerMaxMultiplexInstances);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(
-        workerFactory, useNewWorkerPool, workerMaxInstances, workerMaxMultiplexInstances);
+    return Objects.hash(useNewWorkerPool, workerMaxInstances, workerMaxMultiplexInstances);
   }
 }

src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImpl.java

@@ -72,8 +72,8 @@ public class WorkerPoolImpl implements WorkerPool {
   private final ImmutableMap<String, Integer> multiplexMaxInstances;
   private final ConcurrentHashMap<WorkerKey, WorkerKeyPool> pools = new ConcurrentHashMap<>();
 
-  public WorkerPoolImpl(WorkerPoolConfig config) {
-    this.factory = config.getWorkerFactory();
+  public WorkerPoolImpl(WorkerFactory factory, WorkerPoolConfig config) {
+    this.factory = factory;
     this.singleplexMaxInstances =
         getMaxInstances(config.getWorkerMaxInstances(), DEFAULT_MAX_SINGLEPLEX_WORKERS);
     this.multiplexMaxInstances =

src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImplLegacy.java

@@ -58,7 +58,7 @@ public class WorkerPoolImplLegacy implements WorkerPool {
   /** Map of multiplex worker pools, one per mnemonic. */
   private final ImmutableMap<String, SimpleWorkerPool> multiplexPools;
 
-  public WorkerPoolImplLegacy(WorkerPoolConfig workerPoolConfig) {
+  public WorkerPoolImplLegacy(WorkerFactory factory, WorkerPoolConfig workerPoolConfig) {
     this.workerPoolConfig = workerPoolConfig;
 
     ImmutableMap<String, Integer> config =
@@ -67,8 +67,8 @@ public WorkerPoolImplLegacy(WorkerPoolConfig workerPoolConfig) {
         createConfigFromOptions(
             workerPoolConfig.getWorkerMaxMultiplexInstances(), DEFAULT_MAX_MULTIPLEX_WORKERS);
 
-    workerPools = createWorkerPools(workerPoolConfig.getWorkerFactory(), config);
-    multiplexPools = createWorkerPools(workerPoolConfig.getWorkerFactory(), multiplexConfig);
+    workerPools = createWorkerPools(factory, config);
+    multiplexPools = createWorkerPools(factory, multiplexConfig);
   }
 
   public WorkerPoolConfig getWorkerPoolConfig() {

src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java

@@ -89,16 +89,16 @@ public static List<Object[]> data() throws IOException {
             (WorkerPoolSupplier)
                 (factory, singleplexMaxInstances, multiplexMaxInstances) ->
                     new WorkerPoolImplLegacy(
-                        new WorkerPoolConfig(
-                            factory, singleplexMaxInstances, multiplexMaxInstances)),
+                        factory,
+                        new WorkerPoolConfig(singleplexMaxInstances, multiplexMaxInstances)),
             workerFactorySupplier,
           },
           {
             (WorkerPoolSupplier)
                 (factory, singleplexMaxInstances, multiplexMaxInstances) ->
                     new WorkerPoolImpl(
-                        new WorkerPoolConfig(
-                            factory, singleplexMaxInstances, multiplexMaxInstances)),
+                        factory,
+                        new WorkerPoolConfig(singleplexMaxInstances, multiplexMaxInstances)),
             workerFactorySupplier,
           }
         });

src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java

@@ -138,7 +138,7 @@ public void buildStarting_restartsOnOutputbaseChanges() throws IOException, Abru
         .contains("Worker factory configuration has changed");
     assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
     WorkerKey workerKey = WorkerTestUtils.createWorkerKey(fs, "mnemonic", false);
-    module.getWorkerPoolConfig().getWorkerFactory().create(workerKey);
+    module.workerFactory.create(workerKey);
     assertThat(fs.getPath("/otherRootDir/outputBase/bazel-workers").exists()).isTrue();
     assertThat(oldLog.exists()).isTrue();
   }

src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java

@@ -86,8 +86,8 @@ public static List<Object[]> data() throws IOException {
             (WorkerPoolSupplier)
                 (factory) ->
                     new WorkerPoolImplLegacy(
+                        factory,
                         new WorkerPoolConfig(
-                            factory,
                             /* workerMaxInstances= */ ImmutableList.of(
                                 Maps.immutableEntry("mnem", 2), Maps.immutableEntry("", 1)),
                             /* workerMaxMultiplexInstances= */ ImmutableList.of(
@@ -98,8 +98,8 @@ public static List<Object[]> data() throws IOException {
             (WorkerPoolSupplier)
                 (factory) ->
                     new WorkerPoolImpl(
+                        factory,
                         new WorkerPoolConfig(
-                            factory,
                             /* workerMaxInstances= */ ImmutableList.of(
                                 Maps.immutableEntry("mnem", 2)),
                             /* workerMaxMultiplexInstances= */ ImmutableList.of(