commit 718dfa2d7aec49df352785587436ab28d8d37088
author adonovan <adonovan@google.com> Tue Oct 27 12:47:59 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Oct 27 12:49:13 2020 -0700
tree b883a36450997c1b61a7fe5bde2eb34dff0901b5
parent 41b41947cb776455b33bdd6cf5466a2403f83388

starlark eval: avoid ImmutableSortedMap, which is slow

Instead, sort in toString.

RELNOTES: N/A
PiperOrigin-RevId: 339308485

src/main/java/net/starlark/java/eval/StarlarkSemantics.java

diff --git a/src/main/java/net/starlark/java/eval/StarlarkSemantics.java b/src/main/java/net/starlark/java/eval/StarlarkSemantics.java
index b070419..b83e24a 100644
--- a/src/main/java/net/starlark/java/eval/StarlarkSemantics.java
+++ b/src/main/java/net/starlark/java/eval/StarlarkSemantics.java
@@ -16,9 +16,9 @@
 package net.starlark.java.eval;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableSortedMap;
-import java.util.HashMap;
+import com.google.common.collect.ImmutableMap;
 import java.util.Map;
+import java.util.TreeMap;
 
 /**
  * A StarlarkSemantics is an immutable set of optional name/value pairs that affect the dynamic
@@ -43,15 +43,15 @@
 public final class StarlarkSemantics {
 
   /** Returns the empty semantics, in which every option has its default value. */
-  public static final StarlarkSemantics DEFAULT = new StarlarkSemantics(ImmutableSortedMap.of());
+  public static final StarlarkSemantics DEFAULT = new StarlarkSemantics(ImmutableMap.of());
 
   // A map entry must be accessed by Key iff its name has no [+-] prefix.
   // Key<Boolean> is permitted too.
-  // We use ImmutableSortedMap for the benefit of equals/hashCode/toString.
-  private final ImmutableSortedMap<String, Object> map;
+  // The map keys are sorted but we avoid ImmutableSortedMap due to observed inefficiency.
+  private final ImmutableMap<String, Object> map;
   private final int hashCode;
 
-  private StarlarkSemantics(ImmutableSortedMap<String, Object> map) {
+  private StarlarkSemantics(ImmutableMap<String, Object> map) {
     this.map = map;
     this.hashCode = map.hashCode();
   }
@@ -98,19 +98,19 @@
    * Returns a new builder that initially holds the same key/value pairs as this StarlarkSemantics.
    */
   public Builder toBuilder() {
-    return new Builder(new HashMap<>(map));
+    return new Builder(new TreeMap<>(map));
   }
 
   /** Returns a new empty builder. */
   public static Builder builder() {
-    return new Builder(new HashMap<>());
+    return new Builder(new TreeMap<>());
   }
 
   /** A Builder is a mutable container used to construct an immutable StarlarkSemantics. */
   public static final class Builder {
-    private final HashMap<String, Object> map;
+    private final TreeMap<String, Object> map;
 
-    private Builder(HashMap<String, Object> map) {
+    private Builder(TreeMap<String, Object> map) {
       this.map = map;
     }
 
@@ -139,7 +139,7 @@
 
     /** Returns an immutable StarlarkSemantics. */
     public StarlarkSemantics build() {
-      return new StarlarkSemantics(ImmutableSortedMap.copyOf(map));
+      return new StarlarkSemantics(ImmutableMap.copyOf(map));
     }
   }