From 50795b486fcd3abf615744d15f6c492bc3083f0e Mon Sep 17 00:00:00 2001 From: moste00 Date: Sat, 28 Oct 2023 13:58:00 +0200 Subject: [PATCH] cleaner way to make compact hashes the default --- .../core/hash/HashLiteralNode.java | 20 +++++++------- .../hash/library/PackedHashStoreLibrary.java | 27 ++++++++++++------- .../java/org/truffleruby/options/Options.java | 6 ++--- src/options.yml | 2 +- .../shared/options/OptionsCatalog.java | 10 +++---- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/truffleruby/core/hash/HashLiteralNode.java b/src/main/java/org/truffleruby/core/hash/HashLiteralNode.java index f3c97f069bb1..a024c8631afb 100644 --- a/src/main/java/org/truffleruby/core/hash/HashLiteralNode.java +++ b/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); } } diff --git a/src/main/java/org/truffleruby/core/hash/library/PackedHashStoreLibrary.java b/src/main/java/org/truffleruby/core/hash/library/PackedHashStoreLibrary.java index a8c0d3d7b2ec..1ddb31fadad1 100644 --- a/src/main/java/org/truffleruby/core/hash/library/PackedHashStoreLibrary.java +++ b/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 + 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 { diff --git a/src/main/java/org/truffleruby/options/Options.java b/src/main/java/org/truffleruby/options/Options.java index 543b937730b9..74e34a622b91 100644 --- a/src/main/java/org/truffleruby/options/Options.java +++ b/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": diff --git a/src/options.yml b/src/options.yml index fb09535c2380..a3d44c79e001 100644 --- a/src/options.yml +++ b/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] diff --git a/src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java b/src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java index e58607720a38..76b58a1bb4cb 100644 --- a/src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java +++ b/src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java @@ -64,7 +64,7 @@ public final class OptionsCatalog { public static final OptionKey EXCEPTIONS_WARN_OUT_OF_MEMORY_KEY = new OptionKey<>(true); public static final OptionKey BACKTRACES_INTERLEAVE_JAVA_KEY = new OptionKey<>(false); public static final OptionKey BACKTRACES_OMIT_UNUSED_KEY = new OptionKey<>(true); - public static final OptionKey BIG_HASH_STRATEGY_KEY = new OptionKey<>(false); + public static final OptionKey BIG_HASH_STRATEGY_IS_BUCKETS_KEY = new OptionKey<>(false); public static final OptionKey BACKTRACE_ON_INTERRUPT_KEY = new OptionKey<>(false); public static final OptionKey BACKTRACE_ON_SIGALRM_KEY = new OptionKey<>(!EMBEDDED_KEY.getDefaultValue()); public static final OptionKey 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,