Use hash-based maps in BuildOptions.

Instead of `ImmutableSortedMap`, use a regular `ImmutableMap` that is constructed from sorted entries (to keep order deterministic). Hash-based lookups are much faster than binary search with ` LEXICAL_FRAGMENT_OPTIONS_COMPARATOR`.

Pays down some more of increased cpu seen when removing `BuildConfigurationKeyCache`.

PiperOrigin-RevId: 673010189
Change-Id: Idacee4d2e1cc1becb7245c37287a8e263b1f13c3

src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java

@@ -17,6 +17,7 @@
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.devtools.build.lib.skyframe.serialization.ImmutableMapCodecs.IMMUTABLE_MAP_CODEC;
 import static com.google.devtools.build.lib.skyframe.serialization.strings.UnsafeStringCodec.stringCodec;
+import static java.util.Comparator.naturalOrder;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
@@ -49,9 +50,11 @@
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -415,8 +418,23 @@ public Builder removeStarlarkOption(Label key) {
 
     public BuildOptions build() {
       return new BuildOptions(
-          ImmutableSortedMap.copyOf(fragmentOptions, LEXICAL_FRAGMENT_OPTIONS_COMPARATOR),
-          ImmutableSortedMap.copyOf(starlarkOptions));
+          sortedImmutableHashMap(fragmentOptions, LEXICAL_FRAGMENT_OPTIONS_COMPARATOR),
+          sortedImmutableHashMap(starlarkOptions, naturalOrder()));
+    }
+
+    /**
+     * Constructs a hash-based {@link ImmutableMap} copy of the given map, with an iteration order
+     * defined by the given key comparator.
+     *
+     * <p>The returned map has a deterministic iteration order but is <em>not</em> an {@link
+     * ImmutableSortedMap}, which uses binary search lookups. Hash-based lookups are expected to be
+     * much faster for build options.
+     */
+    private static <K, V> ImmutableMap<K, V> sortedImmutableHashMap(
+        Map<K, V> map, Comparator<K> keyComparator) {
+      List<Map.Entry<K, V>> entries = new ArrayList<>(map.entrySet());
+      entries.sort(Map.Entry.comparingByKey(keyComparator));
+      return ImmutableMap.copyOf(entries);
     }
 
     private final Map<Class<? extends FragmentOptions>, FragmentOptions> fragmentOptions =

src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java

@@ -29,6 +29,7 @@
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
+import com.google.devtools.common.options.Options;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.protobuf.ByteString;
 import com.google.testing.junit.testparameterinjector.TestParameter;
@@ -368,4 +369,43 @@ public void parsingResultMatchNullOption() throws Exception {
 
     assertThat(original.matches(parser)).isFalse();
   }
+
+  @Test
+  public void nativeOptionsOrderedLexicographically() {
+    var options1 = Options.getDefaults(DummyTestOptions.class);
+    var options2 = Options.getDefaults(SecondDummyTestOptions.class);
+
+    BuildOptions forward =
+        BuildOptions.builder().addFragmentOptions(options1).addFragmentOptions(options2).build();
+    BuildOptions backward =
+        BuildOptions.builder().addFragmentOptions(options2).addFragmentOptions(options1).build();
+
+    assertThat(forward.getFragmentClasses())
+        .isInOrder(BuildOptions.LEXICAL_FRAGMENT_OPTIONS_COMPARATOR);
+    assertThat(backward.getFragmentClasses())
+        .isInOrder(BuildOptions.LEXICAL_FRAGMENT_OPTIONS_COMPARATOR);
+    assertThat(forward.getNativeOptions()).containsExactly(options1, options2).inOrder();
+    assertThat(backward.getNativeOptions()).containsExactly(options1, options2).inOrder();
+  }
+
+  @Test
+  public void starlarkOptionsOrderedByLabel() {
+    Label label1 = Label.parseCanonicalUnchecked("//pkg:option1");
+    Label label2 = Label.parseCanonicalUnchecked("//pkg:option2");
+
+    BuildOptions forward =
+        BuildOptions.builder()
+            .addStarlarkOption(label1, true)
+            .addStarlarkOption(label2, false)
+            .build();
+    BuildOptions backward =
+        BuildOptions.builder()
+            .addStarlarkOption(label2, false)
+            .addStarlarkOption(label1, true)
+            .build();
+    assertThat(forward.getStarlarkOptions()).containsExactly(label1, true, label2, false).inOrder();
+    assertThat(backward.getStarlarkOptions())
+        .containsExactly(label1, true, label2, false)
+        .inOrder();
+  }
 }