cleaner way to make compact hashes the default

Author: moste00

src/main/java/org/truffleruby/core/hash/HashLiteralNode.java

@@ -30,14 +30,18 @@ protected HashLiteralNode(RubyNode[] keyValues) {
         this.keyValues = keyValues;
     }
 
-    public static final class BigHashConfiguredStrategy extends RubyBaseNode {
-        private BigHashConfiguredStrategy() {
-            isBuckets = getContext().getOptions().BIG_HASH_STRATEGY;
+    // This singleton serves no other purpose than being able to call .getContext() (an instance method) from
+    static final class CreateBigHashLiteralNode extends RubyBaseNode {
+        private CreateBigHashLiteralNode() {
         }
 
-        // dummy instance, so we can call getContext instance method, never used
-        private static final BigHashConfiguredStrategy bh = new BigHashConfiguredStrategy();
-        public static boolean isBuckets;
+        private static final CreateBigHashLiteralNode INSTANCE = new CreateBigHashLiteralNode();
+
+        private HashLiteralNode execute(RubyNode[] keyValues) {
+            return getContext().getOptions().BIG_HASH_STRATEGY_IS_BUCKETS
+                    ? new BucketsHashStore.GenericHashLiteralNode(keyValues)
+                    : new CompactHashStore.CompactHashLiteralNode(keyValues);
+        }
     }
 
     public static HashLiteralNode create(RubyNode[] keyValues) {
@@ -46,9 +50,7 @@ public static HashLiteralNode create(RubyNode[] keyValues) {
         } else if (keyValues.length <= PackedHashStoreLibrary.MAX_ENTRIES * 2) {
             return PackedHashStoreLibraryFactory.SmallHashLiteralNodeGen.create(keyValues);
         } else {
-            return BigHashConfiguredStrategy.isBuckets
-                    ? new BucketsHashStore.GenericHashLiteralNode(keyValues)
-                    : new CompactHashStore.CompactHashLiteralNode(keyValues);
+            return CreateBigHashLiteralNode.INSTANCE.execute(keyValues);
         }
     }
 

src/main/java/org/truffleruby/core/hash/library/PackedHashStoreLibrary.java

@@ -39,6 +39,7 @@
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Exclusive;
 import com.oracle.truffle.api.dsl.Cached.Shared;
+import com.oracle.truffle.api.dsl.GenerateInline;
 import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.ImportStatic;
 import com.oracle.truffle.api.dsl.Specialization;
@@ -208,6 +209,7 @@ static boolean set(Object[] store, RubyHash hash, Object key, Object value, bool
                 @Cached @Shared CompareHashKeysNode compareHashKeys,
                 @CachedLibrary(limit = "hashStrategyLimit()") HashStoreLibrary hashes,
                 @Cached InlinedConditionProfile withinCapacity,
+                @Cached(inline = true) PromoteToBigHashNode promoteToBigHash,
                 @Bind("this") Node node) {
 
             assert verify(store, hash);
@@ -235,19 +237,11 @@ static boolean set(Object[] store, RubyHash hash, Object key, Object value, bool
                 return true;
             }
 
-            promoteToBigHash(store, hash, size);
+            promoteToBigHash.execute(node, store, hash, size);
 
             hashes.set(hash.store, hash, key2, value, byIdentity);
             return true;
         }
-
-        private static void promoteToBigHash(Object[] store, RubyHash hash, int size) {
-            if (HashLiteralNode.BigHashConfiguredStrategy.isBuckets) {
-                promoteToBuckets(hash, store, size);
-            } else {
-                promoteToCompact(hash, store, size);
-            }
-        }
     }
 
     @ExportMessage
@@ -416,6 +410,21 @@ static boolean verify(Object[] store, RubyHash hash) {
     // endregion
     // region Nodes
 
+    @GenerateUncached
+    @GenerateInline
+    abstract static class PromoteToBigHashNode extends RubyBaseNode {
+        public abstract void execute(Node node, Object[] store, RubyHash hash, int size);
+
+        @Specialization
+        protected void promoteToBigHash(Object[] store, RubyHash hash, int size) {
+            if (getContext().getOptions().BIG_HASH_STRATEGY_IS_BUCKETS) {
+                promoteToBuckets(hash, store, size);
+            } else {
+                promoteToCompact(hash, store, size);
+            }
+        }
+    }
+
     @GenerateUncached
     @ImportStatic(HashGuards.class)
     public abstract static class LookupPackedEntryNode extends RubyBaseNode {

src/main/java/org/truffleruby/options/Options.java

@@ -94,7 +94,7 @@ public final class Options {
     /** --backtraces-interleave-java=false */
     public final boolean BACKTRACES_INTERLEAVE_JAVA;
     /** --buckets-big-hash=false */
-    public final boolean BIG_HASH_STRATEGY;
+    public final boolean BIG_HASH_STRATEGY_IS_BUCKETS;
     /** --backtraces-on-interrupt=false */
     public final boolean BACKTRACE_ON_INTERRUPT;
     /** --backtraces-sigalrm=!EMBEDDED */
@@ -247,7 +247,7 @@ public Options(Env env, OptionValues options, LanguageOptions languageOptions) {
         EXCEPTIONS_WARN_STACKOVERFLOW = options.get(OptionsCatalog.EXCEPTIONS_WARN_STACKOVERFLOW_KEY);
         EXCEPTIONS_WARN_OUT_OF_MEMORY = options.get(OptionsCatalog.EXCEPTIONS_WARN_OUT_OF_MEMORY_KEY);
         BACKTRACES_INTERLEAVE_JAVA = options.get(OptionsCatalog.BACKTRACES_INTERLEAVE_JAVA_KEY);
-        BIG_HASH_STRATEGY = options.get(OptionsCatalog.BIG_HASH_STRATEGY_KEY);
+        BIG_HASH_STRATEGY_IS_BUCKETS = options.get(OptionsCatalog.BIG_HASH_STRATEGY_IS_BUCKETS_KEY);
         BACKTRACE_ON_INTERRUPT = options.get(OptionsCatalog.BACKTRACE_ON_INTERRUPT_KEY);
         BACKTRACE_ON_SIGALRM = options.hasBeenSet(OptionsCatalog.BACKTRACE_ON_SIGALRM_KEY) ? options.get(OptionsCatalog.BACKTRACE_ON_SIGALRM_KEY) : !EMBEDDED;
         BACKTRACE_ON_RAISE = options.get(OptionsCatalog.BACKTRACE_ON_RAISE_KEY);
@@ -379,7 +379,7 @@ public Object fromDescriptor(OptionDescriptor descriptor) {
             case "ruby.backtraces-interleave-java":
                 return BACKTRACES_INTERLEAVE_JAVA;
             case "ruby.buckets-big-hash":
-                return BIG_HASH_STRATEGY;
+                return BIG_HASH_STRATEGY_IS_BUCKETS;
             case "ruby.backtraces-on-interrupt":
                 return BACKTRACE_ON_INTERRUPT;
             case "ruby.backtraces-sigalrm":

src/options.yml

@@ -118,7 +118,7 @@ EXPERT:
     BACKTRACES_OMIT_UNUSED: [backtraces-omit-unused, boolean, true, Omit backtraces that should be unused as they have pure rescue expressions]
 
     # Big Hash Strategy
-    BIG_HASH_STRATEGY: [buckets-big-hash, boolean, false, 'Whether to use chaining-style bukcets hash store for hash tables exceeding the small hash limit']
+    BIG_HASH_STRATEGY_IS_BUCKETS: [buckets-big-hash, boolean, false, 'Whether to use chaining-style bukcets hash store for hash tables exceeding the small hash limit']
 
     # Print backtraces at key events
     BACKTRACE_ON_INTERRUPT: [backtraces-on-interrupt, boolean, false, Show the backtraces of all Threads on Ctrl+C]

src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java

@@ -64,7 +64,7 @@ public final class OptionsCatalog {
     public static final OptionKey<Boolean> EXCEPTIONS_WARN_OUT_OF_MEMORY_KEY = new OptionKey<>(true);
     public static final OptionKey<Boolean> BACKTRACES_INTERLEAVE_JAVA_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> BACKTRACES_OMIT_UNUSED_KEY = new OptionKey<>(true);
-    public static final OptionKey<Boolean> BIG_HASH_STRATEGY_KEY = new OptionKey<>(false);
+    public static final OptionKey<Boolean> BIG_HASH_STRATEGY_IS_BUCKETS_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> BACKTRACE_ON_INTERRUPT_KEY = new OptionKey<>(false);
     public static final OptionKey<Boolean> BACKTRACE_ON_SIGALRM_KEY = new OptionKey<>(!EMBEDDED_KEY.getDefaultValue());
     public static final OptionKey<Boolean> BACKTRACE_ON_RAISE_KEY = new OptionKey<>(false);
@@ -518,8 +518,8 @@ public final class OptionsCatalog {
             .usageSyntax("")
             .build();
 
-    public static final OptionDescriptor BIG_HASH_STRATEGY = OptionDescriptor
-            .newBuilder(BIG_HASH_STRATEGY_KEY, "ruby.buckets-big-hash")
+    public static final OptionDescriptor BIG_HASH_STRATEGY_IS_BUCKETS = OptionDescriptor
+            .newBuilder(BIG_HASH_STRATEGY_IS_BUCKETS_KEY, "ruby.buckets-big-hash")
             .help("Whether to use chaining-style bukcets hash store for hash tables exceeding the small hash limit")
             .category(OptionCategory.EXPERT)
             .stability(OptionStability.EXPERIMENTAL)
@@ -1417,7 +1417,7 @@ public static OptionDescriptor fromName(String name) {
             case "ruby.backtraces-omit-unused":
                 return BACKTRACES_OMIT_UNUSED;
             case "ruby.buckets-big-hash":
-                return BIG_HASH_STRATEGY;
+                return BIG_HASH_STRATEGY_IS_BUCKETS;
             case "ruby.backtraces-on-interrupt":
                 return BACKTRACE_ON_INTERRUPT;
             case "ruby.backtraces-sigalrm":
@@ -1669,7 +1669,7 @@ public static OptionDescriptor[] allDescriptors() {
             EXCEPTIONS_WARN_OUT_OF_MEMORY,
             BACKTRACES_INTERLEAVE_JAVA,
             BACKTRACES_OMIT_UNUSED,
-            BIG_HASH_STRATEGY,
+            BIG_HASH_STRATEGY_IS_BUCKETS,
             BACKTRACE_ON_INTERRUPT,
             BACKTRACE_ON_SIGALRM,
             BACKTRACE_ON_RAISE,