From 10c84d05fcec09bc2c050ded1186f7a40e8ab35a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Jul 2026 16:40:42 -0400 Subject: [PATCH 01/26] Add FlatHashtable: open-addressed find-or-create with static-polymorphism helpers A reusable open-addressed table over self-contained entries (one reference per slot), designed so callers specialize it via a concrete-typed static-final Helper that the JIT devirtualizes and inlines (C++-template-style static polymorphism). Cardinality cap / overflow / size are caller policy; this class is pure mechanism (capacityFor, create, get, getOrCreate). Includes a String-key StringHelper that seals a spread hash. Intended as the shared backing for several open-addressed caches/tables (per-operation sizing hints, UTF8BytesString caches, etc.). Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtable.java | 152 ++++++++++++++++++ .../datadog/trace/util/FlatHashtableTest.java | 92 +++++++++++ 2 files changed, 244 insertions(+) create mode 100644 internal-api/src/main/java/datadog/trace/util/FlatHashtable.java create mode 100644 internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java new file mode 100644 index 00000000000..ac9abf37522 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -0,0 +1,152 @@ +package datadog.trace.util; + +import java.lang.reflect.Array; + +/** + * Open-addressed, single-array find-or-create over self-contained entries — each slot is one + * reference to an entry that carries its own key (and, typically, a cached hash). One array, one + * reference per slot: entry publication is a single reference store, so a reader sees {@code null} or + * a complete entry (never a torn one), and {@code final} identity fields on the entry are visible + * under racy publication. That sidesteps the memory-ordering / visibility problems parallel + * key/hash/value arrays would create — no {@code volatile}, no atomics — as long as the payload is + * one where a stale/lost read is benign (miss → recreate; clobber → one wins). + * + *

Static polymorphism (C++-template-style). The per-use policy is a {@link Helper} — a + * stateless subclass held by each caller as a {@code static final} field declared with the + * concrete helper type (not the {@code Helper} base): + * + *

{@code
+ * private static final MyHelper HELPER = new MyHelper();  // concrete type => exact type pinned
+ * ...
+ * V v = FlatHashtable.getOrCreate(table, key, HELPER);
+ * }
+ * + * Because {@code HELPER} is a compile-time-constant of an exact type at the call site, once these + * small {@code Support} methods inline the JIT devirtualizes and inlines {@code hash}/{@code + * matches}/{@code create} — each call site specializes to straight-line code, one instantiation per + * helper, with no CHA/type-profiling dependence. Keep the methods small so they inline; verify with + * {@code -XX:+PrintInlining} (the failure mode is silent: it compiles and runs, just stays + * megamorphic and slow). {@code Helper} is an abstract class, so a distinct final subclass is required + * anyway — an exact type gives the inliner an unambiguous receiver. + * + *

Contract: {@code table.length} must be a power of two ({@link #capacityFor}). {@code + * helper.hash} should be well-distributed (this class masks it directly). Cardinality cap / overflow + * / a live-size counter are caller policy (this class is pure mechanism): a capped caller does + * {@link #get} first, and only on a miss checks its budget before {@link #getOrCreate} (so hits stay + * a single probe and the create path is warmup-rare). + */ +public final class FlatHashtable { + private FlatHashtable() {} + + /** + * Per-use policy. Extend as a stateless final class and hold a {@code static final} singleton + * of the concrete type (see class doc) so the JIT can specialize each call site. + * + *

An abstract class (not an interface) on purpose: it forces a named helper type (no + * lambdas, which can blur the receiver the inliner needs), and if specialization ever misses, the + * fallback dispatches via {@code invokevirtual} rather than the costlier megamorphic {@code + * invokeinterface}. On the specialized (inlined) path the choice is a wash — this just hedges the + * fallback and lets shared bits be {@code final}-sealed later. + * + * @param lookup key + * @param stored entry — self-contained (carries its own key, ideally a cached hash) + */ + public abstract static class Helper { + /** Hash of {@code key}; should be well-distributed (this table masks it directly). */ + public abstract int hash(K key); + + /** Whether the stored {@code value} entry is the one for {@code key}. */ + public abstract boolean matches(K key, V value); + + /** Mint a new entry for {@code key} (called once, on insert). */ + public abstract V create(K key); + } + + /** + * {@link Helper} specialized for {@code String} keys: seals a spread {@link #hash} so String-key + * callers write only {@link #matches} and {@link #create}. Extend as a stateless final class held in + * a concrete-typed {@code static final} singleton, exactly like {@link Helper} — the {@code final} + * hash resolves directly and the concrete subclass still specializes the same at each call site, so + * there's no cost to the extra layer. + * + * @param stored entry — self-contained (carries its own key, ideally a cached hash) + */ + public abstract static class StringHelper extends Helper { + @Override + public final int hash(String key) { + final int h = key.hashCode(); + return h ^ (h >>> 16); // spread; FlatHashtable masks this directly + } + } + + /** Power-of-two capacity for a cardinality budget: {@code >= 2 * limit} (load factor <= 0.5). */ + public static int capacityFor(int cardinalityLimit) { + if (cardinalityLimit <= 0) { + throw new IllegalArgumentException("cardinalityLimit must be positive: " + cardinalityLimit); + } + return Integer.highestOneBit(cardinalityLimit * 2 - 1) << 1; + } + + /** + * Allocates a correctly-typed table for a cardinality budget ({@link #capacityFor} slots). Passing + * {@code type} makes the array's runtime component type {@code T} rather than {@code Object[]} — + * typed reads, real array-store checks, and a monomorphic element type for the JIT. Callers can't + * {@code new T[]} themselves under erasure; this does the one reflective allocation at construction + * (off any hot path). Note: this {@code create} mints the backing array; {@link Helper#create} + * mints an entry — different types, no ambiguity at the call site. + */ + @SuppressWarnings("unchecked") + public static T[] create(Class type, int cardinalityLimit) { + return (T[]) Array.newInstance(type, capacityFor(cardinalityLimit)); + } + + /** + * Existing entry for {@code key}, or {@code null}. Read-only — never creates. Single probe on a + * hit; walks to the first empty slot (or all the way around) on a miss. + */ + public static V get(V[] table, K key, Helper helper) { + final int mask = table.length - 1; + final int start = helper.hash(key) & mask; + int i = start; + for (; ; ) { + final V e = table[i]; + if (e == null) { + return null; // empty slot terminates the probe (no tombstones) + } + if (helper.matches(key, e)) { + return e; + } + i = (i + 1) & mask; + if (i == start) { + return null; // wrapped ⇒ full, absent + } + } + } + + /** + * Existing entry for {@code key}, or a freshly {@link Helper#create created} + inserted one. + * Returns {@code null} only if the table is full (no empty slot) — the caller supplies its overflow + * default. The insert is a single plain reference store: a concurrent clobber / double-create is + * acceptable only when the payload makes it benign (see class doc). + */ + public static V getOrCreate(V[] table, K key, Helper helper) { + final int mask = table.length - 1; + final int start = helper.hash(key) & mask; + int i = start; + for (; ; ) { + final V e = table[i]; + if (e == null) { + final V created = helper.create(key); + table[i] = created; // single-reference publish; benign clobber (see class doc) + return created; + } + if (helper.matches(key, e)) { + return e; + } + i = (i + 1) & mask; + if (i == start) { + return null; // wrapped ⇒ full + } + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java new file mode 100644 index 00000000000..e6dba7391dc --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -0,0 +1,92 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class FlatHashtableTest { + + /** Self-contained entry: carries its own key (the identity FlatHashtable relies on). */ + static final class Entry { + final String key; + + Entry(String key) { + this.key = key; + } + } + + /** Stateless concrete helper, held as a concrete-typed static final singleton (the JIT idiom). */ + static final class EntryHelper extends FlatHashtable.StringHelper { + @Override + public boolean matches(String key, Entry value) { + return key.equals(value.key); + } + + @Override + public Entry create(String key) { + return new Entry(key); + } + } + + private static final EntryHelper HELPER = new EntryHelper(); + + @Test + void capacityFor_roundsToPowerOfTwoAtLeastTwiceLimit() { + assertEquals(2, FlatHashtable.capacityFor(1)); + assertEquals(8, FlatHashtable.capacityFor(4)); + assertEquals(16, FlatHashtable.capacityFor(6)); // 6*2-1=11 -> 8 -> 16 + } + + @Test + void capacityFor_rejectsNonPositive() { + assertThrows(IllegalArgumentException.class, () -> FlatHashtable.capacityFor(0)); + assertThrows(IllegalArgumentException.class, () -> FlatHashtable.capacityFor(-1)); + } + + @Test + void create_allocatesTypedTableOfCapacity() { + Entry[] table = FlatHashtable.create(Entry.class, 4); + assertEquals(8, table.length); + assertEquals(Entry.class, table.getClass().getComponentType()); + } + + @Test + void getOrCreate_insertsOnceAndReturnsTheExistingEntry() { + Entry[] table = FlatHashtable.create(Entry.class, 8); + Entry first = FlatHashtable.getOrCreate(table, "a", HELPER); + assertEquals("a", first.key); + // A second call must return the SAME instance, not mint a new one. + assertSame(first, FlatHashtable.getOrCreate(table, "a", HELPER)); + assertSame(first, FlatHashtable.get(table, "a", HELPER)); + } + + @Test + void get_returnsNullForAbsentKey() { + Entry[] table = FlatHashtable.create(Entry.class, 8); + assertNull(FlatHashtable.get(table, "missing", HELPER)); + FlatHashtable.getOrCreate(table, "present", HELPER); + assertNull(FlatHashtable.get(table, "still-missing", HELPER)); + } + + @Test + void getOrCreate_returnsNullWhenTableIsFull() { + // capacityFor(1) == 2 slots. + Entry[] table = FlatHashtable.create(Entry.class, 1); + assertTrue(FlatHashtable.getOrCreate(table, "k0", HELPER) != null); + assertTrue(FlatHashtable.getOrCreate(table, "k1", HELPER) != null); + // Both slots occupied by distinct keys -> a third distinct key finds no room. + assertNull(FlatHashtable.getOrCreate(table, "k2", HELPER)); + // ...but an existing key still resolves even when full. + assertSame( + FlatHashtable.get(table, "k0", HELPER), FlatHashtable.getOrCreate(table, "k0", HELPER)); + } + + @Test + void stringHelper_hashIsStableForEqualKeys() { + assertEquals(HELPER.hash("route"), HELPER.hash(new String("route"))); + } +} From 3746ea515f7585747eb8f370d3ccde954dbec486 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Jul 2026 19:31:50 -0400 Subject: [PATCH 02/26] Add FlatHashtable collision/probe coverage tests Force hash collisions via fixed-hash helpers to exercise the linear-probe paths the original tests missed: probe-past-occupied + match-after-probe (in both get and getOrCreate), wraparound to the front, and get()'s full-table wrap-to-null branch. Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtableTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index e6dba7391dc..9bebfa9c45a 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -1,6 +1,7 @@ package datadog.trace.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -34,6 +35,46 @@ public Entry create(String key) { private static final EntryHelper HELPER = new EntryHelper(); + /** All keys hash to slot 0, so inserts chain by linear probing — exercises the probe path. */ + static final class CollidingHelper extends FlatHashtable.Helper { + @Override + public int hash(String key) { + return 0; + } + + @Override + public boolean matches(String key, Entry value) { + return key.equals(value.key); + } + + @Override + public Entry create(String key) { + return new Entry(key); + } + } + + private static final CollidingHelper COLLIDING = new CollidingHelper(); + + /** All keys hash to the last slot ({@code -1 & mask}), so probing wraps around to index 0. */ + static final class LastSlotHelper extends FlatHashtable.Helper { + @Override + public int hash(String key) { + return -1; + } + + @Override + public boolean matches(String key, Entry value) { + return key.equals(value.key); + } + + @Override + public Entry create(String key) { + return new Entry(key); + } + } + + private static final LastSlotHelper LAST_SLOT = new LastSlotHelper(); + @Test void capacityFor_roundsToPowerOfTwoAtLeastTwiceLimit() { assertEquals(2, FlatHashtable.capacityFor(1)); @@ -89,4 +130,49 @@ void getOrCreate_returnsNullWhenTableIsFull() { void stringHelper_hashIsStableForEqualKeys() { assertEquals(HELPER.hash("route"), HELPER.hash(new String("route"))); } + + @Test + void collision_probesPastOccupiedSlots_andResolvesEach() { + Entry[] table = FlatHashtable.create(Entry.class, 4); // 8 slots; COLLIDING sends all to slot 0 + Entry a = FlatHashtable.getOrCreate(table, "a", COLLIDING); + Entry b = FlatHashtable.getOrCreate(table, "b", COLLIDING); // slot 0 taken -> probes to slot 1 + Entry c = FlatHashtable.getOrCreate(table, "c", COLLIDING); // -> slot 2 + + assertNotSame(a, b); + assertNotSame(b, c); + + // each resolves via probe-past-occupied + match-after-probe + assertSame(a, FlatHashtable.get(table, "a", COLLIDING)); + assertSame(b, FlatHashtable.get(table, "b", COLLIDING)); + assertSame(c, FlatHashtable.get(table, "c", COLLIDING)); + + // existing colliding key: found after probing, no new entry minted + assertSame(b, FlatHashtable.getOrCreate(table, "b", COLLIDING)); + + // absent key: probe past the 3 occupied slots, hit an empty slot -> null + assertNull(FlatHashtable.get(table, "absent", COLLIDING)); + } + + @Test + void collision_probeWrapsAroundToFront() { + Entry[] table = + FlatHashtable.create(Entry.class, 1); // 2 slots (0,1), mask=1; LAST_SLOT starts at 1 + Entry k0 = FlatHashtable.getOrCreate(table, "k0", LAST_SLOT); // -> slot 1 + Entry k1 = FlatHashtable.getOrCreate(table, "k1", LAST_SLOT); // slot 1 taken -> wraps to slot 0 + + assertNotSame(k0, k1); + assertSame(k0, FlatHashtable.get(table, "k0", LAST_SLOT)); + // start slot 1 is occupied (no match) -> probe wraps to slot 0 -> match + assertSame(k1, FlatHashtable.get(table, "k1", LAST_SLOT)); + } + + @Test + void get_returnsNullWhenTableFullAndKeyAbsent() { + Entry[] table = FlatHashtable.create(Entry.class, 1); // 2 slots + FlatHashtable.getOrCreate(table, "k0", COLLIDING); + FlatHashtable.getOrCreate(table, "k1", COLLIDING); // fills slots 0 and 1 + + // get() probes both occupied slots, wraps back to start -> null (get's full-wrap branch) + assertNull(FlatHashtable.get(table, "absent", COLLIDING)); + } } From bd24e27f91c522e3ea796953be51199dbafb76a2 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 10:55:41 -0400 Subject: [PATCH 03/26] Reshape FlatHashtable into KeyStrategy/CreateStrategy strategies KeyStrategy (abstract class): hash/matches/hashOf, long hashes for family consistency with Hashtable/ConcurrentHashtable. StringKeyStrategy seals the spread hash; EntryKeyStrategy seals hashOf to a cached Entry.hash (Entry is an optional structure-free base). CreateStrategy (bespoke @FunctionalInterface): create, cold+per-use, lambda-able. @Strategy on both strategy types; @StrategyConsumer on the inlining consumers (get/getOrCreate/insert). Surface: get/getOrCreate (key-taking) + two insert flavors (Entry-based / KeyStrategy-based) over a shared comparison-free placement core; forEach (+ context variant); hash-filtered read-only iterator (not a StrategyConsumer -- interface-dispatched cold traversal). No removal. Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtable.java | 342 ++++++++++++++---- .../datadog/trace/util/FlatHashtableTest.java | 252 ++++++++++--- 2 files changed, 471 insertions(+), 123 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index ac9abf37522..6a934167f2e 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -1,84 +1,153 @@ package datadog.trace.util; +import datadog.trace.api.function.Strategy; +import datadog.trace.api.function.StrategyConsumer; import java.lang.reflect.Array; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.function.BiConsumer; +import java.util.function.Consumer; /** * Open-addressed, single-array find-or-create over self-contained entries — each slot is one * reference to an entry that carries its own key (and, typically, a cached hash). One array, one - * reference per slot: entry publication is a single reference store, so a reader sees {@code null} or - * a complete entry (never a torn one), and {@code final} identity fields on the entry are visible - * under racy publication. That sidesteps the memory-ordering / visibility problems parallel + * reference per slot: entry publication is a single reference store, so a reader sees {@code null} + * or a complete entry (never a torn one), and {@code final} identity fields on the entry are + * visible under racy publication. That sidesteps the memory-ordering / visibility problems parallel * key/hash/value arrays would create — no {@code volatile}, no atomics — as long as the payload is * one where a stale/lost read is benign (miss → recreate; clobber → one wins). * - *

Static polymorphism (C++-template-style). The per-use policy is a {@link Helper} — a - * stateless subclass held by each caller as a {@code static final} field declared with the - * concrete helper type (not the {@code Helper} base): + *

Two strategies, split by concern. The per-use policy is two {@link Strategy strategy} + * objects rather than one: + * + *

* *
{@code
- * private static final MyHelper HELPER = new MyHelper();  // concrete type => exact type pinned
+ * private static final MyKeyStrategy KEYS = new MyKeyStrategy();   // concrete type => exact type pinned
  * ...
- * V v = FlatHashtable.getOrCreate(table, key, HELPER);
+ * E e = FlatHashtable.getOrCreate(table, key, KEYS, MyEntry::new); // non-capturing create
  * }
* - * Because {@code HELPER} is a compile-time-constant of an exact type at the call site, once these - * small {@code Support} methods inline the JIT devirtualizes and inlines {@code hash}/{@code - * matches}/{@code create} — each call site specializes to straight-line code, one instantiation per - * helper, with no CHA/type-profiling dependence. Keep the methods small so they inline; verify with - * {@code -XX:+PrintInlining} (the failure mode is silent: it compiles and runs, just stays - * megamorphic and slow). {@code Helper} is an abstract class, so a distinct final subclass is required - * anyway — an exact type gives the inliner an unambiguous receiver. - * *

Contract: {@code table.length} must be a power of two ({@link #capacityFor}). {@code - * helper.hash} should be well-distributed (this class masks it directly). Cardinality cap / overflow - * / a live-size counter are caller policy (this class is pure mechanism): a capped caller does - * {@link #get} first, and only on a miss checks its budget before {@link #getOrCreate} (so hits stay - * a single probe and the create path is warmup-rare). + * KeyStrategy.hash} should be well-distributed (this class masks it directly). Cardinality cap / + * overflow / a live-size counter are caller policy (this class is pure mechanism): a capped + * caller does {@link #get} first, and only on a miss checks its budget before {@link #getOrCreate} + * (so hits stay a single probe and the create path is warmup-rare). */ public final class FlatHashtable { private FlatHashtable() {} /** - * Per-use policy. Extend as a stateless final class and hold a {@code static final} singleton - * of the concrete type (see class doc) so the JIT can specialize each call site. + * Optional structure-free entry base carrying only a cached {@code hash} — an + * optimization, not plumbing (open addressing needs no {@code next}), so extending it is + * never required: bring any entry type and supply {@link KeyStrategy#hashOf} yourself instead. + * Caller contract: {@code hash} must equal the table's {@link KeyStrategy#hash} for this entry's + * key, so the entry lands where {@link #get} looks. + */ + public abstract static class Entry { + public final long hash; + + protected Entry(long hash) { + this.hash = hash; + } + } + + /** + * Key-identity strategy: how to {@link #hash} a lookup key and {@link #matches} it against a + * stored entry. Extend as a stateless final class and hold a {@code static final} + * singleton of the concrete type so the JIT can specialize each call site (see {@link Strategy}). * - *

An abstract class (not an interface) on purpose: it forces a named helper type (no - * lambdas, which can blur the receiver the inliner needs), and if specialization ever misses, the + *

An abstract class (not an interface) on purpose: it forces a named strategy type (no + * lambdas, which can blur the receiver the inliner needs), and if specialization ever misses the * fallback dispatches via {@code invokevirtual} rather than the costlier megamorphic {@code - * invokeinterface}. On the specialized (inlined) path the choice is a wash — this just hedges the - * fallback and lets shared bits be {@code final}-sealed later. + * invokeinterface}. Key-identity is the hot strategy (every probe), so it takes the + * abstract-class rigor; creation is the cold one, hence {@link CreateStrategy} is a lambda-able + * interface. * * @param lookup key - * @param stored entry — self-contained (carries its own key, ideally a cached hash) + * @param stored entry — self-contained (carries its own key) */ - public abstract static class Helper { - /** Hash of {@code key}; should be well-distributed (this table masks it directly). */ - public abstract int hash(K key); + @Strategy + public abstract static class KeyStrategy { + /** + * Hash of {@code key} ({@code long} for family-wide consistency with Hashtable / + * ConcurrentHashtable and to leave room for composite keys); should be well-distributed (this + * table masks it directly). + */ + public abstract long hash(K key); - /** Whether the stored {@code value} entry is the one for {@code key}. */ - public abstract boolean matches(K key, V value); + /** Whether the stored {@code entry} is the one for {@code key}. */ + public abstract boolean matches(K key, E entry); - /** Mint a new entry for {@code key} (called once, on insert). */ - public abstract V create(K key); + /** + * Hash of a stored {@code entry} — must equal {@link #hash}{@code (key)} for that entry's key, + * so the entry lands where {@link #get} would look for it. Used by the entry-taking {@link + * #insert(Object[], Object, KeyStrategy)} and {@link #iterator} (which have an entry, not a + * key); {@link #get} lookups never call it. When the entry can surface its key this is + * typically {@code hash(entry.key)}; when it caches its own hash (see {@link Entry}), {@link + * EntryKeyStrategy} seals it to that field. + */ + public abstract long hashOf(E entry); } /** - * {@link Helper} specialized for {@code String} keys: seals a spread {@link #hash} so String-key - * callers write only {@link #matches} and {@link #create}. Extend as a stateless final class held in - * a concrete-typed {@code static final} singleton, exactly like {@link Helper} — the {@code final} - * hash resolves directly and the concrete subclass still specializes the same at each call site, so - * there's no cost to the extra layer. + * {@link KeyStrategy} specialized for {@code String} keys: seals a spread {@link #hash} so + * String-key callers write only {@link #matches}. Extend as a stateless final class held in a + * concrete-typed {@code static final} singleton, exactly like {@link KeyStrategy} — the {@code + * final} hash resolves directly and the concrete subclass still specializes the same at each call + * site, so there's no cost to the extra layer. * - * @param stored entry — self-contained (carries its own key, ideally a cached hash) + * @param stored entry — self-contained (carries its own key) */ - public abstract static class StringHelper extends Helper { + public abstract static class StringKeyStrategy extends KeyStrategy { @Override - public final int hash(String key) { + public final long hash(String key) { final int h = key.hashCode(); - return h ^ (h >>> 16); // spread; FlatHashtable masks this directly + return h ^ (h >>> 16); // spread the int entropy; widened to the family-wide long hash width } } + /** + * {@link KeyStrategy} for entries that extend {@link Entry}: seals {@link #hashOf} to the entry's + * cached {@code hash}. Callers still supply {@link #hash} and {@link #matches} (or start from + * {@link StringKeyStrategy} for the {@code hash} seal too). + * + * @param lookup key + * @param stored entry — must extend {@link Entry} + */ + public abstract static class EntryKeyStrategy extends KeyStrategy { + @Override + public final long hashOf(E entry) { + return entry.hash; + } + } + + /** + * Creation strategy: mint a new entry for {@code key} (called once, on insert). A {@link + * FunctionalInterface} — supply a {@code static final} constant or a non-capturing lambda + * (e.g. {@code MyEntry::new}) so it stays a single monomorphic, allocation-free instance; a + * capturing lambda silently re-allocates per call and can de-monomorphize the site (see {@link + * Strategy}). Bespoke rather than {@link java.util.function.Function} so it carries the {@link + * Strategy} contract and reads as {@code create} at the call site. + * + * @param lookup key + * @param stored entry to create + */ + @Strategy + @FunctionalInterface + public interface CreateStrategy { + E create(K key); + } + /** Power-of-two capacity for a cardinality budget: {@code >= 2 * limit} (load factor <= 0.5). */ public static int capacityFor(int cardinalityLimit) { if (cardinalityLimit <= 0) { @@ -88,32 +157,33 @@ public static int capacityFor(int cardinalityLimit) { } /** - * Allocates a correctly-typed table for a cardinality budget ({@link #capacityFor} slots). Passing - * {@code type} makes the array's runtime component type {@code T} rather than {@code Object[]} — - * typed reads, real array-store checks, and a monomorphic element type for the JIT. Callers can't - * {@code new T[]} themselves under erasure; this does the one reflective allocation at construction - * (off any hot path). Note: this {@code create} mints the backing array; {@link Helper#create} - * mints an entry — different types, no ambiguity at the call site. + * Allocates a correctly-typed table for a cardinality budget ({@link #capacityFor} slots). + * Passing {@code type} makes the array's runtime component type {@code E} rather than {@code + * Object[]} — typed reads, real array-store checks, and a monomorphic element type for the JIT. + * Callers can't {@code new E[]} themselves under erasure; this does the one reflective allocation + * at construction (off any hot path). Note: this {@code create} mints the backing array; {@link + * CreateStrategy#create} mints an entry — different types, no ambiguity at the call site. */ @SuppressWarnings("unchecked") - public static T[] create(Class type, int cardinalityLimit) { - return (T[]) Array.newInstance(type, capacityFor(cardinalityLimit)); + public static E[] create(Class type, int cardinalityLimit) { + return (E[]) Array.newInstance(type, capacityFor(cardinalityLimit)); } /** * Existing entry for {@code key}, or {@code null}. Read-only — never creates. Single probe on a * hit; walks to the first empty slot (or all the way around) on a miss. */ - public static V get(V[] table, K key, Helper helper) { + @StrategyConsumer + public static E get(E[] table, K key, KeyStrategy keyStrat) { final int mask = table.length - 1; - final int start = helper.hash(key) & mask; + final int start = (int) (keyStrat.hash(key) & mask); int i = start; for (; ; ) { - final V e = table[i]; + final E e = table[i]; if (e == null) { return null; // empty slot terminates the probe (no tombstones) } - if (helper.matches(key, e)) { + if (keyStrat.matches(key, e)) { return e; } i = (i + 1) & mask; @@ -124,23 +194,25 @@ public static V get(V[] table, K key, Helper helper) { } /** - * Existing entry for {@code key}, or a freshly {@link Helper#create created} + inserted one. - * Returns {@code null} only if the table is full (no empty slot) — the caller supplies its overflow - * default. The insert is a single plain reference store: a concurrent clobber / double-create is - * acceptable only when the payload makes it benign (see class doc). + * Existing entry for {@code key}, or a freshly {@link CreateStrategy#create created} + inserted + * one. Returns {@code null} only if the table is full (no empty slot) — the caller supplies its + * overflow default. The insert is a single plain reference store: a concurrent clobber / + * double-create is acceptable only when the payload makes it benign (see class doc). */ - public static V getOrCreate(V[] table, K key, Helper helper) { + @StrategyConsumer + public static E getOrCreate( + E[] table, K key, KeyStrategy keyStrat, CreateStrategy createStrat) { final int mask = table.length - 1; - final int start = helper.hash(key) & mask; + final int start = (int) (keyStrat.hash(key) & mask); int i = start; for (; ; ) { - final V e = table[i]; + final E e = table[i]; if (e == null) { - final V created = helper.create(key); + final E created = createStrat.create(key); table[i] = created; // single-reference publish; benign clobber (see class doc) return created; } - if (helper.matches(key, e)) { + if (keyStrat.matches(key, e)) { return e; } i = (i + 1) & mask; @@ -149,4 +221,146 @@ public static V getOrCreate(V[] table, K key, Helper helper) { } } } + + /** + * Unconditionally adds {@code entry} at the first empty slot from its {@link Entry#hash home}; + * {@code false} if the table is full. Convenience over the {@link KeyStrategy}-taking overload + * for {@link Entry}-based entries (the home comes from the entry, so no strategy is needed). + * + *

Comparison-free and caller-responsible. It does not check for an existing key, so the + * caller must ensure {@code entry}'s key is absent. A duplicate lands shadowed further + * along the probe run — unreachable by {@link #get}, wasting a slot, and (if the key is later + * removed) able to resurrect stale data. Reach for it only from the expert tier, with that + * contract in hand. + */ + public static boolean insert(E[] table, E entry) { + return placeAt(table, entry, entry.hash); + } + + /** + * {@link #insert(Entry[], Entry)} for any entry type: the home comes from {@link + * KeyStrategy#hashOf}. Same comparison-free, caller-ensures-absence contract (the key type is + * irrelevant here — insert never hashes or matches a key). + */ + @StrategyConsumer + public static boolean insert(E[] table, E entry, KeyStrategy keyStrat) { + return placeAt(table, entry, keyStrat.hashOf(entry)); + } + + /** + * Shared placement core: probe from {@code hash}'s home to the first empty slot; false if full. + */ + private static boolean placeAt(E[] table, E entry, long hash) { + final int mask = table.length - 1; + final int start = (int) (hash & mask); + int i = start; + for (; ; ) { + if (table[i] == null) { + table[i] = entry; // single-reference publish (see class doc) + return true; + } + i = (i + 1) & mask; + if (i == start) { + return false; // wrapped ⇒ full + } + } + } + + /** Applies {@code consumer} to every entry in {@code table} (skipping empty slots); any order. */ + public static void forEach(E[] table, Consumer consumer) { + for (final E e : table) { + if (e != null) { + consumer.accept(e); + } + } + } + + /** + * Context-passing {@link #forEach(Object[], Consumer)}: pair a non-capturing {@link BiConsumer} + * (typically a {@code static final}) with side-band {@code context} to avoid a per-call closure. + */ + public static void forEach( + E[] table, C context, BiConsumer consumer) { + for (final E e : table) { + if (e != null) { + consumer.accept(context, e); + } + } + } + + /** + * Read-only iterator over the entries sharing {@code hash} — walks the probe run from {@code + * hash}'s home and yields each entry whose {@link KeyStrategy#hashOf} equals {@code hash}, + * stopping at the first empty slot (the FlatHashtable analogue of walking a chained bucket). The + * key type is irrelevant, so any {@link KeyStrategy} for {@code E} works. + * + *

Deliberately not a {@link StrategyConsumer}: iteration goes through the {@link + * Iterator} interface and calls {@code hashOf} virtually, so the strategy does not inline here — + * this is a cold traversal, not a hot specialization site. (Still pass a {@code static final} + * strategy to avoid a per-call allocation.) + */ + public static Iterator iterator(E[] table, long hash, KeyStrategy keyStrat) { + return new HashIterator<>(table, hash, keyStrat); + } + + private static final class HashIterator implements Iterator { + private final E[] table; + private final long hash; + private final KeyStrategy keyStrat; + private final int start; + private int i; + private boolean done; + private E lookahead; + + HashIterator(E[] table, long hash, KeyStrategy keyStrat) { + this.table = table; + this.hash = hash; + this.keyStrat = keyStrat; + this.start = (int) (hash & (table.length - 1)); + this.i = this.start; + advance(); + } + + private void advance() { + lookahead = null; + if (done) { + return; + } + final int mask = table.length - 1; + for (; ; ) { + final E e = table[i]; + if (e == null) { + done = true; // probe run ends at the first empty slot + return; + } + final boolean match = keyStrat.hashOf(e) == hash; + i = (i + 1) & mask; + final boolean wrapped = (i == start); + if (match) { + lookahead = e; + done = wrapped; + return; + } + if (wrapped) { + done = true; // walked the whole table without an empty slot + return; + } + } + } + + @Override + public boolean hasNext() { + return lookahead != null; + } + + @Override + public E next() { + final E e = lookahead; + if (e == null) { + throw new NoSuchElementException(); + } + advance(); + return e; + } + } } diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 9bebfa9c45a..551a83b874b 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -1,79 +1,123 @@ package datadog.trace.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Set; import org.junit.jupiter.api.Test; class FlatHashtableTest { /** Self-contained entry: carries its own key (the identity FlatHashtable relies on). */ - static final class Entry { + static final class TestEntry { final String key; - Entry(String key) { + TestEntry(String key) { this.key = key; } } - /** Stateless concrete helper, held as a concrete-typed static final singleton (the JIT idiom). */ - static final class EntryHelper extends FlatHashtable.StringHelper { + /** + * Stateless concrete key strategy over String keys, held as a concrete-typed static final + * singleton (the JIT idiom). {@code hash} is sealed by {@link FlatHashtable.StringKeyStrategy}; + * {@code hashOf} recomputes from the entry's key (this entry doesn't cache its hash). + */ + static final class TestEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { @Override - public boolean matches(String key, Entry value) { - return key.equals(value.key); + public boolean matches(String key, TestEntry entry) { + return key.equals(entry.key); } @Override - public Entry create(String key) { - return new Entry(key); + public long hashOf(TestEntry entry) { + return hash(entry.key); } } - private static final EntryHelper HELPER = new EntryHelper(); + private static final TestEntryKeyStrategy ENTRY_KEY_STRATEGY = new TestEntryKeyStrategy(); + + /** Non-capturing create strategy (a constructor method ref => singleton-cached, alloc-free). */ + private static final FlatHashtable.CreateStrategy CREATE = TestEntry::new; /** All keys hash to slot 0, so inserts chain by linear probing — exercises the probe path. */ - static final class CollidingHelper extends FlatHashtable.Helper { + static final class TestCollidingKeyStrategy extends FlatHashtable.KeyStrategy { @Override - public int hash(String key) { + public long hash(String key) { return 0; } @Override - public boolean matches(String key, Entry value) { - return key.equals(value.key); + public boolean matches(String key, TestEntry entry) { + return key.equals(entry.key); } @Override - public Entry create(String key) { - return new Entry(key); + public long hashOf(TestEntry entry) { + return hash(entry.key); } } - private static final CollidingHelper COLLIDING = new CollidingHelper(); + private static final TestCollidingKeyStrategy COLLIDING_KEY_STRATEGY = + new TestCollidingKeyStrategy(); /** All keys hash to the last slot ({@code -1 & mask}), so probing wraps around to index 0. */ - static final class LastSlotHelper extends FlatHashtable.Helper { + static final class TestLastSlotKeyStrategy extends FlatHashtable.KeyStrategy { @Override - public int hash(String key) { + public long hash(String key) { return -1; } @Override - public boolean matches(String key, Entry value) { - return key.equals(value.key); + public boolean matches(String key, TestEntry entry) { + return key.equals(entry.key); + } + + @Override + public long hashOf(TestEntry entry) { + return hash(entry.key); + } + } + + private static final TestLastSlotKeyStrategy LAST_SLOT_KEY_STRATEGY = + new TestLastSlotKeyStrategy(); + + /** + * Entry that caches its own hash (extends the Entry base) — for the entry-taking insert flavor. + */ + static final class TestHashedEntry extends FlatHashtable.Entry { + final String key; + + TestHashedEntry(String key) { + super( + ENTRY_KEY_STRATEGY.hash(key)); // cache the same hash ENTRY_KEY_STRATEGY uses for lookups + this.key = key; + } + } + + /** Key strategy for {@link TestHashedEntry}: {@code hashOf} is sealed to the cached hash. */ + static final class TestHashedKeyStrategy + extends FlatHashtable.EntryKeyStrategy { + @Override + public long hash(String key) { + return ENTRY_KEY_STRATEGY.hash(key); } @Override - public Entry create(String key) { - return new Entry(key); + public boolean matches(String key, TestHashedEntry entry) { + return key.equals(entry.key); } } - private static final LastSlotHelper LAST_SLOT = new LastSlotHelper(); + private static final TestHashedKeyStrategy HASHED_KEY_STRATEGY = new TestHashedKeyStrategy(); @Test void capacityFor_roundsToPowerOfTwoAtLeastTwiceLimit() { @@ -90,89 +134,179 @@ void capacityFor_rejectsNonPositive() { @Test void create_allocatesTypedTableOfCapacity() { - Entry[] table = FlatHashtable.create(Entry.class, 4); + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); assertEquals(8, table.length); - assertEquals(Entry.class, table.getClass().getComponentType()); + assertEquals(TestEntry.class, table.getClass().getComponentType()); } @Test void getOrCreate_insertsOnceAndReturnsTheExistingEntry() { - Entry[] table = FlatHashtable.create(Entry.class, 8); - Entry first = FlatHashtable.getOrCreate(table, "a", HELPER); + TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); + TestEntry first = FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE); assertEquals("a", first.key); // A second call must return the SAME instance, not mint a new one. - assertSame(first, FlatHashtable.getOrCreate(table, "a", HELPER)); - assertSame(first, FlatHashtable.get(table, "a", HELPER)); + assertSame(first, FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE)); + assertSame(first, FlatHashtable.get(table, "a", ENTRY_KEY_STRATEGY)); } @Test void get_returnsNullForAbsentKey() { - Entry[] table = FlatHashtable.create(Entry.class, 8); - assertNull(FlatHashtable.get(table, "missing", HELPER)); - FlatHashtable.getOrCreate(table, "present", HELPER); - assertNull(FlatHashtable.get(table, "still-missing", HELPER)); + TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); + assertNull(FlatHashtable.get(table, "missing", ENTRY_KEY_STRATEGY)); + FlatHashtable.getOrCreate(table, "present", ENTRY_KEY_STRATEGY, CREATE); + assertNull(FlatHashtable.get(table, "still-missing", ENTRY_KEY_STRATEGY)); } @Test void getOrCreate_returnsNullWhenTableIsFull() { // capacityFor(1) == 2 slots. - Entry[] table = FlatHashtable.create(Entry.class, 1); - assertTrue(FlatHashtable.getOrCreate(table, "k0", HELPER) != null); - assertTrue(FlatHashtable.getOrCreate(table, "k1", HELPER) != null); + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); + assertTrue(FlatHashtable.getOrCreate(table, "k0", ENTRY_KEY_STRATEGY, CREATE) != null); + assertTrue(FlatHashtable.getOrCreate(table, "k1", ENTRY_KEY_STRATEGY, CREATE) != null); // Both slots occupied by distinct keys -> a third distinct key finds no room. - assertNull(FlatHashtable.getOrCreate(table, "k2", HELPER)); + assertNull(FlatHashtable.getOrCreate(table, "k2", ENTRY_KEY_STRATEGY, CREATE)); // ...but an existing key still resolves even when full. assertSame( - FlatHashtable.get(table, "k0", HELPER), FlatHashtable.getOrCreate(table, "k0", HELPER)); + FlatHashtable.get(table, "k0", ENTRY_KEY_STRATEGY), + FlatHashtable.getOrCreate(table, "k0", ENTRY_KEY_STRATEGY, CREATE)); } @Test - void stringHelper_hashIsStableForEqualKeys() { - assertEquals(HELPER.hash("route"), HELPER.hash(new String("route"))); + void stringKeyStrategy_hashIsStableForEqualKeys() { + assertEquals(ENTRY_KEY_STRATEGY.hash("route"), ENTRY_KEY_STRATEGY.hash(new String("route"))); } @Test void collision_probesPastOccupiedSlots_andResolvesEach() { - Entry[] table = FlatHashtable.create(Entry.class, 4); // 8 slots; COLLIDING sends all to slot 0 - Entry a = FlatHashtable.getOrCreate(table, "a", COLLIDING); - Entry b = FlatHashtable.getOrCreate(table, "b", COLLIDING); // slot 0 taken -> probes to slot 1 - Entry c = FlatHashtable.getOrCreate(table, "c", COLLIDING); // -> slot 2 + // 8 slots; COLLIDING sends all to slot 0 + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); + TestEntry a = FlatHashtable.getOrCreate(table, "a", COLLIDING_KEY_STRATEGY, CREATE); + TestEntry b = + FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE); // slot 0 taken -> 1 + TestEntry c = + FlatHashtable.getOrCreate(table, "c", COLLIDING_KEY_STRATEGY, CREATE); // -> slot 2 assertNotSame(a, b); assertNotSame(b, c); // each resolves via probe-past-occupied + match-after-probe - assertSame(a, FlatHashtable.get(table, "a", COLLIDING)); - assertSame(b, FlatHashtable.get(table, "b", COLLIDING)); - assertSame(c, FlatHashtable.get(table, "c", COLLIDING)); + assertSame(a, FlatHashtable.get(table, "a", COLLIDING_KEY_STRATEGY)); + assertSame(b, FlatHashtable.get(table, "b", COLLIDING_KEY_STRATEGY)); + assertSame(c, FlatHashtable.get(table, "c", COLLIDING_KEY_STRATEGY)); // existing colliding key: found after probing, no new entry minted - assertSame(b, FlatHashtable.getOrCreate(table, "b", COLLIDING)); + assertSame(b, FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE)); // absent key: probe past the 3 occupied slots, hit an empty slot -> null - assertNull(FlatHashtable.get(table, "absent", COLLIDING)); + assertNull(FlatHashtable.get(table, "absent", COLLIDING_KEY_STRATEGY)); } @Test void collision_probeWrapsAroundToFront() { - Entry[] table = - FlatHashtable.create(Entry.class, 1); // 2 slots (0,1), mask=1; LAST_SLOT starts at 1 - Entry k0 = FlatHashtable.getOrCreate(table, "k0", LAST_SLOT); // -> slot 1 - Entry k1 = FlatHashtable.getOrCreate(table, "k1", LAST_SLOT); // slot 1 taken -> wraps to slot 0 + // 2 slots (0,1), mask=1; LAST_SLOT starts at 1 + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); + TestEntry k0 = + FlatHashtable.getOrCreate(table, "k0", LAST_SLOT_KEY_STRATEGY, CREATE); // -> slot 1 + TestEntry k1 = + FlatHashtable.getOrCreate( + table, "k1", LAST_SLOT_KEY_STRATEGY, CREATE); // taken -> wraps to 0 assertNotSame(k0, k1); - assertSame(k0, FlatHashtable.get(table, "k0", LAST_SLOT)); + assertSame(k0, FlatHashtable.get(table, "k0", LAST_SLOT_KEY_STRATEGY)); // start slot 1 is occupied (no match) -> probe wraps to slot 0 -> match - assertSame(k1, FlatHashtable.get(table, "k1", LAST_SLOT)); + assertSame(k1, FlatHashtable.get(table, "k1", LAST_SLOT_KEY_STRATEGY)); } @Test void get_returnsNullWhenTableFullAndKeyAbsent() { - Entry[] table = FlatHashtable.create(Entry.class, 1); // 2 slots - FlatHashtable.getOrCreate(table, "k0", COLLIDING); - FlatHashtable.getOrCreate(table, "k1", COLLIDING); // fills slots 0 and 1 + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots + FlatHashtable.getOrCreate(table, "k0", COLLIDING_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "k1", COLLIDING_KEY_STRATEGY, CREATE); // fills slots 0 and 1 // get() probes both occupied slots, wraps back to start -> null (get's full-wrap branch) - assertNull(FlatHashtable.get(table, "absent", COLLIDING)); + assertNull(FlatHashtable.get(table, "absent", COLLIDING_KEY_STRATEGY)); + } + + @Test + void insert_generalFlavor_placesViaHashOfAndResolves() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); + TestEntry e = new TestEntry("a"); + // flavor 2: the home comes from ENTRY_KEY_STRATEGY.hashOf(e) + assertTrue(FlatHashtable.insert(table, e, ENTRY_KEY_STRATEGY)); + assertSame(e, FlatHashtable.get(table, "a", ENTRY_KEY_STRATEGY)); + } + + @Test + void insert_entryFlavor_placesViaCachedHashAndResolves() { + TestHashedEntry[] table = FlatHashtable.create(TestHashedEntry.class, 8); + TestHashedEntry e = new TestHashedEntry("a"); + // flavor 1: the home comes from the Entry's own cached hash, no strategy needed + assertTrue(FlatHashtable.insert(table, e)); + assertSame(e, FlatHashtable.get(table, "a", HASHED_KEY_STRATEGY)); + } + + @Test + void insert_returnsFalseWhenFull() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots + assertTrue(FlatHashtable.insert(table, new TestEntry("k0"), ENTRY_KEY_STRATEGY)); + assertTrue(FlatHashtable.insert(table, new TestEntry("k1"), ENTRY_KEY_STRATEGY)); + assertFalse(FlatHashtable.insert(table, new TestEntry("k2"), ENTRY_KEY_STRATEGY)); // no room + } + + @Test + void forEach_visitsEveryEntry() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); + FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "b", ENTRY_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "c", ENTRY_KEY_STRATEGY, CREATE); + + Set seen = new HashSet<>(); + FlatHashtable.forEach(table, e -> seen.add(e.key)); + assertEquals(new HashSet<>(Arrays.asList("a", "b", "c")), seen); + } + + @Test + void forEach_contextVariant_passesContextWithoutCapture() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); + FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "b", ENTRY_KEY_STRATEGY, CREATE); + + Set seen = new HashSet<>(); + FlatHashtable.forEach(table, seen, (ctx, e) -> ctx.add(e.key)); + assertEquals(new HashSet<>(Arrays.asList("a", "b")), seen); + } + + @Test + void iterator_yieldsEveryEntrySharingTheHash() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // COLLIDING sends all to slot 0 + TestEntry a = FlatHashtable.getOrCreate(table, "a", COLLIDING_KEY_STRATEGY, CREATE); + TestEntry b = FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE); + TestEntry c = FlatHashtable.getOrCreate(table, "c", COLLIDING_KEY_STRATEGY, CREATE); + + Set seen = new HashSet<>(); + Iterator it = FlatHashtable.iterator(table, 0, COLLIDING_KEY_STRATEGY); + while (it.hasNext()) { + seen.add(it.next()); + } + assertEquals(new HashSet<>(Arrays.asList(a, b, c)), seen); + } + + @Test + void iterator_filtersOutEntriesWithADifferentHash() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // entries at slot 0, hashOf == 0 + FlatHashtable.getOrCreate(table, "a", COLLIDING_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE); + + // hash 8 shares the home slot (8 & 7 == 0) but no stored entry has hashOf == 8 + Iterator it = FlatHashtable.iterator(table, 8, COLLIDING_KEY_STRATEGY); + assertFalse(it.hasNext()); + } + + @Test + void iterator_emptyRunHasNoNext() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); + Iterator it = FlatHashtable.iterator(table, 0, COLLIDING_KEY_STRATEGY); + assertFalse(it.hasNext()); + assertThrows(NoSuchElementException.class, it::next); } } From 0e20fa3e9a555857e2d57140ded5b61c38148b6b Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 13:03:53 -0400 Subject: [PATCH 04/26] Benchmark FlatHashtable in the single-threaded map comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds FlatHashtable to SingleThreadedMapBenchmark on the ops it supports (build via comparison-free insert, get, iterate) — its self-contained entry holds the value unboxed vs HashMap. Fixed-capacity so the table is sized to the key count. Uses the INSTANCE-singleton strategy style (private ctor, lazy class-init). Co-Authored-By: Claude Opus 4.8 --- .../util/SingleThreadedMapBenchmark.java | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java index 11572aa923d..51d3e4c8b12 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java @@ -36,6 +36,10 @@ *

  • TreeMap — when a custom Comparator is needed (see CaseInsensitiveMapBenchmark) *
  • LinkedHashMap — only when insertion-order iteration is required; cost is paid at * construction and in per-entry memory + *
  • FlatHashtable — a find-or-create table over self-contained entries (not a general Map: no + * arbitrary put/remove). Compared here on the ops it does support — build, get, iterate — + * where its self-contained entry stores the value unboxed (one object per key, no + * {@code Integer}). Fixed-capacity, so it must be sized to the working set up front. * * *

    Uncontended synchronization tax. A {@link Collections#synchronizedMap} case is included @@ -81,12 +85,54 @@ static TagMap fillTagMap(TagMap map) { return map; } + // FlatHashtable is a find-or-create table over self-contained entries — no arbitrary put/remove, + // so only the comparable ops appear here: build (via the comparison-free insert of distinct + // keys), + // get, and iterate. Its entry carries the value UNBOXED (no Integer), one object per key. + static final class IntEntry { + final String key; + final int value; + + IntEntry(String key, int value) { + this.key = key; + this.value = value; + } + } + + static final class IntEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { + // Canonical exact-typed singleton: one instance, private ctor => the static-poly discipline is + // enforced by the class, not left to each caller to declare correctly. + static final IntEntryKeyStrategy INSTANCE = new IntEntryKeyStrategy(); + + private IntEntryKeyStrategy() {} + + @Override + public boolean matches(String key, IntEntry entry) { + return key.equals(entry.key); + } + + @Override + public long hashOf(IntEntry entry) { + return hash(entry.key); + } + } + + static IntEntry[] newFilledFlat() { + // Sized to the key count (FlatHashtable is fixed-capacity, no resize): load factor <= 0.5. + IntEntry[] table = FlatHashtable.create(IntEntry.class, INSERTION_KEYS.length); + for (int i = 0; i < INSERTION_KEYS.length; ++i) { + FlatHashtable.insert(table, new IntEntry(INSERTION_KEYS[i], i), IntEntryKeyStrategy.INSTANCE); + } + return table; + } + // Per-thread prebuilt maps for the read + clone benchmarks (built once per trial, per thread). HashMap hashMap; Map synchronizedHashMap; TreeMap treeMap; LinkedHashMap linkedHashMap; TagMap tagMap; + IntEntry[] flatTable; int index = 0; @Setup(Level.Trial) @@ -99,6 +145,7 @@ public void setUp() { linkedHashMap = new LinkedHashMap<>(); fill(linkedHashMap); tagMap = fillTagMap(TagMap.create()); + flatTable = newFilledFlat(); } String nextLookupKey() { @@ -159,6 +206,11 @@ public TagMap create_tagMap_via_ledger() { return ledger.build(); } + @Benchmark + public IntEntry[] create_flatHashtable() { + return newFilledFlat(); + } + // ---- copy ---- @Benchmark @@ -200,6 +252,11 @@ public Integer get_synchronizedHashMap() { return synchronizedHashMap.get(nextLookupKey()); } + @Benchmark + public IntEntry get_flatHashtable() { + return FlatHashtable.get(flatTable, nextLookupKey(), IntEntryKeyStrategy.INSTANCE); + } + @Benchmark public void iterate_hashMap(Blackhole blackhole) { for (Map.Entry entry : hashMap.entrySet()) { @@ -219,4 +276,16 @@ public void iterate_synchronizedHashMap(Blackhole blackhole) { } } } + + @Benchmark + public void iterate_flatHashtable(Blackhole blackhole) { + // Context-passing forEach: blackhole rides through as context, so the lambda doesn't capture. + FlatHashtable.forEach( + flatTable, + blackhole, + (bh, e) -> { + bh.consume(e.key); + bh.consume(e.value); + }); + } } From 2ddcd84a59756755b68e2dc26b2bfda7d43e6c6a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 13:52:57 -0400 Subject: [PATCH 05/26] Benchmark FlatHashtable's lock-free read in the thread-safe map comparison Adds FlatHashtable to ThreadSafeMapBenchmark: build + concurrent get on a shared, once-published table (lock-free, no volatile) alongside ConcurrentHashMap / volatile-HashMap / synchronizedHashMap. Fixture mirrors SingleThreadedMapBenchmark (self-contained per benchmark). Co-Authored-By: Claude Opus 4.8 --- .../trace/util/ThreadSafeMapBenchmark.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java index 793627a37e6..51d50bb6b0c 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java @@ -21,6 +21,8 @@ *

  • ConcurrentMap - only when there are simultaneously readers & writers in multiple threads *
  • HashMap via volatile - preferred for background thread updates *
  • synchronized HashMap - when simultaneous readers & writers are uncommon (e.g. tags) + *
  • FlatHashtable - lock-free reads (no lock, no volatile; benign-race) of a fixed, once-built + * keyed set; a find-or-create table, not a general concurrent Map (no arbitrary put/remove) * * *

    @@ -104,6 +106,46 @@ static void fill(Map map) { } } + // FlatHashtable's contribution here is the lock-free concurrent read: get() is a plain array + // probe + // with no lock and no volatile — safe under concurrency because the table is published once (a + // final static field) and each entry's identity fields are final. (Fixture mirrors the one in + // SingleThreadedMapBenchmark; the benchmarks are self-contained.) + static final class IntEntry { + final String key; + final int value; + + IntEntry(String key, int value) { + this.key = key; + this.value = value; + } + } + + static final class IntEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { + static final IntEntryKeyStrategy INSTANCE = new IntEntryKeyStrategy(); + + private IntEntryKeyStrategy() {} + + @Override + public boolean matches(String key, IntEntry entry) { + return key.equals(entry.key); + } + + @Override + public long hashOf(IntEntry entry) { + return hash(entry.key); + } + } + + static IntEntry[] _create_flat() { + // Sized to the key count (FlatHashtable is fixed-capacity, no resize): load factor <= 0.5. + IntEntry[] table = FlatHashtable.create(IntEntry.class, INSERTION_KEYS.length); + for (int i = 0; i < INSERTION_KEYS.length; ++i) { + FlatHashtable.insert(table, new IntEntry(INSERTION_KEYS[i], i), IntEntryKeyStrategy.INSTANCE); + } + return table; + } + static final HashMap _create_hashMap() { HashMap map = new HashMap<>(); fill(map); @@ -177,4 +219,17 @@ public ConcurrentSkipListMap create_concSkipListMap() { public Integer get_concSkipListMap() { return CONC_SKIP_LIST_MAP.get(nextLookupKey()); } + + @Benchmark + public IntEntry[] create_flatHashtable() { + return _create_flat(); + } + + static final IntEntry[] FLAT_TABLE = _create_flat(); + + @Benchmark + public IntEntry get_flatHashtable() { + // Lock-free concurrent read of the shared, once-published table. + return FlatHashtable.get(FLAT_TABLE, nextLookupKey(), IntEntryKeyStrategy.INSTANCE); + } } From 93d5f4ed9471a4663913480659f492df61acab62 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 14:07:50 -0400 Subject: [PATCH 06/26] Make ThreadSafeMapBenchmark lookup index per-thread (remove shared-counter contention) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nextLookupKey used a shared static counter incremented from all @Threads(8) — a cache-line-contended race that floors the fastest reads and masks the very differences the benchmark compares (mirrors the per-thread index SingleThreadedMapBenchmark already uses, and the set-benchmark fix in #11721). Maps stay static/shared; only the lookup index goes per-thread via @State(Scope.Thread). Existing Javadoc numbers predate this and need a rerun. Co-Authored-By: Claude Opus 4.8 --- .../trace/util/ThreadSafeMapBenchmark.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java index 51d50bb6b0c..9b66e727455 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java @@ -9,6 +9,8 @@ import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Threads; import org.openjdk.jmh.annotations.Warmup; @@ -67,6 +69,7 @@ @Warmup(iterations = 2) @Measurement(iterations = 3) @Threads(8) +@State(Scope.Thread) public class ThreadSafeMapBenchmark { static final String[] INSERTION_KEYS = { "foo", "bar", "baz", "quux", "foobar", "foobaz", "key0", "key1", "key2", "key3" @@ -86,16 +89,20 @@ static T init(Supplier supplier) { return supplier.get(); } - static int sharedLookupIndex = 0; + // Per-thread (@State(Scope.Thread)) so cycling the lookup key doesn't contend a shared counter. + // The maps below stay static/shared (the point — concurrent reads of one map); only the index is + // per-thread. A shared counter's cache-line ping-pong would otherwise floor the fastest reads + // (e.g. FlatHashtable's lock-free probe), hiding exactly the differences this benchmark compares. + int lookupIndex = 0; - static String nextLookupKey() { + String nextLookupKey() { return nextLookupKey(EQUAL_KEYS); } - static String nextLookupKey(String[] keys) { - int localIndex = ++sharedLookupIndex; + String nextLookupKey(String[] keys) { + int localIndex = ++lookupIndex; if (localIndex >= keys.length) { - sharedLookupIndex = localIndex = 0; + lookupIndex = localIndex = 0; } return keys[localIndex]; } From e0a5620b4ff62a3d53e6800f7f81e536c5ebdbe4 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 14:44:55 -0400 Subject: [PATCH 07/26] Adopt INSTANCE-singleton convention in FlatHashtable test strategies Each concrete key strategy carries a static final INSTANCE of its exact type + a private ctor (lazy class-init singleton); call sites use X.INSTANCE, dropping the separate module-level constants. Matches the benchmark fixture and the strategy-class style. Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtableTest.java | 148 ++++++++++-------- 1 file changed, 80 insertions(+), 68 deletions(-) diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 551a83b874b..24a74084e1d 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -27,11 +27,16 @@ static final class TestEntry { } /** - * Stateless concrete key strategy over String keys, held as a concrete-typed static final - * singleton (the JIT idiom). {@code hash} is sealed by {@link FlatHashtable.StringKeyStrategy}; - * {@code hashOf} recomputes from the entry's key (this entry doesn't cache its hash). + * Stateless concrete key strategy over String keys, exposed as a canonical {@code INSTANCE} + * singleton (private ctor) so the JIT can specialize each call site. {@code hash} is sealed by + * {@link FlatHashtable.StringKeyStrategy}; {@code hashOf} recomputes from the entry's key (this + * entry doesn't cache its hash). */ static final class TestEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { + static final TestEntryKeyStrategy INSTANCE = new TestEntryKeyStrategy(); + + private TestEntryKeyStrategy() {} + @Override public boolean matches(String key, TestEntry entry) { return key.equals(entry.key); @@ -43,13 +48,15 @@ public long hashOf(TestEntry entry) { } } - private static final TestEntryKeyStrategy ENTRY_KEY_STRATEGY = new TestEntryKeyStrategy(); - /** Non-capturing create strategy (a constructor method ref => singleton-cached, alloc-free). */ private static final FlatHashtable.CreateStrategy CREATE = TestEntry::new; /** All keys hash to slot 0, so inserts chain by linear probing — exercises the probe path. */ static final class TestCollidingKeyStrategy extends FlatHashtable.KeyStrategy { + static final TestCollidingKeyStrategy INSTANCE = new TestCollidingKeyStrategy(); + + private TestCollidingKeyStrategy() {} + @Override public long hash(String key) { return 0; @@ -66,11 +73,12 @@ public long hashOf(TestEntry entry) { } } - private static final TestCollidingKeyStrategy COLLIDING_KEY_STRATEGY = - new TestCollidingKeyStrategy(); - /** All keys hash to the last slot ({@code -1 & mask}), so probing wraps around to index 0. */ static final class TestLastSlotKeyStrategy extends FlatHashtable.KeyStrategy { + static final TestLastSlotKeyStrategy INSTANCE = new TestLastSlotKeyStrategy(); + + private TestLastSlotKeyStrategy() {} + @Override public long hash(String key) { return -1; @@ -87,9 +95,6 @@ public long hashOf(TestEntry entry) { } } - private static final TestLastSlotKeyStrategy LAST_SLOT_KEY_STRATEGY = - new TestLastSlotKeyStrategy(); - /** * Entry that caches its own hash (extends the Entry base) — for the entry-taking insert flavor. */ @@ -97,8 +102,8 @@ static final class TestHashedEntry extends FlatHashtable.Entry { final String key; TestHashedEntry(String key) { - super( - ENTRY_KEY_STRATEGY.hash(key)); // cache the same hash ENTRY_KEY_STRATEGY uses for lookups + // cache the same hash TestEntryKeyStrategy uses for lookups + super(TestEntryKeyStrategy.INSTANCE.hash(key)); this.key = key; } } @@ -106,9 +111,13 @@ static final class TestHashedEntry extends FlatHashtable.Entry { /** Key strategy for {@link TestHashedEntry}: {@code hashOf} is sealed to the cached hash. */ static final class TestHashedKeyStrategy extends FlatHashtable.EntryKeyStrategy { + static final TestHashedKeyStrategy INSTANCE = new TestHashedKeyStrategy(); + + private TestHashedKeyStrategy() {} + @Override public long hash(String key) { - return ENTRY_KEY_STRATEGY.hash(key); + return TestEntryKeyStrategy.INSTANCE.hash(key); } @Override @@ -117,8 +126,6 @@ public boolean matches(String key, TestHashedEntry entry) { } } - private static final TestHashedKeyStrategy HASHED_KEY_STRATEGY = new TestHashedKeyStrategy(); - @Test void capacityFor_roundsToPowerOfTwoAtLeastTwiceLimit() { assertEquals(2, FlatHashtable.capacityFor(1)); @@ -142,98 +149,102 @@ void create_allocatesTypedTableOfCapacity() { @Test void getOrCreate_insertsOnceAndReturnsTheExistingEntry() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - TestEntry first = FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE); + TestEntry first = FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE); assertEquals("a", first.key); // A second call must return the SAME instance, not mint a new one. - assertSame(first, FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE)); - assertSame(first, FlatHashtable.get(table, "a", ENTRY_KEY_STRATEGY)); + assertSame(first, FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE)); + assertSame(first, FlatHashtable.get(table, "a", TestEntryKeyStrategy.INSTANCE)); } @Test void get_returnsNullForAbsentKey() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - assertNull(FlatHashtable.get(table, "missing", ENTRY_KEY_STRATEGY)); - FlatHashtable.getOrCreate(table, "present", ENTRY_KEY_STRATEGY, CREATE); - assertNull(FlatHashtable.get(table, "still-missing", ENTRY_KEY_STRATEGY)); + assertNull(FlatHashtable.get(table, "missing", TestEntryKeyStrategy.INSTANCE)); + FlatHashtable.getOrCreate(table, "present", TestEntryKeyStrategy.INSTANCE, CREATE); + assertNull(FlatHashtable.get(table, "still-missing", TestEntryKeyStrategy.INSTANCE)); } @Test void getOrCreate_returnsNullWhenTableIsFull() { // capacityFor(1) == 2 slots. TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); - assertTrue(FlatHashtable.getOrCreate(table, "k0", ENTRY_KEY_STRATEGY, CREATE) != null); - assertTrue(FlatHashtable.getOrCreate(table, "k1", ENTRY_KEY_STRATEGY, CREATE) != null); + assertTrue( + FlatHashtable.getOrCreate(table, "k0", TestEntryKeyStrategy.INSTANCE, CREATE) != null); + assertTrue( + FlatHashtable.getOrCreate(table, "k1", TestEntryKeyStrategy.INSTANCE, CREATE) != null); // Both slots occupied by distinct keys -> a third distinct key finds no room. - assertNull(FlatHashtable.getOrCreate(table, "k2", ENTRY_KEY_STRATEGY, CREATE)); + assertNull(FlatHashtable.getOrCreate(table, "k2", TestEntryKeyStrategy.INSTANCE, CREATE)); // ...but an existing key still resolves even when full. assertSame( - FlatHashtable.get(table, "k0", ENTRY_KEY_STRATEGY), - FlatHashtable.getOrCreate(table, "k0", ENTRY_KEY_STRATEGY, CREATE)); + FlatHashtable.get(table, "k0", TestEntryKeyStrategy.INSTANCE), + FlatHashtable.getOrCreate(table, "k0", TestEntryKeyStrategy.INSTANCE, CREATE)); } @Test void stringKeyStrategy_hashIsStableForEqualKeys() { - assertEquals(ENTRY_KEY_STRATEGY.hash("route"), ENTRY_KEY_STRATEGY.hash(new String("route"))); + assertEquals( + TestEntryKeyStrategy.INSTANCE.hash("route"), + TestEntryKeyStrategy.INSTANCE.hash(new String("route"))); } @Test void collision_probesPastOccupiedSlots_andResolvesEach() { // 8 slots; COLLIDING sends all to slot 0 TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); - TestEntry a = FlatHashtable.getOrCreate(table, "a", COLLIDING_KEY_STRATEGY, CREATE); - TestEntry b = - FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE); // slot 0 taken -> 1 - TestEntry c = - FlatHashtable.getOrCreate(table, "c", COLLIDING_KEY_STRATEGY, CREATE); // -> slot 2 + TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); + // slot 0 taken -> 1 + TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); + // -> slot 2 + TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingKeyStrategy.INSTANCE, CREATE); assertNotSame(a, b); assertNotSame(b, c); // each resolves via probe-past-occupied + match-after-probe - assertSame(a, FlatHashtable.get(table, "a", COLLIDING_KEY_STRATEGY)); - assertSame(b, FlatHashtable.get(table, "b", COLLIDING_KEY_STRATEGY)); - assertSame(c, FlatHashtable.get(table, "c", COLLIDING_KEY_STRATEGY)); + assertSame(a, FlatHashtable.get(table, "a", TestCollidingKeyStrategy.INSTANCE)); + assertSame(b, FlatHashtable.get(table, "b", TestCollidingKeyStrategy.INSTANCE)); + assertSame(c, FlatHashtable.get(table, "c", TestCollidingKeyStrategy.INSTANCE)); // existing colliding key: found after probing, no new entry minted - assertSame(b, FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE)); + assertSame(b, FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE)); // absent key: probe past the 3 occupied slots, hit an empty slot -> null - assertNull(FlatHashtable.get(table, "absent", COLLIDING_KEY_STRATEGY)); + assertNull(FlatHashtable.get(table, "absent", TestCollidingKeyStrategy.INSTANCE)); } @Test void collision_probeWrapsAroundToFront() { // 2 slots (0,1), mask=1; LAST_SLOT starts at 1 TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); - TestEntry k0 = - FlatHashtable.getOrCreate(table, "k0", LAST_SLOT_KEY_STRATEGY, CREATE); // -> slot 1 - TestEntry k1 = - FlatHashtable.getOrCreate( - table, "k1", LAST_SLOT_KEY_STRATEGY, CREATE); // taken -> wraps to 0 + // -> slot 1 + TestEntry k0 = FlatHashtable.getOrCreate(table, "k0", TestLastSlotKeyStrategy.INSTANCE, CREATE); + // taken -> wraps to 0 + TestEntry k1 = FlatHashtable.getOrCreate(table, "k1", TestLastSlotKeyStrategy.INSTANCE, CREATE); assertNotSame(k0, k1); - assertSame(k0, FlatHashtable.get(table, "k0", LAST_SLOT_KEY_STRATEGY)); + assertSame(k0, FlatHashtable.get(table, "k0", TestLastSlotKeyStrategy.INSTANCE)); // start slot 1 is occupied (no match) -> probe wraps to slot 0 -> match - assertSame(k1, FlatHashtable.get(table, "k1", LAST_SLOT_KEY_STRATEGY)); + assertSame(k1, FlatHashtable.get(table, "k1", TestLastSlotKeyStrategy.INSTANCE)); } @Test void get_returnsNullWhenTableFullAndKeyAbsent() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots - FlatHashtable.getOrCreate(table, "k0", COLLIDING_KEY_STRATEGY, CREATE); - FlatHashtable.getOrCreate(table, "k1", COLLIDING_KEY_STRATEGY, CREATE); // fills slots 0 and 1 + FlatHashtable.getOrCreate(table, "k0", TestCollidingKeyStrategy.INSTANCE, CREATE); + // fills slots 0 and 1 + FlatHashtable.getOrCreate(table, "k1", TestCollidingKeyStrategy.INSTANCE, CREATE); // get() probes both occupied slots, wraps back to start -> null (get's full-wrap branch) - assertNull(FlatHashtable.get(table, "absent", COLLIDING_KEY_STRATEGY)); + assertNull(FlatHashtable.get(table, "absent", TestCollidingKeyStrategy.INSTANCE)); } @Test void insert_generalFlavor_placesViaHashOfAndResolves() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); TestEntry e = new TestEntry("a"); - // flavor 2: the home comes from ENTRY_KEY_STRATEGY.hashOf(e) - assertTrue(FlatHashtable.insert(table, e, ENTRY_KEY_STRATEGY)); - assertSame(e, FlatHashtable.get(table, "a", ENTRY_KEY_STRATEGY)); + // flavor 2: the home comes from TestEntryKeyStrategy.INSTANCE.hashOf(e) + assertTrue(FlatHashtable.insert(table, e, TestEntryKeyStrategy.INSTANCE)); + assertSame(e, FlatHashtable.get(table, "a", TestEntryKeyStrategy.INSTANCE)); } @Test @@ -242,23 +253,24 @@ void insert_entryFlavor_placesViaCachedHashAndResolves() { TestHashedEntry e = new TestHashedEntry("a"); // flavor 1: the home comes from the Entry's own cached hash, no strategy needed assertTrue(FlatHashtable.insert(table, e)); - assertSame(e, FlatHashtable.get(table, "a", HASHED_KEY_STRATEGY)); + assertSame(e, FlatHashtable.get(table, "a", TestHashedKeyStrategy.INSTANCE)); } @Test void insert_returnsFalseWhenFull() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots - assertTrue(FlatHashtable.insert(table, new TestEntry("k0"), ENTRY_KEY_STRATEGY)); - assertTrue(FlatHashtable.insert(table, new TestEntry("k1"), ENTRY_KEY_STRATEGY)); - assertFalse(FlatHashtable.insert(table, new TestEntry("k2"), ENTRY_KEY_STRATEGY)); // no room + assertTrue(FlatHashtable.insert(table, new TestEntry("k0"), TestEntryKeyStrategy.INSTANCE)); + assertTrue(FlatHashtable.insert(table, new TestEntry("k1"), TestEntryKeyStrategy.INSTANCE)); + // no room + assertFalse(FlatHashtable.insert(table, new TestEntry("k2"), TestEntryKeyStrategy.INSTANCE)); } @Test void forEach_visitsEveryEntry() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE); - FlatHashtable.getOrCreate(table, "b", ENTRY_KEY_STRATEGY, CREATE); - FlatHashtable.getOrCreate(table, "c", ENTRY_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestEntryKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "c", TestEntryKeyStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); FlatHashtable.forEach(table, e -> seen.add(e.key)); @@ -268,8 +280,8 @@ void forEach_visitsEveryEntry() { @Test void forEach_contextVariant_passesContextWithoutCapture() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - FlatHashtable.getOrCreate(table, "a", ENTRY_KEY_STRATEGY, CREATE); - FlatHashtable.getOrCreate(table, "b", ENTRY_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestEntryKeyStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); FlatHashtable.forEach(table, seen, (ctx, e) -> ctx.add(e.key)); @@ -279,12 +291,12 @@ void forEach_contextVariant_passesContextWithoutCapture() { @Test void iterator_yieldsEveryEntrySharingTheHash() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // COLLIDING sends all to slot 0 - TestEntry a = FlatHashtable.getOrCreate(table, "a", COLLIDING_KEY_STRATEGY, CREATE); - TestEntry b = FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE); - TestEntry c = FlatHashtable.getOrCreate(table, "c", COLLIDING_KEY_STRATEGY, CREATE); + TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); + TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingKeyStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); - Iterator it = FlatHashtable.iterator(table, 0, COLLIDING_KEY_STRATEGY); + Iterator it = FlatHashtable.iterator(table, 0, TestCollidingKeyStrategy.INSTANCE); while (it.hasNext()) { seen.add(it.next()); } @@ -294,18 +306,18 @@ void iterator_yieldsEveryEntrySharingTheHash() { @Test void iterator_filtersOutEntriesWithADifferentHash() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // entries at slot 0, hashOf == 0 - FlatHashtable.getOrCreate(table, "a", COLLIDING_KEY_STRATEGY, CREATE); - FlatHashtable.getOrCreate(table, "b", COLLIDING_KEY_STRATEGY, CREATE); + FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); // hash 8 shares the home slot (8 & 7 == 0) but no stored entry has hashOf == 8 - Iterator it = FlatHashtable.iterator(table, 8, COLLIDING_KEY_STRATEGY); + Iterator it = FlatHashtable.iterator(table, 8, TestCollidingKeyStrategy.INSTANCE); assertFalse(it.hasNext()); } @Test void iterator_emptyRunHasNoNext() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); - Iterator it = FlatHashtable.iterator(table, 0, COLLIDING_KEY_STRATEGY); + Iterator it = FlatHashtable.iterator(table, 0, TestCollidingKeyStrategy.INSTANCE); assertFalse(it.hasNext()); assertThrows(NoSuchElementException.class, it::next); } From 547f68f6b110a74bb1712157be82391387ab8573 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 15:06:59 -0400 Subject: [PATCH 08/26] Defeat CHA in the FlatHashtable map benchmarks to prove structural devirt Load two decoy KeyStrategy implementors alongside the real one in both map benchmarks so KeyStrategy.hash and KeyStrategy.matches each have >=2 concrete implementors before the hot method compiles. This denies C2 the single- implementor CHA devirtualization of keyStrat.hash/matches inside get(), so the steady-state numbers reflect the no-CHA regime rather than a deopt-guarded bet. Verified with -XX:+PrintInlining (Zulu 21; Java 8/ARM64 is fine for inlining- decision inspection but not for throughput): with CHA impossible, the concrete StringKeyStrategy::hash / IntEntryKeyStrategy::matches still inline (hot) and no type-profile/morphic markers appear on any FlatHashtable/KeyStrategy method. The devirtualization is therefore exact-type, from the constant INSTANCE propagated through the inlined get -- the structural monomorphization the @Strategy contract promises, not a CHA or type-profile speculation. Co-Authored-By: Claude Opus 4.8 --- .../util/SingleThreadedMapBenchmark.java | 53 +++++++++++++++++++ .../trace/util/ThreadSafeMapBenchmark.java | 53 +++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java index 51d3e4c8b12..6409bf045e8 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java @@ -117,6 +117,59 @@ public long hashOf(IntEntry entry) { } } + // --- CHA-defeat decoys --------------------------------------------------------------------- + // Never used to build a table; loaded (in setUp) only so KeyStrategy.hash and KeyStrategy.matches + // each have >=2 concrete implementors. That denies C2 the single-implementor CHA devirtualization + // of keyStrat.hash/matches inside get(). If the strategy calls still inline afterward, the win is + // structural (the constant INSTANCE's exact type propagated through the inlined get), not a CHA + // bet that would deopt when a second subclass loads. + + // Second StringKeyStrategy impl -> KeyStrategy.matches is now polymorphic. + static final class DecoyStringKeyStrategy extends FlatHashtable.StringKeyStrategy { + static final DecoyStringKeyStrategy INSTANCE = new DecoyStringKeyStrategy(); + + private DecoyStringKeyStrategy() {} + + @Override + public boolean matches(String key, IntEntry entry) { + return key == entry.key; // deliberately different body from IntEntryKeyStrategy + } + + @Override + public long hashOf(IntEntry entry) { + return hash(entry.key); + } + } + + // Direct KeyStrategy impl with its own hash -> KeyStrategy.hash is now polymorphic too. + static final class DecoyKeyStrategy extends FlatHashtable.KeyStrategy { + static final DecoyKeyStrategy INSTANCE = new DecoyKeyStrategy(); + + private DecoyKeyStrategy() {} + + @Override + public long hash(String key) { + return key.length(); + } + + @Override + public boolean matches(String key, IntEntry entry) { + return key.equals(entry.key); + } + + @Override + public long hashOf(IntEntry entry) { + return entry.key.length(); + } + } + + // Referenced only so these three concrete KeyStrategy implementors load at benchmark class-init, + // before the hot method compiles — see the CHA-defeat note above. + @SuppressWarnings("unused") + static final Object[] CHA_DEFEAT = { + IntEntryKeyStrategy.INSTANCE, DecoyStringKeyStrategy.INSTANCE, DecoyKeyStrategy.INSTANCE + }; + static IntEntry[] newFilledFlat() { // Sized to the key count (FlatHashtable is fixed-capacity, no resize): load factor <= 0.5. IntEntry[] table = FlatHashtable.create(IntEntry.class, INSERTION_KEYS.length); diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java index 9b66e727455..94925664938 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java @@ -144,6 +144,59 @@ public long hashOf(IntEntry entry) { } } + // --- CHA-defeat decoys --------------------------------------------------------------------- + // These are never used to build a table; they exist only to be *loaded* (see loadStrategies), + // so KeyStrategy.hash and KeyStrategy.matches each have >=2 concrete implementors. That denies + // C2 the single-implementor CHA devirtualization of keyStrat.hash/matches inside get(). If the + // strategy calls still inline afterward, the win is structural (the constant INSTANCE's exact + // type propagated through the inlined get), not a CHA bet that would deopt on a second subclass. + + // Second StringKeyStrategy impl -> KeyStrategy.matches is now polymorphic. + static final class DecoyStringKeyStrategy extends FlatHashtable.StringKeyStrategy { + static final DecoyStringKeyStrategy INSTANCE = new DecoyStringKeyStrategy(); + + private DecoyStringKeyStrategy() {} + + @Override + public boolean matches(String key, IntEntry entry) { + return key == entry.key; // deliberately different body from IntEntryKeyStrategy + } + + @Override + public long hashOf(IntEntry entry) { + return hash(entry.key); + } + } + + // Direct KeyStrategy impl with its own hash -> KeyStrategy.hash is now polymorphic too. + static final class DecoyKeyStrategy extends FlatHashtable.KeyStrategy { + static final DecoyKeyStrategy INSTANCE = new DecoyKeyStrategy(); + + private DecoyKeyStrategy() {} + + @Override + public long hash(String key) { + return key.length(); + } + + @Override + public boolean matches(String key, IntEntry entry) { + return key.equals(entry.key); + } + + @Override + public long hashOf(IntEntry entry) { + return entry.key.length(); + } + } + + // Referenced only so these three concrete KeyStrategy implementors load at benchmark class-init, + // before the hot method compiles — see the CHA-defeat note above. + @SuppressWarnings("unused") + static final Object[] CHA_DEFEAT = { + IntEntryKeyStrategy.INSTANCE, DecoyStringKeyStrategy.INSTANCE, DecoyKeyStrategy.INSTANCE + }; + static IntEntry[] _create_flat() { // Sized to the key count (FlatHashtable is fixed-capacity, no resize): load factor <= 0.5. IntEntry[] table = FlatHashtable.create(IntEntry.class, INSERTION_KEYS.length); From 357af142cbd72c5dd9c4bd14b82fa6b82fbf4c20 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 17 Jul 2026 16:56:33 -0400 Subject: [PATCH 09/26] Add case-insensitive strategy, load-factor control, and resize to FlatHashtable Rounds out the FlatHashtable toolbox and adds a case-insensitive map arm to the comparison benchmark: - Strings.caseInsensitiveHashCode: a hashCode consistent with String.equalsIgnoreCase (same two-way fold as regionMatches(ignoreCase)), allocation-free. Reusable primitive; the strategy below composes it. - FlatHashtable.CaseInsensitiveStringKeyStrategy: case-insensitive sibling of StringKeyStrategy, sealing hash to the primitive. - The table now owns the spread (home()): a golden-ratio Fibonacci mix robust to weak/int-derived and full 64-bit hashes alike, so a KeyStrategy returns a plain hashCode without pre-mixing. StringKeyStrategy/CI/Entry now carry raw hashes. - Load-factor control: DEFAULT_LOAD_FACTOR (0.5) / LOW_LOAD_FACTOR (0.25) constants plus create(Class,int,float) / capacityFor(int,float); the 2-arg forms are kept and delegate to the default. - resize(...) and resizingInsert(...) (Entry and KeyStrategy flavors): explicit, caller-invoked growth for the rare full case, keeping get/insert resize-free on the hot path. resizingInsert returns the (possibly new) array either way. Benchmark: CaseInsensitiveMapBenchmark gains a FlatHashtable arm (dogfooding the shared strategy + primitive) and a DEFAULT-vs-LOW load-factor pair, and moves the lookup index per-thread (@State(Scope.Thread)) so the shared counter doesn't floor the fastest reads. On Zulu 21 the CI FlatHashtable is ~2x the (previously recommended) TreeMap at the same zero allocation, and matches HashMap's throughput without HashMap's per-lookup folded-String garbage; LOW_LOAD_FACTOR is a wash for the fold-dominated CI lookup. Co-Authored-By: Claude Opus 4.8 --- .../util/CaseInsensitiveMapBenchmark.java | 141 ++++++++++--- .../datadog/trace/util/FlatHashtable.java | 185 ++++++++++++++++-- .../main/java/datadog/trace/util/Strings.java | 18 ++ .../datadog/trace/util/FlatHashtableTest.java | 142 +++++++++++++- .../StringsCaseInsensitiveHashCodeTest.java | 57 ++++++ 5 files changed, 492 insertions(+), 51 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java diff --git a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java index 48500669cd5..514bd7df0cc 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -7,6 +7,8 @@ import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Threads; import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; @@ -15,42 +17,49 @@ * * *

      - * Benchmark to illustrate the trade-offs around case-insensitive Map look-ups - using either... - *
    • (RECOMMENDED) TreeMap with Comparator of String::compareToIgnoreCase - *
    • HashMap with look-ups using String::toCase + * Benchmark for the trade-offs around case-insensitive Map look-ups, comparing: + *
    • TreeMap with a {@code String::compareToIgnoreCase} comparator — allocation-free, O(log n) + *
    • HashMap keyed on {@code toLowerCase()} — O(1) but allocates a folded String per look-up + *
    • FlatHashtable with a {@link FlatHashtable.CaseInsensitiveStringKeyStrategy} — O(1) probe, + * allocation-free (case folded inside hash/matches), value stored unboxed *
    * - *

    For case-insensitive lookups, TreeMap map creation is consistently faster because it avoids - * String::toCase calls. + *

    Takeaways. FlatHashtable is ~2x the (previously recommended) TreeMap at the same zero + * allocation, and matches HashMap's look-up throughput without HashMap's per-look-up folded + * String (which drives the multi-threaded GC pressure). The case-insensitive hash is the + * consistent-for-all-inputs two-way fold ({@link + * datadog.trace.util.Strings#caseInsensitiveHashCode} — see its note); a cheaper ASCII-only fold + * would recover a few percent for header-name-only hot paths, deliberately not the default. {@code + * LOW_LOAD_FACTOR} makes no difference here (the fold, not the probe count, dominates), so the + * default 0.5 is used. * - *

    Despite calls to String::toCase, HashMap lookups are faster in single threaded - * microbenchmark by 50% but are worse when frequently called in a multi-threaded system. + *

    Numbers below: MacBook M1, Zulu 21, per-thread lookup index, @Fork(5). + * 1 thread * - *

    With many threads, the extra allocation from calling String::toCase leads to frequent GCs - * which has adverse impacts on the whole system. - * MacBook M1 with 1 thread (Java 21) + * Benchmark Mode Cnt Score Error Units + * create_flatHashtable thrpt 15 3723141.4 ± 63717.9 ops/s + * create_hashMap thrpt 15 905452.5 ± 16561.3 ops/s + * create_treeMap thrpt 15 1208339.4 ± 84364.2 ops/s * - * Benchmark Mode Cnt Score Error Units - * CaseInsensitiveMapBenchmark.create_hashMap thrpt 6 994213.041 ± 15718.903 ops/s - * CaseInsensitiveMapBenchmark.create_treeMap thrpt 6 1522900.015 ± 21646.688 ops/s - * - * CaseInsensitiveMapBenchmark.get_hashMap thrpt 6 69149862.293 ± 9168648.566 ops/s - * CaseInsensitiveMapBenchmark.get_treeMap thrpt 6 42796699.230 ± 9029447.805 ops/s + * lookup_flatHashtable thrpt 15 75874505.0 ± 3722582.3 ops/s + * lookup_flatHashtable_lowLoad thrpt 15 75686682.1 ± 1879579.4 ops/s + * lookup_hashMap thrpt 15 80319813.9 ± 7410634.9 ops/s + * lookup_treeMap thrpt 15 45926358.7 ± 1917349.2 ops/s * - * MacBook M1 with 8 threads (Java 21) - * - * Benchmark Mode Cnt Score Error Units - * CaseInsensitiveMapBenchmark.create_hashMap thrpt 6 6641003.483 ± 543210.409 ops/s - * CaseInsensitiveMapBenchmark.create_treeMap thrpt 6 10030191.764 ± 1308865.113 ops/s + * 8 threads (with -prof gc; alloc = gc.alloc.rate.norm) * - * CaseInsensitiveMapBenchmark.get_hashMap thrpt 6 38748031.837 ± 9012072.804 ops/s - * CaseInsensitiveMapBenchmark.get_treeMap thrpt 6 173495470.789 ± 27824904.999 ops/s + * Benchmark Mode Cnt Score Error Units alloc + * lookup_flatHashtable thrpt 15 558144937.2 ± 22797680.7 ops/s ~0 B/op + * lookup_flatHashtable_lowLoad thrpt 15 564984154.1 ± 25899687.5 ops/s ~0 B/op + * lookup_hashMap thrpt 15 529773720.8 ± 82928000.6 ops/s 24.0 B/op (151 GCs) + * lookup_treeMap thrpt 15 262110611.8 ± 30486484.7 ops/s ~0 B/op * */ @Fork(2) @Warmup(iterations = 2) @Measurement(iterations = 3) @Threads(8) +@State(Scope.Thread) public class CaseInsensitiveMapBenchmark { static final String[] PREFIXES = {"foo", "bar", "baz", "quux"}; @@ -87,12 +96,17 @@ static T init(Supplier supplier) { return keys; }); - static int sharedLookupIndex = 0; + // Per-thread (@State(Scope.Thread)) so cycling the lookup key doesn't contend a shared counter. + // The maps stay static/shared (read-only after class-init); only the index is per-thread. A + // shared + // counter's cache-line ping-pong would floor the fastest lookups (the flat probe) at @Threads(8), + // masking exactly the differences this benchmark compares. + int lookupIndex = 0; - static String nextLookupKey() { - int localIndex = ++sharedLookupIndex; + String nextLookupKey() { + int localIndex = ++lookupIndex; if (localIndex >= LOOKUP_KEYS.length) { - sharedLookupIndex = localIndex = 0; + lookupIndex = localIndex = 0; } return LOOKUP_KEYS[localIndex]; } @@ -178,5 +192,78 @@ public Integer lookup_treeMap() { return TREE_MAP.get(nextLookupKey()); } + // FlatHashtable with a case-insensitive KeyStrategy: the strategy folds case inside hash/matches, + // so lookups are O(1) (single probe) AND allocation-free (no String::toCase) — TreeMap's zero- + // alloc property without TreeMap's O(log n) comparison walk. Value is stored unboxed. Read-only + // after build, so reads are lock-free (see FlatHashtable / ThreadSafeMapBenchmark). + static final class CIEntry extends FlatHashtable.Entry { + final String key; // original case preserved + final int value; + + CIEntry(String key, long hash, int value) { + super(hash); // cache the (char-by-char) case-insensitive hash + this.key = key; + this.value = value; + } + } + + // Dogfoods the shared toolbox pieces: the CI hash is Strings.caseInsensitiveHashCode (sealed by + // CaseInsensitiveStringKeyStrategy), the table owns the spread. Only matches/hashOf are bespoke. + static final class CaseInsensitiveKeyStrategy + extends FlatHashtable.CaseInsensitiveStringKeyStrategy { + static final CaseInsensitiveKeyStrategy INSTANCE = new CaseInsensitiveKeyStrategy(); + + private CaseInsensitiveKeyStrategy() {} + + @Override + public boolean matches(String key, CIEntry entry) { + return key.equalsIgnoreCase(entry.key); // case-folded, allocation-free + } + + @Override + public long hashOf(CIEntry entry) { + return entry.hash; // CIEntry caches its (raw, case-insensitive) hash + } + } + + static CIEntry[] _create_flat(float loadFactor) { + // 16 distinct case-insensitive keys (foo-0..quux-3). + CIEntry[] table = + FlatHashtable.create(CIEntry.class, PREFIXES.length * NUM_SUFFIXES, loadFactor); + for (int suffix = 0; suffix < NUM_SUFFIXES; ++suffix) { + for (String prefix : PREFIXES) { + String key = prefix + "-" + suffix; + long hash = CaseInsensitiveKeyStrategy.INSTANCE.hash(key); + FlatHashtable.insert( + table, new CIEntry(key, hash, suffix), CaseInsensitiveKeyStrategy.INSTANCE); + } + } + // The HashMap/TreeMap builds' second loop (UPPER_PREFIXES, suffix 0 & 2) only OVERWRITES values + // case-insensitively — it adds no new keys, and values don't affect lookup throughput — so the + // read set is these same 16 keys. + return table; + } + + @Benchmark + public CIEntry[] create_flatHashtable() { + return _create_flat(FlatHashtable.DEFAULT_LOAD_FACTOR); + } + + static final CIEntry[] FLAT_TABLE = _create_flat(FlatHashtable.DEFAULT_LOAD_FACTOR); + static final CIEntry[] FLAT_TABLE_LOW = _create_flat(FlatHashtable.LOW_LOAD_FACTOR); + + @Benchmark + public CIEntry lookup_flatHashtable() { + // Lock-free, allocation-free, single-probe case-insensitive lookup. + return FlatHashtable.get(FLAT_TABLE, nextLookupKey(), CaseInsensitiveKeyStrategy.INSTANCE); + } + + @Benchmark + public CIEntry lookup_flatHashtable_lowLoad() { + // Same, but at LOW_LOAD_FACTOR (4x): does the sparser table shave probes for the (mostly + // hash-fold-dominated) CI lookup, or is it a wash? — see the delta to lookup_flatHashtable. + return FlatHashtable.get(FLAT_TABLE_LOW, nextLookupKey(), CaseInsensitiveKeyStrategy.INSTANCE); + } + // TODO: Add ConcurrentSkipListMap & synchronized HashMap & TreeMap } diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index 6a934167f2e..fbc9cdc4597 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -38,10 +38,10 @@ * } * *

    Contract: {@code table.length} must be a power of two ({@link #capacityFor}). {@code - * KeyStrategy.hash} should be well-distributed (this class masks it directly). Cardinality cap / - * overflow / a live-size counter are caller policy (this class is pure mechanism): a capped - * caller does {@link #get} first, and only on a miss checks its budget before {@link #getOrCreate} - * (so hits stay a single probe and the create path is warmup-rare). + * KeyStrategy.hash} may return a plain {@code hashCode} — the table owns the spread ({@link + * #home}). Cardinality cap / overflow / a live-size counter are caller policy (this class is + * pure mechanism): a capped caller does {@link #get} first, and only on a miss checks its budget + * before {@link #getOrCreate} (so hits stay a single probe and the create path is warmup-rare). */ public final class FlatHashtable { private FlatHashtable() {} @@ -51,7 +51,8 @@ private FlatHashtable() {} * optimization, not plumbing (open addressing needs no {@code next}), so extending it is * never required: bring any entry type and supply {@link KeyStrategy#hashOf} yourself instead. * Caller contract: {@code hash} must equal the table's {@link KeyStrategy#hash} for this entry's - * key, so the entry lands where {@link #get} looks. + * key (the raw hash — the table applies its own spread), so the entry lands where {@link + * #get} looks. */ public abstract static class Entry { public final long hash; @@ -80,8 +81,8 @@ protected Entry(long hash) { public abstract static class KeyStrategy { /** * Hash of {@code key} ({@code long} for family-wide consistency with Hashtable / - * ConcurrentHashtable and to leave room for composite keys); should be well-distributed (this - * table masks it directly). + * ConcurrentHashtable and to leave room for composite keys). Return a plain {@code hashCode} — + * the table {@linkplain #home spreads} it before masking, so there is no need to pre-mix. */ public abstract long hash(K key); @@ -100,19 +101,35 @@ public abstract static class KeyStrategy { } /** - * {@link KeyStrategy} specialized for {@code String} keys: seals a spread {@link #hash} so - * String-key callers write only {@link #matches}. Extend as a stateless final class held in a - * concrete-typed {@code static final} singleton, exactly like {@link KeyStrategy} — the {@code - * final} hash resolves directly and the concrete subclass still specializes the same at each call - * site, so there's no cost to the extra layer. + * {@link KeyStrategy} specialized for {@code String} keys: seals {@link #hash} to {@link + * String#hashCode} so String-key callers write only {@link #matches}. Extend as a stateless final + * class held in a concrete-typed {@code static final} singleton, exactly like {@link KeyStrategy} + * — the {@code final} hash resolves directly and the concrete subclass still specializes the same + * at each call site, so there's no cost to the extra layer. (No spread here — the table + * {@linkplain #home spreads} the raw hashCode itself.) * * @param stored entry — self-contained (carries its own key) */ public abstract static class StringKeyStrategy extends KeyStrategy { @Override public final long hash(String key) { - final int h = key.hashCode(); - return h ^ (h >>> 16); // spread the int entropy; widened to the family-wide long hash width + return key.hashCode(); // raw; the table spreads before masking + } + } + + /** + * {@link KeyStrategy} for {@code String} keys compared case-insensitively — the case-insensitive + * sibling of {@link StringKeyStrategy}. Seals {@link #hash} to {@link + * Strings#caseInsensitiveHashCode}, which is consistent with {@link String#equalsIgnoreCase} + * (callers implement {@link #matches} with {@code equalsIgnoreCase}). Extend as a stateless final + * class held in a concrete-typed {@code static final} singleton. + * + * @param stored entry — self-contained (carries its own key) + */ + public abstract static class CaseInsensitiveStringKeyStrategy extends KeyStrategy { + @Override + public final long hash(String key) { + return Strings.caseInsensitiveHashCode(key); // raw; the table spreads before masking } } @@ -148,12 +165,42 @@ public interface CreateStrategy { E create(K key); } - /** Power-of-two capacity for a cardinality budget: {@code >= 2 * limit} (load factor <= 0.5). */ + /** + * Balanced default load factor — target fill {@code <= 0.5} ({@code >= 2x} capacity). Linear + * probing then costs ~1.5 probes on a hit, ~2.5 on a miss (Knuth); the general-purpose sweet + * spot. + */ + public static final float DEFAULT_LOAD_FACTOR = 0.5f; + + /** + * Sparse load factor — target fill {@code <= 0.25} ({@code >= 4x} capacity): ~1.2 probes on a + * hit, ~1.4 on a miss. For miss-heavy hot paths (membership checks) where the extra empty slots + * are cheap and shaving the (quadratic-in-load) miss cost is worth the memory. Measure before + * preferring it to {@link #DEFAULT_LOAD_FACTOR}. There is deliberately no higher-than-default + * constant — open addressing degrades sharply past 0.5 (~8.5 probes/miss at 0.75). + */ + public static final float LOW_LOAD_FACTOR = 0.25f; + + /** Power-of-two capacity for a cardinality budget at the {@link #DEFAULT_LOAD_FACTOR}. */ public static int capacityFor(int cardinalityLimit) { + return capacityFor(cardinalityLimit, DEFAULT_LOAD_FACTOR); + } + + /** + * Power-of-two capacity for a cardinality budget at {@code loadFactor}: the smallest power of two + * {@code >= ceil(cardinalityLimit / loadFactor)}. Because it rounds up to a power of two, the + * achieved fill is often below {@code loadFactor} (never above) — you always get at least the + * headroom you asked for. + */ + public static int capacityFor(int cardinalityLimit, float loadFactor) { if (cardinalityLimit <= 0) { throw new IllegalArgumentException("cardinalityLimit must be positive: " + cardinalityLimit); } - return Integer.highestOneBit(cardinalityLimit * 2 - 1) << 1; + if (!(loadFactor > 0f && loadFactor < 1f)) { + throw new IllegalArgumentException("loadFactor must be in (0, 1): " + loadFactor); + } + int min = (int) Math.ceil(cardinalityLimit / (double) loadFactor); + return Integer.highestOneBit(min - 1) << 1; } /** @@ -169,6 +216,15 @@ public static E[] create(Class type, int cardinalityLimit) { return (E[]) Array.newInstance(type, capacityFor(cardinalityLimit)); } + /** + * {@link #create(Class, int)} at an explicit {@code loadFactor} (see {@link #capacityFor(int, + * float)} and the {@link #DEFAULT_LOAD_FACTOR} / {@link #LOW_LOAD_FACTOR} constants). + */ + @SuppressWarnings("unchecked") + public static E[] create(Class type, int cardinalityLimit, float loadFactor) { + return (E[]) Array.newInstance(type, capacityFor(cardinalityLimit, loadFactor)); + } + /** * Existing entry for {@code key}, or {@code null}. Read-only — never creates. Single probe on a * hit; walks to the first empty slot (or all the way around) on a miss. @@ -176,7 +232,7 @@ public static E[] create(Class type, int cardinalityLimit) { @StrategyConsumer public static E get(E[] table, K key, KeyStrategy keyStrat) { final int mask = table.length - 1; - final int start = (int) (keyStrat.hash(key) & mask); + final int start = home(keyStrat.hash(key), mask); int i = start; for (; ; ) { final E e = table[i]; @@ -203,7 +259,7 @@ public static E get(E[] table, K key, KeyStrategy keyStrat) { public static E getOrCreate( E[] table, K key, KeyStrategy keyStrat, CreateStrategy createStrat) { final int mask = table.length - 1; - final int start = (int) (keyStrat.hash(key) & mask); + final int start = home(keyStrat.hash(key), mask); int i = start; for (; ; ) { final E e = table[i]; @@ -252,7 +308,7 @@ public static boolean insert(E[] table, E entry, KeyStrategy keyStrat) */ private static boolean placeAt(E[] table, E entry, long hash) { final int mask = table.length - 1; - final int start = (int) (hash & mask); + final int start = home(hash, mask); int i = start; for (; ; ) { if (table[i] == null) { @@ -266,6 +322,95 @@ private static boolean placeAt(E[] table, E entry, long hash) { } } + /** + * Placement slot for {@code hash} in a table of {@code mask + 1} slots. The table owns the + * spread: a golden-ratio (Fibonacci) multiply diffuses the hash across all bits — robust to weak + * or {@code int}-derived {@code hashCode}s and to full 64-bit composite hashes alike — then the + * low index bits are taken. So a {@link KeyStrategy} may return a plain {@code hashCode} without + * pre-mixing. Package-private so tests can predict slots. + */ + static int home(long hash, int mask) { + long z = hash * 0x9E3779B97F4A7C15L; // 2^64 / golden ratio; odd ⇒ a bijection (loses no bits) + z ^= z >>> 32; // fold the well-mixed high half down into the low bits the mask keeps + return (int) z & mask; + } + + /** + * Doubles capacity and rehashes every entry into a new table — call when {@link #insert} returns + * {@code false} and you want to grow rather than reject; the caller stores the returned array + * back. Convenience over the {@link KeyStrategy}-taking overload for {@link Entry}-based entries + * (the home comes from {@link Entry#hash}). See {@link #resizingInsert(Object[], Object)} to do + * both in one call, and its note on growing over unbounded key domains. + */ + public static E[] resize(E[] table) { + E[] grown = allocateGrown(table); + for (final E e : table) { + if (e != null) { + placeAt(grown, e, e.hash); + } + } + return grown; + } + + /** + * {@link #resize(Entry[])} for any entry type: each entry's home comes from {@link + * KeyStrategy#hashOf}. Not a {@link StrategyConsumer} — the rehash is a cold, one-off traversal, + * not a hot specialization site. + */ + public static E[] resize(E[] table, KeyStrategy keyStrat) { + E[] grown = allocateGrown(table); + for (final E e : table) { + if (e != null) { + placeAt(grown, e, keyStrat.hashOf(e)); + } + } + return grown; + } + + /** + * A new, empty table of twice the capacity, of the same runtime component type as {@code table}. + */ + @SuppressWarnings("unchecked") + private static E[] allocateGrown(E[] table) { + return (E[]) Array.newInstance(table.getClass().getComponentType(), table.length << 1); + } + + /** + * {@link #insert(Entry[], Entry) insert} that grows on demand: adds {@code entry}, {@link + * #resize(Entry[]) resizing} first if the table is full, and returns the table to store back — + * the same array if it fit, a new larger one if it grew: + * + *

    {@code
    +   * table = FlatHashtable.resizingInsert(table, entry); // always reassign
    +   * }
    + * + * Same comparison-free, caller-ensures-absence contract as {@link #insert}. + * + *

    Grows unboundedly. Unlike {@code insert}'s {@code false}, this hides the full signal, + * so it is the easiest place to leak memory: use it only for a genuinely bounded key domain, + * never over externally-controlled cardinality. + */ + public static E[] resizingInsert(E[] table, E entry) { + E[] t = table; + while (!insert(t, entry)) { + t = resize(t); // one doubling always suffices; the loop is belt-and-braces + } + return t; + } + + /** + * {@link #resizingInsert(Entry[], Entry)} for any entry type (home via {@link + * KeyStrategy#hashOf}). Same grows-unboundedly caution. + */ + @StrategyConsumer + public static E[] resizingInsert(E[] table, E entry, KeyStrategy keyStrat) { + E[] t = table; + while (!insert(t, entry, keyStrat)) { + t = resize(t, keyStrat); + } + return t; + } + /** Applies {@code consumer} to every entry in {@code table} (skipping empty slots); any order. */ public static void forEach(E[] table, Consumer consumer) { for (final E e : table) { @@ -316,7 +461,7 @@ private static final class HashIterator implements Iterator { this.table = table; this.hash = hash; this.keyStrat = keyStrat; - this.start = (int) (hash & (table.length - 1)); + this.start = home(hash, table.length - 1); this.i = this.start; advance(); } diff --git a/internal-api/src/main/java/datadog/trace/util/Strings.java b/internal-api/src/main/java/datadog/trace/util/Strings.java index 603a7665c04..5ea52a0ce14 100644 --- a/internal-api/src/main/java/datadog/trace/util/Strings.java +++ b/internal-api/src/main/java/datadog/trace/util/Strings.java @@ -353,4 +353,22 @@ public static boolean regionContains(String s, int beginIndex, int endIndex, Str int idx = s.indexOf(needle, beginIndex); return idx >= 0 && idx + needle.length() <= endIndex; } + + /** + * A {@code hashCode} consistent with {@link String#equalsIgnoreCase}: any two strings that are + * equal ignoring case produce the same value. Same polynomial as {@link String#hashCode} but over + * the case-folded characters, so it never allocates (no {@code toLowerCase} copy). + * + *

    Uses the same two-way fold {@code String.equalsIgnoreCase} / {@code + * String.regionMatches(ignoreCase)} use ({@code toLowerCase(toUpperCase(c))}), so the two stay + * consistent for all inputs, not just ASCII — pairing a one-way fold here with {@code + * equalsIgnoreCase} would risk silent false misses on the Unicode characters where they diverge. + */ + public static int caseInsensitiveHashCode(String s) { + int h = 0; + for (int i = 0, len = s.length(); i < len; ++i) { + h = 31 * h + Character.toLowerCase(Character.toUpperCase(s.charAt(i))); + } + return h; + } } diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 24a74084e1d..7bf39c7f78f 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -73,15 +73,32 @@ public long hashOf(TestEntry entry) { } } - /** All keys hash to the last slot ({@code -1 & mask}), so probing wraps around to index 0. */ + /** + * Smallest hash {@code >= 1} that the table places on {@code slot} of a {@code mask + 1} table. + */ + private static long hashLandingOn(int slot, int mask) { + for (long h = 1; h < 1_000_000L; ++h) { + if (FlatHashtable.home(h, mask) == slot) { + return h; + } + } + throw new AssertionError("no hash found landing on slot " + slot); + } + + /** + * All keys hash to the last slot of a 2-slot table (so probing wraps around to index 0). The + * table owns the spread now, so we compute a hash that lands there rather than assuming {@code -1 + * & mask}. + */ static final class TestLastSlotKeyStrategy extends FlatHashtable.KeyStrategy { static final TestLastSlotKeyStrategy INSTANCE = new TestLastSlotKeyStrategy(); + private static final long LAST_SLOT_HASH = hashLandingOn(1, 1); // slot 1 of a 2-slot table private TestLastSlotKeyStrategy() {} @Override public long hash(String key) { - return -1; + return LAST_SLOT_HASH; } @Override @@ -126,6 +143,26 @@ public boolean matches(String key, TestHashedEntry entry) { } } + /** + * Case-insensitive key strategy: {@code hash} sealed by the CI base, {@code matches} folds case. + */ + static final class TestCaseInsensitiveKeyStrategy + extends FlatHashtable.CaseInsensitiveStringKeyStrategy { + static final TestCaseInsensitiveKeyStrategy INSTANCE = new TestCaseInsensitiveKeyStrategy(); + + private TestCaseInsensitiveKeyStrategy() {} + + @Override + public boolean matches(String key, TestEntry entry) { + return key.equalsIgnoreCase(entry.key); + } + + @Override + public long hashOf(TestEntry entry) { + return hash(entry.key); + } + } + @Test void capacityFor_roundsToPowerOfTwoAtLeastTwiceLimit() { assertEquals(2, FlatHashtable.capacityFor(1)); @@ -309,8 +346,10 @@ void iterator_filtersOutEntriesWithADifferentHash() { FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); - // hash 8 shares the home slot (8 & 7 == 0) but no stored entry has hashOf == 8 - Iterator it = FlatHashtable.iterator(table, 8, TestCollidingKeyStrategy.INSTANCE); + // a hash that shares the entries' home slot (0) but that no stored entry has as its hashOf + long sameHomeOtherHash = hashLandingOn(0, table.length - 1); + Iterator it = + FlatHashtable.iterator(table, sameHomeOtherHash, TestCollidingKeyStrategy.INSTANCE); assertFalse(it.hasNext()); } @@ -321,4 +360,99 @@ void iterator_emptyRunHasNoNext() { assertFalse(it.hasNext()); assertThrows(NoSuchElementException.class, it::next); } + + @Test + void capacityFor_honorsLoadFactor() { + // default 0.5 -> >= 2x; LOW 0.25 -> >= 4x. + assertEquals(8, FlatHashtable.capacityFor(4, FlatHashtable.DEFAULT_LOAD_FACTOR)); + assertEquals(16, FlatHashtable.capacityFor(4, FlatHashtable.LOW_LOAD_FACTOR)); + // capacityFor(cardinalityLimit) delegates to the default. + assertEquals(FlatHashtable.capacityFor(6), FlatHashtable.capacityFor(6, 0.5f)); + } + + @Test + void capacityFor_rejectsLoadFactorOutOfRange() { + assertThrows(IllegalArgumentException.class, () -> FlatHashtable.capacityFor(4, 0f)); + assertThrows(IllegalArgumentException.class, () -> FlatHashtable.capacityFor(4, 1f)); + assertThrows(IllegalArgumentException.class, () -> FlatHashtable.capacityFor(4, -0.1f)); + } + + @Test + void create_honorsLoadFactor() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4, FlatHashtable.LOW_LOAD_FACTOR); + assertEquals(16, table.length); + } + + @Test + void resize_generalFlavor_growsAndKeepsEveryEntryFindable() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots + FlatHashtable.insert(table, new TestEntry("a"), TestEntryKeyStrategy.INSTANCE); + FlatHashtable.insert(table, new TestEntry("b"), TestEntryKeyStrategy.INSTANCE); + + TestEntry[] grown = FlatHashtable.resize(table, TestEntryKeyStrategy.INSTANCE); + assertEquals(4, grown.length); + assertNotSame(table, grown); + assertEquals("a", FlatHashtable.get(grown, "a", TestEntryKeyStrategy.INSTANCE).key); + assertEquals("b", FlatHashtable.get(grown, "b", TestEntryKeyStrategy.INSTANCE).key); + } + + @Test + void resize_entryFlavor_growsAndKeepsEveryEntryFindable() { + TestHashedEntry[] table = FlatHashtable.create(TestHashedEntry.class, 1); // 2 slots + FlatHashtable.insert(table, new TestHashedEntry("a")); + FlatHashtable.insert(table, new TestHashedEntry("b")); + + TestHashedEntry[] grown = FlatHashtable.resize(table); + assertEquals(4, grown.length); + assertEquals("a", FlatHashtable.get(grown, "a", TestHashedKeyStrategy.INSTANCE).key); + assertEquals("b", FlatHashtable.get(grown, "b", TestHashedKeyStrategy.INSTANCE).key); + } + + @Test + void resizingInsert_generalFlavor_growsPastCapacityAndReturnsTheTable() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots + for (int i = 0; i < 10; ++i) { + table = + FlatHashtable.resizingInsert( + table, new TestEntry("k" + i), TestEntryKeyStrategy.INSTANCE); + } + assertTrue(table.length >= 16); // grew from 2 to hold 10 at load factor <= 0.5 + for (int i = 0; i < 10; ++i) { + assertEquals("k" + i, FlatHashtable.get(table, "k" + i, TestEntryKeyStrategy.INSTANCE).key); + } + } + + @Test + void resizingInsert_entryFlavor_growsPastCapacityAndReturnsTheTable() { + TestHashedEntry[] table = FlatHashtable.create(TestHashedEntry.class, 1); // 2 slots + for (int i = 0; i < 10; ++i) { + table = FlatHashtable.resizingInsert(table, new TestHashedEntry("k" + i)); + } + assertTrue(table.length >= 16); + for (int i = 0; i < 10; ++i) { + assertEquals("k" + i, FlatHashtable.get(table, "k" + i, TestHashedKeyStrategy.INSTANCE).key); + } + } + + @Test + void caseInsensitiveStrategy_matchesRegardlessOfCase() { + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); + TestEntry stored = + FlatHashtable.getOrCreate( + table, "Content-Type", TestCaseInsensitiveKeyStrategy.INSTANCE, CREATE); + + // Look-ups in any case resolve to the same stored entry, allocation-free. + assertSame( + stored, FlatHashtable.get(table, "content-type", TestCaseInsensitiveKeyStrategy.INSTANCE)); + assertSame( + stored, FlatHashtable.get(table, "CONTENT-TYPE", TestCaseInsensitiveKeyStrategy.INSTANCE)); + assertSame( + stored, FlatHashtable.get(table, "cOnTeNt-TyPe", TestCaseInsensitiveKeyStrategy.INSTANCE)); + // getOrCreate with a differently-cased key does not mint a second entry. + assertSame( + stored, + FlatHashtable.getOrCreate( + table, "CONTENT-TYPE", TestCaseInsensitiveKeyStrategy.INSTANCE, CREATE)); + assertNull(FlatHashtable.get(table, "content-length", TestCaseInsensitiveKeyStrategy.INSTANCE)); + } } diff --git a/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java b/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java new file mode 100644 index 00000000000..94295c44a43 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java @@ -0,0 +1,57 @@ +package datadog.trace.util; + +import static datadog.trace.util.Strings.caseInsensitiveHashCode; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import org.junit.jupiter.api.Test; + +class StringsCaseInsensitiveHashCodeTest { + + @Test + void equalIgnoringCaseProducesEqualHash() { + assertEquals(caseInsensitiveHashCode("Content-Type"), caseInsensitiveHashCode("content-type")); + assertEquals(caseInsensitiveHashCode("Content-Type"), caseInsensitiveHashCode("CONTENT-TYPE")); + assertEquals(caseInsensitiveHashCode("Content-Type"), caseInsensitiveHashCode("cOnTeNt-TyPe")); + } + + @Test + void emptyStringHashesToZero() { + // Matches String.hashCode("") == 0. + assertEquals(0, caseInsensitiveHashCode("")); + } + + @Test + void distinctContentHashesDiffer() { + // Not a guarantee in general, but these representative keys must not collide. + assertNotEquals(caseInsensitiveHashCode("foo"), caseInsensitiveHashCode("bar")); + assertNotEquals(caseInsensitiveHashCode("Accept"), caseInsensitiveHashCode("Host")); + } + + @Test + void staysConsistentWithEqualsIgnoreCase() { + // For any pair, equalsIgnoreCase => equal hash. (The converse — unequal hash implies not + // equalsIgnoreCase — is what a table relies on to never miss a present key.) + String[] samples = { + "Accept", + "accept", + "ACCEPT", + "Accept-Encoding", + "accept-encoding", + "X-Forwarded-For", + "x-forwarded-for", + "Host", + "host" + }; + for (String a : samples) { + for (String b : samples) { + if (a.equalsIgnoreCase(b)) { + assertEquals( + caseInsensitiveHashCode(a), + caseInsensitiveHashCode(b), + () -> "hash mismatch for equalIgnoreCase pair"); + } + } + } + } +} From 2ea26a36aa6ad3de658feca6fc6d0a5575fe13c4 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Sun, 19 Jul 2026 09:55:09 -0400 Subject: [PATCH 10/26] Split FlatHashtable strategy into HashStrategy/MatchingStrategy + specialize the iterator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape the single KeyStrategy into concern-split roles so each operation asks only for what it needs, and so the iterator can be specialized without stubbing out unused methods: - HashStrategy (hashOf) — entry side: insert / iterator / resize. - MatchingStrategy (matches, + hashKey defaulting to key.hashCode()) — key side: get / getOrCreate. A functional interface; override hashKey only for custom hashing (e.g. case-insensitive). - EntryStrategy implements both — the do-everything base a full user extends. - CaseInsensitiveStringStrategy seals hashKey to Strings.caseInsensitiveHashCode; StringKeyStrategy/EntryKeyStrategy are gone (the String hash is now the default). Specialize the iterator via the CacheHelper static-polymorphism move: HashIterator is an abstract base with final template methods (advanceWith/nextWith) taking the strategy; StrategyHashIterator holds it in a field (general), EntryHashIterator feeds the constant Entry-hash singleton so hashOf inlines to entry.hash. The public iterator(...) API and Iterator return type are unchanged; the call site specializes by its own monomorphism. FlatHashtableIteratorBenchmark demonstrates why the specialization is more robust: @Setup poisons the shared hashOf profile with four distinct strategy types, after which iterate_general (single strategy, looks monomorphic) pays ~2.1x because HotSpot keeps one per-bci profile shared across all inlining contexts, while iterate_specialized is immune (constant type-flow, not profile). Unpoisoned the two tie. Tests and the three map benchmarks updated to the new hierarchy. Co-Authored-By: Claude Opus 4.8 --- .../util/CaseInsensitiveMapBenchmark.java | 8 +- .../util/FlatHashtableIteratorBenchmark.java | 180 ++++++++++ .../util/SingleThreadedMapBenchmark.java | 45 +-- .../trace/util/ThreadSafeMapBenchmark.java | 42 +-- .../datadog/trace/util/FlatHashtable.java | 331 +++++++++++------- .../datadog/trace/util/FlatHashtableTest.java | 269 ++++++++------ 6 files changed, 586 insertions(+), 289 deletions(-) create mode 100644 internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java diff --git a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java index 514bd7df0cc..7952c4f1707 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -208,15 +208,15 @@ static final class CIEntry extends FlatHashtable.Entry { } // Dogfoods the shared toolbox pieces: the CI hash is Strings.caseInsensitiveHashCode (sealed by - // CaseInsensitiveStringKeyStrategy), the table owns the spread. Only matches/hashOf are bespoke. + // CaseInsensitiveStringStrategy), the table owns the spread. Only matches/hashOf are bespoke. static final class CaseInsensitiveKeyStrategy - extends FlatHashtable.CaseInsensitiveStringKeyStrategy { + extends FlatHashtable.CaseInsensitiveStringStrategy { static final CaseInsensitiveKeyStrategy INSTANCE = new CaseInsensitiveKeyStrategy(); private CaseInsensitiveKeyStrategy() {} @Override - public boolean matches(String key, CIEntry entry) { + public boolean matches(CIEntry entry, String key) { return key.equalsIgnoreCase(entry.key); // case-folded, allocation-free } @@ -233,7 +233,7 @@ static CIEntry[] _create_flat(float loadFactor) { for (int suffix = 0; suffix < NUM_SUFFIXES; ++suffix) { for (String prefix : PREFIXES) { String key = prefix + "-" + suffix; - long hash = CaseInsensitiveKeyStrategy.INSTANCE.hash(key); + long hash = CaseInsensitiveKeyStrategy.INSTANCE.hashKey(key); FlatHashtable.insert( table, new CIEntry(key, hash, suffix), CaseInsensitiveKeyStrategy.INSTANCE); } diff --git a/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java new file mode 100644 index 00000000000..09686ef6518 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java @@ -0,0 +1,180 @@ +package datadog.trace.util; + +import java.util.Iterator; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Tests the static-polymorphism iterator specialization: walking a shared-hash bucket through the + * general {@code iterator(table, hash, hashStrategy)} (strategy held in a field -> {@code + * hashOf} dispatched via the shared type profile) vs the Entry-specialized {@code + * iterator(table, hash)} (feeds a constant strategy into the shared traversal template -> {@code + * hashOf} inlines to {@code entry.hash} by constant type-flow). Both return {@code Iterator} and + * reuse the same core; only the strategy call's dispatch differs. + * + *

    Why the general path is fragile. When only one strategy type flows through it, C2 + * devirtualizes {@code hashOf} from the type profile and the two arms tie — type-profile does the + * specialization's job. But HotSpot keeps one {@code hashOf} profile per bytecode index, + * shared across every inlining context (no context-sensitive profiling), so {@code setUp} + * poisons that profile by driving the general iterator with four distinct strategy types. After + * that, {@code iterate_general} — though it uses a single strategy and looks monomorphic — pays a + * virtual {@code hashOf} per entry, degraded by callers it has nothing to do with. {@code + * iterate_specialized} is immune: the constant gives exact-type devirt regardless of the profile. + * That contrast is the point — the specialization turns a profile-dependent inline into a + * structural one. Confirm with {@code -XX:+PrintInlining}. + */ +@Fork(2) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 3, time = 1) +@Threads(1) +@State(Scope.Thread) +public class FlatHashtableIteratorBenchmark { + // All entries share this hash -> one contiguous probe-run "bucket" the iterator walks. + static final long BUCKET_HASH = 0x123456789ABCDEF0L; + static final int BUCKET_SIZE = 16; + + static final class ItEntry extends FlatHashtable.Entry { + final int value; + + ItEntry(long hash, int value) { + super(hash); + this.value = value; + } + } + + // Four DISTINCT hashOf implementors, all reading entry.hash (identical behavior, different + // types). + // Loading them defeats CHA; driving all four through the shared traversal in setUp poisons the + // hashOf profile to megamorphic, so type-profile can't cleanly devirtualize it for the general + // path — the case the Entry-specialized iterator is immune to. + // + // Measured (MacBook M1, Zulu 21, poisoned profile): iterate_general ~18.0M vs iterate_specialized + // ~37.8M ops/s (2.1x). Unpoisoned, the two tie (~37.8M) — type-profile does the general path's + // devirt then; the poisoning is what type-profile can't survive and the constant can. + static final class ItHashStrategyA implements FlatHashtable.HashStrategy { + static final ItHashStrategyA INSTANCE = new ItHashStrategyA(); + + private ItHashStrategyA() {} + + @Override + public long hashOf(ItEntry entry) { + return entry.hash; + } + } + + static final class ItHashStrategyB implements FlatHashtable.HashStrategy { + static final ItHashStrategyB INSTANCE = new ItHashStrategyB(); + + private ItHashStrategyB() {} + + @Override + public long hashOf(ItEntry entry) { + return entry.hash; + } + } + + static final class ItHashStrategyC implements FlatHashtable.HashStrategy { + static final ItHashStrategyC INSTANCE = new ItHashStrategyC(); + + private ItHashStrategyC() {} + + @Override + public long hashOf(ItEntry entry) { + return entry.hash; + } + } + + static final class ItHashStrategyD implements FlatHashtable.HashStrategy { + static final ItHashStrategyD INSTANCE = new ItHashStrategyD(); + + private ItHashStrategyD() {} + + @Override + public long hashOf(ItEntry entry) { + return entry.hash; + } + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + static final FlatHashtable.HashStrategy[] STRATEGIES = + new FlatHashtable.HashStrategy[] { + ItHashStrategyA.INSTANCE, + ItHashStrategyB.INSTANCE, + ItHashStrategyC.INSTANCE, + ItHashStrategyD.INSTANCE, + }; + + ItEntry[] table; + + // Kept live so the profile-poisoning loop below can't be dead-code-eliminated. + static long POISON_SINK; + + @Setup(Level.Trial) + public void setUp() { + // Sparse so the bucket is one contiguous run with a clean terminating empty slot. + table = FlatHashtable.create(ItEntry.class, BUCKET_SIZE, FlatHashtable.LOW_LOAD_FACTOR); + for (int i = 0; i < BUCKET_SIZE; ++i) { + FlatHashtable.insert(table, new ItEntry(BUCKET_HASH, i)); // all share the hash => one bucket + } + // Poison the shared HashIterator.advanceWith hashOf profile: drive the GENERAL iterator with + // four distinct strategy types, hot enough that C2 records the site as megamorphic. HotSpot + // keeps one per-bci profile shared across all inlining contexts (no context-sensitive profiling + // — tried in Zing, never robust), so those unrelated callers permanently pollute the profile. + long sink = 0; + for (int r = 0; r < 200_000; ++r) { + sink += poison(); + } + POISON_SINK = sink; + } + + /** Drives the general iterator once per distinct strategy type — the profile poisoner. */ + private long poison() { + long sink = 0; + for (FlatHashtable.HashStrategy s : STRATEGIES) { + Iterator it = FlatHashtable.iterator(table, BUCKET_HASH, s); + while (it.hasNext()) { + sink += it.next().value; + } + } + return sink; + } + + /** + * General iterator with a SINGLE strategy — looks monomorphic, but its {@code hashOf} still + * dispatches virtually because {@code setUp} already poisoned the shared profile with other + * strategy types. This is the cross-caller pollution failure mode: a caller degraded by callers + * it has nothing to do with. (Unpoisoned, this ties {@code iterate_specialized} — type-profile + * then devirtualizes it; the poisoning is what type-profile can't survive and the constant can.) + */ + @Benchmark + public long iterate_general() { + long sum = 0; + Iterator it = FlatHashtable.iterator(table, BUCKET_HASH, ItHashStrategyA.INSTANCE); + while (it.hasNext()) { + sum += it.next().value; + } + return sum; + } + + /** + * Entry-specialized iterator: feeds the constant Entry-hash strategy into the shared template, so + * {@code hashOf} inlines to {@code entry.hash} structurally (constant type-flow, not + * profile) — immune to the poisoned profile that sinks {@code iterate_general}. + */ + @Benchmark + public long iterate_specialized() { + long sum = 0; + Iterator it = FlatHashtable.iterator(table, BUCKET_HASH); // Entry overload + while (it.hasNext()) { + sum += it.next().value; // hashOf inlined to entry.hash via the constant strategy + } + return sum; + } +} diff --git a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java index 6409bf045e8..cb792cc1ca9 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java @@ -99,7 +99,7 @@ static final class IntEntry { } } - static final class IntEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { + static final class IntEntryKeyStrategy extends FlatHashtable.EntryStrategy { // Canonical exact-typed singleton: one instance, private ctor => the static-poly discipline is // enforced by the class, not left to each caller to declare correctly. static final IntEntryKeyStrategy INSTANCE = new IntEntryKeyStrategy(); @@ -107,53 +107,54 @@ static final class IntEntryKeyStrategy extends FlatHashtable.StringKeyStrategy=2 concrete implementors. That denies C2 the single-implementor CHA devirtualization - // of keyStrat.hash/matches inside get(). If the strategy calls still inline afterward, the win is - // structural (the constant INSTANCE's exact type propagated through the inlined get), not a CHA - // bet that would deopt when a second subclass loads. + // of matchStrat.hashKey/matches inside get(). If the strategy calls still inline afterward, the + // win is structural (the constant INSTANCE's exact type propagated through the inlined get), not + // a + // CHA bet that would deopt when a second subclass loads. - // Second StringKeyStrategy impl -> KeyStrategy.matches is now polymorphic. - static final class DecoyStringKeyStrategy extends FlatHashtable.StringKeyStrategy { - static final DecoyStringKeyStrategy INSTANCE = new DecoyStringKeyStrategy(); + // Second matches impl -> MatchingStrategy.matches is polymorphic. + static final class DecoyMatchStrategy extends FlatHashtable.EntryStrategy { + static final DecoyMatchStrategy INSTANCE = new DecoyMatchStrategy(); - private DecoyStringKeyStrategy() {} + private DecoyMatchStrategy() {} @Override - public boolean matches(String key, IntEntry entry) { + public boolean matches(IntEntry entry, String key) { return key == entry.key; // deliberately different body from IntEntryKeyStrategy } @Override public long hashOf(IntEntry entry) { - return hash(entry.key); + return entry.key.hashCode(); } } - // Direct KeyStrategy impl with its own hash -> KeyStrategy.hash is now polymorphic too. - static final class DecoyKeyStrategy extends FlatHashtable.KeyStrategy { - static final DecoyKeyStrategy INSTANCE = new DecoyKeyStrategy(); + // Overrides hashKey -> MatchingStrategy.hashKey is polymorphic too (default + this override). + static final class DecoyHashKeyStrategy extends FlatHashtable.EntryStrategy { + static final DecoyHashKeyStrategy INSTANCE = new DecoyHashKeyStrategy(); - private DecoyKeyStrategy() {} + private DecoyHashKeyStrategy() {} @Override - public long hash(String key) { + public long hashKey(String key) { return key.length(); } @Override - public boolean matches(String key, IntEntry entry) { + public boolean matches(IntEntry entry, String key) { return key.equals(entry.key); } @@ -163,11 +164,11 @@ public long hashOf(IntEntry entry) { } } - // Referenced only so these three concrete KeyStrategy implementors load at benchmark class-init, - // before the hot method compiles — see the CHA-defeat note above. + // Referenced only so these three concrete implementors load at benchmark class-init, before the + // hot method compiles — see the CHA-defeat note above. @SuppressWarnings("unused") static final Object[] CHA_DEFEAT = { - IntEntryKeyStrategy.INSTANCE, DecoyStringKeyStrategy.INSTANCE, DecoyKeyStrategy.INSTANCE + IntEntryKeyStrategy.INSTANCE, DecoyMatchStrategy.INSTANCE, DecoyHashKeyStrategy.INSTANCE }; static IntEntry[] newFilledFlat() { diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java index 94925664938..1b686591ea4 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java @@ -128,59 +128,59 @@ static final class IntEntry { } } - static final class IntEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { + static final class IntEntryKeyStrategy extends FlatHashtable.EntryStrategy { static final IntEntryKeyStrategy INSTANCE = new IntEntryKeyStrategy(); private IntEntryKeyStrategy() {} @Override - public boolean matches(String key, IntEntry entry) { + public boolean matches(IntEntry entry, String key) { return key.equals(entry.key); } @Override public long hashOf(IntEntry entry) { - return hash(entry.key); + return entry.key.hashCode(); // consistent with the default hashKey } } // --- CHA-defeat decoys --------------------------------------------------------------------- - // These are never used to build a table; they exist only to be *loaded* (see loadStrategies), - // so KeyStrategy.hash and KeyStrategy.matches each have >=2 concrete implementors. That denies - // C2 the single-implementor CHA devirtualization of keyStrat.hash/matches inside get(). If the + // These are never used to build a table; they exist only to be *loaded* (see CHA_DEFEAT), so + // MatchingStrategy.matches and .hashKey each have >=2 concrete implementors. That denies C2 the + // single-implementor CHA devirtualization of matchStrat.hashKey/matches inside get(). If the // strategy calls still inline afterward, the win is structural (the constant INSTANCE's exact // type propagated through the inlined get), not a CHA bet that would deopt on a second subclass. - // Second StringKeyStrategy impl -> KeyStrategy.matches is now polymorphic. - static final class DecoyStringKeyStrategy extends FlatHashtable.StringKeyStrategy { - static final DecoyStringKeyStrategy INSTANCE = new DecoyStringKeyStrategy(); + // Second matches impl -> MatchingStrategy.matches is polymorphic. + static final class DecoyMatchStrategy extends FlatHashtable.EntryStrategy { + static final DecoyMatchStrategy INSTANCE = new DecoyMatchStrategy(); - private DecoyStringKeyStrategy() {} + private DecoyMatchStrategy() {} @Override - public boolean matches(String key, IntEntry entry) { + public boolean matches(IntEntry entry, String key) { return key == entry.key; // deliberately different body from IntEntryKeyStrategy } @Override public long hashOf(IntEntry entry) { - return hash(entry.key); + return entry.key.hashCode(); } } - // Direct KeyStrategy impl with its own hash -> KeyStrategy.hash is now polymorphic too. - static final class DecoyKeyStrategy extends FlatHashtable.KeyStrategy { - static final DecoyKeyStrategy INSTANCE = new DecoyKeyStrategy(); + // Overrides hashKey -> MatchingStrategy.hashKey is polymorphic too (default + this override). + static final class DecoyHashKeyStrategy extends FlatHashtable.EntryStrategy { + static final DecoyHashKeyStrategy INSTANCE = new DecoyHashKeyStrategy(); - private DecoyKeyStrategy() {} + private DecoyHashKeyStrategy() {} @Override - public long hash(String key) { + public long hashKey(String key) { return key.length(); } @Override - public boolean matches(String key, IntEntry entry) { + public boolean matches(IntEntry entry, String key) { return key.equals(entry.key); } @@ -190,11 +190,11 @@ public long hashOf(IntEntry entry) { } } - // Referenced only so these three concrete KeyStrategy implementors load at benchmark class-init, - // before the hot method compiles — see the CHA-defeat note above. + // Referenced only so these three concrete implementors load at benchmark class-init, before the + // hot method compiles — see the CHA-defeat note above. @SuppressWarnings("unused") static final Object[] CHA_DEFEAT = { - IntEntryKeyStrategy.INSTANCE, DecoyStringKeyStrategy.INSTANCE, DecoyKeyStrategy.INSTANCE + IntEntryKeyStrategy.INSTANCE, DecoyMatchStrategy.INSTANCE, DecoyHashKeyStrategy.INSTANCE }; static IntEntry[] _create_flat() { diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index fbc9cdc4597..6a3003c9d41 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -17,31 +17,37 @@ * key/hash/value arrays would create — no {@code volatile}, no atomics — as long as the payload is * one where a stale/lost read is benign (miss → recreate; clobber → one wins). * - *

    Two strategies, split by concern. The per-use policy is two {@link Strategy strategy} - * objects rather than one: + *

    Strategy roles, split by concern. The per-use policy is a small set of {@link Strategy + * strategy} objects rather than one, so a caller supplies only what an operation needs: * *

      - *
    • a {@link KeyStrategy} — how to {@link KeyStrategy#hash hash} a lookup key and {@link - * KeyStrategy#matches match} it against a stored entry. Key-identity is intrinsic to the key - * type, so this is shared and reused (one {@link StringKeyStrategy} serves every - * String-keyed table); it is on the hot path (every probe), so it is an abstract class held - * as a concrete-typed {@code static final} constant to specialize (see {@link Strategy}). - *
    • a {@link CreateStrategy} — how to mint an entry for a key. Creation varies per use case and - * is on the cold path (once per key, at warmup), so it is a {@link FunctionalInterface} you - * can supply as a non-capturing lambda. + *
    • a {@link MatchingStrategy} — the key side: {@link MatchingStrategy#hashKey hash a + * lookup key} (defaults to {@code hashCode}) and {@link MatchingStrategy#matches match} it + * against a stored entry. Used by {@link #get} / {@link #getOrCreate}. + *
    • a {@link HashStrategy} — the entry side: {@link HashStrategy#hashOf hash a stored + * entry}. Used by {@link #insert} / {@link #iterator} / {@link #resize} (which have an entry, + * not a key). For {@link Entry}-based tables this is just the cached {@link Entry#hash}, so + * those get dedicated overloads that need no strategy at all. + *
    • an {@link EntryStrategy} — both of the above, for a user that does lookups and + * inserts; extend this one abstract class and you have the whole policy. + *
    • a {@link CreateStrategy} — how to mint an entry for a key. Cold (once per key, at warmup), + * so it is a {@link FunctionalInterface} you can supply as a non-capturing lambda. *
    * *
    {@code
    - * private static final MyKeyStrategy KEYS = new MyKeyStrategy();   // concrete type => exact type pinned
    + * private static final MyStrategy S = new MyStrategy();          // concrete type => exact type pinned
      * ...
    - * E e = FlatHashtable.getOrCreate(table, key, KEYS, MyEntry::new); // non-capturing create
    + * E e = FlatHashtable.getOrCreate(table, key, S, MyEntry::new);  // non-capturing create
      * }
    * - *

    Contract: {@code table.length} must be a power of two ({@link #capacityFor}). {@code - * KeyStrategy.hash} may return a plain {@code hashCode} — the table owns the spread ({@link - * #home}). Cardinality cap / overflow / a live-size counter are caller policy (this class is - * pure mechanism): a capped caller does {@link #get} first, and only on a miss checks its budget - * before {@link #getOrCreate} (so hits stay a single probe and the create path is warmup-rare). + *

    Contract: {@code table.length} must be a power of two ({@link #capacityFor}). Both + * {@link MatchingStrategy#hashKey} and {@link HashStrategy#hashOf} may return a plain {@code + * hashCode} — the table owns the spread ({@link #home}) — but they must be consistent: + * {@code hashKey(key)} must equal {@code hashOf(entry)} for that key's entry, so a lookup lands + * where the entry was placed (trivially true when both default to {@code hashCode}). Cardinality + * cap / overflow / a live-size counter are caller policy (this class is pure mechanism): a + * capped caller does {@link #get} first, and only on a miss checks its budget before {@link + * #getOrCreate} (so hits stay a single probe and the create path is warmup-rare). */ public final class FlatHashtable { private FlatHashtable() {} @@ -49,8 +55,8 @@ private FlatHashtable() {} /** * Optional structure-free entry base carrying only a cached {@code hash} — an * optimization, not plumbing (open addressing needs no {@code next}), so extending it is - * never required: bring any entry type and supply {@link KeyStrategy#hashOf} yourself instead. - * Caller contract: {@code hash} must equal the table's {@link KeyStrategy#hash} for this entry's + * never required: bring any entry type and supply a {@link HashStrategy} yourself instead. Caller + * contract: {@code hash} must equal the table's {@link MatchingStrategy#hashKey} for this entry's * key (the raw hash — the table applies its own spread), so the entry lands where {@link * #get} looks. */ @@ -63,91 +69,80 @@ protected Entry(long hash) { } /** - * Key-identity strategy: how to {@link #hash} a lookup key and {@link #matches} it against a - * stored entry. Extend as a stateless final class and hold a {@code static final} - * singleton of the concrete type so the JIT can specialize each call site (see {@link Strategy}). + * Entry-side strategy: the hash of a stored {@code entry} — its home is {@link #home}{@code + * (hashOf(entry))}. Used by {@link #insert} / {@link #iterator} / {@link #resize}, which have an + * entry rather than a lookup key. Must be consistent with {@link MatchingStrategy#hashKey} (see + * the class contract). Supply a {@code static final} constant or a non-capturing lambda so it + * stays a single monomorphic instance (see {@link Strategy}); for {@link Entry}-based tables use + * the strategy-free overloads, which read {@link Entry#hash} directly. * - *

    An abstract class (not an interface) on purpose: it forces a named strategy type (no - * lambdas, which can blur the receiver the inliner needs), and if specialization ever misses the - * fallback dispatches via {@code invokevirtual} rather than the costlier megamorphic {@code - * invokeinterface}. Key-identity is the hot strategy (every probe), so it takes the - * abstract-class rigor; creation is the cold one, hence {@link CreateStrategy} is a lambda-able - * interface. + * @param stored entry + */ + @Strategy + @FunctionalInterface + public interface HashStrategy { + long hashOf(E entry); + } + + /** + * Key-side strategy: whether a stored {@code entry} is the one for a lookup {@code key} ({@link + * #matches}), and how to hash that key ({@link #hashKey}). {@code hashKey} defaults to {@code + * key.hashCode()} — override it only when the key's identity needs different hashing (e.g. + * case-insensitive), and then keep it consistent with the table's {@link HashStrategy#hashOf}. + * Used by {@link #get} / {@link #getOrCreate}. + * + *

    A {@link FunctionalInterface} ({@code matches} is the sole abstract method), so the common + * case can be a non-capturing lambda; a strategy that also customizes hashing is a named class + * that overrides {@code hashKey}. * + * @param stored entry * @param lookup key - * @param stored entry — self-contained (carries its own key) */ @Strategy - public abstract static class KeyStrategy { - /** - * Hash of {@code key} ({@code long} for family-wide consistency with Hashtable / - * ConcurrentHashtable and to leave room for composite keys). Return a plain {@code hashCode} — - * the table {@linkplain #home spreads} it before masking, so there is no need to pre-mix. - */ - public abstract long hash(K key); - + @FunctionalInterface + public interface MatchingStrategy { /** Whether the stored {@code entry} is the one for {@code key}. */ - public abstract boolean matches(K key, E entry); + boolean matches(E entry, K key); /** - * Hash of a stored {@code entry} — must equal {@link #hash}{@code (key)} for that entry's key, - * so the entry lands where {@link #get} would look for it. Used by the entry-taking {@link - * #insert(Object[], Object, KeyStrategy)} and {@link #iterator} (which have an entry, not a - * key); {@link #get} lookups never call it. When the entry can surface its key this is - * typically {@code hash(entry.key)}; when it caches its own hash (see {@link Entry}), {@link - * EntryKeyStrategy} seals it to that field. + * Hash of {@code key}; defaults to {@code key.hashCode()} (the table applies its own spread). */ - public abstract long hashOf(E entry); + default long hashKey(K key) { + return key.hashCode(); + } } /** - * {@link KeyStrategy} specialized for {@code String} keys: seals {@link #hash} to {@link - * String#hashCode} so String-key callers write only {@link #matches}. Extend as a stateless final - * class held in a concrete-typed {@code static final} singleton, exactly like {@link KeyStrategy} - * — the {@code final} hash resolves directly and the concrete subclass still specializes the same - * at each call site, so there's no cost to the extra layer. (No spread here — the table - * {@linkplain #home spreads} the raw hashCode itself.) + * The whole policy for a table you both look up in and insert into: a {@link MatchingStrategy} + * (key side) and a {@link HashStrategy} (entry side) in one. Extend as a stateless final + * class held in a concrete-typed {@code static final} singleton so the JIT specializes each call + * site (see {@link Strategy}); an abstract class (not an interface) so a specialization + * miss falls back to {@code invokevirtual} rather than the costlier megamorphic {@code + * invokeinterface}. * - * @param stored entry — self-contained (carries its own key) + * @param stored entry + * @param lookup key */ - public abstract static class StringKeyStrategy extends KeyStrategy { - @Override - public final long hash(String key) { - return key.hashCode(); // raw; the table spreads before masking - } - } + @Strategy + public abstract static class EntryStrategy + implements HashStrategy, MatchingStrategy {} /** - * {@link KeyStrategy} for {@code String} keys compared case-insensitively — the case-insensitive - * sibling of {@link StringKeyStrategy}. Seals {@link #hash} to {@link - * Strings#caseInsensitiveHashCode}, which is consistent with {@link String#equalsIgnoreCase} - * (callers implement {@link #matches} with {@code equalsIgnoreCase}). Extend as a stateless final - * class held in a concrete-typed {@code static final} singleton. + * {@link EntryStrategy} for {@code String} keys compared case-insensitively: seals {@link + * #hashKey} to {@link Strings#caseInsensitiveHashCode} (consistent with {@link + * String#equalsIgnoreCase}, which callers use in {@link #matches}). Callers still supply {@link + * #matches} (typically {@code key.equalsIgnoreCase(entry.key)}) and {@link #hashOf} (the same + * case-insensitive hash of the entry's key, or the cached {@link Entry#hash}). * * @param stored entry — self-contained (carries its own key) */ - public abstract static class CaseInsensitiveStringKeyStrategy extends KeyStrategy { + public abstract static class CaseInsensitiveStringStrategy extends EntryStrategy { @Override - public final long hash(String key) { + public final long hashKey(String key) { return Strings.caseInsensitiveHashCode(key); // raw; the table spreads before masking } } - /** - * {@link KeyStrategy} for entries that extend {@link Entry}: seals {@link #hashOf} to the entry's - * cached {@code hash}. Callers still supply {@link #hash} and {@link #matches} (or start from - * {@link StringKeyStrategy} for the {@code hash} seal too). - * - * @param lookup key - * @param stored entry — must extend {@link Entry} - */ - public abstract static class EntryKeyStrategy extends KeyStrategy { - @Override - public final long hashOf(E entry) { - return entry.hash; - } - } - /** * Creation strategy: mint a new entry for {@code key} (called once, on insert). A {@link * FunctionalInterface} — supply a {@code static final} constant or a non-capturing lambda @@ -156,12 +151,12 @@ public final long hashOf(E entry) { * Strategy}). Bespoke rather than {@link java.util.function.Function} so it carries the {@link * Strategy} contract and reads as {@code create} at the call site. * - * @param lookup key * @param stored entry to create + * @param lookup key */ @Strategy @FunctionalInterface - public interface CreateStrategy { + public interface CreateStrategy { E create(K key); } @@ -230,16 +225,16 @@ public static E[] create(Class type, int cardinalityLimit, float loadFact * hit; walks to the first empty slot (or all the way around) on a miss. */ @StrategyConsumer - public static E get(E[] table, K key, KeyStrategy keyStrat) { + public static E get(E[] table, K key, MatchingStrategy matchStrat) { final int mask = table.length - 1; - final int start = home(keyStrat.hash(key), mask); + final int start = home(matchStrat.hashKey(key), mask); int i = start; for (; ; ) { final E e = table[i]; if (e == null) { return null; // empty slot terminates the probe (no tombstones) } - if (keyStrat.matches(key, e)) { + if (matchStrat.matches(e, key)) { return e; } i = (i + 1) & mask; @@ -256,10 +251,10 @@ public static E get(E[] table, K key, KeyStrategy keyStrat) { * double-create is acceptable only when the payload makes it benign (see class doc). */ @StrategyConsumer - public static E getOrCreate( - E[] table, K key, KeyStrategy keyStrat, CreateStrategy createStrat) { + public static E getOrCreate( + E[] table, K key, MatchingStrategy matchStrat, CreateStrategy createStrat) { final int mask = table.length - 1; - final int start = home(keyStrat.hash(key), mask); + final int start = home(matchStrat.hashKey(key), mask); int i = start; for (; ; ) { final E e = table[i]; @@ -268,7 +263,7 @@ public static E getOrCreate( table[i] = created; // single-reference publish; benign clobber (see class doc) return created; } - if (keyStrat.matches(key, e)) { + if (matchStrat.matches(e, key)) { return e; } i = (i + 1) & mask; @@ -280,7 +275,7 @@ public static E getOrCreate( /** * Unconditionally adds {@code entry} at the first empty slot from its {@link Entry#hash home}; - * {@code false} if the table is full. Convenience over the {@link KeyStrategy}-taking overload + * {@code false} if the table is full. Convenience over the {@link HashStrategy}-taking overload * for {@link Entry}-based entries (the home comes from the entry, so no strategy is needed). * *

    Comparison-free and caller-responsible. It does not check for an existing key, so the @@ -295,12 +290,11 @@ public static boolean insert(E[] table, E entry) { /** * {@link #insert(Entry[], Entry)} for any entry type: the home comes from {@link - * KeyStrategy#hashOf}. Same comparison-free, caller-ensures-absence contract (the key type is - * irrelevant here — insert never hashes or matches a key). + * HashStrategy#hashOf}. Same comparison-free, caller-ensures-absence contract. */ @StrategyConsumer - public static boolean insert(E[] table, E entry, KeyStrategy keyStrat) { - return placeAt(table, entry, keyStrat.hashOf(entry)); + public static boolean insert(E[] table, E entry, HashStrategy hashStrat) { + return placeAt(table, entry, hashStrat.hashOf(entry)); } /** @@ -326,8 +320,8 @@ private static boolean placeAt(E[] table, E entry, long hash) { * Placement slot for {@code hash} in a table of {@code mask + 1} slots. The table owns the * spread: a golden-ratio (Fibonacci) multiply diffuses the hash across all bits — robust to weak * or {@code int}-derived {@code hashCode}s and to full 64-bit composite hashes alike — then the - * low index bits are taken. So a {@link KeyStrategy} may return a plain {@code hashCode} without - * pre-mixing. Package-private so tests can predict slots. + * low index bits are taken. So a strategy may return a plain {@code hashCode} without pre-mixing. + * Package-private so tests can predict slots. */ static int home(long hash, int mask) { long z = hash * 0x9E3779B97F4A7C15L; // 2^64 / golden ratio; odd ⇒ a bijection (loses no bits) @@ -338,8 +332,8 @@ static int home(long hash, int mask) { /** * Doubles capacity and rehashes every entry into a new table — call when {@link #insert} returns * {@code false} and you want to grow rather than reject; the caller stores the returned array - * back. Convenience over the {@link KeyStrategy}-taking overload for {@link Entry}-based entries - * (the home comes from {@link Entry#hash}). See {@link #resizingInsert(Object[], Object)} to do + * back. Convenience over the {@link HashStrategy}-taking overload for {@link Entry}-based entries + * (the home comes from {@link Entry#hash}). See {@link #resizingInsert(Entry[], Entry)} to do * both in one call, and its note on growing over unbounded key domains. */ public static E[] resize(E[] table) { @@ -354,14 +348,14 @@ public static E[] resize(E[] table) { /** * {@link #resize(Entry[])} for any entry type: each entry's home comes from {@link - * KeyStrategy#hashOf}. Not a {@link StrategyConsumer} — the rehash is a cold, one-off traversal, + * HashStrategy#hashOf}. Not a {@link StrategyConsumer} — the rehash is a cold, one-off traversal, * not a hot specialization site. */ - public static E[] resize(E[] table, KeyStrategy keyStrat) { + public static E[] resize(E[] table, HashStrategy hashStrat) { E[] grown = allocateGrown(table); for (final E e : table) { if (e != null) { - placeAt(grown, e, keyStrat.hashOf(e)); + placeAt(grown, e, hashStrat.hashOf(e)); } } return grown; @@ -400,13 +394,13 @@ public static E[] resizingInsert(E[] table, E entry) { /** * {@link #resizingInsert(Entry[], Entry)} for any entry type (home via {@link - * KeyStrategy#hashOf}). Same grows-unboundedly caution. + * HashStrategy#hashOf}). Same grows-unboundedly caution. */ @StrategyConsumer - public static E[] resizingInsert(E[] table, E entry, KeyStrategy keyStrat) { + public static E[] resizingInsert(E[] table, E entry, HashStrategy hashStrat) { E[] t = table; - while (!insert(t, entry, keyStrat)) { - t = resize(t, keyStrat); + while (!insert(t, entry, hashStrat)) { + t = resize(t, hashStrat); } return t; } @@ -435,38 +429,62 @@ public static void forEach( /** * Read-only iterator over the entries sharing {@code hash} — walks the probe run from {@code - * hash}'s home and yields each entry whose {@link KeyStrategy#hashOf} equals {@code hash}, - * stopping at the first empty slot (the FlatHashtable analogue of walking a chained bucket). The - * key type is irrelevant, so any {@link KeyStrategy} for {@code E} works. + * hash}'s home and yields each entry whose {@link HashStrategy#hashOf} equals {@code hash}, + * stopping at the first empty slot (the FlatHashtable analogue of walking a chained bucket). * - *

    Deliberately not a {@link StrategyConsumer}: iteration goes through the {@link - * Iterator} interface and calls {@code hashOf} virtually, so the strategy does not inline here — - * this is a cold traversal, not a hot specialization site. (Still pass a {@code static final} + *

    This general overload holds the strategy in a field, so {@code hashOf} is called virtually + * (not inlined). For {@link Entry}-based tables prefer {@link #iterator(Entry[], long)}, which + * specializes the traversal so {@code hashOf} inlines. (Still pass a {@code static final} * strategy to avoid a per-call allocation.) */ - public static Iterator iterator(E[] table, long hash, KeyStrategy keyStrat) { - return new HashIterator<>(table, hash, keyStrat); + public static Iterator iterator(E[] table, long hash, HashStrategy hashStrat) { + return new StrategyHashIterator<>(table, hash, hashStrat); } - private static final class HashIterator implements Iterator { - private final E[] table; - private final long hash; - private final KeyStrategy keyStrat; - private final int start; - private int i; - private boolean done; - private E lookahead; + /** + * {@link #iterator(Object[], long, HashStrategy)} for {@link Entry}-based tables: the filter hash + * comes from {@link Entry#hash}, so no strategy is needed. Returns a specialized iterator that + * feeds the {@link Entry}-hash strategy singleton into the shared traversal template as a + * constant, so {@code hashOf} devirtualizes to {@code entry.hash} and inlines — a monomorphic + * call site thus gets a devirtualized pull-based traversal while keeping the plain {@link + * Iterator} API and reusing the same core (see the {@link HashIterator} base). Same {@code + * Iterator} return type as the general overload; the call site specializes by its own + * monomorphism. + */ + public static Iterator iterator(E[] table, long hash) { + return new EntryHashIterator<>(table, hash); + } - HashIterator(E[] table, long hash, KeyStrategy keyStrat) { + /** + * Shared iterator core. The traversal lives in {@code final} template methods parameterized by + * the strategy ({@code advanceWith}/{@code nextWith}); each concrete subclass implements {@link + * #next} by handing in its strategy source — a field (general) or a {@code static final} constant + * (Entry). Feeding a constant into the {@code final} template is what lets the specialized + * subclass inline {@code hashOf} (the CacheHelper static-polymorphism move), so the two share all + * the mechanism yet differ only in whether the strategy call devirtualizes. + */ + private abstract static class HashIterator implements Iterator { + final E[] table; + final long hash; + final int start; + int i; + boolean done; + E lookahead; + + HashIterator(E[] table, long hash) { this.table = table; this.hash = hash; - this.keyStrat = keyStrat; this.start = home(hash, table.length - 1); this.i = this.start; - advance(); + // Priming advance() is left to the concrete ctor: it needs the subclass's strategy source, + // which isn't set until after super(). } - private void advance() { + /** + * Template traversal core, parameterized by the strategy. {@code final} so that a subclass + * passing a constant strategy inlines this and devirtualizes {@code hashOf}. + */ + final void advanceWith(HashStrategy hashStrat) { lookahead = null; if (done) { return; @@ -478,7 +496,7 @@ private void advance() { done = true; // probe run ends at the first empty slot return; } - final boolean match = keyStrat.hashOf(e) == hash; + final boolean match = hashStrat.hashOf(e) == hash; i = (i + 1) & mask; final boolean wrapped = (i == start); if (match) { @@ -493,19 +511,66 @@ private void advance() { } } + final E nextWith(HashStrategy hashStrat) { + final E e = lookahead; + if (e == null) { + throw new NoSuchElementException(); + } + advanceWith(hashStrat); + return e; + } + @Override - public boolean hasNext() { + public final boolean hasNext() { return lookahead != null; } + // Abstract so each subclass injects its own strategy source into nextWith(); that binding is + // what lets the Entry variant inline hashOf while the general one stays virtual. + @Override + public abstract E next(); + } + + /** General iterator: strategy held in a field, so {@code hashOf} stays a virtual call. */ + private static final class StrategyHashIterator extends HashIterator { + private final HashStrategy hashStrat; + + StrategyHashIterator(E[] table, long hash, HashStrategy hashStrat) { + super(table, hash); + this.hashStrat = hashStrat; + advanceWith(hashStrat); // prime + } + @Override public E next() { - final E e = lookahead; - if (e == null) { - throw new NoSuchElementException(); - } - advance(); - return e; + return nextWith(hashStrat); } } + + /** + * Entry iterator: feeds the constant {@link #ENTRY_HASH} singleton, so {@code hashOf} inlines to + * {@code entry.hash}. + */ + private static final class EntryHashIterator extends HashIterator { + EntryHashIterator(E[] table, long hash) { + super(table, hash); + advanceWith(entryHash()); // prime with the constant + } + + @Override + public E next() { + return nextWith(entryHash()); + } + } + + /** + * Stateless {@link HashStrategy} that reads the cached {@link Entry#hash} — the constant fed into + * the Entry iterator template so {@code hashOf} devirtualizes. + */ + private static final HashStrategy ENTRY_HASH = entry -> entry.hash; + + @SuppressWarnings("unchecked") + private static HashStrategy entryHash() { + return (HashStrategy) ENTRY_HASH; // safe: hashOf only reads Entry.hash, present on all E + } } diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 7bf39c7f78f..1b165bb8285 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -27,49 +27,49 @@ static final class TestEntry { } /** - * Stateless concrete key strategy over String keys, exposed as a canonical {@code INSTANCE} - * singleton (private ctor) so the JIT can specialize each call site. {@code hash} is sealed by - * {@link FlatHashtable.StringKeyStrategy}; {@code hashOf} recomputes from the entry's key (this - * entry doesn't cache its hash). + * Stateless concrete strategy over String keys, exposed as a canonical {@code INSTANCE} singleton + * (private ctor) so the JIT can specialize each call site. {@code hashKey} defaults to {@code + * key.hashCode()}; {@code hashOf} recomputes from the entry's key (this entry doesn't cache it), + * consistently with that default. */ - static final class TestEntryKeyStrategy extends FlatHashtable.StringKeyStrategy { - static final TestEntryKeyStrategy INSTANCE = new TestEntryKeyStrategy(); + static final class TestEntryStrategy extends FlatHashtable.EntryStrategy { + static final TestEntryStrategy INSTANCE = new TestEntryStrategy(); - private TestEntryKeyStrategy() {} + private TestEntryStrategy() {} @Override - public boolean matches(String key, TestEntry entry) { + public boolean matches(TestEntry entry, String key) { return key.equals(entry.key); } @Override public long hashOf(TestEntry entry) { - return hash(entry.key); + return entry.key.hashCode(); // consistent with the default hashKey (key.hashCode()) } } /** Non-capturing create strategy (a constructor method ref => singleton-cached, alloc-free). */ - private static final FlatHashtable.CreateStrategy CREATE = TestEntry::new; + private static final FlatHashtable.CreateStrategy CREATE = TestEntry::new; - /** All keys hash to slot 0, so inserts chain by linear probing — exercises the probe path. */ - static final class TestCollidingKeyStrategy extends FlatHashtable.KeyStrategy { - static final TestCollidingKeyStrategy INSTANCE = new TestCollidingKeyStrategy(); + /** All keys/entries hash to slot 0, so inserts chain by linear probing — exercises the probe. */ + static final class TestCollidingStrategy extends FlatHashtable.EntryStrategy { + static final TestCollidingStrategy INSTANCE = new TestCollidingStrategy(); - private TestCollidingKeyStrategy() {} + private TestCollidingStrategy() {} @Override - public long hash(String key) { + public long hashKey(String key) { return 0; } @Override - public boolean matches(String key, TestEntry entry) { + public boolean matches(TestEntry entry, String key) { return key.equals(entry.key); } @Override public long hashOf(TestEntry entry) { - return hash(entry.key); + return 0; } } @@ -90,25 +90,25 @@ private static long hashLandingOn(int slot, int mask) { * table owns the spread now, so we compute a hash that lands there rather than assuming {@code -1 * & mask}. */ - static final class TestLastSlotKeyStrategy extends FlatHashtable.KeyStrategy { - static final TestLastSlotKeyStrategy INSTANCE = new TestLastSlotKeyStrategy(); + static final class TestLastSlotStrategy extends FlatHashtable.EntryStrategy { + static final TestLastSlotStrategy INSTANCE = new TestLastSlotStrategy(); private static final long LAST_SLOT_HASH = hashLandingOn(1, 1); // slot 1 of a 2-slot table - private TestLastSlotKeyStrategy() {} + private TestLastSlotStrategy() {} @Override - public long hash(String key) { + public long hashKey(String key) { return LAST_SLOT_HASH; } @Override - public boolean matches(String key, TestEntry entry) { + public boolean matches(TestEntry entry, String key) { return key.equals(entry.key); } @Override public long hashOf(TestEntry entry) { - return hash(entry.key); + return LAST_SLOT_HASH; } } @@ -119,47 +119,46 @@ static final class TestHashedEntry extends FlatHashtable.Entry { final String key; TestHashedEntry(String key) { - // cache the same hash TestEntryKeyStrategy uses for lookups - super(TestEntryKeyStrategy.INSTANCE.hash(key)); + super(key.hashCode()); // cache the default hashKey (key.hashCode()) this.key = key; } } - /** Key strategy for {@link TestHashedEntry}: {@code hashOf} is sealed to the cached hash. */ - static final class TestHashedKeyStrategy - extends FlatHashtable.EntryKeyStrategy { - static final TestHashedKeyStrategy INSTANCE = new TestHashedKeyStrategy(); + /** Strategy for {@link TestHashedEntry}: {@code hashOf} reads the cached hash. */ + static final class TestHashedStrategy + extends FlatHashtable.EntryStrategy { + static final TestHashedStrategy INSTANCE = new TestHashedStrategy(); - private TestHashedKeyStrategy() {} + private TestHashedStrategy() {} @Override - public long hash(String key) { - return TestEntryKeyStrategy.INSTANCE.hash(key); + public boolean matches(TestHashedEntry entry, String key) { + return key.equals(entry.key); } @Override - public boolean matches(String key, TestHashedEntry entry) { - return key.equals(entry.key); + public long hashOf(TestHashedEntry entry) { + return entry.hash; } } /** - * Case-insensitive key strategy: {@code hash} sealed by the CI base, {@code matches} folds case. + * Case-insensitive strategy: {@code hashKey} sealed by the CI base, {@code matches} folds case. */ - static final class TestCaseInsensitiveKeyStrategy - extends FlatHashtable.CaseInsensitiveStringKeyStrategy { - static final TestCaseInsensitiveKeyStrategy INSTANCE = new TestCaseInsensitiveKeyStrategy(); + static final class TestCaseInsensitiveStrategy + extends FlatHashtable.CaseInsensitiveStringStrategy { + static final TestCaseInsensitiveStrategy INSTANCE = new TestCaseInsensitiveStrategy(); - private TestCaseInsensitiveKeyStrategy() {} + private TestCaseInsensitiveStrategy() {} @Override - public boolean matches(String key, TestEntry entry) { + public boolean matches(TestEntry entry, String key) { return key.equalsIgnoreCase(entry.key); } @Override public long hashOf(TestEntry entry) { - return hash(entry.key); + return Strings.caseInsensitiveHashCode(entry.key); } } @@ -186,67 +185,65 @@ void create_allocatesTypedTableOfCapacity() { @Test void getOrCreate_insertsOnceAndReturnsTheExistingEntry() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - TestEntry first = FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE); + TestEntry first = FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); assertEquals("a", first.key); // A second call must return the SAME instance, not mint a new one. - assertSame(first, FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE)); - assertSame(first, FlatHashtable.get(table, "a", TestEntryKeyStrategy.INSTANCE)); + assertSame(first, FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE)); + assertSame(first, FlatHashtable.get(table, "a", TestEntryStrategy.INSTANCE)); } @Test void get_returnsNullForAbsentKey() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - assertNull(FlatHashtable.get(table, "missing", TestEntryKeyStrategy.INSTANCE)); - FlatHashtable.getOrCreate(table, "present", TestEntryKeyStrategy.INSTANCE, CREATE); - assertNull(FlatHashtable.get(table, "still-missing", TestEntryKeyStrategy.INSTANCE)); + assertNull(FlatHashtable.get(table, "missing", TestEntryStrategy.INSTANCE)); + FlatHashtable.getOrCreate(table, "present", TestEntryStrategy.INSTANCE, CREATE); + assertNull(FlatHashtable.get(table, "still-missing", TestEntryStrategy.INSTANCE)); } @Test void getOrCreate_returnsNullWhenTableIsFull() { // capacityFor(1) == 2 slots. TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); - assertTrue( - FlatHashtable.getOrCreate(table, "k0", TestEntryKeyStrategy.INSTANCE, CREATE) != null); - assertTrue( - FlatHashtable.getOrCreate(table, "k1", TestEntryKeyStrategy.INSTANCE, CREATE) != null); + assertTrue(FlatHashtable.getOrCreate(table, "k0", TestEntryStrategy.INSTANCE, CREATE) != null); + assertTrue(FlatHashtable.getOrCreate(table, "k1", TestEntryStrategy.INSTANCE, CREATE) != null); // Both slots occupied by distinct keys -> a third distinct key finds no room. - assertNull(FlatHashtable.getOrCreate(table, "k2", TestEntryKeyStrategy.INSTANCE, CREATE)); + assertNull(FlatHashtable.getOrCreate(table, "k2", TestEntryStrategy.INSTANCE, CREATE)); // ...but an existing key still resolves even when full. assertSame( - FlatHashtable.get(table, "k0", TestEntryKeyStrategy.INSTANCE), - FlatHashtable.getOrCreate(table, "k0", TestEntryKeyStrategy.INSTANCE, CREATE)); + FlatHashtable.get(table, "k0", TestEntryStrategy.INSTANCE), + FlatHashtable.getOrCreate(table, "k0", TestEntryStrategy.INSTANCE, CREATE)); } @Test - void stringKeyStrategy_hashIsStableForEqualKeys() { + void hashKey_isStableForEqualKeys() { assertEquals( - TestEntryKeyStrategy.INSTANCE.hash("route"), - TestEntryKeyStrategy.INSTANCE.hash(new String("route"))); + TestEntryStrategy.INSTANCE.hashKey("route"), + TestEntryStrategy.INSTANCE.hashKey(new String("route"))); } @Test void collision_probesPastOccupiedSlots_andResolvesEach() { // 8 slots; COLLIDING sends all to slot 0 TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); - TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); + TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); // slot 0 taken -> 1 - TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); // -> slot 2 - TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingKeyStrategy.INSTANCE, CREATE); + TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingStrategy.INSTANCE, CREATE); assertNotSame(a, b); assertNotSame(b, c); // each resolves via probe-past-occupied + match-after-probe - assertSame(a, FlatHashtable.get(table, "a", TestCollidingKeyStrategy.INSTANCE)); - assertSame(b, FlatHashtable.get(table, "b", TestCollidingKeyStrategy.INSTANCE)); - assertSame(c, FlatHashtable.get(table, "c", TestCollidingKeyStrategy.INSTANCE)); + assertSame(a, FlatHashtable.get(table, "a", TestCollidingStrategy.INSTANCE)); + assertSame(b, FlatHashtable.get(table, "b", TestCollidingStrategy.INSTANCE)); + assertSame(c, FlatHashtable.get(table, "c", TestCollidingStrategy.INSTANCE)); // existing colliding key: found after probing, no new entry minted - assertSame(b, FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE)); + assertSame(b, FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE)); // absent key: probe past the 3 occupied slots, hit an empty slot -> null - assertNull(FlatHashtable.get(table, "absent", TestCollidingKeyStrategy.INSTANCE)); + assertNull(FlatHashtable.get(table, "absent", TestCollidingStrategy.INSTANCE)); } @Test @@ -254,34 +251,34 @@ void collision_probeWrapsAroundToFront() { // 2 slots (0,1), mask=1; LAST_SLOT starts at 1 TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // -> slot 1 - TestEntry k0 = FlatHashtable.getOrCreate(table, "k0", TestLastSlotKeyStrategy.INSTANCE, CREATE); + TestEntry k0 = FlatHashtable.getOrCreate(table, "k0", TestLastSlotStrategy.INSTANCE, CREATE); // taken -> wraps to 0 - TestEntry k1 = FlatHashtable.getOrCreate(table, "k1", TestLastSlotKeyStrategy.INSTANCE, CREATE); + TestEntry k1 = FlatHashtable.getOrCreate(table, "k1", TestLastSlotStrategy.INSTANCE, CREATE); assertNotSame(k0, k1); - assertSame(k0, FlatHashtable.get(table, "k0", TestLastSlotKeyStrategy.INSTANCE)); + assertSame(k0, FlatHashtable.get(table, "k0", TestLastSlotStrategy.INSTANCE)); // start slot 1 is occupied (no match) -> probe wraps to slot 0 -> match - assertSame(k1, FlatHashtable.get(table, "k1", TestLastSlotKeyStrategy.INSTANCE)); + assertSame(k1, FlatHashtable.get(table, "k1", TestLastSlotStrategy.INSTANCE)); } @Test void get_returnsNullWhenTableFullAndKeyAbsent() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots - FlatHashtable.getOrCreate(table, "k0", TestCollidingKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "k0", TestCollidingStrategy.INSTANCE, CREATE); // fills slots 0 and 1 - FlatHashtable.getOrCreate(table, "k1", TestCollidingKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "k1", TestCollidingStrategy.INSTANCE, CREATE); // get() probes both occupied slots, wraps back to start -> null (get's full-wrap branch) - assertNull(FlatHashtable.get(table, "absent", TestCollidingKeyStrategy.INSTANCE)); + assertNull(FlatHashtable.get(table, "absent", TestCollidingStrategy.INSTANCE)); } @Test void insert_generalFlavor_placesViaHashOfAndResolves() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); TestEntry e = new TestEntry("a"); - // flavor 2: the home comes from TestEntryKeyStrategy.INSTANCE.hashOf(e) - assertTrue(FlatHashtable.insert(table, e, TestEntryKeyStrategy.INSTANCE)); - assertSame(e, FlatHashtable.get(table, "a", TestEntryKeyStrategy.INSTANCE)); + // flavor 2: the home comes from TestEntryStrategy.INSTANCE.hashOf(e) + assertTrue(FlatHashtable.insert(table, e, TestEntryStrategy.INSTANCE)); + assertSame(e, FlatHashtable.get(table, "a", TestEntryStrategy.INSTANCE)); } @Test @@ -290,24 +287,24 @@ void insert_entryFlavor_placesViaCachedHashAndResolves() { TestHashedEntry e = new TestHashedEntry("a"); // flavor 1: the home comes from the Entry's own cached hash, no strategy needed assertTrue(FlatHashtable.insert(table, e)); - assertSame(e, FlatHashtable.get(table, "a", TestHashedKeyStrategy.INSTANCE)); + assertSame(e, FlatHashtable.get(table, "a", TestHashedStrategy.INSTANCE)); } @Test void insert_returnsFalseWhenFull() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots - assertTrue(FlatHashtable.insert(table, new TestEntry("k0"), TestEntryKeyStrategy.INSTANCE)); - assertTrue(FlatHashtable.insert(table, new TestEntry("k1"), TestEntryKeyStrategy.INSTANCE)); + assertTrue(FlatHashtable.insert(table, new TestEntry("k0"), TestEntryStrategy.INSTANCE)); + assertTrue(FlatHashtable.insert(table, new TestEntry("k1"), TestEntryStrategy.INSTANCE)); // no room - assertFalse(FlatHashtable.insert(table, new TestEntry("k2"), TestEntryKeyStrategy.INSTANCE)); + assertFalse(FlatHashtable.insert(table, new TestEntry("k2"), TestEntryStrategy.INSTANCE)); } @Test void forEach_visitsEveryEntry() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "b", TestEntryKeyStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "c", TestEntryKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "c", TestEntryStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); FlatHashtable.forEach(table, e -> seen.add(e.key)); @@ -317,8 +314,8 @@ void forEach_visitsEveryEntry() { @Test void forEach_contextVariant_passesContextWithoutCapture() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - FlatHashtable.getOrCreate(table, "a", TestEntryKeyStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "b", TestEntryKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestEntryStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); FlatHashtable.forEach(table, seen, (ctx, e) -> ctx.add(e.key)); @@ -328,12 +325,12 @@ void forEach_contextVariant_passesContextWithoutCapture() { @Test void iterator_yieldsEveryEntrySharingTheHash() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // COLLIDING sends all to slot 0 - TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); - TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); - TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingKeyStrategy.INSTANCE, CREATE); + TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); - Iterator it = FlatHashtable.iterator(table, 0, TestCollidingKeyStrategy.INSTANCE); + Iterator it = FlatHashtable.iterator(table, 0, TestCollidingStrategy.INSTANCE); while (it.hasNext()) { seen.add(it.next()); } @@ -343,20 +340,20 @@ void iterator_yieldsEveryEntrySharingTheHash() { @Test void iterator_filtersOutEntriesWithADifferentHash() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // entries at slot 0, hashOf == 0 - FlatHashtable.getOrCreate(table, "a", TestCollidingKeyStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "b", TestCollidingKeyStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); // a hash that shares the entries' home slot (0) but that no stored entry has as its hashOf long sameHomeOtherHash = hashLandingOn(0, table.length - 1); Iterator it = - FlatHashtable.iterator(table, sameHomeOtherHash, TestCollidingKeyStrategy.INSTANCE); + FlatHashtable.iterator(table, sameHomeOtherHash, TestCollidingStrategy.INSTANCE); assertFalse(it.hasNext()); } @Test void iterator_emptyRunHasNoNext() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); - Iterator it = FlatHashtable.iterator(table, 0, TestCollidingKeyStrategy.INSTANCE); + Iterator it = FlatHashtable.iterator(table, 0, TestCollidingStrategy.INSTANCE); assertFalse(it.hasNext()); assertThrows(NoSuchElementException.class, it::next); } @@ -386,14 +383,14 @@ void create_honorsLoadFactor() { @Test void resize_generalFlavor_growsAndKeepsEveryEntryFindable() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots - FlatHashtable.insert(table, new TestEntry("a"), TestEntryKeyStrategy.INSTANCE); - FlatHashtable.insert(table, new TestEntry("b"), TestEntryKeyStrategy.INSTANCE); + FlatHashtable.insert(table, new TestEntry("a"), TestEntryStrategy.INSTANCE); + FlatHashtable.insert(table, new TestEntry("b"), TestEntryStrategy.INSTANCE); - TestEntry[] grown = FlatHashtable.resize(table, TestEntryKeyStrategy.INSTANCE); + TestEntry[] grown = FlatHashtable.resize(table, TestEntryStrategy.INSTANCE); assertEquals(4, grown.length); assertNotSame(table, grown); - assertEquals("a", FlatHashtable.get(grown, "a", TestEntryKeyStrategy.INSTANCE).key); - assertEquals("b", FlatHashtable.get(grown, "b", TestEntryKeyStrategy.INSTANCE).key); + assertEquals("a", FlatHashtable.get(grown, "a", TestEntryStrategy.INSTANCE).key); + assertEquals("b", FlatHashtable.get(grown, "b", TestEntryStrategy.INSTANCE).key); } @Test @@ -404,8 +401,8 @@ void resize_entryFlavor_growsAndKeepsEveryEntryFindable() { TestHashedEntry[] grown = FlatHashtable.resize(table); assertEquals(4, grown.length); - assertEquals("a", FlatHashtable.get(grown, "a", TestHashedKeyStrategy.INSTANCE).key); - assertEquals("b", FlatHashtable.get(grown, "b", TestHashedKeyStrategy.INSTANCE).key); + assertEquals("a", FlatHashtable.get(grown, "a", TestHashedStrategy.INSTANCE).key); + assertEquals("b", FlatHashtable.get(grown, "b", TestHashedStrategy.INSTANCE).key); } @Test @@ -413,12 +410,11 @@ void resizingInsert_generalFlavor_growsPastCapacityAndReturnsTheTable() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots for (int i = 0; i < 10; ++i) { table = - FlatHashtable.resizingInsert( - table, new TestEntry("k" + i), TestEntryKeyStrategy.INSTANCE); + FlatHashtable.resizingInsert(table, new TestEntry("k" + i), TestEntryStrategy.INSTANCE); } assertTrue(table.length >= 16); // grew from 2 to hold 10 at load factor <= 0.5 for (int i = 0; i < 10; ++i) { - assertEquals("k" + i, FlatHashtable.get(table, "k" + i, TestEntryKeyStrategy.INSTANCE).key); + assertEquals("k" + i, FlatHashtable.get(table, "k" + i, TestEntryStrategy.INSTANCE).key); } } @@ -430,29 +426,84 @@ void resizingInsert_entryFlavor_growsPastCapacityAndReturnsTheTable() { } assertTrue(table.length >= 16); for (int i = 0; i < 10; ++i) { - assertEquals("k" + i, FlatHashtable.get(table, "k" + i, TestHashedKeyStrategy.INSTANCE).key); + assertEquals("k" + i, FlatHashtable.get(table, "k" + i, TestHashedStrategy.INSTANCE).key); + } + } + + /** Entry with an explicitly-set cached hash — lets a test force a shared-hash bucket. */ + static final class TestHashEntry extends FlatHashtable.Entry { + final String key; + + TestHashEntry(String key, long hash) { + super(hash); + this.key = key; } } + @Test + void entryIterator_yieldsEveryEntrySharingTheHash() { + TestHashEntry[] table = FlatHashtable.create(TestHashEntry.class, 4); + TestHashEntry a = new TestHashEntry("a", 0); + TestHashEntry b = new TestHashEntry("b", 0); + TestHashEntry c = new TestHashEntry("c", 0); + FlatHashtable.insert(table, a); + FlatHashtable.insert(table, b); + FlatHashtable.insert(table, c); + + Set seen = new HashSet<>(); + // 2-arg Entry overload -> the specialized iterator (constant strategy, inlined hashOf). + Iterator it = FlatHashtable.iterator(table, 0); + while (it.hasNext()) { + seen.add(it.next()); + } + assertEquals(new HashSet<>(Arrays.asList(a, b, c)), seen); + } + + @Test + void entryIterator_filtersOutEntriesWithADifferentHash() { + TestHashEntry[] table = FlatHashtable.create(TestHashEntry.class, 4); // 8 slots, mask 7 + FlatHashtable.insert(table, new TestHashEntry("a", 0)); + FlatHashtable.insert(table, new TestHashEntry("b", 0)); + // shares the home slot (0) but a different hash -> must be filtered out + long otherHash = hashLandingOn(0, table.length - 1); + FlatHashtable.insert(table, new TestHashEntry("c", otherHash)); + + int yielded = 0; + Iterator it = FlatHashtable.iterator(table, 0); + while (it.hasNext()) { + assertEquals(0, it.next().hash); + yielded++; + } + assertEquals(2, yielded); + } + + @Test + void entryIterator_emptyRunHasNoNext() { + TestHashEntry[] table = FlatHashtable.create(TestHashEntry.class, 4); + Iterator it = FlatHashtable.iterator(table, 0); + assertFalse(it.hasNext()); + assertThrows(NoSuchElementException.class, it::next); + } + @Test void caseInsensitiveStrategy_matchesRegardlessOfCase() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); TestEntry stored = FlatHashtable.getOrCreate( - table, "Content-Type", TestCaseInsensitiveKeyStrategy.INSTANCE, CREATE); + table, "Content-Type", TestCaseInsensitiveStrategy.INSTANCE, CREATE); // Look-ups in any case resolve to the same stored entry, allocation-free. assertSame( - stored, FlatHashtable.get(table, "content-type", TestCaseInsensitiveKeyStrategy.INSTANCE)); + stored, FlatHashtable.get(table, "content-type", TestCaseInsensitiveStrategy.INSTANCE)); assertSame( - stored, FlatHashtable.get(table, "CONTENT-TYPE", TestCaseInsensitiveKeyStrategy.INSTANCE)); + stored, FlatHashtable.get(table, "CONTENT-TYPE", TestCaseInsensitiveStrategy.INSTANCE)); assertSame( - stored, FlatHashtable.get(table, "cOnTeNt-TyPe", TestCaseInsensitiveKeyStrategy.INSTANCE)); + stored, FlatHashtable.get(table, "cOnTeNt-TyPe", TestCaseInsensitiveStrategy.INSTANCE)); // getOrCreate with a differently-cased key does not mint a second entry. assertSame( stored, FlatHashtable.getOrCreate( - table, "CONTENT-TYPE", TestCaseInsensitiveKeyStrategy.INSTANCE, CREATE)); - assertNull(FlatHashtable.get(table, "content-length", TestCaseInsensitiveKeyStrategy.INSTANCE)); + table, "CONTENT-TYPE", TestCaseInsensitiveStrategy.INSTANCE, CREATE)); + assertNull(FlatHashtable.get(table, "content-length", TestCaseInsensitiveStrategy.INSTANCE)); } } From 58f712dc91cc907ae793bf1c2e13b4407ce3c5fa Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Sun, 19 Jul 2026 10:01:44 -0400 Subject: [PATCH 11/26] Finalize iterator benchmark numbers at F5 (2.10x, tight CIs) Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtableIteratorBenchmark.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java index 09686ef6518..43d928176b1 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/FlatHashtableIteratorBenchmark.java @@ -55,9 +55,10 @@ static final class ItEntry extends FlatHashtable.Entry { // hashOf profile to megamorphic, so type-profile can't cleanly devirtualize it for the general // path — the case the Entry-specialized iterator is immune to. // - // Measured (MacBook M1, Zulu 21, poisoned profile): iterate_general ~18.0M vs iterate_specialized - // ~37.8M ops/s (2.1x). Unpoisoned, the two tie (~37.8M) — type-profile does the general path's - // devirt then; the poisoning is what type-profile can't survive and the constant can. + // Measured (MacBook M1, Zulu 21, F5, poisoned profile): iterate_general 18.06M ±0.18 vs + // iterate_specialized 37.88M ±0.16 ops/s (2.10x, tight non-overlapping CIs). Unpoisoned, the two + // tie (~37.8M) — type-profile does the general path's devirt then; the poisoning is what + // type-profile can't survive and the constant can. static final class ItHashStrategyA implements FlatHashtable.HashStrategy { static final ItHashStrategyA INSTANCE = new ItHashStrategyA(); From c6a166c9c702959a15029dc063fcd5b81da82b7c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Sun, 19 Jul 2026 10:19:15 -0400 Subject: [PATCH 12/26] Refresh CaseInsensitiveMapBenchmark Javadoc with post-reshape F5 numbers Co-Authored-By: Claude Opus 4.8 --- .../util/CaseInsensitiveMapBenchmark.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java index 7952c4f1707..686a521cf3c 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -37,22 +37,22 @@ * 1 thread * * Benchmark Mode Cnt Score Error Units - * create_flatHashtable thrpt 15 3723141.4 ± 63717.9 ops/s - * create_hashMap thrpt 15 905452.5 ± 16561.3 ops/s - * create_treeMap thrpt 15 1208339.4 ± 84364.2 ops/s + * create_flatHashtable thrpt 15 3794154.9 ± 52254.3 ops/s + * create_hashMap thrpt 15 954573.2 ± 34051.9 ops/s + * create_treeMap thrpt 15 1233590.4 ± 9950.1 ops/s * - * lookup_flatHashtable thrpt 15 75874505.0 ± 3722582.3 ops/s - * lookup_flatHashtable_lowLoad thrpt 15 75686682.1 ± 1879579.4 ops/s - * lookup_hashMap thrpt 15 80319813.9 ± 7410634.9 ops/s - * lookup_treeMap thrpt 15 45926358.7 ± 1917349.2 ops/s + * lookup_flatHashtable thrpt 15 75350287.4 ± 4128577.5 ops/s + * lookup_flatHashtable_lowLoad thrpt 15 77127204.7 ± 2546322.0 ops/s + * lookup_hashMap thrpt 15 76615721.1 ± 4615488.0 ops/s + * lookup_treeMap thrpt 15 45777645.5 ± 4551223.1 ops/s * * 8 threads (with -prof gc; alloc = gc.alloc.rate.norm) * * Benchmark Mode Cnt Score Error Units alloc - * lookup_flatHashtable thrpt 15 558144937.2 ± 22797680.7 ops/s ~0 B/op - * lookup_flatHashtable_lowLoad thrpt 15 564984154.1 ± 25899687.5 ops/s ~0 B/op - * lookup_hashMap thrpt 15 529773720.8 ± 82928000.6 ops/s 24.0 B/op (151 GCs) - * lookup_treeMap thrpt 15 262110611.8 ± 30486484.7 ops/s ~0 B/op + * lookup_flatHashtable thrpt 15 537007985.7 ± 21864181.3 ops/s ~0 B/op + * lookup_flatHashtable_lowLoad thrpt 15 540434673.5 ± 20451984.4 ops/s ~0 B/op + * lookup_hashMap thrpt 15 441875038.1 ± 110408182.2 ops/s 24.0 B/op (129 GCs) + * lookup_treeMap thrpt 15 251195415.1 ± 14662568.3 ops/s ~0 B/op * */ @Fork(2) From 10ee8e35f4e98839f476c0cab82fc1404a1d3111 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 20 Jul 2026 08:23:50 -0400 Subject: [PATCH 13/26] Cover FlatHashtable resize empty-slot skip and iterator full-table wrap Closes the two jacoco branch gaps the reshape left: resize now runs over a partially-filled table (exercising the null-slot skip in the rehash loop), and two iterator tests walk a full colliding table (the match-on-wrapping-slot and absent-hash walked-whole-table paths in HashIterator.advanceWith). All FlatHashtable classes are now 100% instruction- and branch-covered. Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtableTest.java | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 1b165bb8285..666f7520fb9 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -358,6 +358,34 @@ void iterator_emptyRunHasNoNext() { assertThrows(NoSuchElementException.class, it::next); } + @Test + void iterator_fullTable_yieldsMatchesIncludingTheWrappingSlot() { + // 2 slots, both filled by colliding (hash 0) entries -> the probe has no empty slot to stop at, + // so the traversal must yield the entry on the wrapping slot and then terminate on wrap. + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); + TestEntry a = FlatHashtable.getOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); + + Set seen = new HashSet<>(); + Iterator it = FlatHashtable.iterator(table, 0, TestCollidingStrategy.INSTANCE); + while (it.hasNext()) { + seen.add(it.next()); + } + assertEquals(new HashSet<>(Arrays.asList(a, b)), seen); + } + + @Test + void iterator_fullTable_absentHash_terminatesOnWrap() { + // Full table, iterating a hash no stored entry has (all hashOf == 0) -> the traversal walks + // every slot and wraps without ever hitting an empty one, then reports no elements. + TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); + FlatHashtable.getOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); + + Iterator it = FlatHashtable.iterator(table, 5, TestCollidingStrategy.INSTANCE); + assertFalse(it.hasNext()); + } + @Test void capacityFor_honorsLoadFactor() { // default 0.5 -> >= 2x; LOW 0.25 -> >= 4x. @@ -382,12 +410,13 @@ void create_honorsLoadFactor() { @Test void resize_generalFlavor_growsAndKeepsEveryEntryFindable() { - TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots + // 8 slots with only 2 entries -> the rehash loop skips empty slots as well as copying entries. + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); FlatHashtable.insert(table, new TestEntry("a"), TestEntryStrategy.INSTANCE); FlatHashtable.insert(table, new TestEntry("b"), TestEntryStrategy.INSTANCE); TestEntry[] grown = FlatHashtable.resize(table, TestEntryStrategy.INSTANCE); - assertEquals(4, grown.length); + assertEquals(16, grown.length); assertNotSame(table, grown); assertEquals("a", FlatHashtable.get(grown, "a", TestEntryStrategy.INSTANCE).key); assertEquals("b", FlatHashtable.get(grown, "b", TestEntryStrategy.INSTANCE).key); @@ -395,12 +424,12 @@ void resize_generalFlavor_growsAndKeepsEveryEntryFindable() { @Test void resize_entryFlavor_growsAndKeepsEveryEntryFindable() { - TestHashedEntry[] table = FlatHashtable.create(TestHashedEntry.class, 1); // 2 slots + TestHashedEntry[] table = FlatHashtable.create(TestHashedEntry.class, 4); // 8 slots, 2 entries FlatHashtable.insert(table, new TestHashedEntry("a")); FlatHashtable.insert(table, new TestHashedEntry("b")); TestHashedEntry[] grown = FlatHashtable.resize(table); - assertEquals(4, grown.length); + assertEquals(16, grown.length); assertEquals("a", FlatHashtable.get(grown, "a", TestHashedStrategy.INSTANCE).key); assertEquals("b", FlatHashtable.get(grown, "b", TestHashedStrategy.INSTANCE).key); } From cb9b9f6a03581b7e1a7c67fd21619069aa5c4608 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 20 Jul 2026 11:16:05 -0400 Subject: [PATCH 14/26] Address Codex review on #11980: fair CI create arm + clarify caseInsensitiveHashCode semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CaseInsensitiveMapBenchmark: _create_flat now mirrors the HashMap/TreeMap builds' second loop (8 UPPER_PREFIXES case-insensitive collisions) via getOrCreate — all hits, so it does the same probe/match work the maps' overwrite puts do (the non-capturing create never fires, nothing allocates). All three create arms now run the same 24 ops, so create_* is comparable. Refreshed create numbers (create_flatHashtable 2.16M, was an unfair 3.79M measuring only 16 inserts). - Strings.caseInsensitiveHashCode javadoc: note it folds per-char exactly as String.equalsIgnoreCase itself does, so a supplementary case pair (U+10400 / U+10428) is treated as distinct by both and the two stay consistent — a code-point fold would make the hash inconsistent with equalsIgnoreCase, not more correct. Co-Authored-By: Claude Opus 4.8 --- .../util/CaseInsensitiveMapBenchmark.java | 24 ++++++++++++++----- .../main/java/datadog/trace/util/Strings.java | 6 +++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java index 686a521cf3c..9236ae35c79 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -37,9 +37,9 @@ * 1 thread * * Benchmark Mode Cnt Score Error Units - * create_flatHashtable thrpt 15 3794154.9 ± 52254.3 ops/s - * create_hashMap thrpt 15 954573.2 ± 34051.9 ops/s - * create_treeMap thrpt 15 1233590.4 ± 9950.1 ops/s + * create_flatHashtable thrpt 15 2158595.7 ± 73576.7 ops/s + * create_hashMap thrpt 15 944890.9 ± 34398.7 ops/s + * create_treeMap thrpt 15 1285085.3 ± 133648.6 ops/s * * lookup_flatHashtable thrpt 15 75350287.4 ± 4128577.5 ops/s * lookup_flatHashtable_lowLoad thrpt 15 77127204.7 ± 2546322.0 ops/s @@ -226,6 +226,11 @@ public long hashOf(CIEntry entry) { } } + // Never fires here (the mirror loop below only hits already-present keys), so it neither + // allocates nor captures; the value is unused. Kept static/non-capturing to avoid per-call cost. + static final FlatHashtable.CreateStrategy CI_CREATE = + key -> new CIEntry(key, CaseInsensitiveKeyStrategy.INSTANCE.hashKey(key), 0); + static CIEntry[] _create_flat(float loadFactor) { // 16 distinct case-insensitive keys (foo-0..quux-3). CIEntry[] table = @@ -238,9 +243,16 @@ static CIEntry[] _create_flat(float loadFactor) { table, new CIEntry(key, hash, suffix), CaseInsensitiveKeyStrategy.INSTANCE); } } - // The HashMap/TreeMap builds' second loop (UPPER_PREFIXES, suffix 0 & 2) only OVERWRITES values - // case-insensitively — it adds no new keys, and values don't affect lookup throughput — so the - // read set is these same 16 keys. + // Mirror the HashMap/TreeMap builds' second loop (UPPER_PREFIXES, suffix 0 & 2): 8 case- + // insensitive collisions. getOrCreate finds the already-present lower-case entry (a hit -> the + // create never fires, nothing allocates), doing the same probe/match work the maps' overwrite + // puts do — so all three create arms perform the same 24 operations and are comparable. + for (int suffix = 0; suffix < NUM_SUFFIXES; suffix += 2) { + for (String prefix : UPPER_PREFIXES) { + FlatHashtable.getOrCreate( + table, prefix + "-" + suffix, CaseInsensitiveKeyStrategy.INSTANCE, CI_CREATE); + } + } return table; } diff --git a/internal-api/src/main/java/datadog/trace/util/Strings.java b/internal-api/src/main/java/datadog/trace/util/Strings.java index 5ea52a0ce14..bf862c44802 100644 --- a/internal-api/src/main/java/datadog/trace/util/Strings.java +++ b/internal-api/src/main/java/datadog/trace/util/Strings.java @@ -363,6 +363,12 @@ public static boolean regionContains(String s, int beginIndex, int endIndex, Str * String.regionMatches(ignoreCase)} use ({@code toLowerCase(toUpperCase(c))}), so the two stay * consistent for all inputs, not just ASCII — pairing a one-way fold here with {@code * equalsIgnoreCase} would risk silent false misses on the Unicode characters where they diverge. + * + *

    Folds per {@code char} (UTF-16 unit), which is exactly what {@code equalsIgnoreCase} itself + * does — so a supplementary case pair (e.g. U+10400 / U+10428) is treated as distinct by + * both, and the two remain consistent. This mirrors {@code equalsIgnoreCase} rather than doing + * full code-point case folding; a code-point fold would unify pairs {@code equalsIgnoreCase} does + * not, making the hash inconsistent with it. */ public static int caseInsensitiveHashCode(String s) { int h = 0; From 3fc2869f25a564d7572629682827060747f1eec7 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 20 Jul 2026 12:11:42 -0400 Subject: [PATCH 15/26] Document FlatHashtable as the bounded-by-construction safety primitive + default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Class javadoc: frame fixed capacity as a feature — create() forces you to size the table (surfacing the cardinality question that, unasked, becomes an unbounded-growth leak in an embedded agent). It caps rather than churns; growth is an explicit opt-in. Positions it as the general-purpose default of the family. Co-Authored-By: Claude Opus 4.8 --- .../main/java/datadog/trace/util/FlatHashtable.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index 6a3003c9d41..2cbb334f8c3 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -17,6 +17,19 @@ * key/hash/value arrays would create — no {@code volatile}, no atomics — as long as the payload is * one where a stale/lost read is benign (miss → recreate; clobber → one wins). * + *

    Bounded by construction — and that is a feature, not just a limit. {@link #create} + * takes a cardinality budget, so you cannot build one without deciding how big it may get — + * the question whose unasked version becomes an unbounded-growth leak in a long-lived agent living + * in someone else's process. A regular {@code Map}'s auto-resize lets you forget that (fine when + * you own the heap; the wrong default when you are a guest in one). This table never grows on its + * own: {@link #get} / {@link #getOrCreate} / {@link #insert} cap rather than churn — a full + * table degrades to recompute-on-miss with bounded memory and no reallocation — and growth is an + * explicit, deliberate {@link #resize} / {@link #resizingInsert}. So it defaults to the + * bounded-footprint posture the agent needs, with unbounded growth an opt-in you have to reach for + * (and one that, over externally-controlled keys, is the leak this structure otherwise prevents — + * see {@link #resizingInsert}). The trade only pays when a miss is benign (a cache / interner), not + * for a must-hold-everything map. + * *

    Strategy roles, split by concern. The per-use policy is a small set of {@link Strategy * strategy} objects rather than one, so a caller supplies only what an operation needs: * From 1e2e5eef687d68610be211a474a052acac695b17 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 28 Jul 2026 17:08:38 -0400 Subject: [PATCH 16/26] Add D1/D2 convenience tables to FlatHashtable Mirror Hashtable.D1/D2 as an entry-centric convenience layer over the (untouched) strategy-based static core: nested D1/D2 hold a table + size and delegate placement/growth to the core, while D1.Entry/D2.Entry carry the key(s), matches(), and cached hash. Chosen at construction via distinct factories rather than an ambiguous (Class, int) constructor: - createFixed(maxCapacity) bounded; getOrCreate caps (returns null) - createGrowable(initialCapacity) doubles on demand; never caps No remove (the open-addressed core has no tombstones); true-map needs are left to a future LightMap. Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtable.java | 390 ++++++++++++++++++ 1 file changed, 390 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index 2cbb334f8c3..ef0e0bf08e1 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -3,8 +3,10 @@ import datadog.trace.api.function.Strategy; import datadog.trace.api.function.StrategyConsumer; import java.lang.reflect.Array; +import java.util.Arrays; import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.Objects; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -81,6 +83,394 @@ protected Entry(long hash) { } } + /** + * Single-key, {@code HashMap}-style convenience over the {@linkplain FlatHashtable static core}: + * {@link #get} / {@link #getOrCreate} / {@link #insert} / {@link #forEach} without writing a + * {@link MatchingStrategy}. Reach for it when you want something quick that beats {@code + * HashMap} — the entry carries its own value fields, so updating an existing value is + * allocation-free (look up once, then write the returned entry). + * + *

    Fixed or growable, chosen at construction. {@link #createFixed} keeps the raw core's + * bounded posture — the table holds up to {@code maxCapacity} entries, then {@link #getOrCreate} + * caps and returns {@code null} (the caller supplies the overflow default). {@link + * #createGrowable} trades that for {@code HashMap}-like ergonomics — its {@code initialCapacity} + * is a sizing hint, not a cap, the table doubles when it fills past its load factor, and {@code + * getOrCreate} never returns {@code null}. The distinct factory names make the choice explicit at + * the call site (there's no ambiguous {@code (Class, int)} constructor); {@code Capacity} always + * counts entries — contrast the chained {@code Hashtable.D1}, whose factory counts + * buckets. + * + *

    Entry-centric, not strategy-based. Supply a {@link D1.Entry} subclass carrying the + * key and value fields; key equality is {@link Object#equals} by default (override {@link + * Entry#matches(Object)} for e.g. reference equality on interned keys). The strategy-based core + * is untouched and remains the expert path. + * + *

    No {@code remove}. The open-addressed core has no tombstones (an empty slot + * terminates a probe), so deletion isn't supported — this layer is for accumulate / interner / + * counter workloads, not churn. + * + *

    Not thread-safe. + * + * @param the key type + * @param the user's {@link D1.Entry D1.Entry<K>} subclass + */ + public static final class D1> { + /** + * Abstract base for {@link D1} entries. Subclass to add value fields you wish to mutate in + * place after retrieving the entry via {@link D1#get}. The key is captured at construction and + * stored alongside its precomputed hash (via {@link FlatHashtable.Entry}). + * + * @param the key type + */ + public abstract static class Entry extends FlatHashtable.Entry { + final K key; + + protected Entry(K key) { + super(hash(key)); + this.key = key; + } + + public final K key() { + return key; + } + + public boolean matches(Object key) { + return Objects.equals(this.key, key); + } + + /** + * Returns the lookup hash for {@code key}. Null keys map to {@link Long#MIN_VALUE} so they + * don't collide with a real key that hashes to 0; real-key collisions are resolved by {@link + * #matches(Object)}. + */ + public static long hash(Object key) { + return (key == null) ? Long.MIN_VALUE : key.hashCode(); + } + } + + private final boolean growable; + private final float loadFactor; + private TEntry[] table; + private int size; + private int limit; // grow trigger when growable; hard cap when fixed + + private D1(TEntry[] table, float loadFactor, boolean growable, int limit) { + this.growable = growable; + this.loadFactor = loadFactor; + this.table = table; + this.size = 0; + this.limit = limit; + } + + /** + * A bounded {@link D1} holding up to {@code maxCapacity} entries at the {@link + * #DEFAULT_LOAD_FACTOR}, then capping ({@link #getOrCreate} returns {@code null}). + */ + public static > D1 createFixed(Class type, int maxCapacity) { + return createFixed(type, maxCapacity, DEFAULT_LOAD_FACTOR); + } + + public static > D1 createFixed( + Class type, int maxCapacity, float loadFactor) { + return new D1<>(create(type, maxCapacity, loadFactor), loadFactor, false, maxCapacity); + } + + /** + * A growable {@link D1} sized initially for {@code initialCapacity} entries at the {@link + * #DEFAULT_LOAD_FACTOR}; doubles on demand, so it never caps. + */ + public static > D1 createGrowable( + Class type, int initialCapacity) { + return createGrowable(type, initialCapacity, DEFAULT_LOAD_FACTOR); + } + + public static > D1 createGrowable( + Class type, int initialCapacity, float loadFactor) { + E[] table = create(type, initialCapacity, loadFactor); + return new D1<>(table, loadFactor, true, (int) (table.length * loadFactor)); + } + + public int size() { + return size; + } + + /** Existing entry for {@code key}, or {@code null}. Read-only — never creates. */ + public TEntry get(K key) { + final long keyHash = Entry.hash(key); + final TEntry[] table = this.table; + final int mask = table.length - 1; + final int start = home(keyHash, mask); + int i = start; + for (; ; ) { + final TEntry e = table[i]; + if (e == null) { + return null; // empty slot terminates the probe (no tombstones) + } + if (e.hash == keyHash && e.matches(key)) { + return e; + } + i = (i + 1) & mask; + if (i == start) { + return null; // wrapped ⇒ absent (can't happen once growth keeps a free slot) + } + } + } + + /** + * Existing entry for {@code key}, or a freshly {@link CreateStrategy#create created} + inserted + * one. A growable table never returns {@code null}; a fixed one returns {@code null} when full + * and {@code key} is absent (the caller supplies the overflow default). A hit is always + * returned even at capacity — the cap blocks only creation, not lookup. + */ + public TEntry getOrCreate(K key, CreateStrategy createStrat) { + final TEntry existing = get(key); + if (existing != null) { + return existing; + } + if (size >= limit) { + if (!growable) { + return null; // fixed table full ⇒ caller supplies the overflow default + } + grow(); + } + final TEntry created = createStrat.create(key); + FlatHashtable.insert(table, created); // fits: cap/growth keeps a free slot + size++; + return created; + } + + /** + * Unconditionally adds {@code entry} ({@code true}), or {@code false} if a fixed table is full. + * Comparison-free and caller-responsible (same contract as {@link FlatHashtable#insert}): the + * caller must ensure {@code entry}'s key is absent, else it lands shadowed. + */ + public boolean insert(TEntry entry) { + if (size >= limit) { + if (!growable) { + return false; // fixed table full + } + grow(); + } + FlatHashtable.insert(table, entry); + size++; + return true; + } + + private void grow() { + table = resize(table); + limit = (int) (table.length * loadFactor); + } + + public void clear() { + Arrays.fill(table, null); + size = 0; + } + + public void forEach(Consumer consumer) { + FlatHashtable.forEach(table, consumer); + } + + /** + * Context-passing {@link #forEach(Consumer)}: pair a non-capturing {@link BiConsumer} with + * side-band {@code context} to avoid a per-call closure. + */ + public void forEach(C context, BiConsumer consumer) { + FlatHashtable.forEach(table, context, consumer); + } + } + + /** + * Two-key (composite-key) analogue of {@link D1}. Both key parts pass directly through {@link + * #get} / {@link #getOrCreate}, so a lookup allocates no {@code Pair} — the win over {@code + * HashMap}. Same fixed-or-growable ({@link #createFixed} / {@link #createGrowable}), + * entry-centric, no-{@code remove}, not-thread-safe contract as {@link D1}. + * + * @param first key type + * @param second key type + * @param the user's {@link D2.Entry D2.Entry<K1, K2>} subclass + */ + public static final class D2> { + /** + * Abstract base for {@link D2} entries. Subclass to add value fields you wish to mutate in + * place. Both key parts are captured at construction alongside their combined hash. + * + * @param first key type + * @param second key type + */ + public abstract static class Entry extends FlatHashtable.Entry { + final K1 key1; + final K2 key2; + + protected Entry(K1 key1, K2 key2) { + super(hash(key1, key2)); + this.key1 = key1; + this.key2 = key2; + } + + public final K1 key1() { + return key1; + } + + public final K2 key2() { + return key2; + } + + public boolean matches(K1 key1, K2 key2) { + return Objects.equals(this.key1, key1) && Objects.equals(this.key2, key2); + } + + /** + * Returns the combined lookup hash for both key parts via {@link + * LongHashingUtils#hash(Object, Object)}. Null parts contribute {@code 0} (not a sentinel, + * unlike {@link D1.Entry}); {@link #matches(Object, Object)} resolves any collision. + */ + public static long hash(Object key1, Object key2) { + return LongHashingUtils.hash(key1, key2); + } + } + + private final boolean growable; + private final float loadFactor; + private TEntry[] table; + private int size; + private int limit; // grow trigger when growable; hard cap when fixed + + private D2(TEntry[] table, float loadFactor, boolean growable, int limit) { + this.growable = growable; + this.loadFactor = loadFactor; + this.table = table; + this.size = 0; + this.limit = limit; + } + + /** + * A bounded {@link D2} holding up to {@code maxCapacity} entries at the {@link + * #DEFAULT_LOAD_FACTOR}, then capping ({@link #getOrCreate} returns {@code null}). + */ + public static > D2 createFixed( + Class type, int maxCapacity) { + return createFixed(type, maxCapacity, DEFAULT_LOAD_FACTOR); + } + + public static > D2 createFixed( + Class type, int maxCapacity, float loadFactor) { + return new D2<>(create(type, maxCapacity, loadFactor), loadFactor, false, maxCapacity); + } + + /** + * A growable {@link D2} sized initially for {@code initialCapacity} entries at the {@link + * #DEFAULT_LOAD_FACTOR}; doubles on demand, so it never caps. + */ + public static > D2 createGrowable( + Class type, int initialCapacity) { + return createGrowable(type, initialCapacity, DEFAULT_LOAD_FACTOR); + } + + public static > D2 createGrowable( + Class type, int initialCapacity, float loadFactor) { + E[] table = create(type, initialCapacity, loadFactor); + return new D2<>(table, loadFactor, true, (int) (table.length * loadFactor)); + } + + public int size() { + return size; + } + + /** Existing entry for {@code (key1, key2)}, or {@code null}. Read-only — never creates. */ + public TEntry get(K1 key1, K2 key2) { + final long keyHash = Entry.hash(key1, key2); + final TEntry[] table = this.table; + final int mask = table.length - 1; + final int start = home(keyHash, mask); + int i = start; + for (; ; ) { + final TEntry e = table[i]; + if (e == null) { + return null; + } + if (e.hash == keyHash && e.matches(key1, key2)) { + return e; + } + i = (i + 1) & mask; + if (i == start) { + return null; + } + } + } + + /** + * Two-key analogue of {@link D1#getOrCreate}: growable never returns {@code null}; fixed + * returns {@code null} when full and {@code (key1, key2)} is absent. + */ + public TEntry getOrCreate(K1 key1, K2 key2, CreateStrategy2 createStrat) { + final TEntry existing = get(key1, key2); + if (existing != null) { + return existing; + } + if (size >= limit) { + if (!growable) { + return null; // fixed table full ⇒ caller supplies the overflow default + } + grow(); + } + final TEntry created = createStrat.create(key1, key2); + FlatHashtable.insert(table, created); + size++; + return created; + } + + /** + * Unconditionally adds {@code entry} ({@code true}), or {@code false} if a fixed table is full. + * Comparison-free and caller-responsible: the caller must ensure {@code (key1, key2)} is + * absent. + */ + public boolean insert(TEntry entry) { + if (size >= limit) { + if (!growable) { + return false; // fixed table full + } + grow(); + } + FlatHashtable.insert(table, entry); + size++; + return true; + } + + private void grow() { + table = resize(table); + limit = (int) (table.length * loadFactor); + } + + public void clear() { + Arrays.fill(table, null); + size = 0; + } + + public void forEach(Consumer consumer) { + FlatHashtable.forEach(table, consumer); + } + + public void forEach(C context, BiConsumer consumer) { + FlatHashtable.forEach(table, context, consumer); + } + } + + /** + * Two-key creation strategy for {@link D2#getOrCreate}: mint a new entry for {@code (key1, + * key2)}. Like {@link CreateStrategy}, supply a {@code static final} constant or a + * non-capturing lambda (e.g. {@code MyEntry::new}) so it stays a single monomorphic, + * allocation-free instance. + * + * @param stored entry to create + * @param first key type + * @param second key type + */ + @Strategy + @FunctionalInterface + public interface CreateStrategy2 { + E create(K1 key1, K2 key2); + } + /** * Entry-side strategy: the hash of a stored {@code entry} — its home is {@link #home}{@code * (hashOf(entry))}. Used by {@link #insert} / {@link #iterator} / {@link #resize}, which have an From 5f36a5ea6780b760aa0ef064684a5d4d3a21137f Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 28 Jul 2026 17:08:40 -0400 Subject: [PATCH 17/26] Test FlatHashtable.D1/D2 conveniences Cover get/getOrCreate/insert/forEach/clear/size, key() accessors, null-key handling (D1 sentinel; D2 null parts), collision-by-equality, fixed capping (getOrCreate->null, insert->false when full), and growable growth past the initial capacity. Co-Authored-By: Claude Opus 4.8 --- .../trace/util/FlatHashtableD1Test.java | 268 ++++++++++++++++++ .../trace/util/FlatHashtableD2Test.java | 203 +++++++++++++ 2 files changed, 471 insertions(+) create mode 100644 internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java create mode 100644 internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java new file mode 100644 index 00000000000..4fc6838974b --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java @@ -0,0 +1,268 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class FlatHashtableD1Test { + + static final class StringIntEntry extends FlatHashtable.D1.Entry { + int value; + + StringIntEntry(String key, int value) { + super(key); + this.value = value; + } + } + + /** Key whose hashCode is fully controllable, to force probe collisions deterministically. */ + static final class CollidingKey { + final String label; + final int hash; + + CollidingKey(String label, int hash) { + this.label = label; + this.hash = hash; + } + + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof CollidingKey)) { + return false; + } + CollidingKey that = (CollidingKey) o; + return hash == that.hash && label.equals(that.label); + } + } + + static final class CollidingKeyEntry extends FlatHashtable.D1.Entry { + int value; + + CollidingKeyEntry(CollidingKey key, int value) { + super(key); + this.value = value; + } + } + + private static FlatHashtable.D1 growable(int initialCapacity) { + return FlatHashtable.D1.createGrowable(StringIntEntry.class, initialCapacity); + } + + private static FlatHashtable.D1 fixed(int maxCapacity) { + return FlatHashtable.D1.createFixed(StringIntEntry.class, maxCapacity); + } + + @Test + void emptyTableLookupReturnsNull() { + FlatHashtable.D1 table = growable(8); + assertNull(table.get("missing")); + assertEquals(0, table.size()); + } + + @Test + void insertedEntryIsRetrievable() { + FlatHashtable.D1 table = growable(8); + StringIntEntry e = new StringIntEntry("foo", 1); + assertTrue(table.insert(e)); + assertEquals(1, table.size()); + assertSame(e, table.get("foo")); + } + + @Test + void multipleInsertsRetrievableSeparately() { + FlatHashtable.D1 table = growable(16); + StringIntEntry a = new StringIntEntry("alpha", 1); + StringIntEntry b = new StringIntEntry("beta", 2); + StringIntEntry c = new StringIntEntry("gamma", 3); + table.insert(a); + table.insert(b); + table.insert(c); + assertEquals(3, table.size()); + assertSame(a, table.get("alpha")); + assertSame(b, table.get("beta")); + assertSame(c, table.get("gamma")); + } + + @Test + void keyAccessorExposesConstructionKey() { + StringIntEntry e = new StringIntEntry("foo", 1); + assertEquals("foo", e.key()); + } + + @Test + void inPlaceMutationVisibleViaSubsequentGet() { + FlatHashtable.D1 table = growable(8); + table.insert(new StringIntEntry("counter", 0)); + for (int i = 0; i < 10; i++) { + StringIntEntry e = table.get("counter"); + e.value++; + } + assertEquals(10, table.get("counter").value); + } + + @Test + void clearEmptiesTheTable() { + FlatHashtable.D1 table = growable(8); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + table.clear(); + assertEquals(0, table.size()); + assertNull(table.get("a")); + // Reinsertion works after clear. + table.insert(new StringIntEntry("a", 99)); + assertEquals(99, table.get("a").value); + } + + @Test + void forEachVisitsEveryInsertedEntry() { + FlatHashtable.D1 table = growable(8); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + table.insert(new StringIntEntry("c", 3)); + Map seen = new HashMap<>(); + table.forEach(e -> seen.put(e.key(), e.value)); + assertEquals(3, seen.size()); + assertEquals(1, seen.get("a")); + assertEquals(2, seen.get("b")); + assertEquals(3, seen.get("c")); + } + + @Test + void forEachWithContextPassesContextToConsumer() { + FlatHashtable.D1 table = growable(8); + table.insert(new StringIntEntry("a", 10)); + table.insert(new StringIntEntry("b", 20)); + Map seen = new HashMap<>(); + table.forEach(seen, (ctx, e) -> ctx.put(e.key(), e.value)); + assertEquals(2, seen.size()); + assertEquals(10, seen.get("a")); + assertEquals(20, seen.get("b")); + } + + @Test + void nullKeyIsPermittedAndDistinctFromAbsent() { + FlatHashtable.D1 table = growable(8); + assertNull(table.get(null)); + StringIntEntry nullKeyed = new StringIntEntry(null, 7); + table.insert(nullKeyed); + assertSame(nullKeyed, table.get(null)); + assertEquals(1, table.size()); + // A real key that hashes to 0 must not collide with the null-key sentinel. + StringIntEntry zeroHashed = new StringIntEntry("", 0); + assertEquals(0, "".hashCode()); + table.insert(zeroHashed); + assertSame(zeroHashed, table.get("")); + assertSame(nullKeyed, table.get(null)); + } + + @Test + void hashCollisionsResolveByEquality() { + // Two distinct keys with the same hashCode: the probe must distinguish them via matches(). + FlatHashtable.D1 table = + FlatHashtable.D1.createGrowable(CollidingKeyEntry.class, 4); + CollidingKey k1 = new CollidingKey("first", 17); + CollidingKey k2 = new CollidingKey("second", 17); + CollidingKeyEntry e1 = new CollidingKeyEntry(k1, 100); + CollidingKeyEntry e2 = new CollidingKeyEntry(k2, 200); + table.insert(e1); + table.insert(e2); + assertEquals(2, table.size()); + assertSame(e1, table.get(k1)); + assertSame(e2, table.get(k2)); + } + + @Test + void getOrCreateOnMissBuildsEntryViaCreator() { + FlatHashtable.D1 table = growable(8); + int[] createCount = {0}; + StringIntEntry created = + table.getOrCreate( + "foo", + k -> { + createCount[0]++; + return new StringIntEntry(k, 42); + }); + assertNotNull(created); + assertEquals("foo", created.key()); + assertEquals(42, created.value); + assertEquals(1, table.size()); + assertEquals(1, createCount[0]); + assertSame(created, table.get("foo")); + } + + @Test + void getOrCreateOnHitSkipsCreator() { + FlatHashtable.D1 table = growable(8); + StringIntEntry seeded = new StringIntEntry("foo", 1); + table.insert(seeded); + int[] createCount = {0}; + StringIntEntry got = + table.getOrCreate( + "foo", + k -> { + createCount[0]++; + return new StringIntEntry(k, 999); + }); + assertSame(seeded, got); + assertEquals(1, table.size()); + assertEquals(0, createCount[0]); + } + + @Test + void growableGrowsPastInitialCapacity() { + // initialCapacity 1 => a tiny 2-slot table; inserting far past it must trigger growth. + FlatHashtable.D1 table = growable(1); + for (int i = 0; i < 50; i++) { + assertTrue(table.insert(new StringIntEntry("k" + i, i))); + } + assertEquals(50, table.size()); + for (int i = 0; i < 50; i++) { + assertEquals(i, table.get("k" + i).value); + } + } + + @Test + void growableGetOrCreateNeverReturnsNull() { + FlatHashtable.D1 table = growable(1); + for (int i = 0; i < 50; i++) { + StringIntEntry e = table.getOrCreate("k" + i, k -> new StringIntEntry(k, 0)); + assertNotNull(e); + } + assertEquals(50, table.size()); + } + + @Test + void fixedGetOrCreateCapsWhenFull() { + FlatHashtable.D1 table = fixed(2); + assertNotNull(table.getOrCreate("a", k -> new StringIntEntry(k, 1))); + assertNotNull(table.getOrCreate("b", k -> new StringIntEntry(k, 2))); + assertEquals(2, table.size()); + // At capacity, a new key can't be created -> null (caller's overflow default). + assertNull(table.getOrCreate("c", k -> new StringIntEntry(k, 3))); + assertEquals(2, table.size()); + // ...but an existing key still resolves even at capacity (cap blocks creation, not lookup). + StringIntEntry a = table.get("a"); + assertSame(a, table.getOrCreate("a", k -> new StringIntEntry(k, 99))); + } + + @Test + void fixedInsertReturnsFalseWhenFull() { + FlatHashtable.D1 table = fixed(2); + assertTrue(table.insert(new StringIntEntry("a", 1))); + assertTrue(table.insert(new StringIntEntry("b", 2))); + assertFalse(table.insert(new StringIntEntry("c", 3))); + assertEquals(2, table.size()); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java new file mode 100644 index 00000000000..e299c8a7b23 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java @@ -0,0 +1,203 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class FlatHashtableD2Test { + + static final class PairEntry extends FlatHashtable.D2.Entry { + int value; + + PairEntry(String key1, Integer key2, int value) { + super(key1, key2); + this.value = value; + } + } + + private static FlatHashtable.D2 growable(int initialCapacity) { + return FlatHashtable.D2.createGrowable(PairEntry.class, initialCapacity); + } + + private static FlatHashtable.D2 fixed(int maxCapacity) { + return FlatHashtable.D2.createFixed(PairEntry.class, maxCapacity); + } + + @Test + void emptyTableLookupReturnsNull() { + FlatHashtable.D2 table = growable(8); + assertNull(table.get("missing", 1)); + assertEquals(0, table.size()); + } + + @Test + void insertedEntryIsRetrievableByBothParts() { + FlatHashtable.D2 table = growable(8); + PairEntry e = new PairEntry("a", 1, 100); + assertTrue(table.insert(e)); + assertEquals(1, table.size()); + assertSame(e, table.get("a", 1)); + } + + @Test + void distinctCompositeKeysStaySeparate() { + FlatHashtable.D2 table = growable(16); + PairEntry a1 = new PairEntry("a", 1, 10); + PairEntry a2 = new PairEntry("a", 2, 20); // same key1, different key2 + PairEntry b1 = new PairEntry("b", 1, 30); // different key1, same key2 + table.insert(a1); + table.insert(a2); + table.insert(b1); + assertEquals(3, table.size()); + assertSame(a1, table.get("a", 1)); + assertSame(a2, table.get("a", 2)); + assertSame(b1, table.get("b", 1)); + // A composite that was never inserted is absent. + assertNull(table.get("b", 2)); + } + + @Test + void keyAccessorsExposeConstructionKeys() { + PairEntry e = new PairEntry("a", 1, 100); + assertEquals("a", e.key1()); + assertEquals(1, e.key2()); + } + + @Test + void inPlaceMutationVisibleViaSubsequentGet() { + FlatHashtable.D2 table = growable(8); + table.insert(new PairEntry("counter", 7, 0)); + for (int i = 0; i < 10; i++) { + PairEntry e = table.get("counter", 7); + e.value++; + } + assertEquals(10, table.get("counter", 7).value); + } + + @Test + void clearEmptiesTheTable() { + FlatHashtable.D2 table = growable(8); + table.insert(new PairEntry("a", 1, 1)); + table.insert(new PairEntry("b", 2, 2)); + table.clear(); + assertEquals(0, table.size()); + assertNull(table.get("a", 1)); + } + + @Test + void forEachVisitsEveryInsertedEntry() { + FlatHashtable.D2 table = growable(8); + table.insert(new PairEntry("a", 1, 1)); + table.insert(new PairEntry("b", 2, 2)); + table.insert(new PairEntry("c", 3, 3)); + Map seen = new HashMap<>(); + table.forEach(e -> seen.put(e.key1() + e.key2(), e.value)); + assertEquals(3, seen.size()); + assertEquals(1, seen.get("a1")); + assertEquals(2, seen.get("b2")); + assertEquals(3, seen.get("c3")); + } + + @Test + void forEachWithContextPassesContextToConsumer() { + FlatHashtable.D2 table = growable(8); + table.insert(new PairEntry("a", 1, 10)); + table.insert(new PairEntry("b", 2, 20)); + Map seen = new HashMap<>(); + table.forEach(seen, (ctx, e) -> ctx.put(e.key1() + e.key2(), e.value)); + assertEquals(2, seen.size()); + assertEquals(10, seen.get("a1")); + assertEquals(20, seen.get("b2")); + } + + @Test + void nullKeyPartsArePermitted() { + FlatHashtable.D2 table = growable(8); + PairEntry bothNull = new PairEntry(null, null, 1); + table.insert(bothNull); + assertSame(bothNull, table.get(null, null)); + assertNull(table.get("a", null)); + assertNull(table.get(null, 1)); + assertEquals(1, table.size()); + } + + @Test + void getOrCreateOnMissBuildsEntryViaCreator() { + FlatHashtable.D2 table = growable(8); + int[] createCount = {0}; + PairEntry created = + table.getOrCreate( + "foo", + 1, + (k1, k2) -> { + createCount[0]++; + return new PairEntry(k1, k2, 42); + }); + assertNotNull(created); + assertEquals("foo", created.key1()); + assertEquals(1, created.key2()); + assertEquals(42, created.value); + assertEquals(1, table.size()); + assertEquals(1, createCount[0]); + assertSame(created, table.get("foo", 1)); + } + + @Test + void getOrCreateOnHitSkipsCreator() { + FlatHashtable.D2 table = growable(8); + PairEntry seeded = new PairEntry("foo", 1, 1); + table.insert(seeded); + int[] createCount = {0}; + PairEntry got = + table.getOrCreate( + "foo", + 1, + (k1, k2) -> { + createCount[0]++; + return new PairEntry(k1, k2, 999); + }); + assertSame(seeded, got); + assertEquals(1, table.size()); + assertEquals(0, createCount[0]); + } + + @Test + void growableGrowsPastInitialCapacity() { + FlatHashtable.D2 table = growable(1); + for (int i = 0; i < 50; i++) { + assertTrue(table.insert(new PairEntry("k", i, i))); + } + assertEquals(50, table.size()); + for (int i = 0; i < 50; i++) { + assertEquals(i, table.get("k", i).value); + } + } + + @Test + void fixedGetOrCreateCapsWhenFull() { + FlatHashtable.D2 table = fixed(2); + assertNotNull(table.getOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 1))); + assertNotNull(table.getOrCreate("b", 2, (k1, k2) -> new PairEntry(k1, k2, 2))); + assertEquals(2, table.size()); + assertNull(table.getOrCreate("c", 3, (k1, k2) -> new PairEntry(k1, k2, 3))); + assertEquals(2, table.size()); + PairEntry a = table.get("a", 1); + assertSame(a, table.getOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 99))); + } + + @Test + void fixedInsertReturnsFalseWhenFull() { + FlatHashtable.D2 table = fixed(2); + assertTrue(table.insert(new PairEntry("a", 1, 1))); + assertTrue(table.insert(new PairEntry("b", 2, 2))); + assertFalse(table.insert(new PairEntry("c", 3, 3))); + assertEquals(2, table.size()); + } +} From 381cdfc4fcd5e6a3f7081007b6a2dad2ffec4fac Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 29 Jul 2026 12:40:53 -0400 Subject: [PATCH 18/26] FlatHashtable: annotate nullability (@Nonnull/@Nullable) Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/FlatHashtable.java | 124 ++++++++++++------ 1 file changed, 82 insertions(+), 42 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index ef0e0bf08e1..0cafd460e71 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -9,6 +9,8 @@ import java.util.Objects; import java.util.function.BiConsumer; import java.util.function.Consumer; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Open-addressed, single-array find-or-create over self-contained entries — each slot is one @@ -125,16 +127,17 @@ public static final class D1> { public abstract static class Entry extends FlatHashtable.Entry { final K key; - protected Entry(K key) { + protected Entry(@Nullable K key) { super(hash(key)); this.key = key; } + @Nullable public final K key() { return key; } - public boolean matches(Object key) { + public boolean matches(@Nullable Object key) { return Objects.equals(this.key, key); } @@ -143,7 +146,7 @@ public boolean matches(Object key) { * don't collide with a real key that hashes to 0; real-key collisions are resolved by {@link * #matches(Object)}. */ - public static long hash(Object key) { + public static long hash(@Nullable Object key) { return (key == null) ? Long.MIN_VALUE : key.hashCode(); } } @@ -166,12 +169,15 @@ private D1(TEntry[] table, float loadFactor, boolean growable, int limit) { * A bounded {@link D1} holding up to {@code maxCapacity} entries at the {@link * #DEFAULT_LOAD_FACTOR}, then capping ({@link #getOrCreate} returns {@code null}). */ - public static > D1 createFixed(Class type, int maxCapacity) { + @Nonnull + public static > D1 createFixed( + @Nonnull Class type, int maxCapacity) { return createFixed(type, maxCapacity, DEFAULT_LOAD_FACTOR); } + @Nonnull public static > D1 createFixed( - Class type, int maxCapacity, float loadFactor) { + @Nonnull Class type, int maxCapacity, float loadFactor) { return new D1<>(create(type, maxCapacity, loadFactor), loadFactor, false, maxCapacity); } @@ -179,13 +185,15 @@ public static > D1 createFixed( * A growable {@link D1} sized initially for {@code initialCapacity} entries at the {@link * #DEFAULT_LOAD_FACTOR}; doubles on demand, so it never caps. */ + @Nonnull public static > D1 createGrowable( - Class type, int initialCapacity) { + @Nonnull Class type, int initialCapacity) { return createGrowable(type, initialCapacity, DEFAULT_LOAD_FACTOR); } + @Nonnull public static > D1 createGrowable( - Class type, int initialCapacity, float loadFactor) { + @Nonnull Class type, int initialCapacity, float loadFactor) { E[] table = create(type, initialCapacity, loadFactor); return new D1<>(table, loadFactor, true, (int) (table.length * loadFactor)); } @@ -195,7 +203,8 @@ public int size() { } /** Existing entry for {@code key}, or {@code null}. Read-only — never creates. */ - public TEntry get(K key) { + @Nullable + public TEntry get(@Nullable K key) { final long keyHash = Entry.hash(key); final TEntry[] table = this.table; final int mask = table.length - 1; @@ -222,7 +231,8 @@ public TEntry get(K key) { * and {@code key} is absent (the caller supplies the overflow default). A hit is always * returned even at capacity — the cap blocks only creation, not lookup. */ - public TEntry getOrCreate(K key, CreateStrategy createStrat) { + @Nullable + public TEntry getOrCreate(@Nullable K key, @Nonnull CreateStrategy createStrat) { final TEntry existing = get(key); if (existing != null) { return existing; @@ -244,7 +254,7 @@ public TEntry getOrCreate(K key, CreateStrategy createStrat) { * Comparison-free and caller-responsible (same contract as {@link FlatHashtable#insert}): the * caller must ensure {@code entry}'s key is absent, else it lands shadowed. */ - public boolean insert(TEntry entry) { + public boolean insert(@Nonnull TEntry entry) { if (size >= limit) { if (!growable) { return false; // fixed table full @@ -266,7 +276,7 @@ public void clear() { size = 0; } - public void forEach(Consumer consumer) { + public void forEach(@Nonnull Consumer consumer) { FlatHashtable.forEach(table, consumer); } @@ -274,7 +284,7 @@ public void forEach(Consumer consumer) { * Context-passing {@link #forEach(Consumer)}: pair a non-capturing {@link BiConsumer} with * side-band {@code context} to avoid a per-call closure. */ - public void forEach(C context, BiConsumer consumer) { + public void forEach(C context, @Nonnull BiConsumer consumer) { FlatHashtable.forEach(table, context, consumer); } } @@ -301,21 +311,23 @@ public abstract static class Entry extends FlatHashtable.Entry { final K1 key1; final K2 key2; - protected Entry(K1 key1, K2 key2) { + protected Entry(@Nullable K1 key1, @Nullable K2 key2) { super(hash(key1, key2)); this.key1 = key1; this.key2 = key2; } + @Nullable public final K1 key1() { return key1; } + @Nullable public final K2 key2() { return key2; } - public boolean matches(K1 key1, K2 key2) { + public boolean matches(@Nullable K1 key1, @Nullable K2 key2) { return Objects.equals(this.key1, key1) && Objects.equals(this.key2, key2); } @@ -324,7 +336,7 @@ public boolean matches(K1 key1, K2 key2) { * LongHashingUtils#hash(Object, Object)}. Null parts contribute {@code 0} (not a sentinel, * unlike {@link D1.Entry}); {@link #matches(Object, Object)} resolves any collision. */ - public static long hash(Object key1, Object key2) { + public static long hash(@Nullable Object key1, @Nullable Object key2) { return LongHashingUtils.hash(key1, key2); } } @@ -347,13 +359,15 @@ private D2(TEntry[] table, float loadFactor, boolean growable, int limit) { * A bounded {@link D2} holding up to {@code maxCapacity} entries at the {@link * #DEFAULT_LOAD_FACTOR}, then capping ({@link #getOrCreate} returns {@code null}). */ + @Nonnull public static > D2 createFixed( - Class type, int maxCapacity) { + @Nonnull Class type, int maxCapacity) { return createFixed(type, maxCapacity, DEFAULT_LOAD_FACTOR); } + @Nonnull public static > D2 createFixed( - Class type, int maxCapacity, float loadFactor) { + @Nonnull Class type, int maxCapacity, float loadFactor) { return new D2<>(create(type, maxCapacity, loadFactor), loadFactor, false, maxCapacity); } @@ -361,13 +375,15 @@ public static > D2 createFixed( * A growable {@link D2} sized initially for {@code initialCapacity} entries at the {@link * #DEFAULT_LOAD_FACTOR}; doubles on demand, so it never caps. */ + @Nonnull public static > D2 createGrowable( - Class type, int initialCapacity) { + @Nonnull Class type, int initialCapacity) { return createGrowable(type, initialCapacity, DEFAULT_LOAD_FACTOR); } + @Nonnull public static > D2 createGrowable( - Class type, int initialCapacity, float loadFactor) { + @Nonnull Class type, int initialCapacity, float loadFactor) { E[] table = create(type, initialCapacity, loadFactor); return new D2<>(table, loadFactor, true, (int) (table.length * loadFactor)); } @@ -377,7 +393,8 @@ public int size() { } /** Existing entry for {@code (key1, key2)}, or {@code null}. Read-only — never creates. */ - public TEntry get(K1 key1, K2 key2) { + @Nullable + public TEntry get(@Nullable K1 key1, @Nullable K2 key2) { final long keyHash = Entry.hash(key1, key2); final TEntry[] table = this.table; final int mask = table.length - 1; @@ -402,7 +419,11 @@ public TEntry get(K1 key1, K2 key2) { * Two-key analogue of {@link D1#getOrCreate}: growable never returns {@code null}; fixed * returns {@code null} when full and {@code (key1, key2)} is absent. */ - public TEntry getOrCreate(K1 key1, K2 key2, CreateStrategy2 createStrat) { + @Nullable + public TEntry getOrCreate( + @Nullable K1 key1, + @Nullable K2 key2, + @Nonnull CreateStrategy2 createStrat) { final TEntry existing = get(key1, key2); if (existing != null) { return existing; @@ -424,7 +445,7 @@ public TEntry getOrCreate(K1 key1, K2 key2, CreateStrategy2 crea * Comparison-free and caller-responsible: the caller must ensure {@code (key1, key2)} is * absent. */ - public boolean insert(TEntry entry) { + public boolean insert(@Nonnull TEntry entry) { if (size >= limit) { if (!growable) { return false; // fixed table full @@ -446,11 +467,11 @@ public void clear() { size = 0; } - public void forEach(Consumer consumer) { + public void forEach(@Nonnull Consumer consumer) { FlatHashtable.forEach(table, consumer); } - public void forEach(C context, BiConsumer consumer) { + public void forEach(C context, @Nonnull BiConsumer consumer) { FlatHashtable.forEach(table, context, consumer); } } @@ -468,7 +489,8 @@ public void forEach(C context, BiConsumer consume @Strategy @FunctionalInterface public interface CreateStrategy2 { - E create(K1 key1, K2 key2); + @Nonnull + E create(@Nullable K1 key1, @Nullable K2 key2); } /** @@ -484,7 +506,7 @@ public interface CreateStrategy2 { @Strategy @FunctionalInterface public interface HashStrategy { - long hashOf(E entry); + long hashOf(@Nonnull E entry); } /** @@ -505,7 +527,7 @@ public interface HashStrategy { @FunctionalInterface public interface MatchingStrategy { /** Whether the stored {@code entry} is the one for {@code key}. */ - boolean matches(E entry, K key); + boolean matches(@Nonnull E entry, K key); /** * Hash of {@code key}; defaults to {@code key.hashCode()} (the table applies its own spread). @@ -560,7 +582,8 @@ public final long hashKey(String key) { @Strategy @FunctionalInterface public interface CreateStrategy { - E create(K key); + @Nonnull + E create(@Nullable K key); } /** @@ -610,7 +633,8 @@ public static int capacityFor(int cardinalityLimit, float loadFactor) { * CreateStrategy#create} mints an entry — different types, no ambiguity at the call site. */ @SuppressWarnings("unchecked") - public static E[] create(Class type, int cardinalityLimit) { + @Nonnull + public static E[] create(@Nonnull Class type, int cardinalityLimit) { return (E[]) Array.newInstance(type, capacityFor(cardinalityLimit)); } @@ -619,7 +643,8 @@ public static E[] create(Class type, int cardinalityLimit) { * float)} and the {@link #DEFAULT_LOAD_FACTOR} / {@link #LOW_LOAD_FACTOR} constants). */ @SuppressWarnings("unchecked") - public static E[] create(Class type, int cardinalityLimit, float loadFactor) { + @Nonnull + public static E[] create(@Nonnull Class type, int cardinalityLimit, float loadFactor) { return (E[]) Array.newInstance(type, capacityFor(cardinalityLimit, loadFactor)); } @@ -628,7 +653,9 @@ public static E[] create(Class type, int cardinalityLimit, float loadFact * hit; walks to the first empty slot (or all the way around) on a miss. */ @StrategyConsumer - public static E get(E[] table, K key, MatchingStrategy matchStrat) { + @Nullable + public static E get( + @Nonnull E[] table, K key, @Nonnull MatchingStrategy matchStrat) { final int mask = table.length - 1; final int start = home(matchStrat.hashKey(key), mask); int i = start; @@ -654,8 +681,12 @@ public static E get(E[] table, K key, MatchingStrategy matchStrat) * double-create is acceptable only when the payload makes it benign (see class doc). */ @StrategyConsumer + @Nullable public static E getOrCreate( - E[] table, K key, MatchingStrategy matchStrat, CreateStrategy createStrat) { + @Nonnull E[] table, + K key, + @Nonnull MatchingStrategy matchStrat, + @Nonnull CreateStrategy createStrat) { final int mask = table.length - 1; final int start = home(matchStrat.hashKey(key), mask); int i = start; @@ -687,7 +718,7 @@ public static E getOrCreate( * removed) able to resurrect stale data. Reach for it only from the expert tier, with that * contract in hand. */ - public static boolean insert(E[] table, E entry) { + public static boolean insert(@Nonnull E[] table, @Nonnull E entry) { return placeAt(table, entry, entry.hash); } @@ -696,7 +727,8 @@ public static boolean insert(E[] table, E entry) { * HashStrategy#hashOf}. Same comparison-free, caller-ensures-absence contract. */ @StrategyConsumer - public static boolean insert(E[] table, E entry, HashStrategy hashStrat) { + public static boolean insert( + @Nonnull E[] table, @Nonnull E entry, @Nonnull HashStrategy hashStrat) { return placeAt(table, entry, hashStrat.hashOf(entry)); } @@ -739,7 +771,8 @@ static int home(long hash, int mask) { * (the home comes from {@link Entry#hash}). See {@link #resizingInsert(Entry[], Entry)} to do * both in one call, and its note on growing over unbounded key domains. */ - public static E[] resize(E[] table) { + @Nonnull + public static E[] resize(@Nonnull E[] table) { E[] grown = allocateGrown(table); for (final E e : table) { if (e != null) { @@ -754,7 +787,8 @@ public static E[] resize(E[] table) { * HashStrategy#hashOf}. Not a {@link StrategyConsumer} — the rehash is a cold, one-off traversal, * not a hot specialization site. */ - public static E[] resize(E[] table, HashStrategy hashStrat) { + @Nonnull + public static E[] resize(@Nonnull E[] table, @Nonnull HashStrategy hashStrat) { E[] grown = allocateGrown(table); for (final E e : table) { if (e != null) { @@ -787,7 +821,8 @@ private static E[] allocateGrown(E[] table) { * so it is the easiest place to leak memory: use it only for a genuinely bounded key domain, * never over externally-controlled cardinality. */ - public static E[] resizingInsert(E[] table, E entry) { + @Nonnull + public static E[] resizingInsert(@Nonnull E[] table, @Nonnull E entry) { E[] t = table; while (!insert(t, entry)) { t = resize(t); // one doubling always suffices; the loop is belt-and-braces @@ -800,7 +835,9 @@ public static E[] resizingInsert(E[] table, E entry) { * HashStrategy#hashOf}). Same grows-unboundedly caution. */ @StrategyConsumer - public static E[] resizingInsert(E[] table, E entry, HashStrategy hashStrat) { + @Nonnull + public static E[] resizingInsert( + @Nonnull E[] table, @Nonnull E entry, @Nonnull HashStrategy hashStrat) { E[] t = table; while (!insert(t, entry, hashStrat)) { t = resize(t, hashStrat); @@ -809,7 +846,7 @@ public static E[] resizingInsert(E[] table, E entry, HashStrategy hashStr } /** Applies {@code consumer} to every entry in {@code table} (skipping empty slots); any order. */ - public static void forEach(E[] table, Consumer consumer) { + public static void forEach(@Nonnull E[] table, @Nonnull Consumer consumer) { for (final E e : table) { if (e != null) { consumer.accept(e); @@ -822,7 +859,7 @@ public static void forEach(E[] table, Consumer consumer) { * (typically a {@code static final}) with side-band {@code context} to avoid a per-call closure. */ public static void forEach( - E[] table, C context, BiConsumer consumer) { + @Nonnull E[] table, C context, @Nonnull BiConsumer consumer) { for (final E e : table) { if (e != null) { consumer.accept(context, e); @@ -840,7 +877,9 @@ public static void forEach( * specializes the traversal so {@code hashOf} inlines. (Still pass a {@code static final} * strategy to avoid a per-call allocation.) */ - public static Iterator iterator(E[] table, long hash, HashStrategy hashStrat) { + @Nonnull + public static Iterator iterator( + @Nonnull E[] table, long hash, @Nonnull HashStrategy hashStrat) { return new StrategyHashIterator<>(table, hash, hashStrat); } @@ -854,7 +893,8 @@ public static Iterator iterator(E[] table, long hash, HashStrategy has * Iterator} return type as the general overload; the call site specializes by its own * monomorphism. */ - public static Iterator iterator(E[] table, long hash) { + @Nonnull + public static Iterator iterator(@Nonnull E[] table, long hash) { return new EntryHashIterator<>(table, hash); } From e650cd0d451b00a83f86a5b662bcc88a610765d2 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 19 Aug 2026 10:33:33 -0400 Subject: [PATCH 19/26] Cover D2 hash-collision matches() and getOrCreate growth for coverage gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FlatHashtable$D2$Entry sat at 50% branch coverage — matches() was only ever exercised on the all-match path, so both short-circuit rejections (key1 differs; key1 equal but key2 differs) were untested, and D2.getOrCreate's growable grow() path was likewise unhit. Both are reachable via the instance API; add a forced composite-hash-collision test (constant-hash CollidingKey) and a growable getOrCreate growth test. D2$Entry 50%->100%; D1/D2 branch to 94.4% (the only residual misses are the provably-unreachable full-table wrap guards in get(), which the free-slot invariant prevents). Co-Authored-By: Claude Opus 4.8 --- .../trace/util/FlatHashtableD2Test.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java index e299c8a7b23..5d19af4fecf 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java @@ -22,6 +22,40 @@ static final class PairEntry extends FlatHashtable.D2.Entry { } } + /** A key whose hash is fixed at construction so composite-hash collisions can be forced. */ + static final class CollidingKey { + final String label; + final int hash; + + CollidingKey(String label, int hash) { + this.label = label; + this.hash = hash; + } + + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof CollidingKey)) { + return false; + } + CollidingKey that = (CollidingKey) o; + return hash == that.hash && label.equals(that.label); + } + } + + static final class CollidingPairEntry extends FlatHashtable.D2.Entry { + int value; + + CollidingPairEntry(CollidingKey key1, CollidingKey key2, int value) { + super(key1, key2); + this.value = value; + } + } + private static FlatHashtable.D2 growable(int initialCapacity) { return FlatHashtable.D2.createGrowable(PairEntry.class, initialCapacity); } @@ -200,4 +234,36 @@ void fixedInsertReturnsFalseWhenFull() { assertFalse(table.insert(new PairEntry("c", 3, 3))); assertEquals(2, table.size()); } + + @Test + void hashCollisionsResolveByKeyEquality() { + FlatHashtable.D2 table = + FlatHashtable.D2.createGrowable(CollidingPairEntry.class, 8); + CollidingKey key1 = new CollidingKey("a", 7); + CollidingKey key2 = new CollidingKey("x", 3); + CollidingPairEntry e = new CollidingPairEntry(key1, key2, 100); + table.insert(e); + + // Same combined hash (31*7 + 3) as the stored entry, but key1 differs by equals: the entry's + // matches() must reject on key1, so the lookup misses rather than returning the wrong entry. + assertNull(table.get(new CollidingKey("b", 7), key2)); + // Same combined hash again, key1 equal but key2 differs by equals: matches() must reject on + // key2. + assertNull(table.get(key1, new CollidingKey("y", 3))); + // The genuine key still resolves. + assertSame(e, table.get(key1, key2)); + } + + @Test + void growableGetOrCreateGrowsPastInitialCapacity() { + FlatHashtable.D2 table = growable(1); + for (int i = 0; i < 50; i++) { + PairEntry e = table.getOrCreate("k", i, (k1, k2) -> new PairEntry(k1, k2, k2)); + assertNotNull(e); + } + assertEquals(50, table.size()); + for (int i = 0; i < 50; i++) { + assertEquals(i, table.get("k", i).value); + } + } } From b6b4a8dbb8381573533ff3aac94f28bddc32394d Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 19 Aug 2026 11:08:46 -0400 Subject: [PATCH 20/26] Guard capacityFor against capacities exceeding Integer.MAX_VALUE Large cardinality budgets (>536870912 at DEFAULT_LOAD_FACTOR, >268435456 at LOW_LOAD_FACTOR) overflow the power-of-two rounding to Integer.MIN_VALUE, which previously surfaced as an opaque NegativeArraySizeException from deep inside create(). Throw a clear IllegalArgumentException at the precondition instead. Flagged by Datadog Autotest on #11980. Co-Authored-By: Claude Opus 4.8 --- .../main/java/datadog/trace/util/FlatHashtable.java | 11 ++++++++++- .../java/datadog/trace/util/FlatHashtableTest.java | 12 ++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index 0cafd460e71..50ca556ea54 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -621,7 +621,16 @@ public static int capacityFor(int cardinalityLimit, float loadFactor) { throw new IllegalArgumentException("loadFactor must be in (0, 1): " + loadFactor); } int min = (int) Math.ceil(cardinalityLimit / (double) loadFactor); - return Integer.highestOneBit(min - 1) << 1; + int capacity = Integer.highestOneBit(min - 1) << 1; + if (capacity <= 0) { + throw new IllegalArgumentException( + "cardinalityLimit " + + cardinalityLimit + + " at loadFactor " + + loadFactor + + " requires a capacity larger than Integer.MAX_VALUE"); + } + return capacity; } /** diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 666f7520fb9..42aa2000abd 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -402,6 +402,18 @@ void capacityFor_rejectsLoadFactorOutOfRange() { assertThrows(IllegalArgumentException.class, () -> FlatHashtable.capacityFor(4, -0.1f)); } + @Test + void capacityFor_rejectsCardinalityRequiringCapacityAboveMaxInt() { + // 536870913 at 0.5 needs 2^31 slots, which overflows a signed int -> clear error, not + // an opaque NegativeArraySizeException from create(). + assertThrows( + IllegalArgumentException.class, + () -> FlatHashtable.capacityFor(536870913, FlatHashtable.DEFAULT_LOAD_FACTOR)); + assertThrows( + IllegalArgumentException.class, + () -> FlatHashtable.capacityFor(268435457, FlatHashtable.LOW_LOAD_FACTOR)); + } + @Test void create_honorsLoadFactor() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4, FlatHashtable.LOW_LOAD_FACTOR); From 6f25e2a1a211596d220fb30bce370a094a73c3e3 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 20 Aug 2026 11:44:44 -0400 Subject: [PATCH 21/26] Fix caseInsensitiveHashCode to fold by code point, not by char On JDK 9+, String.equalsIgnoreCase reconstructs supplementary code points before folding case, so a per-char fold hashes some equalsIgnoreCase-equal pairs (e.g. U+10400 / U+10428) differently. Through CaseInsensitiveStringStrategy this was a silent false miss on FlatHashtable.get, not just a wasted collision. The prior per-char fold was deliberate (see cb9b9f6a03) based on matching equalsIgnoreCase's own JDK 8 behavior; JDK 9 changed that behavior, invalidating the assumption. Verified empirically against JDK 8/17/25. Keeps an ASCII fast path so the common case stays as cheap as the old per-char loop. Co-Authored-By: Claude Sonnet 5 --- .../main/java/datadog/trace/util/Strings.java | 37 ++++++++++++--- .../datadog/trace/util/FlatHashtableTest.java | 23 ++++++++++ .../StringsCaseInsensitiveHashCodeTest.java | 46 +++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/util/Strings.java b/internal-api/src/main/java/datadog/trace/util/Strings.java index bf862c44802..7bd2055f32c 100644 --- a/internal-api/src/main/java/datadog/trace/util/Strings.java +++ b/internal-api/src/main/java/datadog/trace/util/Strings.java @@ -364,16 +364,39 @@ public static boolean regionContains(String s, int beginIndex, int endIndex, Str * consistent for all inputs, not just ASCII — pairing a one-way fold here with {@code * equalsIgnoreCase} would risk silent false misses on the Unicode characters where they diverge. * - *

    Folds per {@code char} (UTF-16 unit), which is exactly what {@code equalsIgnoreCase} itself - * does — so a supplementary case pair (e.g. U+10400 / U+10428) is treated as distinct by - * both, and the two remain consistent. This mirrors {@code equalsIgnoreCase} rather than doing - * full code-point case folding; a code-point fold would unify pairs {@code equalsIgnoreCase} does - * not, making the hash inconsistent with it. + *

    Folds by code point, not by {@code char} (UTF-16 unit). On JDK 9+, {@code equalsIgnoreCase} + * itself reconstructs supplementary code points before folding (e.g. it treats U+10400 and + * U+10428 as equal), so a per-{@code char} fold would make this hash inconsistent with it for + * such pairs -- a supplementary-plane analog of the same silent-false-miss risk called out above. + * A per-{@code char} fold matched {@code equalsIgnoreCase} only on JDK 8, where it did not + * reconstruct code points; folding by code point keeps the two consistent on every supported JDK. + * (On JDK 8, where {@code equalsIgnoreCase} still treats such pairs as distinct, folding them + * together here is merely an extra, harmless collision -- consistency only needs to hold in the + * equal-hash direction, not the reverse.) */ public static int caseInsensitiveHashCode(String s) { int h = 0; - for (int i = 0, len = s.length(); i < len; ++i) { - h = 31 * h + Character.toLowerCase(Character.toUpperCase(s.charAt(i))); + final int len = s.length(); + for (int i = 0; i < len; ) { + final char c = s.charAt(i); + if (c < 0x80) { + // ASCII fast path: case folding is exactly A-Z -> a-z, and every ASCII char is its own + // code point, so this is equivalent to (and cheaper than) the code-point fold below. + h = 31 * h + ((c >= 'A' && c <= 'Z') ? (c + 32) : c); + ++i; + continue; + } + final int cp = s.codePointAt(i); + final int folded = Character.toLowerCase(Character.toUpperCase(cp)); + if (Character.isSupplementaryCodePoint(folded)) { + // Fold back into the two chars equalsIgnoreCase itself compares, so the polynomial stays + // over the same "characters" for both single- and multi-char inputs. + h = 31 * h + Character.highSurrogate(folded); + h = 31 * h + Character.lowSurrogate(folded); + } else { + h = 31 * h + folded; + } + i += Character.charCount(cp); } return h; } diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 42aa2000abd..0df0aa09551 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -547,4 +547,27 @@ void caseInsensitiveStrategy_matchesRegardlessOfCase() { table, "CONTENT-TYPE", TestCaseInsensitiveStrategy.INSTANCE, CREATE)); assertNull(FlatHashtable.get(table, "content-length", TestCaseInsensitiveStrategy.INSTANCE)); } + + /** + * End-to-end regression for the supplementary-plane false-miss: a per-{@code char} hash fold + * hashed this pair differently even where {@code equalsIgnoreCase} treats them as equal (JDK 9+), + * so {@code get} would stop probing at the first empty slot and never find the stored entry. + * Folding by code point keeps the hash consistent with {@code matches}, so the lookup finds it. + * Guarded by {@code equalsIgnoreCase} itself so the test is correct on JDK 8 too, where the pair + * is merely a benign collision {@code matches} resolves to no match. + */ + @Test + void caseInsensitiveStrategy_doesNotFalseMissOnSupplementaryCasePair() { + String s1 = new String(Character.toChars(0x10400)); // DESERET CAPITAL LETTER LONG I + String s2 = new String(Character.toChars(0x10428)); // DESERET SMALL LETTER LONG I + TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); + TestEntry stored = + FlatHashtable.getOrCreate(table, s1, TestCaseInsensitiveStrategy.INSTANCE, CREATE); + + if (s1.equalsIgnoreCase(s2)) { + assertSame(stored, FlatHashtable.get(table, s2, TestCaseInsensitiveStrategy.INSTANCE)); + } else { + assertNull(FlatHashtable.get(table, s2, TestCaseInsensitiveStrategy.INSTANCE)); + } + } } diff --git a/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java b/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java index 94295c44a43..52195d73693 100644 --- a/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java +++ b/internal-api/src/test/java/datadog/trace/util/StringsCaseInsensitiveHashCodeTest.java @@ -54,4 +54,50 @@ void staysConsistentWithEqualsIgnoreCase() { } } } + + /** + * A supplementary-plane pair that {@code equalsIgnoreCase} unifies on JDK 9+ (code-point-based + * folding) but a per-{@code char} hash fold kept distinct -- the bug this fold fixes. Folding by + * code point unifies the hash too, so this holds on every JDK regardless of what {@code + * equalsIgnoreCase} itself does with the pair on that JDK (see {@link + * #staysConsistentAcrossAllDefinedCodePoints} for the guarded, cross-JDK-safe version of this + * check). + */ + @Test + void foldsSupplementaryCasePairTogether() { + String s1 = new String(Character.toChars(0x10400)); // DESERET CAPITAL LETTER LONG I + String s2 = new String(Character.toChars(0x10428)); // DESERET SMALL LETTER LONG I + assertEquals(caseInsensitiveHashCode(s1), caseInsensitiveHashCode(s2)); + } + + /** + * Exhaustive sweep guarded by {@code equalsIgnoreCase} itself, so it stays portable across JDKs: + * on JDK 8 (no code-point reconstruction) the supplementary pairs above simply never enter the + * guard, and the sweep still passes. + */ + @Test + void staysConsistentAcrossAllDefinedCodePoints() { + for (int cp = 0; cp <= Character.MAX_CODE_POINT; cp++) { + if (!Character.isDefined(cp) || Character.isSurrogate((char) cp)) continue; + String s = new String(Character.toChars(cp)); + for (int variant : + new int[] { + Character.toUpperCase(cp), Character.toLowerCase(cp), Character.toTitleCase(cp) + }) { + String t = new String(Character.toChars(variant)); + if (s.equalsIgnoreCase(t)) { + final String fs = s; + final String ft = t; + assertEquals( + caseInsensitiveHashCode(s), + caseInsensitiveHashCode(t), + () -> + "hash mismatch for equalsIgnoreCase pair U+" + + Integer.toHexString(fs.codePointAt(0)) + + " / U+" + + Integer.toHexString(ft.codePointAt(0))); + } + } + } + } } From 27d68fde3e4aa5cae388db9dc562230a6346297f Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 20 Aug 2026 11:52:01 -0400 Subject: [PATCH 22/26] Guard FlatHashtable's growth arithmetic against Integer.MAX_VALUE overflow table.length << 1 wraps to a negative value once length reaches 1 << 30, which previously surfaced as an opaque NegativeArraySizeException from Array.newInstance deep inside resize()/grow(). Extract the doubling into grownLength(int), guarded like capacityFor's existing overflow check, so the failure is a clear IllegalStateException and the arithmetic is testable without allocating a multi-GB array. Co-Authored-By: Claude Sonnet 5 --- .../java/datadog/trace/util/FlatHashtable.java | 18 +++++++++++++++++- .../datadog/trace/util/FlatHashtableTest.java | 13 +++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index 50ca556ea54..e67f3424554 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -812,7 +812,23 @@ public static E[] resize(@Nonnull E[] table, @Nonnull HashStrategy hashSt */ @SuppressWarnings("unchecked") private static E[] allocateGrown(E[] table) { - return (E[]) Array.newInstance(table.getClass().getComponentType(), table.length << 1); + return (E[]) Array.newInstance(table.getClass().getComponentType(), grownLength(table.length)); + } + + /** + * {@code currentLength << 1}, guarded against overflow. {@code currentLength << 1} overflows to a + * negative value at {@code currentLength == 1 << 30}; without this check the failure surfaces as + * an opaque {@code NegativeArraySizeException} from {@code Array.newInstance} in {@link + * #allocateGrown}. Same overflow class as {@link #capacityFor}'s guard -- this is the growth-time + * counterpart. + */ + static int grownLength(int currentLength) { + int grownLength = currentLength << 1; + if (grownLength <= 0) { + throw new IllegalStateException( + "cannot grow a table of length " + currentLength + " past Integer.MAX_VALUE"); + } + return grownLength; } /** diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java index 0df0aa09551..b1099210611 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -414,6 +414,19 @@ void capacityFor_rejectsCardinalityRequiringCapacityAboveMaxInt() { () -> FlatHashtable.capacityFor(268435457, FlatHashtable.LOW_LOAD_FACTOR)); } + @Test + void grownLength_doublesNormally() { + assertEquals(8, FlatHashtable.grownLength(4)); + assertEquals(1 << 30, FlatHashtable.grownLength(1 << 29)); + } + + @Test + void grownLength_rejectsOverflowingCurrentLength() { + // 1 << 30 doubled overflows a signed int -> clear error, not an opaque + // NegativeArraySizeException from Array.newInstance() deep inside resize()/grow(). + assertThrows(IllegalStateException.class, () -> FlatHashtable.grownLength(1 << 30)); + } + @Test void create_honorsLoadFactor() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4, FlatHashtable.LOW_LOAD_FACTOR); From 1e419f850e5278c4bd7051153bd6db02bc511637 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 20 Aug 2026 11:55:10 -0400 Subject: [PATCH 23/26] Tighten FlatHashtable's concurrency and null-key contract wording Class javadoc previously implied general lock-free safety; clarify that only the slot reference and an entry's final fields get safe-publication guarantees -- non-final payload fields and race-losing entries built by getOrCreate are not safely published, so concurrent creation of entries with meaningful mutable state needs fully-final entries. Also document that MatchingStrategy#hashKey's default NPEs on a null key, unlike D1.Entry which maps null to a hash sentinel -- the raw core takes no position on null keys and a strategy needing them must handle null itself. Co-Authored-By: Claude Sonnet 5 --- .../java/datadog/trace/util/FlatHashtable.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index e67f3424554..30bf2c064b0 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -18,8 +18,17 @@ * reference per slot: entry publication is a single reference store, so a reader sees {@code null} * or a complete entry (never a torn one), and {@code final} identity fields on the entry are * visible under racy publication. That sidesteps the memory-ordering / visibility problems parallel - * key/hash/value arrays would create — no {@code volatile}, no atomics — as long as the payload is - * one where a stale/lost read is benign (miss → recreate; clobber → one wins). + * key/hash/value arrays would create — no {@code volatile}, no atomics. + * + *

    Concurrent use is racy by design, not lock-free-safe in general. The single-reference + * guarantee above covers only the slot reference and an entry's {@code final} fields; a non-final + * payload field written after construction is not safely published by a racing {@link + * #getOrCreate}, and a freshly built entry that loses the slot race is discarded without ever being + * retained by the table. That is fine for build-then-publish usage (populate on one thread, e.g. a + * static-final table, then read from many) and for a payload where a stale/default read or a + * discarded race-loser is benign (miss → recreate; clobber → one wins). For concurrent + * creation of entries with meaningful post-construction state, keep entry state fully {@code + * final} — do not rely on this class for safe publication of mutable entry fields. * *

    Bounded by construction — and that is a feature, not just a limit. {@link #create} * takes a cardinality budget, so you cannot build one without deciding how big it may get — @@ -531,6 +540,10 @@ public interface MatchingStrategy { /** * Hash of {@code key}; defaults to {@code key.hashCode()} (the table applies its own spread). + * This default NPEs on a {@code null} key — unlike {@link D1.Entry}, which maps a {@code null} + * key to a dedicated hash sentinel, the raw core takes no position on {@code null} keys; a + * strategy that must support them overrides {@code hashKey} (and {@code matches}) to handle + * {@code null} itself. */ default long hashKey(K key) { return key.hashCode(); From 57385c4999537bd54cb4661266ecff884773869c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 20 Aug 2026 11:59:13 -0400 Subject: [PATCH 24/26] Fix broken javadoc link in CaseInsensitiveMapBenchmark Referenced a nonexistent CaseInsensitiveStringKeyStrategy; the actual type is FlatHashtable.CaseInsensitiveStringStrategy. Co-Authored-By: Claude Sonnet 5 --- .../java/datadog/trace/util/CaseInsensitiveMapBenchmark.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java index 7a00ce7808b..fcad859024e 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -18,7 +18,7 @@ * Benchmark for the trade-offs around case-insensitive Map look-ups, comparing: *

  • TreeMap with a {@code String::compareToIgnoreCase} comparator — allocation-free, O(log n) *
  • HashMap keyed on {@code toLowerCase()} — O(1) but allocates a folded String per look-up - *
  • FlatHashtable with a {@link FlatHashtable.CaseInsensitiveStringKeyStrategy} — O(1) probe, + *
  • FlatHashtable with a {@link FlatHashtable.CaseInsensitiveStringStrategy} — O(1) probe, * allocation-free (case folded inside hash/matches), value stored unboxed * * From 6874e4f86e69373ca46b5b03aeb15188fec62e73 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 20 Aug 2026 12:50:30 -0400 Subject: [PATCH 25/26] Document resizingInsert's probe-to-full growth trigger resizingInsert only grows when insert reports the table full, since the raw core carries no live-size counter to compare against a load factor. That sits against the class's load-factor argument (DEFAULT_LOAD_FACTOR/ LOW_LOAD_FACTOR), which sizes a fixed table up front rather than driving insert-time growth. Document the distinction: a caller that sizes up front (the expected usage) never sees this; one that grows an under-sized table from scratch via resizingInsert runs fill past where those load-factor figures apply. D1/D2, which do track size, can and do grow at a load factor of their own. Co-Authored-By: Claude Sonnet 5 --- .../main/java/datadog/trace/util/FlatHashtable.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java index 30bf2c064b0..4dc6bf5a2ec 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -858,6 +858,17 @@ static int grownLength(int currentLength) { *

    Grows unboundedly. Unlike {@code insert}'s {@code false}, this hides the full signal, * so it is the easiest place to leak memory: use it only for a genuinely bounded key domain, * never over externally-controlled cardinality. + * + *

    Grows at 100% fill, not at a load factor. The raw core carries no live-size counter + * (see the class contract), so it has no cheap way to know the current fill short of a full table + * scan; growing only when {@link #insert} reports "full" is the only trigger available without + * one. {@link #DEFAULT_LOAD_FACTOR} and {@link #LOW_LOAD_FACTOR}'s Knuth figures size a + * fixed table (via {@link #capacityFor} / {@link #create}) up front for its expected + * cardinality; they are not an insert-time growth trigger here. A caller that sizes up front — + * the expected usage — never sees the difference; one that instead leans on {@code + * resizingInsert} to grow an under-sized table from scratch runs fill up through 0.75, 0.9, and + * 1.0 before doubling, past where those figures stop applying. A caller that already tracks live + * size (as {@link D1} / {@link D2} do) can grow at a load factor of its own choosing instead. */ @Nonnull public static E[] resizingInsert(@Nonnull E[] table, @Nonnull E entry) { @@ -870,7 +881,7 @@ public static E[] resizingInsert(@Nonnull E[] table, @Nonnull /** * {@link #resizingInsert(Entry[], Entry)} for any entry type (home via {@link - * HashStrategy#hashOf}). Same grows-unboundedly caution. + * HashStrategy#hashOf}). Same grows-unboundedly caution and same probe-to-full growth trigger. */ @StrategyConsumer @Nonnull From e7eb628afa21d9f4a981d32962eefad41990dba3 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 20 Aug 2026 16:40:56 -0400 Subject: [PATCH 26/26] Fix FlatHashtable benchmark to overwrite value on collision, matching maps' put() getOrCreate only finds the existing entry on a hit and never updates it, so the mirrored collision loop was skipping the value overwrite that HashMap/TreeMap's put() performs -- giving FlatHashtable's create benchmark a false advantage by doing less work than the arms it's compared against. Co-Authored-By: Claude Sonnet 5 --- .../trace/util/CaseInsensitiveMapBenchmark.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java index fcad859024e..9cbbddcf299 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -196,7 +196,7 @@ public Integer lookup_treeMap() { // after build, so reads are lock-free (see FlatHashtable / ThreadSafeMapBenchmark). static final class CIEntry extends FlatHashtable.Entry { final String key; // original case preserved - final int value; + int value; // mutable: the collision loop below overwrites it on a hit, mirroring put() CIEntry(String key, long hash, int value) { super(hash); // cache the (char-by-char) case-insensitive hash @@ -243,12 +243,17 @@ static CIEntry[] _create_flat(float loadFactor) { } // Mirror the HashMap/TreeMap builds' second loop (UPPER_PREFIXES, suffix 0 & 2): 8 case- // insensitive collisions. getOrCreate finds the already-present lower-case entry (a hit -> the - // create never fires, nothing allocates), doing the same probe/match work the maps' overwrite - // puts do — so all three create arms perform the same 24 operations and are comparable. + // create never fires, nothing allocates) and then the value is overwritten explicitly -- getOr- + // Create itself never updates an existing entry, so without this the FlatHashtable arm would do + // less work (and end up with different final values) than the maps' overwriting put(), a false + // performance advantage. With the overwrite, all three create arms perform the same 24 + // operations and end up with the same final values. for (int suffix = 0; suffix < NUM_SUFFIXES; suffix += 2) { for (String prefix : UPPER_PREFIXES) { - FlatHashtable.getOrCreate( - table, prefix + "-" + suffix, CaseInsensitiveKeyStrategy.INSTANCE, CI_CREATE); + String key = prefix + "-" + suffix; + CIEntry entry = + FlatHashtable.getOrCreate(table, key, CaseInsensitiveKeyStrategy.INSTANCE, CI_CREATE); + entry.value = suffix + 1; } } return table;