Set default value of number of workers only on createWorkerPools.

Before there was 3 places with initialisation of default worker numbers:
1) Default value from `WorkerOptions.MultiResourceConverter` in case of null options
2) Default value from `createConfigFromOptions` in `WorkerPool`
3) Default value from `createWorkerPools` in `WorkerPool`

I left only 2nd way to determine the default value of size of worker pool.

PiperOrigin-RevId: 459472421
Change-Id: I35ff1dfb0e3855497ca701e365b3ace769a2f52d

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

@@ -49,28 +49,30 @@ public class WorkerOptions extends OptionsBase {
   /**
    * Defines a resource converter for named values in the form [name=]value, where the value is
    * {@link ResourceConverter.FLAG_SYNTAX}. If no name is provided (used when setting a default),
-   * the empty string is used as the key. The default value for unspecified mnemonics is {@value
-   * DEFAULT_VALUE}. "auto" currently returns the default.
+   * the empty string is used as the key. The default value for unspecified mnemonics is defined in
+   * {@link WorkerPool.createWorkerPools}. "auto" currently returns the default.
    */
   public static class MultiResourceConverter extends Converter.Contextless<Entry<String, Integer>> {
 
-    public static final int DEFAULT_VALUE = 4;
-
-    static ResourceConverter valueConverter =
-        new ResourceConverter(() -> DEFAULT_VALUE, 0, Integer.MAX_VALUE);
+    static final ResourceConverter valueConverter =
+        new ResourceConverter(() -> 0, 0, Integer.MAX_VALUE);
 
     @Override
     public Map.Entry<String, Integer> convert(String input) throws OptionsParsingException {
       // TODO(steinman): Make auto value return a reasonable multiplier of host capacity.
-      if (input == null || "null".equals(input)) {
-        input = "auto";
+      if (input == null || "null".equals(input) || "auto".equals(input)) {
+        return Maps.immutableEntry(null, null);
       }
       int pos = input.indexOf('=');
       if (pos < 0) {
         return Maps.immutableEntry("", valueConverter.convert(input, /*conversionContext=*/ null));
       }
       String name = input.substring(0, pos);
       String value = input.substring(pos + 1);
+      if ("auto".equals(value)) {
+        return Maps.immutableEntry(name, null);
+      }
+
       return Maps.immutableEntry(name, valueConverter.convert(value, /*conversionContext=*/ null));
     }
 

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

@@ -16,7 +16,6 @@
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.worker.WorkerOptions.MultiResourceConverter;
 import java.io.IOException;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -64,15 +63,14 @@ public WorkerPool(WorkerPoolConfig workerPoolConfig) {
     highPriorityWorkerMnemonics =
         ImmutableSet.copyOf((Iterable<String>) workerPoolConfig.getHighPriorityWorkers());
 
-    Map<String, Integer> config = createConfigFromOptions(workerPoolConfig.getWorkerMaxInstances());
-    Map<String, Integer> multiplexConfig =
-        createConfigFromOptions(workerPoolConfig.getWorkerMaxMultiplexInstances());
+    ImmutableMap<String, Integer> config =
+        createConfigFromOptions(workerPoolConfig.getWorkerMaxInstances(), DEFAULT_MAX_WORKERS);
+    ImmutableMap<String, Integer> multiplexConfig =
+        createConfigFromOptions(
+            workerPoolConfig.getWorkerMaxMultiplexInstances(), DEFAULT_MAX_MULTIPLEX_WORKERS);
 
-    workerPools =
-        createWorkerPools(workerPoolConfig.getWorkerFactory(), config, DEFAULT_MAX_WORKERS);
-    multiplexPools =
-        createWorkerPools(
-            workerPoolConfig.getWorkerFactory(), multiplexConfig, DEFAULT_MAX_MULTIPLEX_WORKERS);
+    workerPools = createWorkerPools(workerPoolConfig.getWorkerFactory(), config);
+    multiplexPools = createWorkerPools(workerPoolConfig.getWorkerFactory(), multiplexConfig);
   }
 
   public WorkerPoolConfig getWorkerPoolConfig() {
@@ -85,29 +83,28 @@ public WorkerPoolConfig getWorkerPoolConfig() {
    */
   @Nonnull
   private static ImmutableMap<String, Integer> createConfigFromOptions(
-      List<Entry<String, Integer>> options) {
+      List<Entry<String, Integer>> options, int defaultMaxWorkers) {
     LinkedHashMap<String, Integer> newConfigBuilder = new LinkedHashMap<>();
     for (Map.Entry<String, Integer> entry : options) {
-      newConfigBuilder.put(entry.getKey(), entry.getValue());
+      if (entry.getValue() != null) {
+        newConfigBuilder.put(entry.getKey(), entry.getValue());
+      } else if (entry.getKey() != null) {
+        newConfigBuilder.put(entry.getKey(), defaultMaxWorkers);
+      }
     }
-
     if (!newConfigBuilder.containsKey("")) {
       // Empty string gives the number of workers for any type of worker not explicitly specified.
-      // If no value is given, use the default, 2.
-      newConfigBuilder.put("", MultiResourceConverter.DEFAULT_VALUE);
+      // If no value is given, use the default.
+      newConfigBuilder.put("", defaultMaxWorkers);
     }
-
     return ImmutableMap.copyOf(newConfigBuilder);
   }
 
   private static ImmutableMap<String, SimpleWorkerPool> createWorkerPools(
-      WorkerFactory factory, Map<String, Integer> config, int defaultMaxWorkers) {
+      WorkerFactory factory, Map<String, Integer> config) {
     ImmutableMap.Builder<String, SimpleWorkerPool> workerPoolsBuilder = ImmutableMap.builder();
     config.forEach(
         (key, value) -> workerPoolsBuilder.put(key, new SimpleWorkerPool(factory, value)));
-    if (!config.containsKey("")) {
-      workerPoolsBuilder.put("", new SimpleWorkerPool(factory, defaultMaxWorkers));
-    }
     return workerPoolsBuilder.build();
   }
 

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

@@ -38,8 +38,7 @@ public void setUp() {
 
   @Test
   public void convert_mnemonicEqualsAuto_returnsDefault() throws OptionsParsingException {
-    assertThat(multiResourceConverter.convert("someMnemonic=auto").getValue())
-        .isEqualTo(MultiResourceConverter.DEFAULT_VALUE);
+    assertThat(multiResourceConverter.convert("someMnemonic=auto").getValue()).isNull();
   }
 
   @Test
@@ -51,8 +50,7 @@ public void convert_mnemonicEqualsKeyword_equalsResourceConverterConvertKeyword(
 
   @Test
   public void convert_auto_returnsDefault() throws OptionsParsingException {
-    assertThat(multiResourceConverter.convert("auto").getValue())
-        .isEqualTo(MultiResourceConverter.DEFAULT_VALUE);
+    assertThat(multiResourceConverter.convert("auto").getValue()).isNull();
   }
 
   @Test
@@ -70,6 +68,6 @@ public void convert_mnemonic_savesCorrectKey() throws Exception {
 
   @Test
   public void convert_auto_setsEmptyStringAKADefaultAsKey() throws Exception {
-    assertThat(multiResourceConverter.convert("auto").getKey()).isEmpty();
+    assertThat(multiResourceConverter.convert("auto").getKey()).isNull();
   }
 }