diff --git a/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/CardinalityLimitReporterBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/CardinalityLimitReporterBenchmark.java new file mode 100644 index 00000000000..6566216153f --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/common/metrics/CardinalityLimitReporterBenchmark.java @@ -0,0 +1,61 @@ +package datadog.trace.common.metrics; + +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +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; + +/** + * Steady-state {@code record()} acceptance check: once every tag in the working set has an entry, + * every call should be a lookup + in-place count bump through the {@link + * datadog.trace.util.Hashtable.D1#tryGetOrCreate} {@code Maybe}, with no per-call allocation. Run + * with {@code -prof gc} -- B/op should read ~0. + * + *

Not thread-safe by design (see {@link CardinalityLimitReporter}'s class javadoc), so each + * thread gets its own reporter and tag pool rather than sharing one instance. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 5, time = 2) +@Fork(3) +@Threads(8) +public class CardinalityLimitReporterBenchmark { + + private static final int DISTINCT_TAGS = 32; + + private CardinalityLimitReporter reporter; + private String[] tags; + private int cursor; + + @Setup(Level.Trial) + public void setup() { + this.reporter = new CardinalityLimitReporter(); + this.tags = new String[DISTINCT_TAGS]; + for (int i = 0; i < DISTINCT_TAGS; i++) { + tags[i] = "tag-" + i; + } + // Pre-populate every entry so the measured path is pure lookup + update, not creation. + for (String tag : tags) { + reporter.record(tag, 1); + } + } + + @Benchmark + public void record() { + String tag = tags[cursor++ & (DISTINCT_TAGS - 1)]; + long count = 1L + (ThreadLocalRandom.current().nextLong() & 0xFF); + reporter.record(tag, count); + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java index 1214b246470..41f280faee7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateEntry.java @@ -244,6 +244,14 @@ public int getHitCount() { return hitCount; } + /** + * {@code true} if nothing hit this entry in the current reporting cycle, making it the first + * thing worth evicting when the table is full. + */ + public boolean isStale() { + return hitCount == 0; + } + public int getErrorCount() { return errorCount; } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java index b120cb8b915..1984e8f4f20 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateTable.java @@ -2,7 +2,6 @@ import datadog.trace.core.monitor.HealthMetrics; import datadog.trace.util.Hashtable; -import datadog.trace.util.Hashtable.MutatingTableIterator; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -25,17 +24,9 @@ */ final class AggregateTable { - private final Hashtable.Entry[] buckets; - private final int maxAggregates; - private final AggregateEntry.Canonical canonical; - private int size; + private final Hashtable.State state; - /** - * Bucket index where the last {@link #evictOneStale} successfully removed an entry. The next call - * resumes from this bucket so a fast-evicting workload doesn't repeatedly re-walk the same hot - * entries clustered near bucket 0. Reset to {@code 0} by {@link #clear}. - */ - private int evictCursor; + private final AggregateEntry.Canonical canonical; AggregateTable(int maxAggregates) { this(maxAggregates, AdditionalTagsSchema.EMPTY); @@ -47,8 +38,7 @@ final class AggregateTable { AggregateTable( int maxAggregates, CoreHandlers handlers, AdditionalTagsSchema additionalTagsSchema) { - this.buckets = Hashtable.Support.create(maxAggregates, Hashtable.Support.MAX_RATIO); - this.maxAggregates = maxAggregates; + this.state = Hashtable.createCapped(maxAggregates); this.canonical = new AggregateEntry.Canonical(handlers, additionalTagsSchema); } @@ -56,82 +46,58 @@ void resetCoreHandlers(HealthMetrics healthMetrics, CardinalityLimitReporter rep canonical.handlers.reset(healthMetrics, reporter); } + /** + * Live aggregate count. Exact from this class's point of view: {@link Hashtable#estimateSize} is + * an estimate only across a reservation window, and {@link #findOrInsert} reserves and links + * without yielding, so no caller can observe one. + */ int size() { - return size; + return Hashtable.estimateSize(state); } boolean isEmpty() { - return size == 0; + return Hashtable.isLikelyEmpty(state); } /** * Returns the {@link AggregateEntry} to update for {@code snapshot}, lazily creating one on miss. * Returns {@code null} when the table is at capacity and no stale entry can be evicted -- the - * caller should drop the data point in that case. + * caller should drop the data point in that case (reported via {@code onStatsAggregateDropped}). + * Dropping the new key rather than evicting an established one is deliberate: the cap is sized to + * the steady-state working set, so a full table of entries that were all used this cycle means + * the new key is the outlier. + * + *

Cardinality limiting (see {@link MetricCardinalityLimits#USE_BLOCKED_SENTINEL}) reduces how + * often eviction fires but doesn't eliminate it. Over-cap values for a single field collapse into + * the shared {@code tracer_blocked_value} sentinel, so no one field can fill the table on its + * own. But distinct in-budget combinations across fields (resource x service x operation x ...) + * can still drive the entry count to {@code maxAggregates}, so eviction remains the backstop. + * + *

The scan that finds a stale entry, and its resume-where-it-left-off amortization, live in + * {@link Hashtable#tryReserveOrEvict} -- this class only supplies {@link AggregateEntry#isStale}. */ AggregateEntry findOrInsert(SpanSnapshot snapshot) { canonical.populateFrom(snapshot); long keyHash = canonical.keyHash; - for (AggregateEntry candidate = Hashtable.Support.bucket(buckets, keyHash); + for (AggregateEntry candidate = Hashtable.bucketFor(state, keyHash); candidate != null; candidate = candidate.next()) { if (candidate.keyHash == keyHash && canonical.matches(candidate)) { return candidate; } } - // Miss path. - if (size >= maxAggregates && !evictOneStale()) { + // Miss path. Reserve before building the entry so a refused insert costs no allocation; the + // reservation evicts a stale entry to make room if the table is already full. + if (!Hashtable.tryReserveOrEvict(state, AggregateEntry::isStale)) { return null; } AggregateEntry entry = canonical.createEntry(); - Hashtable.Support.insertHeadEntry(buckets, keyHash, entry); - size++; + Hashtable.insertReserved(state, keyHash, entry); return entry; } - /** - * Unlinks the first entry whose {@code getHitCount() == 0}, resuming the scan from {@link - * #evictCursor} so consecutive evictions amortize to O(1) per call. Worst case for a single call - * is still O(N) when nearly every entry is hot, but a sustained eviction stream never re-scans - * the hot prefix more than twice across N evictions. - * - *

If the table is full and every entry was used in this cycle, drop the new key (reported via - * {@code onStatsAggregateDropped}) rather than evicting an established one. Cap is sized to the - * steady-state working set, so eviction is rare in the common case. - * - *

Cardinality limiting (see {@link MetricCardinalityLimits#USE_BLOCKED_SENTINEL}) reduces how - * often this fires but doesn't eliminate it. Over-cap values for a single field collapse into the - * shared {@code tracer_blocked_value} sentinel, so no one field can fill the table on its own. - * But distinct in-budget combinations across fields (resource x service x operation x ...) can - * still drive the entry count to {@code maxAggregates}, so this cursor-resumed scan remains the - * backstop. - */ - private boolean evictOneStale() { - // Two passes -- [cursor, length) then [0, cursor) -- using the half-open-range iterator. The - // second pass is naturally empty when cursor==0, so no extra check needed. - return evictOneStaleInRange(evictCursor, buckets.length) - || evictOneStaleInRange(0, evictCursor); - } - - /** Scans {@code [startBucket, endBucket)} for the first stale entry and unlinks it. */ - private boolean evictOneStaleInRange(int startBucket, int endBucket) { - MutatingTableIterator iter = - Hashtable.Support.mutatingTableIterator(buckets, startBucket, endBucket); - while (iter.hasNext()) { - AggregateEntry e = iter.next(); - if (e.getHitCount() == 0) { - int bucket = iter.currentBucket(); - iter.remove(); - size--; - evictCursor = bucket; - return true; - } - } - return false; - } - void forEach(Consumer consumer) { - Hashtable.Support.forEach(buckets, consumer); + Hashtable.forEach(state, consumer); } /** @@ -139,26 +105,16 @@ void forEach(Consumer consumer) { * each invocation -- pass a non-capturing {@link BiConsumer} (typically a {@code static final}) * plus whatever side-band state it needs as {@code context}. */ - void forEach(T context, BiConsumer consumer) { - Hashtable.Support.forEach(buckets, context, consumer); + void forEach(C context, BiConsumer consumer) { + Hashtable.forEach(state, context, consumer); } /** Removes entries whose {@code getHitCount() == 0}. */ void expungeStaleAggregates() { - for (MutatingTableIterator iter = - Hashtable.Support.mutatingTableIterator(buckets); - iter.hasNext(); ) { - AggregateEntry e = iter.next(); - if (e.getHitCount() == 0) { - iter.remove(); - size--; - } - } + Hashtable.evictAll(state, AggregateEntry::isStale); } void clear() { - Hashtable.Support.clear(buckets); - size = 0; - evictCursor = 0; + Hashtable.clear(state); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/metrics/CardinalityLimitReporter.java b/dd-trace-core/src/main/java/datadog/trace/common/metrics/CardinalityLimitReporter.java index 215c278bef3..307f6c5492c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/metrics/CardinalityLimitReporter.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/metrics/CardinalityLimitReporter.java @@ -32,9 +32,10 @@ final class CardinalityLimitReporter { // Distinct blocked tag names in a window: 9 property fields + the configured peer tags + up to // AdditionalTagsSchema.MAX_ADDITIONAL_TAG_KEYS + base.service, with headroom for the brief - // overlap - // of old and new peer names across a schema rebuild. Fixed capacity; the table chains on overflow - // rather than dropping, so an underestimate only adds chain depth on this cold path. + // overlap of old and new peer names across a schema rebuild. Fixed, strict-cap capacity: if this + // is ever underestimated, excess distinct tags are silently dropped from the summary rather than + // recorded (see the null-check in record()) -- this is a cold, best-effort logging path, not a + // correctness-sensitive one. private static final int TAG_CAPACITY = 64; // Rough width of one "=, " entry, used to pre-size the summary builder. Cold path, so @@ -43,7 +44,8 @@ final class CardinalityLimitReporter { private final RatelimitedLogger rlLog; // Tag name -> blocked count accumulated since the last emitted summary. - private final Hashtable.D1 blockedByTag = new Hashtable.D1<>(TAG_CAPACITY); + private final Hashtable.D1 blockedByTag = + Hashtable.D1.createCapped(TagBlockEntry.class, TAG_CAPACITY); CardinalityLimitReporter() { this(new RatelimitedLogger(log, 5, MINUTES)); @@ -53,10 +55,15 @@ final class CardinalityLimitReporter { this.rlLog = rlLog; } - /** Records {@code count} values blocked for {@code tag} in the current reporting cycle. */ + /** + * Records {@code count} values blocked for {@code tag} in the current reporting cycle. + * + *

A refused create -- the tag table is itself at capacity -- is deliberately ignored: this is + * a log sink, and the durable counts still reach {@code onTagCardinalityBlocked}. + */ void record(String tag, long count) { if (count > 0) { - blockedByTag.getOrCreate(tag, TagBlockEntry::new).count += count; + blockedByTag.tryGetOrCreate(tag, TagBlockEntry::new).update(count, TagBlockEntry::inc); } } @@ -101,5 +108,9 @@ private static final class TagBlockEntry extends Hashtable.D1.Entry { TagBlockEntry(String tag) { super(tag); } + + void inc(long n) { + count += n; + } } } diff --git a/internal-api/src/jmh/java/datadog/trace/api/TagMapAccessBenchmark.java b/internal-api/src/jmh/java/datadog/trace/api/TagMapAccessBenchmark.java index de33c957e9a..3cd021a65b4 100644 --- a/internal-api/src/jmh/java/datadog/trace/api/TagMapAccessBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/api/TagMapAccessBenchmark.java @@ -1,5 +1,6 @@ package datadog.trace.api; +import datadog.trace.util.BenchmarkUtils; import java.util.HashMap; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -56,6 +57,31 @@ * TagMapAccessBenchmark.insert_hashMap_builderStyle thrpt 5 28057827.189 ± 1359655.664 ops/s * TagMapAccessBenchmark.insert_via_ledger thrpt 5 41169656.095 ± 773264.754 ops/s * + * + *

Rerun on JDK 8 with a new top-level {@code @Setup(Level.Trial)} calling {@link + * BenchmarkUtils#polluteHashDispatch()} (this file had none before). M ops/s, 8 threads: + * + * 

{@code
+ * getEntry                        83   getObject                    87
+ * insert                          37   insert_hashMap                48
+ * insert_hashMap_builderStyle     20   insert_via_ledger             37*
+ * }
+ * + *

* = error bar about a quarter of the mean at {@code @Fork(2)} — directional only. + * + *

Every number here is 9-29% below the Java 17 table above, with no clear split between the + * TagMap paths and the HashMap paths this pollution should affect. The table above is Java 17; this + * rerun is JDK 8, whose C2 backend for Apple Silicon (AArch64) is far less mature than JDK 17+'s — + * a broad-based slowdown across every entry is expected from that JDK gap alone, independent of + * pollution — the same JDK-crossing explanation applies to {@link + * datadog.trace.util.CaseInsensitiveMapBenchmark}'s rerun. ({@link + * datadog.trace.util.HashtableD1Benchmark} and {@link datadog.trace.util.HashtableD2Benchmark} saw + * a similar broad drop despite holding the JDK constant — that one is same-session run-to-run + * noise, not a JDK effect.) The relative story survives: {@code insert_hashMap} (48M) still beats + * {@code insert} (37M) for plain insertion, and {@code insert_via_ledger} (37M) still clearly beats + * the HashMap builder-style path (20M); {@code insert_via_ledger} landing roughly level with {@code + * insert} here (vs. clearly behind it in the table above) is within that path's own wide error bar, + * not a new finding. */ @BenchmarkMode(Mode.Throughput) @OutputTimeUnit(TimeUnit.SECONDS) @@ -99,6 +125,11 @@ public class TagMapAccessBenchmark { * Pre-populated read map, PER-THREAD ({@code Scope.Thread}): each thread owns its own map so * reads don't contend on shared mutable state under {@code @Threads(8)}. */ + @Setup(Level.Trial) + public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + } + @State(Scope.Thread) public static class ReadMap { TagMap map; diff --git a/internal-api/src/jmh/java/datadog/trace/util/BenchmarkUtils.java b/internal-api/src/jmh/java/datadog/trace/util/BenchmarkUtils.java new file mode 100644 index 00000000000..e785757a515 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/BenchmarkUtils.java @@ -0,0 +1,118 @@ +package datadog.trace.util; + +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** Shared setup helpers for JMH benchmarks in this module. */ +public final class BenchmarkUtils { + private BenchmarkUtils() {} + + private static final Object[] DEFAULT_DECOY_KEYS = { + "decoy", 1, 1L, 1.0d, Boolean.TRUE, new Object() + }; + + /** + * Exercises {@link HashSet}/{@link java.util.HashMap}, the tracer's {@link + * CollectionUtils#tryMakeImmutableSet} immutable sets, and {@link ConcurrentHashMap} with several + * distinct key classes, so each structure's internal {@code hashCode()}/{@code equals()} dispatch + * -- a call site shared JVM-wide by every instance of that structure in the process, regardless + * of which specific instance or call site invokes {@code add}/{@code contains}/{@code get} -- is + * already megamorphic before a benchmark measures lookups against a single key type. + * + *

This matches production: those shared internal call sites are hit by every hash-based + * structure in the JVM across whatever key types the whole application uses, so they're + * realistically almost always megamorphic. An isolated benchmark that only ever looks up one key + * type (e.g. {@code String}) would otherwise leave them artificially monomorphic for the entire + * run, understating real dispatch cost. + * + *

{@code HashSet} is backed by {@code HashMap} in the JDK, so polluting it also covers plain + * {@code HashMap} and {@code LinkedHashMap} (which extends {@code HashMap}) -- they share the + * same internal dispatch call site. {@code ConcurrentHashMap} does not: it's an unrelated class + * with its own {@code hashCode()}/{@code equals()} call sites, so it needs its own scratch + * instance (also covers {@code ConcurrentHashMap#newKeySet()}, which is backed by a {@code + * ConcurrentHashMap}). Structures that dispatch on {@code compareTo} instead ({@code TreeMap}, + * {@code TreeSet}, {@code ConcurrentSkipListMap}) aren't affected by any of this and don't need + * pollution. + * + *

Deliberately does not touch the benchmark's own {@code contains}/{@code add}/{@code get} + * call sites -- those are realistically free to specialize per caller, the way a genuinely hot, + * narrowly-typed call site would in production. + * + *

Not to be confused with the CHA-defeat decoys in {@code SingleThreadedMapBenchmark}/{@code + * ThreadSafeMapBenchmark} ({@code KeyStrategy} implementors referenced only so they're loaded, + * never invoked): that technique denies class-hierarchy analysis a single-implementor bet for a + * narrow, dd-trace-java-owned interface, and works by class-loading alone. It doesn't apply here + * -- {@code Object.hashCode()}/{@code equals()} already have countless implementors loaded in any + * real JVM, so a single-implementor CHA bet was never available for them. What gates their + * dispatch is the interpreter's per-call-site type profile, which only invocation can pollute -- + * hence this helper actually calls {@code add}/{@code contains}/{@code get}, rather than just + * loading classes. + */ + public static void polluteHashDispatch() { + polluteHashDispatch(DEFAULT_DECOY_KEYS); + } + + public static void polluteHashDispatch(Object... decoyKeys) { + populateTypeProfileMutable(new HashSet<>(), decoyKeys); + populateTypeProfile(CollectionUtils.tryMakeImmutableSet(Arrays.asList(decoyKeys)), decoyKeys); + populateTypeProfileMutableMap(new ConcurrentHashMap<>(), decoyKeys); + } + + /** + * The entry point most benchmarks should reach for: pass the same kind of collection instance + * under test (or an equivalent scratch instance). Works for both mutable and immutable + * collections since it only drives {@code contains()} -- the operation every {@link + * java.util.Set} supports, and the one these lookup benchmarks actually measure. + */ + public static void populateTypeProfile(Collection populated) { + populateTypeProfile(populated, DEFAULT_DECOY_KEYS); + } + + public static void populateTypeProfile(Collection populated, Object... decoyKeys) { + for (Object key : decoyKeys) { + populated.contains(key); + } + } + + /** + * Lower-level control: also drives {@code add()} dispatch, so {@code scratch} must genuinely + * support mutation, and lets the caller pick the decoy keys. Reach for this only when {@code + * add()} dispatch matters too, or the default decoys aren't the right shape. + */ + public static void populateTypeProfileMutable(Collection scratch, Object... decoyKeys) { + for (Object key : decoyKeys) { + scratch.add(key); + scratch.contains(key); + } + } + + /** + * {@link Map} counterpart to {@link #populateTypeProfile(Collection)}: pass the map instance + * under test (or an equivalent scratch instance) to drive its {@code get()} dispatch. Safe + * against immutable maps too, since it only calls {@code get()}. + */ + public static void populateTypeProfileMap(Map populated) { + populateTypeProfileMap(populated, DEFAULT_DECOY_KEYS); + } + + public static void populateTypeProfileMap(Map populated, Object... decoyKeys) { + for (Object key : decoyKeys) { + populated.get(key); + } + } + + /** + * Lower-level control, {@link Map} counterpart to {@link #populateTypeProfileMutable}: also + * drives {@code put()} dispatch, so {@code scratch} must genuinely support mutation. + */ + public static void populateTypeProfileMutableMap( + Map scratch, Object... decoyKeys) { + for (Object key : decoyKeys) { + scratch.put(key, key); + scratch.get(key); + } + } +} 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 9cbbddcf299..ec067cefce2 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/CaseInsensitiveMapBenchmark.java @@ -6,8 +6,10 @@ import java.util.function.Supplier; 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; @@ -52,6 +54,31 @@ * 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 * + * + *

Rerun on JDK 8 with {@link BenchmarkUtils#polluteHashDispatch()} added to a new + * {@code @Setup(Level.Trial)} (this file had none before), at this file's actual {@code @Fork(2)} + * (the numbers above are from an ad hoc higher-fork run; not directly comparable). M ops/s, 8 + * threads: + * + * 

{@code
+ * create_baseline        26    create_flatHashtable   13
+ * create_hashMap          9    create_treeMap          7
+ *
+ * lookup_baseline      2618    lookup_flatHashtable  415
+ * lookup_flatHashtable_lowLoad 415  lookup_hashMap    367*
+ * lookup_treeMap        209
+ * }
+ * + *

* = error bar over a third of the mean at {@code @Fork(2)} — directional only. + * + *

All four {@code lookup_*} numbers sit 17-23% below the table above (415 vs 537 flatHashtable, + * 367 vs 442 hashMap, 209 vs 251 treeMap) despite {@code flatHashtable} and {@code treeMap} using + * neither {@code java.util.HashMap} nor {@code hashCode()}/{@code equals()} dispatch — so this drop + * isn't attributable to pollution. The likelier explanation: the table above is Zulu 21, this rerun + * is JDK 8, and JDK 8's C2 backend for Apple Silicon (AArch64) is far less mature than JDK 17+'s — + * a broad-based slowdown across every entry, pollution-affected or not, is expected from that JDK + * gap alone on this machine. The relative ranking — {@code flatHashtable} > {@code hashMap} + * > {@code treeMap} — is unchanged. */ @Fork(2) @Warmup(iterations = 2) @@ -101,6 +128,11 @@ static T init(Supplier supplier) { // masking exactly the differences this benchmark compares. int lookupIndex = 0; + @Setup(Level.Trial) + public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + } + String nextLookupKey() { int localIndex = ++lookupIndex; if (localIndex >= LOOKUP_KEYS.length) { @@ -242,9 +274,10 @@ 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) and then the value is overwritten explicitly -- getOr- - // Create itself never updates an existing entry, so without this the FlatHashtable arm would do + // insensitive collisions. tryGetOrCreate finds the already-present lower-case entry (a hit, so + // the create never fires and nothing allocates) and then the value is overwritten explicitly -- + // tryGetOrCreate 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. @@ -252,7 +285,8 @@ static CIEntry[] _create_flat(float loadFactor) { for (String prefix : UPPER_PREFIXES) { String key = prefix + "-" + suffix; CIEntry entry = - FlatHashtable.getOrCreate(table, key, CaseInsensitiveKeyStrategy.INSTANCE, CI_CREATE); + FlatHashtable.tryGetOrCreate( + table, key, CaseInsensitiveKeyStrategy.INSTANCE, CI_CREATE); entry.value = suffix + 1; } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/HashtableD1Benchmark.java b/internal-api/src/jmh/java/datadog/trace/util/HashtableD1Benchmark.java index f8ba7177e88..f9bcb0de96e 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/HashtableD1Benchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/HashtableD1Benchmark.java @@ -38,10 +38,14 @@ *

  • iterate — walk every entry and consume its key + value. * * - *

    Update is where Hashtable dominates: D1 is ~14x faster, because the HashMap path - * allocates per call (a {@code Long}) and the resulting GC pressure throttles throughput under - * multiple threads. Add is roughly comparable (both allocate one entry per insert). - * Iterate is essentially a wash — both are bucket walks. + *

    Update is where Hashtable dominates: D1 is ~14x faster on JDK 8 (see the Java 17 rerun + * below for a narrower but still decisive margin), because the HashMap path allocates per call (a + * {@code Long}) and the resulting GC pressure throttles throughput under multiple threads. This is + * the headline case for {@code Hashtable}: a simple counter/tally with a primitive value is exactly + * where HashMap's autoboxing tax bites hardest, and {@code Hashtable.D1} sidesteps it entirely by + * mutating a field on the retrieved entry in place. Add is roughly comparable (both allocate + * one entry per insert). Iterate is essentially a wash on JDK 8, though not on Java 17 (see + * below). * MacBook M1 8 threads (Java 8) * * Benchmark Mode Cnt Score Error Units @@ -54,6 +58,72 @@ * HashtableD1Benchmark.iterate_hashMap thrpt 6 20.043 ± 0.752 ops/us * HashtableD1Benchmark.iterate_hashtable thrpt 6 22.208 ± 0.956 ops/us * + * + *

    Rerun with {@link BenchmarkUtils#polluteHashDispatch()} added to {@code D1State.setUp()} (same + * machine/JVM/config): every number moved down somewhat (add_hashMap 188→101, update_hashtable + * 1810→1465, iterate_hashtable 22→17 ops/us), including {@code *_hashtable}. That's expected to be + * a no-op for {@code *_hashtable}: {@link Hashtable.D1.Entry#hash} and {@link + * Hashtable.D1.Entry#matches} are call sites private to {@code Hashtable.java}, structurally + * distinct from {@code java.util.HashMap}/{@code HashSet}'s internal {@code hashCode()}/{@code + * equals()} call sites — JIT type profiles are keyed per call site, so {@code + * polluteHashDispatch()} cannot reach them regardless of key-type overlap. Since the JDK and + * machine were held constant across this rerun (unlike the JDK 8-vs-17 comparisons in {@link + * datadog.trace.util.CaseInsensitiveMapBenchmark} and {@link + * datadog.trace.api.TagMapAccessBenchmark}), the drop here is same-session run-to-run noise + * (thermal/power, not controlled for) rather than either a pollution effect or a JDK effect. The + * relative conclusion (D1 dominates {@code update}, is roughly comparable on {@code add}, + * ties on {@code iterate}) is unchanged either way. + * + *

    Separately rerun on Zulu 17.0.7 (native AArch64, same machine, pollution wiring unchanged; JMH + * auto-detected the cheap "compiler" Blackhole mode here, unlike JDK 8, so absolute numbers below + * are not comparable to the JDK 8 tables above — see {@code HashtableD2Benchmark}'s javadoc for the + * full caveat). M ops/us, 8 threads: + * + * 

    {@code
+ * add_hashMap        1502.6   add_hashtable      1377.3
+ * update_hashMap      644.2   update_hashtable   2706.5
+ * iterate_hashMap      19.3   iterate_hashtable    78.0
+ * }
    + * + *

    Within this single run (so the cross-JDK Blackhole-mode confound doesn't apply to the ratios), + * {@code update_hashtable} still wins by ~4.2x — down from ~14x on JDK 8, because Java 17's + * allocator/GC absorbs {@code update_hashMap}'s per-call {@code Long} boxing far better than JDK 8 + * did (update_hashMap itself got ~5x faster; update_hashtable only ~1.5x faster). {@code + * iterate_hashtable} also now clearly wins (~4.0x), flipping from JDK 8's "wash" — HashMap's {@code + * entrySet()} iterator does more per-entry work than a modern JIT's allocation improvements erase. + * {@code add} is the one case that flips the other way: {@code add_hashMap} edges out {@code + * add_hashtable} slightly (1502.6 vs 1377.3). Net takeaway: {@code Hashtable} is a strong + * substitute for {@code HashMap} particularly for simple counter/tally use cases with a primitive + * value, where avoiding the per-update boxing allocation pays off even on a JVM with much better + * allocation handling than JDK 8 had. + * + *

    Rerun on the capped/{@code State}-backed table (5 forks, 15 datapoints/method, Zulu 17.0.7 + * AArch64, 8 threads). Not comparable to the table above: JMH auto-detected the {@code full + * + dont-inline} Blackhole here rather than the cheap {@code compiler} one, on the same JVM build + * and JMH 1.37 -- the mode is auto-detected per run and is not stable across runs, so every + * absolute number in this file is conditional on a mode that JMH does not record beside it. Compare + * within a table, never across. M ops/us: + * + * 

    {@code
+ * add_hashMap        1204.8   add_hashtable       974.4
+ * update_hashMap      577.2   update_hashtable   1862.6
+ * iterate_hashMap      15.9   iterate_hashtable    21.5
+ * }
    + * + *

    Within this run: {@code update_hashtable} wins by ~3.2x and {@code iterate_hashtable} by + * ~1.35x, while {@code add_hashtable} now loses by ~19% -- no longer the "roughly + * comparable" of the JDK 8 table, and a wider gap than the slight edge HashMap held in the previous + * Java 17 run. {@code add} is where the capped table's bookkeeping is least amortized: both sides + * allocate one entry per insert, so there is no boxing win to offset it, and the loop does nothing + * else. The counter/tally path -- the case {@code Hashtable} exists for -- is unaffected. + * + *

    That is the right side of the trade for this family. {@code Hashtable} and {@link + * ConcurrentHashtable} are designed for workloads where updates dominate: the table is + * populated once and then hit repeatedly, so per-insert cost amortizes away and in-place mutation + * of a primitive field is the operation that runs hot. Paying on {@code add} to make {@code update} + * faster is the trade those workloads want. {@code FlatHashtable} and {@code TagMap} sit at the + * other end -- built up and read, not updated in a loop -- so this result does not transfer to + * them, and neither does the reasoning that justifies it. */ @Fork(2) @Warmup(iterations = 2) @@ -101,9 +171,14 @@ public static class D1State { int cursor; final BhD1Consumer consumer = new BhD1Consumer(); + // Level.Iteration, not Trial: this rebuilds the table and the HashMap, so each iteration must + // start from a fresh, identically-sized state rather than inheriting mutated counters. The + // pollution call rides along -- it is idempotent and untimed, so repeating it costs nothing. @Setup(Level.Iteration) public void setUp() { - table = new Hashtable.D1<>(CAPACITY); + BenchmarkUtils.polluteHashDispatch(); + + table = Hashtable.D1.createCapped(D1Counter.class, CAPACITY); hashMap = new HashMap<>(CAPACITY); keys = SOURCE_KEYS; for (int i = 0; i < N_KEYS; ++i) { diff --git a/internal-api/src/jmh/java/datadog/trace/util/HashtableD2Benchmark.java b/internal-api/src/jmh/java/datadog/trace/util/HashtableD2Benchmark.java index 6f46a702005..4f233b8524b 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/HashtableD2Benchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/HashtableD2Benchmark.java @@ -42,10 +42,13 @@ *

    The D2 variants additionally pay for a composite-key wrapper allocation in the HashMap path * (Java has no built-in tuple-as-key) — D2 sidesteps it by taking both key parts directly. * - *

    Update is where Hashtable dominates: D2 is ~26x faster, because the HashMap path - * allocates per call (a {@code Long}, plus a {@code Key2}) and the resulting GC pressure throttles - * throughput under multiple threads. Add is ~3x faster for D2 (Hashtable sidesteps the - * {@code Key2} allocation). Iterate is essentially a wash — both are bucket walks. + *

    Update is where Hashtable dominates: D2 is ~26x faster on JDK 8 (see the Java 17 rerun + * below for a narrower but still decisive margin), because the HashMap path allocates per call (a + * {@code Long}, plus a {@code Key2}) and the resulting GC pressure throttles throughput under + * multiple threads. Like D1, this is the headline case for {@code Hashtable}: a simple + * counter/tally with a primitive value is exactly where HashMap's autoboxing tax bites hardest. + * Add is ~3x faster for D2 (Hashtable sidesteps the {@code Key2} allocation). Iterate + * is essentially a wash on JDK 8, though not on Java 17 (see below). * MacBook M1 8 threads (Java 8) * * Benchmark Mode Cnt Score Error Units @@ -58,6 +61,46 @@ * HashtableD2Benchmark.iterate_hashMap thrpt 6 19.508 ± 0.760 ops/us * HashtableD2Benchmark.iterate_hashtable thrpt 6 16.968 ± 0.371 ops/us * + * + *

    Rerun with {@link BenchmarkUtils#polluteHashDispatch()} added to {@code D2State.setUp()} (same + * machine/JVM/config): results were noisy and inconsistent with a clean pollution story — + * add_hashMap actually rose (77→103), while add_hashtable fell sharply (217→118, error bars wider + * than the mean both times); update_hashtable fell (1446→1225) and both iterate numbers fell + * (19.5→15.4, 17.0→13.1). As with {@link HashtableD1Benchmark}, {@code *_hashtable} is expected to + * be a no-op here: {@link Hashtable.D2.Entry#hash} and {@link Hashtable.D2.Entry#matches} are call + * sites private to {@code Hashtable.java}, structurally distinct from {@code + * java.util.HashMap}/{@code HashSet}'s internal dispatch call sites — pollution cannot reach them. + * With the JDK and machine held constant across this rerun, the drop is same-session run-to-run + * noise (thermal/power, not controlled for) rather than a genuine pollution effect. Treat these two + * runs as not directly comparable on absolute numbers. The relative conclusion (D2 dominates + * {@code update}, wins {@code add} by avoiding the {@code Key2} allocation, ties on {@code + * iterate}) is unchanged either way. + * + *

    Separately rerun on Zulu 17.0.7 (native AArch64, same machine, pollution wiring unchanged). + * JMH auto-detected the cheap "compiler" Blackhole mode on Java 17 (its log explicitly warns that + * Blackhole-mode differences between JVMs can swing results significantly), which JDK 8 cannot use + * — so absolute numbers below are not comparable to the JDK 8 tables above; only within-run + * ratios are, since both benchmark methods in a given run get identical Blackhole treatment. M + * ops/us, 8 threads: + * + * 

    {@code
+ * add_hashMap         656.7   add_hashtable     1185.5
+ * update_hashMap      196.7   update_hashtable  2292.2
+ * iterate_hashMap      20.5   iterate_hashtable   69.2
+ * }
    + * + *

    {@code update_hashtable} still wins decisively (~11.6x, down from ~26x on JDK 8 — Java 17's + * allocator/GC absorbs {@code update_hashMap}'s per-call {@code Long}+{@code Key2} boxing far + * better than JDK 8 did: update_hashMap got ~3.5x faster, update_hashtable only ~1.6x faster). + * Unlike JDK 8, Hashtable now wins clearly on every operation: {@code add_hashtable} wins + * ~1.8x (vs. JDK 8's ~3x — HashMap's {@code Key2} allocation also got relatively cheaper), and + * {@code iterate_hashtable} flips from JDK 8's wash to a ~3.4x win (HashMap's {@code entrySet()} + * iterator does more per-entry work than a modern JIT's allocation improvements erase). Net + * takeaway, consistent with {@link HashtableD1Benchmark}: {@code Hashtable} is a strong substitute + * for {@code HashMap} particularly for simple counter/tally use cases with a primitive value, where + * avoiding the per-update boxing allocation pays off even on a JVM with much better allocation + * handling than JDK 8 had — and for D2 specifically, avoiding the composite-key wrapper allocation + * pays off across the board, not just on {@code update}. */ @Fork(2) @Warmup(iterations = 2) @@ -136,9 +179,14 @@ public static class D2State { int cursor; final BhD2Consumer consumer = new BhD2Consumer(); + // Level.Iteration, not Trial: this rebuilds the table and the HashMap, so each iteration must + // start from a fresh, identically-sized state rather than inheriting mutated counters. The + // pollution call rides along -- it is idempotent and untimed, so repeating it costs nothing. @Setup(Level.Iteration) public void setUp() { - table = new Hashtable.D2<>(CAPACITY); + BenchmarkUtils.polluteHashDispatch(); + + table = Hashtable.D2.createCapped(D2Counter.class, CAPACITY); hashMap = new HashMap<>(CAPACITY); k1s = SOURCE_K1; k2s = SOURCE_K2; diff --git a/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java index 39aaf82183c..5826e92e10c 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java @@ -44,40 +44,50 @@ * {@code *_sameKey} variants reuse the original interned key instances to show the identity fast * path — which is the common tracer case, since map keys are typically interned tag-name constants. * - *

    JDK 17 results (Apple M1, quiet machine, {@code @Fork(5)}, {@code @Threads(8)}; M ops/s). - * {@code get} uses distinct keys (exercises {@code equals()}); {@code sameKey} reuses the interned - * key (the {@code ==} fast path — the common tracer case): + *

    {@link BenchmarkUtils#polluteHashDispatch()} runs in {@link #setUp}, for the same reason as + * {@link ImmutableSetBenchmark}: {@code Object.hashCode()}/{@code equals()} are JVM-wide shared + * call sites, hit by every hash-based structure in the process (this benchmark's {@code HashMap}, + * {@code LinkedHashMap}, and {@code Map.copyOf}/{@code MapN} all dispatch through them for their + * {@code String} keys) — realistically almost always megamorphic, so leaving them monomorphic for + * the whole run would understate real dispatch cost. * - * 

    {@code
- * Structure                           get sameKey
- * stringIndex_embedded (static)      1498    2081    (fastest)
- * stringIndex (inst)                 1363    1900
- * hashMap                            1216    1850
- * linkedHashMap                      1214       -
- * tagMap                             1167    1386
- * tracerImmutableMap                 1049    1364    (MapN)
- * treeMap                             656       -
- * }
    - * - *

    {@code iterate} (full traversal): + *

    JDK 8 results (Apple M1, quiet machine, {@code @Fork(5)}, {@code @Threads(8)}, with {@link + * BenchmarkUtils#polluteHashDispatch()} in effect; M ops/s). {@code get} uses distinct keys + * (exercises {@code equals()}); {@code sameKey} reuses the interned key (the {@code ==} fast path — + * the common tracer case): * * 

    {@code
- * tagMap.forEach        148    (fastest)
- * linkedHashMap         136
- * tracerImmutableMap    135    (MapN)
- * treeMap               134
- * hashMap               104
- * tagMap (iterator)      96
+ * Structure                get sameKey iterate iterate_forEach
+ * hashMap                 1202    1438     120        -
+ * linkedHashMap           1097       -     127        -
+ * treeMap                  487       -     121        -
+ * tagMap                  1005    1235     109       121
+ * tracerImmutableMap      1052    1249     123        -   (MapN)
+ * stringIndex             1366    1724       -        -
+ * stringIndex_embedded    1479    1846       -        -   (fastest get)
  * }
    * *

    Key findings: * *

      - *
    • StringIndex-as-map ({@code EmbeddingSupport}) is the fastest {@code get} — beating {@code - * HashMap} and {@code Map.copyOf}/{@code MapN}, most on the interned path; the instance - * wrapper trails it by ~10%. (vs {@code MapN} the edge is speed + the slot/parallel-array - * capability, not footprint — see {@link ImmutableSetBenchmark}.) - *
    • {@code TagMap.forEach} (148) beats its own {@code iterator} (96) by ~1.5x: TagMap's + *
    • StringIndex-as-map is a reliable {@code get} win, and it holds up under type-profile + * pollution: {@code stringIndex_embedded} (the {@code static final}-array form) and {@code + * stringIndex} (the instance wrapper) both beat every {@code Map} here by a wide margin, on + * both the {@code equals()} and identity-fast-path lookups. Unlike {@link + * ImmutableSetBenchmark}'s {@code hitFresh} case, there's no bimodality here — this win is + * unconditional, not access-pattern-dependent. + *
    • This table still compares boxed {@code Integer} values ({@code hashMap}/{@code + * linkedHashMap}/{@code treeMap}/{@code tracerImmutableMap} all return {@code Integer}; + * {@code tagMap}/{@code stringIndex} happen to expose primitive {@code int} accessors, but + * that's not exercised as a differentiator here). StringIndex's edge should widen further + * against a {@code Map} once the comparison is against genuinely autoboxed + * reads on both sides — not yet measured. + *
    • {@code treeMap}'s {@code get} has a wide error bar (±129, one fork's measurement iterations + * dropped to ~230-300, the rest sit around 490-610) — a known {@code TreeMap} comparison + * characteristic (uses {@code compareTo} not {@code hashCode}/{@code equals}, so it's + * unaffected by dispatch pollution), not the same warmup-race bimodality investigated in + * {@link ImmutableSetBenchmark}. + *
    • {@code TagMap.forEach} (121) beats its own {@code iterator} (109) by ~10%: TagMap's * structure makes a faithful external {@code Iterator} expensive (externalized cursor + * skip-empty + per-call re-entry + the iterator allocation) — all of which internal {@code * forEach} avoids. Traverse TagMap via {@code forEach}, never its iterator; that gap only @@ -145,6 +155,8 @@ static void fill(Map map) { @Setup(Level.Trial) public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + hashMap = new HashMap<>(); fill(hashMap); linkedHashMap = new LinkedHashMap<>(); diff --git a/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java index f0604e6ddd4..ce5e7dc5ee2 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java @@ -44,37 +44,75 @@ * indirection cost of the wrapper. *
    * - *

    Lookups are interned (the {@code ==} fast path where a structure has one); misses are short - * and never present. + *

    Hit lookups come in two flavors, because reusing the exact same key instances for both + * building a structure and measuring lookups against it is its own validity bug, independent of + * hash-dispatch pollution: {@link String#equals} takes an {@code ==} fast path, and {@link + * String#hashCode()} caches its result in a field on first call, so a key instance that was already + * inserted (or previously looked up) pays neither real cost again. * - *

    JDK 17 results (Apple M1, quiet machine, {@code @Fork(5)}, {@code @Threads(8)}; M ops/s = - * millions): + *

      + *
    • {@code hit} -- looks up the same interned {@link #STRINGS} literals used to build every + * structure. This is realistic and worth keeping (fixed header/config-key lookups commonly + * are interned literals), but identity with the stored element is inherent to interning, not + * a choice this benchmark makes -- two equal literals are always the same instance. + *
    • {@code hitFresh} -- looks up {@link #FRESH_STRINGS}, separate {@code new String(..)} + * instances never touched by {@link #setUp}, so each carries an uncached hash and forces a + * real {@code equals()} beyond the {@code ==} check. Models keys arriving from + * parsing/concatenation/deserialization rather than literals. Only meaningful for the + * hash-based structures ({@code hashSet}, {@code tracerImmutableSet}, {@code stringIndex}, + * {@code stringIndex_embedded}); not measured for {@code array}/{@code sortedArray}/{@code + * treeSet}. + *
    + * + *

    Misses are already representative of both effects for free: {@link #MISSES} is built via + * concatenation (never interned) and never touched by {@link #setUp}. + * + *

    Full re-run, all six structures across {@code hit}/{@code hitFresh}/{@code miss} together, + * with {@link BenchmarkUtils#polluteHashDispatch()} in effect (Apple M1, Java 8u382 -- the + * repo-default {@code jmh} test launcher, no {@code -PtestJvm} override --, {@code @Fork(5)}, + * {@code @Threads(8)}; M ops/s = millions): * * 

    {@code
- * Structure                           hit    miss
- * stringIndex_embedded (static)      2320    2159    (fastest)
- * hashSet                            2198    2134
- * stringIndex (inst)                 2098  1548 *    (* miss bimodal -- see caveat)
- * tracerImmutableSet                 1914    1663    (Set.copyOf / SetN)
- * array                               941     589
- * sortedArray                         685     610
- * treeSet                             657     610
+ * Structure                    hit   hitFresh    miss
+ * stringIndex_embedded (static) 2098      1563    2030    (fastest hit and miss)
+ * hashSet                       1723      1276    1823
+ * stringIndex (inst)            1883      1184 *  1700 *  (* bimodal -- see caveat)
+ * tracerImmutableSet            1632      1232    1625    (Set.copyOf / SetN)
+ * array                          854         -     495
+ * sortedArray                    713         -     613
+ * treeSet                        646         -     544
  * }
    * *

    Key findings: * *

      - *
    • The static {@code EmbeddingSupport} path is the fastest — it beats {@code HashSet} on hit - * and miss and crushes the scan/search/tree forms. + *
    • The static {@code EmbeddingSupport} path is the fastest on both hit and miss -- it beats + * {@code HashSet} on both and crushes the scan/search/tree forms. *
    • {@code stringIndex} (the instance wrapper) trails {@code EmbeddingSupport} by the - * field-load indirection (~10% on hit), landing near {@code HashSet} — fine off the hot path, - * prefer {@code EmbeddingSupport} on it. - *
    • {@link java.util.Set#copyOf} ({@code SetN}, the agent's compact fixed-set form) is ~1.2x - * behind {@code EmbeddingSupport} on hit but the most compact (~27% smaller — no - * cached hashes, no 2x table). So StringIndex's edge over {@code SetN} is speed + the {@code - * indexOf}->parallel-array capability, not footprint; over {@code HashSet} it wins both. - *
    • {@code array} / {@code sortedArray} / {@code treeSet} trail the hashed structures, most on + * field-load indirection. It reliably beats {@code HashSet} on {@code hit} (its actual design + * case: repeated lookups of a known, fixed name set). On {@code miss} and {@code hitFresh} it + * is not a reliable win -- both are bimodal across forks (see caveat below) and the + * mean in each case already sits at or below {@code HashSet}'s steady figure. For miss- or + * fresh-key-heavy membership use, prefer {@link StringIndex.EmbeddingSupport} directly rather + * than assuming the wrapper is strictly better than a plain {@code Set}. This doesn't apply + * to {@code StringIndex}'s parallel-value (map) use case ({@code mapValues}/{@code lookup}) + * -- that win comes from avoiding boxing and node overhead entirely and is unaffected by any + * of this. + *
    • {@link java.util.Set#copyOf} ({@code SetN}, the agent's compact fixed-set form) trails + * {@code EmbeddingSupport} on every scenario but remains the most compact (~27% + * smaller -- no cached hashes, no 2x table). So StringIndex's edge over {@code SetN} is speed + * + the {@code indexOf}->parallel-array capability, not footprint. + *
    • {@code array} / {@code sortedArray} / {@code treeSet} trail every hashed structure, most on * miss. + *
    • {@code hitFresh} is the slowest of the three scenarios for every hash-based + * structure -- clearly below both {@code hit} and {@code miss}, not merely below {@code hit} + * as previously guessed. This makes sense once the two failure shapes are compared: a miss + * usually short-circuits on the first hash mismatch during probing and rarely reaches {@code + * equals()}, while a {@code hitFresh} lookup must probe until it finds the match and pay a + * real, uncached {@code equals()} there -- so it is not simply "the honest version of hit", + * it exercises a genuinely more expensive path than either {@code hit} (cached hash + {@code + * ==}) or {@code miss} (hash-only rejection). Superseded an earlier partial-data guess that + * {@code hitFresh} would land at the same cost as {@code miss}. *
    * *

    Caveat — the instance {@code stringIndex} miss is bimodal across forks (confirmed at @@ -100,6 +138,21 @@ public class ImmutableSetBenchmark { /** Distinct String instances that are never present, for the miss path. */ static final String[] MISSES = newMisses(); + /** + * Equal-content, non-interned, never-before-hashed copies of {@link #STRINGS}, built once here + * and never touched by {@link #setUp} -- so a lookup against them can't ride the {@code ==} fast + * path or a hash cached during set construction. See {@code hitFresh} in the class javadoc. + */ + static final String[] FRESH_STRINGS = newFreshStrings(); + + static String[] newFreshStrings() { + String[] fresh = new String[STRINGS.length]; + for (int i = 0; i < STRINGS.length; ++i) { + fresh[i] = new String(STRINGS[i]); + } + return fresh; + } + static String[] newMisses() { String[] misses = new String[STRINGS.length * 4]; for (int i = 0; i < misses.length; ++i) { @@ -131,6 +184,8 @@ static String[] newMisses() { @Setup(Level.Trial) public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + array = STRINGS; sortedArray = Arrays.copyOf(STRINGS, STRINGS.length); Arrays.sort(sortedArray); @@ -144,6 +199,7 @@ public void setUp() { @State(Scope.Thread) public static class Cursor { int hitIndex = 0; + int hitFreshIndex = 0; int missIndex = 0; String nextHit() { @@ -155,6 +211,16 @@ String nextHit() { return STRINGS[i]; } + /** See {@code hitFresh} in the class javadoc. */ + String nextHitFresh() { + int i = hitFreshIndex + 1; + if (i >= FRESH_STRINGS.length) { + i = 0; + } + hitFreshIndex = i; + return FRESH_STRINGS[i]; + } + String nextMiss() { int i = missIndex + 1; if (i >= MISSES.length) { @@ -199,6 +265,11 @@ public boolean hashSet_hit(Cursor cursor) { return hashSet.contains(cursor.nextHit()); } + @Benchmark + public boolean hashSet_hitFresh(Cursor cursor) { + return hashSet.contains(cursor.nextHitFresh()); + } + @Benchmark public boolean hashSet_miss(Cursor cursor) { return hashSet.contains(cursor.nextMiss()); @@ -219,6 +290,11 @@ public boolean tracerImmutableSet_hit(Cursor cursor) { return tracerImmutableSet.contains(cursor.nextHit()); } + @Benchmark + public boolean tracerImmutableSet_hitFresh(Cursor cursor) { + return tracerImmutableSet.contains(cursor.nextHitFresh()); + } + @Benchmark public boolean tracerImmutableSet_miss(Cursor cursor) { return tracerImmutableSet.contains(cursor.nextMiss()); @@ -229,6 +305,11 @@ public boolean stringIndex_hit(Cursor cursor) { return stringIndex.contains(cursor.nextHit()); } + @Benchmark + public boolean stringIndex_hitFresh(Cursor cursor) { + return stringIndex.contains(cursor.nextHitFresh()); + } + @Benchmark public boolean stringIndex_miss(Cursor cursor) { return stringIndex.contains(cursor.nextMiss()); @@ -236,11 +317,16 @@ public boolean stringIndex_miss(Cursor cursor) { @Benchmark public boolean stringIndex_embedded_hit(Cursor cursor) { - return StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, cursor.nextHit()) >= 0; + return StringIndex.EmbeddingSupport.contains(SI_HASHES, SI_NAMES, cursor.nextHit()); + } + + @Benchmark + public boolean stringIndex_embedded_hitFresh(Cursor cursor) { + return StringIndex.EmbeddingSupport.contains(SI_HASHES, SI_NAMES, cursor.nextHitFresh()); } @Benchmark public boolean stringIndex_embedded_miss(Cursor cursor) { - return StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, cursor.nextMiss()) >= 0; + return StringIndex.EmbeddingSupport.contains(SI_HASHES, SI_NAMES, cursor.nextMiss()); } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/MaybeUsagePatternsBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/MaybeUsagePatternsBenchmark.java new file mode 100644 index 00000000000..07a05f69627 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/MaybeUsagePatternsBenchmark.java @@ -0,0 +1,191 @@ +package datadog.trace.util; + +import java.util.function.BiConsumer; +import java.util.function.ObjLongConsumer; +import javax.annotation.Nullable; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +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; + +/** + * A do/don't guide for using {@link Maybe}, not a research instrument like {@code + * datadog.trace.util.escape.EscapeShapeBenchmark} (which this class's arms are built on top of). + * Read {@code gc.alloc.rate.norm} — the "good" arm in each pair is expected to read 0 B/op on every + * JDK the way {@code EscapeShapeBenchmark}'s {@code singleSite}/{@code passedToInlinedStrategy} + * arms do; the paired "bad" arm exists to make the regression visible rather than theoretical. Run + * as + * + * 

    + * ./gradlew :internal-api:jmh -Pjmh.includes=MaybeUsagePatternsBenchmark -Pjmh.profilers=gc -PtestJvm=17
+ *
    + * + * This is the intended backing example for a perf-review check like "EA-dependent elision on a hot + * path where a structural alternative exists at parity → prefer the deterministic form": both pairs + * below have a same-cost deterministic form available, so reviewing a real diff against these arms + * is a matter of asking "which arm does this call site look like," not re-deriving the + * escape-analysis argument each time. + * + *

    The boxed-context pair is the sharper illustration of that phrase than it first looks + * like. {@code badBoxedContextUpdateInlined} was expected to allocate the boxed {@code Long} + * and, measured here, does not -- with the whole {@code update} call inlined, C2 scalar-replaces + * the box the same as it would any other short-lived object. That is exactly the "EA-dependent" + * half of that phrase: {@link Maybe#update(long, ObjLongConsumer)} has no box to eliminate + * in the first place, so it reads 0 B/op regardless of whether the mutator lambda's own inlining + * holds; the generic-context form's 0 B/op is contingent on that specific inlining, which {@code + * badBoxedContextUpdateUninlined} demonstrates by taking it away via the same {@code + * -XX:CompileCommand=dontinline} technique {@code EscapeShapeBenchmark} uses for its {@code + * UninlinedStrategy} arm. This is narrower than immunity to every inlining failure: if the + * producing method or the {@code update} call itself fails to inline -- a different boundary, + * exercised by {@code EscapeShapeBenchmark}'s {@code passedToUninlinedStrategy} arm (24 B/op) -- + * the {@code Maybe} wrapper itself becomes a real allocation for either overload. + */ +@Fork( + value = 2, + jvmArgsAppend = { + "-XX:CompileCommand=dontinline,datadog.trace.util.MaybeUsagePatternsBenchmark$UninlinedBoxedAdder::accept" + }) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(java.util.concurrent.TimeUnit.NANOSECONDS) +@State(Scope.Thread) +public class MaybeUsagePatternsBenchmark { + + static final class Widget { + long count; + } + + /** A non-capturing updater, as {@link Maybe#update(long, ObjLongConsumer)} expects. */ + static final ObjLongConsumer ADD_PRIMITIVE = (w, delta) -> w.count += delta; + + /** + * The same update expressed through the generic-context overload instead. {@code Long} is not + * assignable from {@code long} without boxing, so calling {@link Maybe#update(Object, + * BiConsumer)} with a {@code long} argument boxes it every time — the exact per-call allocation + * {@link Maybe#update(long, ObjLongConsumer)} exists to avoid. Kept as a {@code + * BiConsumer} rather than inlined at the call site so the two arms below differ + * only in which overload is selected, not in lambda shape. + */ + static final BiConsumer ADD_BOXED_INLINED = (w, delta) -> w.count += delta; + + /** + * Same logic as {@link #ADD_BOXED_INLINED}, but as a named class rather than a lambda so {@code + * -XX:CompileCommand=dontinline} (see this class's {@link Fork} annotation) has a concrete method + * to target -- kept out of line the same way {@code EscapeShapeBenchmark}'s {@code + * UninlinedStrategy} is, by the {@code CompileCommand} rather than {@code CompilerControl}, since + * JMH's processor only reads that annotation from {@code @Benchmark} methods. + */ + static final class UninlinedBoxedAdder implements BiConsumer { + @Override + public void accept(Widget w, Long delta) { + w.count += delta; + } + } + + static final BiConsumer ADD_BOXED_UNINLINED = new UninlinedBoxedAdder(); + + /** + * Deliberately outside {@code Long}'s [-128, 127] cache range -- a cached delta like {@code 1L} + * would make {@link #badBoxedContextUpdateUninlined} read 0 B/op too, for a reason with nothing + * to do with which overload got picked. + */ + static final long DELTA = 1_000L; + + private final Widget[] table = new Widget[8]; + private int counter; + + public MaybeUsagePatternsBenchmark() { + for (int i = 0; i < table.length; i++) { + // Half the slots stay null so every arm below actually exercises the refused/empty path, + // not just the present one -- see EscapeShapeBenchmark's `alternate()` javadoc for why an + // always-taken branch would quietly turn these into single-site arms and lie. + if ((i & 1) == 0) { + table[i] = new Widget(); + } + } + } + + private int nextKey() { + return (counter++) & (table.length - 1); + } + + @Nullable + private Widget lookup(int key) { + return table[key]; + } + + /** + * GOOD: exactly one {@code Maybe.of(...)} call site, fed by delegating to the existing nullable + * method. See {@link Maybe}'s class javadoc for why this is the recommended shape. + */ + private Maybe tryLookupDelegating(int key) { + return Maybe.of(lookup(key)); + } + + /** + * BAD: a {@code Maybe.of(...)} call site per branch. Both branches return the same wrapper type, + * so this looks equivalent to {@link #tryLookupDelegating} at every call site that uses it — the + * difference only shows up here, in the allocation profile of the method that builds the {@code + * Maybe}, which is exactly why it is easy to introduce by accident. + */ + private Maybe tryLookupMultiSite(int key) { + Widget w = lookup(key); + if (w != null) { + return Maybe.of(w); + } else { + return Maybe.of(null); + } + } + + @Benchmark + public void goodSingleConstructionSite(Blackhole bh) { + Maybe t = tryLookupDelegating(nextKey()); + bh.consume(t.isPresent()); + } + + @Benchmark + public void badMultiConstructionSite(Blackhole bh) { + Maybe t = tryLookupMultiSite(nextKey()); + bh.consume(t.isPresent()); + } + + @Benchmark + public void goodPrimitiveContextUpdate(Blackhole bh) { + Maybe t = tryLookupDelegating(nextKey()); + t.update(DELTA, ADD_PRIMITIVE); + bh.consume(t.isPresent()); + } + + /** + * Reads 0 B/op here despite boxing {@link #DELTA} on every call -- this call site stays inlined, + * so C2 scalar-replaces the {@code Long} the same as any other non-escaping object. See {@link + * #badBoxedContextUpdateUninlined} for what that 0 is actually contingent on. + */ + @Benchmark + public void badBoxedContextUpdateInlined(Blackhole bh) { + Maybe t = tryLookupDelegating(nextKey()); + t.update(DELTA, ADD_BOXED_INLINED); + bh.consume(t.isPresent()); + } + + /** + * The same boxing, with only the inlining taken away (via {@link UninlinedBoxedAdder} and this + * class's {@code CompileCommand}). Whatever this costs above {@link #goodPrimitiveContextUpdate} + * is the box {@link #badBoxedContextUpdateInlined} was quietly relying on EA to remove. + */ + @Benchmark + public void badBoxedContextUpdateUninlined(Blackhole bh) { + Maybe t = tryLookupDelegating(nextKey()); + t.update(DELTA, ADD_BOXED_UNINLINED); + bh.consume(t.isPresent()); + } +} 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 cb792cc1ca9..26753867d18 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedMapBenchmark.java @@ -50,6 +50,48 @@ * unsynchronized {@code hashMap} {@code get}/{@code iterate} methods are the in-harness baseline; * the tax is the delta to the {@code synchronizedHashMap} equivalents. Comparing across JVM * versions at stock flags shows the biased-locking effect. (Results pending a fresh multi-JVM run.) + * + *

    JDK 8 results (Apple M1, {@code @Fork(2)}, {@code @Threads(8)}, with {@link + * BenchmarkUtils#polluteHashDispatch()} in effect; M ops/s): + * + * 

    {@code
+ * create_hashMap                 79   create_hashMap_sized     38*
+ * create_synchronizedHashMap    8.6   create_treeMap            5*
+ * create_linkedHashMap            8*  create_tagMap            10
+ * create_tagMap_via_ledger        9   create_flatHashtable    190
+ *
+ * clone_hashMap                  68*  clone_synchronizedHashMap 58
+ * clone_treeMap                 100   clone_linkedHashMap       94
+ * clone_tagMap                  249
+ *
+ * get_hashMap                   164   get_synchronizedHashMap   67
+ * get_flatHashtable              196
+ *
+ * iterate_hashMap                119  iterate_synchronizedHashMap 81
+ * iterate_flatHashtable           14*
+ * }
    + * + *

    * = error bar as wide as (or wider than) the mean at {@code @Fork(2)} — treat these as + * directional, not decisive; a {@code @Fork(5)} rerun would tighten them (see {@code + * ThreadSafeMapBenchmark}'s Javadoc for the same caveat pattern). The construction benchmarks are + * consistently the noisy ones; the read/clone/iterate benchmarks are comparatively tight. + * + *

    Key findings: + * + *

      + *
    • {@code flatHashtable} dominates both {@code create} (190M) and {@code get} (196M) — the + * unboxed, self-contained entry and comparison-free insert pay off, consistent with every + * other FlatHashtable comparison in this module. + *
    • {@code tagMap} clone (249M) is ~3.7x {@code hashMap} clone (68M) — the same story {@link + * datadog.trace.api.TagMapAccessBenchmark} reports from an earlier (unpolluted, Java 17) run + * at ~4.6x; the ratio survives pollution and a different JDK, even though the absolute + * numbers aren't directly comparable across those two runs. + *
    • The uncontended synchronization tax is large here even though this run is on JDK 8, where + * biased locking is enabled by default: {@code get_hashMap} (164M) → {@code + * get_synchronizedHashMap} (67M) is a ~59% hit, and {@code iterate} (119M → 81M) is ~32%. + * That's a bigger tax than the "biased locking should make uncontended locking nearly free" + * story predicts — not root-caused here, left as an open question rather than papered over. + *
    */ @Fork(2) @Warmup(iterations = 2) @@ -191,6 +233,8 @@ static IntEntry[] newFilledFlat() { @Setup(Level.Trial) public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + hashMap = new HashMap<>(); fill(hashMap); synchronizedHashMap = Collections.synchronizedMap(new HashMap<>(hashMap)); diff --git a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedSetBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedSetBenchmark.java index e145e6bbe8b..2b19e3ac48f 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedSetBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/SingleThreadedSetBenchmark.java @@ -45,16 +45,36 @@ * create_treeSet 36 * } * + *

    JDK 8 results (Apple M1, {@code @Fork(2)}, {@code @Threads(8)}, with {@link + * BenchmarkUtils#polluteHashDispatch()} in effect; M ops/s): + * + * 

    {@code
+ * contains_hashSet            1421
+ * contains_synchronizedSet     746    (~48% slower — the uncontended sync tax)
+ * iterate_hashSet              134
+ * iterate_synchronizedSet      129    (one monitor acquire amortized over the walk)
+ *
+ * create_hashSet         67    clone_hashSet          56
+ * create_hashSet_sized   83*   clone_synchronizedSet  48*
+ * create_linkedHashSet   63    clone_linkedHashSet    56
+ * create_synchronizedSet 71*   clone_treeSet          77
+ * create_treeSet         38
+ * }
    + * + *

    * = error bar over half the mean at {@code @Fork(2)} — directional only. + * *

    Key findings: * *

      - *
    • Uncontended synchronization tax on {@code contains} is ~37% (1291 → 808M ops/s) even - * with no contention and biased locking disabled (Java 17, JEP 374) — the full per-lock CAS - * cost. On {@code iterate} it nearly vanishes: a single monitor acquire amortized over the - * traversal. - *
    • Construction: {@code TreeSet} is the slowest to build (~36M); the {@code synchronizedSet} - * wrapper adds a modest cost over plain {@code HashSet}. (Allocation-path numbers carry more - * run-to-run variance than the read paths.) + *
    • Uncontended synchronization tax holds up under pollution and on a different JDK: + * {@code contains} is ~48% slower synchronized (1421 → 746M ops/s on JDK 8, vs. ~37% on Java + * 17) — same story, somewhat larger tax. {@code iterate}'s tax stays small either way (~4% + * here): one monitor acquire amortized over the walk. + *
    • Type-profile pollution didn't change the qualitative story from the original Java 17 run — + * {@code contains_hashSet} and {@code iterate_hashSet} land in the same range (1291 vs 1421M, + * 91 vs 134M) rather than collapsing, unlike {@link ImmutableSetBenchmark}'s {@code hitFresh} + * case. Construction numbers remain the noisiest (several {@code @Fork(2)} error bars exceed + * half the mean); {@code TreeSet} stays the slowest to build across both runs. *
    */ @Fork(2) @@ -94,6 +114,8 @@ static void fill(Set set) { @Setup(Level.Trial) public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + hashSet = new HashSet<>(Arrays.asList(ELEMENTS)); synchronizedSet = Collections.synchronizedSet(new HashSet<>(hashSet)); treeSet = new TreeSet<>(Arrays.asList(ELEMENTS)); 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 63ccb734e9c..53a009597c5 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapBenchmark.java @@ -8,8 +8,10 @@ import java.util.function.Supplier; 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; @@ -62,6 +64,29 @@ * ThreadSafeMapBenchmark.get_hashMap_synchronized thrpt 6 20123419.604 ± 4858466.787 ops/s * ThreadSafeMapBenchmark.get_hashMap_volatile thrpt 6 286024211.995 ± 114449056.603 ops/s *     + * + *

    Rerun on JDK 8 with a new {@code @Setup(Level.Trial)} calling {@link + * BenchmarkUtils#polluteHashDispatch()} (not present before this change — this file had no + * {@code @Setup} at all). Not directly comparable to the Java 21 numbers above (different JDK, and + * this is the first run with pollution), so treat this as its own baseline rather than a delta: + * + * 

    {@code
+ * create_concHashMap            47   create_concSkipListMap  17
+ * create_hashMap                130  create_hashMap_synchronized 92*
+ * create_flatHashtable          178
+ *
+ * get_concHashMap                977  get_concSkipListMap    309
+ * get_flatHashtable             1654  get_hashMap_synchronized 26
+ * get_hashMap_volatile          1257
+ * }
    + * + *

    * = error bar over half the mean at {@code @Fork(2)} — directional only. + * + *

    {@code get_concSkipListMap} (309M) is ~11x the Java 21 measurement above (27M) — far more than + * a JDK swap plausibly explains for an O(log n), {@code compareTo}-dispatched structure that this + * pollution change doesn't touch. Flagging this as an open anomaly rather than a finding: don't + * treat it as "ConcurrentSkipListMap got faster" without a controlled re-run isolating the JDK + * variable. */ @Fork(2) @Warmup(iterations = 2) @@ -93,6 +118,11 @@ static T init(Supplier supplier) { // (e.g. FlatHashtable's lock-free probe), hiding exactly the differences this benchmark compares. int lookupIndex = 0; + @Setup(Level.Trial) + public void setUp() { + BenchmarkUtils.polluteHashDispatch(); + } + String nextLookupKey() { return nextLookupKey(EQUAL_KEYS); } diff --git a/internal-api/src/jmh/java/datadog/trace/util/escape/EscapeShapeBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/escape/EscapeShapeBenchmark.java new file mode 100644 index 00000000000..8f9ab1c63fe --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/escape/EscapeShapeBenchmark.java @@ -0,0 +1,404 @@ +package datadog.trace.util.escape; + +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +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; + +/** + * Minimal code shapes, each isolating one thing that is believed to decide whether C2 can delete a + * short-lived object. Read {@code gc.alloc.rate.norm} — bytes per operation — not the timings: a + * shape that scalar-replaces reports 0, and one that does not reports the object's real size. Run + * it as + * + * 

    ./gradlew :internal-api:jmh -Pjmh.includes=EscapeShape -Pjmh.profilers=gc -PtestJvm=17
+ *
    + * + * and read the same rows across {@code -PtestJvm} 8, 11, 17, 21 and 25. The point is the matrix of + * shape against JDK, so that "will this allocate" stops being a question two people answer from + * memory. Nothing here is specific to any one caller: the arms model a generic two-outcome wrapper + * (something that is either present with a value or absent) and carry over unchanged to {@code + * Maybe}, because the shapes under test are about the compiler's allocation behavior, not about + * what the wrapped value represents. + * + *

    The underlying idea, before the terminology: the compiler can sometimes prove a short-lived + * object never needs to outlive the method that created it, and when it can, it skips putting that + * object on the heap at all -- it keeps the object's fields as plain local values instead. Three + * terms for that recur below. Escape analysis (EA) is the compiler's proof step -- showing + * an allocated object's lifetime is confined to the method (or thread) that created it, i.e. it + * never escapes into a field, a return value visible outside, or a call the compiler cannot see + * into. Scalar replacement is what C2 (HotSpot's JIT) does once that proof holds: the object + * itself disappears, and its individual fields live in registers or on the stack instead, so no + * heap allocation happens -- the arms below that read 0 B/op are exactly the ones EA proved safe. + * {@code ReduceAllocationMerges} (JDK-8287061) extends that same proof to one harder case: + * an if/else (or similar branch) where each side allocates its own object -- say {@code x = new + * Foo()} in one branch and {@code x = new Bar()} in the other -- and the code after the branch + * reads {@code x} without knowing which allocation actually ran. Before JDK 21, C2 could not + * scalar-replace either allocation once they were merged like this, even if each individually would + * have qualified on its own; {@code ReduceAllocationMerges} is what lets it do so starting at JDK + * 21, which is why a few rows below only drop to 0 starting at JDK 21/25 rather than on every JDK. + * All of this is specific to HotSpot's C2 JIT; none of it has been checked against OpenJ9 or + * GraalVM, which use different compilers with different heuristics and may not scalar-replace the + * same shapes. + * + *

    Every arm consumes the object's fields rather than the object. Handing the reference + * to a {@link Blackhole} would make it escape by construction and every row would read the same. + * + *

    Bytes per operation, one machine, {@code -Pjmh.forks=1}. A 16-byte object allocated on half + * the operations reads as 8. Columns are the JDK the fork ran on, which is not necessarily + * the JDK on the shell's path — take it from JMH's own {@code # VM version} line. + * + * 

    + * shape                                 JDK 8  JDK 11  JDK 17  JDK 21  JDK 25   what it isolates
+ * singleSite                                0       ?       0       ?       0   the floor
+ * flagOnOneAllocation                       0       ?       0       ?       0   outcome in a field
+ * closedInFinally                           0       ?       0       ?       0   try/finally
+ * closedInFinallyWithThrow                  0       ?       0       ?       ?   ... with the handler taken
+ * flagOnOneAllocationClosedInFinally        0       ?       0       ?       0   flag field, whole
+ * passedToInlinedStrategy                   0       ?       0       ?       0   @Strategy boundary
+ * backingMonomorphic                        0       ?       0       ?       0   one backing
+ * backingBimorphic                          0       ?       0       ?       0   two backings
+ * mergeWithNull                             8       ?       8       ?       0   merge with null
+ * mergeWithStatic                           8       ?       8       ?       8   merge with a singleton
+ * mergeWithStaticClosedInFinally            8       ?       8       ?       8   ... the same, whole
+ * mergeOfTwoAllocations                    16       ?      16       ?      16   merge of two allocations
+ * passedToUninlinedStrategy                24       ?      24       ?      24   the same boundary, uninlined
+ * backingMegamorphic                       24       ?      24       ?      24   three backings
+ *
    + * + *

    JDK 8 column measured 2026-08-27 (Zulu 8.72.0.17, this machine, {@code -Pjmh.fork=1}): every + * arm lands on the same B/op as the 17/25 columns it was checked against, including {@code + * mergeWithNull} staying at 8 rather than following JDK 25's drop to 0 — the {@code + * ReduceAllocationMerges} relaxation is JDK 21+ only, so 8's floor for this shape is the older, + * unconditional one. + * + *

    What the two measured columns say so far: + * + *

      + *
    • try/finally is free, including with the handler taken often enough to be compiled rather + * than left as an uncommon trap. It was the suspected culprit and it is not one. Note the + * catch is in the same method, so C2 can reduce the throw to control flow; this does not + * exercise an unwind through frames. + *
    • A merge with a static allocates on every JDK measured, JDK 25 included. The JDK 21 + * allocation-merge work shows up only in the {@code mergeWithNull} row, which goes 8 to 0; a + * merge of two live allocations still allocates at 25, because the merged reference is called + * through rather than only read from. + *
    • Moving the outcome into a field of a single allocation costs nothing, with or without the + * {@code finally}. That is the whole fix. + *
    • Inlining is the gate, and the strategy discipline is what holds it open: the same object + * through the same call boundary is 0 when the callee inlines and 24 when it does not. + *
    • Two backings behind a template method are free; three are not. The inheritance layout is + * not costing anything today, and would cost 24 bytes an operation the day a third arrives. + *
    + */ +@Fork( + value = 2, + jvmArgsAppend = { + "-XX:CompileCommand=dontinline,datadog.trace.util.escape.EscapeShapeBenchmark$UninlinedStrategy::apply" + }) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Thread) +public class EscapeShapeBenchmark { + + /** + * Minimal two-method interface -- a value to read and a close to call -- standing in for any + * short-lived object more complex than a single field. + */ + interface Outcome { + int value(); + + void close(); + } + + static final class SingleAllocation implements Outcome { + private final int seed; + + SingleAllocation(int seed) { + this.seed = seed; + } + + @Override + public int value() { + return seed + 1; + } + + @Override + public void close() {} + } + + /** A second allocation site, for the merge that C2 has some chance with. */ + static final class AlternateAllocation implements Outcome { + private final int seed; + + AlternateAllocation(int seed) { + this.seed = seed; + } + + @Override + public int value() { + return seed + 2; + } + + @Override + public void close() {} + } + + /** The absent outcome, reachable from a static, so the merge it takes part in is not local. */ + static final Outcome STATIC_SINGLETON = + new Outcome() { + @Override + public int value() { + return 0; + } + + @Override + public void close() {} + }; + + /** One allocation site carrying the outcome in a field: the shape that survives. */ + static final class FlaggedAllocation { + private final boolean present; + private final int seed; + + FlaggedAllocation(boolean present, int seed) { + this.present = present; + this.seed = seed; + } + + int value() { + return present ? seed + 1 : 0; + } + + void close() {} + } + + /** + * A non-capturing strategy held in a static final field of concrete type, as {@code @Strategy} + * requires. + */ + interface OutcomeStrategy { + int apply(FlaggedAllocation cell); + } + + static final OutcomeStrategy INLINED = FlaggedAllocation::value; + + /** + * Kept out of line by the {@code CompileCommand} in {@link Fork}, not by {@link CompilerControl}: + * JMH's processor only collects that annotation from {@code @Benchmark} methods, so putting it + * here emits no hint at all and the arm silently becomes a duplicate of the inlined one. Check + * the timing against {@code passedToInlinedStrategy} before believing this row — a call that + * really did not inline cannot cost the same as no call. + */ + static final class UninlinedStrategy implements OutcomeStrategy { + @Override + public int apply(FlaggedAllocation cell) { + return cell.value(); + } + } + + static final OutcomeStrategy UNINLINED = new UninlinedStrategy(); + + /** + * The template-method shape: a final method on a base type calling out to an abstract one, with + * the object under test riding along as the argument. How many concrete subclasses are loaded is + * the whole experiment — C2 inlines a monomorphic call outright and a bimorphic one behind a type + * guard, but gives up at three, and a call it does not inline turns its argument into an escape. + */ + abstract static class Backing { + final int admit(FlaggedAllocation cell) { + return store(cell); + } + + abstract int store(FlaggedAllocation cell); + } + + static final class ArrayBacking extends Backing { + @Override + int store(FlaggedAllocation cell) { + return cell.value(); + } + } + + static final class LinkedBacking extends Backing { + @Override + int store(FlaggedAllocation cell) { + return cell.value() + 1; + } + } + + static final class ThirdBacking extends Backing { + @Override + int store(FlaggedAllocation cell) { + return cell.value() + 2; + } + } + + // All three the same length, so the index arithmetic and the bounds check are identical and the + // only difference between the arms is how many types reach the call site. + // + // Unexplained: the monomorphic arm times slower than the bimorphic one (2.14 against 1.26 ns on + // 17), and equalising the lengths did not change it, so it is not the index arithmetic. Both + // eliminate their allocation, which is what this matrix is for, so the timing oddity does not + // touch any conclusion drawn here — but do not quote these two timings against each other until + // someone has read the assembly. + private final Backing[] one = {new ArrayBacking(), new ArrayBacking(), new ArrayBacking()}; + private final Backing[] two = {new ArrayBacking(), new LinkedBacking(), new ArrayBacking()}; + private final Backing[] three = {new ArrayBacking(), new LinkedBacking(), new ThirdBacking()}; + + // The three arms below are deliberately copy-pasted rather than sharing a helper. A shared helper + // would carry one profile for all three call sites, so the megamorphic arm would poison the other + // two and the matrix would report the same answer three times. + + @Benchmark + public void backingMonomorphic(Blackhole bh) { + Backing backing = one[(counter++ & 0x7fffffff) % one.length]; + FlaggedAllocation cell = new FlaggedAllocation(true, counter); + bh.consume(backing.admit(cell)); + } + + @Benchmark + public void backingBimorphic(Blackhole bh) { + Backing backing = two[(counter++ & 0x7fffffff) % two.length]; + FlaggedAllocation cell = new FlaggedAllocation(true, counter); + bh.consume(backing.admit(cell)); + } + + @Benchmark + public void backingMegamorphic(Blackhole bh) { + Backing backing = three[(counter++ & 0x7fffffff) % three.length]; + FlaggedAllocation cell = new FlaggedAllocation(true, counter); + bh.consume(backing.admit(cell)); + } + + /** + * Alternates so both sides of every branch are taken and the profile is honest. A branch C2 never + * sees taken becomes an uncommon trap, which would quietly turn the merge arms into single-site + * arms and make the whole matrix a lie. + */ + private int counter; + + private boolean alternate() { + return (counter++ & 1) == 0; + } + + @Benchmark + public void singleSite(Blackhole bh) { + SingleAllocation cell = new SingleAllocation(counter++); + bh.consume(cell.value()); + } + + @Benchmark + public void mergeOfTwoAllocations(Blackhole bh) { + Outcome cell = alternate() ? new SingleAllocation(counter) : new AlternateAllocation(counter); + bh.consume(cell.value()); + } + + @Benchmark + public void mergeWithStatic(Blackhole bh) { + Outcome cell = alternate() ? new SingleAllocation(counter) : STATIC_SINGLETON; + bh.consume(cell.value()); + } + + @Benchmark + public void mergeWithNull(Blackhole bh) { + SingleAllocation cell = alternate() ? new SingleAllocation(counter) : null; + bh.consume(cell == null ? 0 : cell.value()); + } + + @Benchmark + public void flagOnOneAllocation(Blackhole bh) { + FlaggedAllocation cell = new FlaggedAllocation(alternate(), counter); + bh.consume(cell.value()); + } + + @Benchmark + public void closedInFinally(Blackhole bh) { + SingleAllocation cell = new SingleAllocation(counter++); + try { + bh.consume(cell.value()); + } finally { + cell.close(); + } + } + + /** Preallocated and stackless, so the arm measures control flow rather than fillInStackTrace. */ + static final class Failure extends RuntimeException { + static final Failure INSTANCE = new Failure(); + + private Failure() { + super("failure", null, false, false); + } + } + + /** + * The same try/finally, with the handler actually taken often enough to be compiled rather than + * left as an uncommon trap. This is the case {@link #closedInFinally} does not cover: there, C2 + * has never seen the exception path, so there is no code for the object to be live into. + */ + @Benchmark + public void closedInFinallyWithThrow(Blackhole bh) { + SingleAllocation cell = new SingleAllocation(counter++); + try { + if ((counter & 15) == 0) { + throw Failure.INSTANCE; + } + bh.consume(cell.value()); + } catch (Failure failure) { + bh.consume(cell.value() + 1); + } finally { + cell.close(); + } + } + + /** The Optional-style shape, whole: a singleton for one outcome, under try/finally. */ + @Benchmark + public void mergeWithStaticClosedInFinally(Blackhole bh) { + Outcome cell = alternate() ? new SingleAllocation(counter) : STATIC_SINGLETON; + try { + bh.consume(cell.value()); + } finally { + cell.close(); + } + } + + /** The single-site shape, whole: one allocation carrying a flag, under try/finally. */ + @Benchmark + public void flagOnOneAllocationClosedInFinally(Blackhole bh) { + FlaggedAllocation cell = new FlaggedAllocation(alternate(), counter); + try { + bh.consume(cell.value()); + } finally { + cell.close(); + } + } + + /** + * A non-escaping object handed across a call boundary the strategy discipline keeps inlinable. + */ + @Benchmark + public void passedToInlinedStrategy(Blackhole bh) { + FlaggedAllocation cell = new FlaggedAllocation(alternate(), counter); + bh.consume(INLINED.apply(cell)); + } + + /** + * The same, with only the inlining taken away. Whatever this costs is what the discipline buys. + */ + @Benchmark + public void passedToUninlinedStrategy(Blackhole bh) { + FlaggedAllocation cell = new FlaggedAllocation(alternate(), counter); + bh.consume(UNINLINED.apply(cell)); + } +} 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 4dc6bf5a2ec..6b78c1c4539 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -23,9 +23,9 @@ *

    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 + * #tryGetOrCreate}, 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. @@ -35,21 +35,41 @@ * 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 + * own: {@link #get} / {@link #tryGetOrCreate} / {@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. * + *

    Choosing between the three tables

    + * + *
      + *
    1. Concurrent access? Use {@code ConcurrentHashtable} -- the only thread-safe one of + * the three. This class is racy by design (see above), and {@code Hashtable} is not + * thread-safe at all. + *
    2. Otherwise: does the population reset wholesale, or evolve? A table cleared as a unit + * -- once per cycle, per request, or built and then discarded -- wants this class, whose open + * addressing has no tombstones and so offers no removal beyond clearing. A table whose + * entries come and go independently wants the chained {@code Hashtable}, which removes and + * evicts in place. + *
    + * + *

    Lifetime is the usual shorthand for that second question and mostly works, because a + * short-lived table never needs to remove -- it just dies. The case it mis-sorts is a long-lived + * table that resets on a cycle: that is a sequence of short lives, and belongs with the short-lived + * ones. Compare a table that evicts stale entries one at a time while the busy ones survive the + * cycle (evolving -- {@code Hashtable}) against one that clears every entry each time it reports + * (resets -- this class). + * *

    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 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}. + * against a stored entry. Used by {@link #get} / {@link #tryGetOrCreate}. *
    • 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 @@ -63,7 +83,7 @@ * 
      {@code
  * private static final MyStrategy S = new MyStrategy();          // concrete type => exact type pinned
  * ...
- * E e = FlatHashtable.getOrCreate(table, key, S, MyEntry::new);  // non-capturing create
+ * E e = FlatHashtable.tryGetOrCreate(table, key, S, MyEntry::new);  // non-capturing create
  * }
      * *

      Contract: {@code table.length} must be a power of two ({@link #capacityFor}). Both @@ -73,7 +93,7 @@ * 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). + * #tryGetOrCreate} (so hits stay a single probe and the create path is warmup-rare). */ public final class FlatHashtable { private FlatHashtable() {} @@ -96,20 +116,21 @@ 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 #get} / {@link #tryGetOrCreate} / {@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. + * bounded posture — the table holds up to {@code maxCapacity} entries, then {@link + * #tryGetOrCreateOrNull} 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 tryGetOrCreateOrNull} 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, matching the chained {@code + * Hashtable.D1.createCapped}. Across the family a table factory's number is always entries — only + * the low-level array allocators take a bucket count. * *

      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 @@ -176,7 +197,7 @@ 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}). + * #DEFAULT_LOAD_FACTOR}, then capping ({@link #tryGetOrCreateOrNull} returns {@code null}). */ @Nonnull public static > D1 createFixed( @@ -236,12 +257,33 @@ public TEntry get(@Nullable K key) { /** * 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. + * one, wrapped in a {@link Maybe}. A growable table's {@link Maybe} is always present; a fixed + * one's is absent 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. + * + *

      The {@code try} prefix marks "this may refuse" — a growable table simply never exercises + * it. + * + *

      Delegates to {@link #tryGetOrCreateOrNull} as the sole {@link Maybe#of} call site -- see + * {@link Maybe}'s class javadoc for why that shape is required to stay allocation-free. + */ + @Nonnull + public Maybe tryGetOrCreate( + @Nullable K key, @Nonnull CreateStrategy createStrat) { + return Maybe.of(tryGetOrCreateOrNull(key, createStrat)); + } + + /** + * Low-level, {@code null}-returning form of {@link #tryGetOrCreate}. Prefer the {@link Maybe} + * form above for new call sites; this one remains as an escape hatch for callers where the + * {@link Maybe} allocation-free contract doesn't fit or that pre-date it. Under-promising + * refusal here costs an NPE at the cap; over-promising it costs a redundant null check on a + * growable table -- so it errs toward {@code try}. */ @Nullable - public TEntry getOrCreate(@Nullable K key, @Nonnull CreateStrategy createStrat) { + public TEntry tryGetOrCreateOrNull( + @Nullable K key, @Nonnull CreateStrategy createStrat) { final TEntry existing = get(key); if (existing != null) { return existing; @@ -300,7 +342,7 @@ public void forEach(C context, @Nonnull BiConsumer}. Same fixed-or-growable ({@link #createFixed} / {@link #createGrowable}), * entry-centric, no-{@code remove}, not-thread-safe contract as {@link D1}. * @@ -366,7 +408,7 @@ 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}). + * #DEFAULT_LOAD_FACTOR}, then capping ({@link #tryGetOrCreateOrNull} returns {@code null}). */ @Nonnull public static > D2 createFixed( @@ -425,11 +467,25 @@ public TEntry get(@Nullable K1 key1, @Nullable 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. + * Two-key analogue of {@link D1#tryGetOrCreate}: {@link Maybe}-wrapped form, delegating to + * {@link #tryGetOrCreateOrNull} as the sole {@link Maybe#of} call site. Growable's {@link + * Maybe} is always present; fixed's is absent when full and {@code (key1, key2)} is absent. + */ + @Nonnull + public Maybe tryGetOrCreate( + @Nullable K1 key1, + @Nullable K2 key2, + @Nonnull CreateStrategy2 createStrat) { + return Maybe.of(tryGetOrCreateOrNull(key1, key2, createStrat)); + } + + /** + * Two-key analogue of {@link D1#tryGetOrCreateOrNull}: low-level, {@code null}-returning form + * of {@link #tryGetOrCreate}. Growable never returns {@code null}; fixed returns {@code null} + * when full and {@code (key1, key2)} is absent. */ @Nullable - public TEntry getOrCreate( + public TEntry tryGetOrCreateOrNull( @Nullable K1 key1, @Nullable K2 key2, @Nonnull CreateStrategy2 createStrat) { @@ -486,7 +542,7 @@ public void forEach(C context, @Nonnull BiConsumernon-capturing lambda (e.g. {@code MyEntry::new}) so it stays a single monomorphic, * allocation-free instance. @@ -523,7 +579,7 @@ public interface HashStrategy { * #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}. + * Used by {@link #get} / {@link #tryGetOrCreate}. * *

      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 @@ -704,7 +760,7 @@ public static E get( */ @StrategyConsumer @Nullable - public static E getOrCreate( + public static E tryGetOrCreate( @Nonnull E[] table, K key, @Nonnull MatchingStrategy matchStrat, diff --git a/internal-api/src/main/java/datadog/trace/util/Hashtable.java b/internal-api/src/main/java/datadog/trace/util/Hashtable.java index a2fbfc62ad1..81e5daa7a0f 100644 --- a/internal-api/src/main/java/datadog/trace/util/Hashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/Hashtable.java @@ -1,5 +1,6 @@ package datadog.trace.util; +import java.lang.reflect.Array; import java.util.Arrays; import java.util.Iterator; import java.util.NoSuchElementException; @@ -8,6 +9,10 @@ import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.ObjLongConsumer; +import java.util.function.Predicate; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Light weight simple Hashtable system that can be useful when HashMap would be unnecessarily @@ -23,10 +28,33 @@ * Convenience classes are provided for lower key dimensions. * *

      For higher key dimensions, client code must implement its own class, but can still use the - * support class to ease the implementation complexity. + * static building blocks on this class to ease the implementation complexity. + * + *

      Choosing between the three tables

      + * + *
        + *
      1. Concurrent access? Use {@code ConcurrentHashtable} -- the only thread-safe one of + * the three. {@code FlatHashtable} is racy by design, and this class is not thread-safe at + * all. + *
      2. Otherwise: does the population reset wholesale, or evolve? A table cleared as a unit + * -- once per cycle, per request, or built and then discarded -- wants {@code FlatHashtable}, + * whose open addressing has no tombstones and so offers no removal beyond clearing. A table + * whose entries come and go independently wants this one, where chaining removes and evicts + * in place. + *
      + * + *

      Lifetime is the usual shorthand for that second question and mostly works, because a + * short-lived table never needs to remove -- it just dies. The case it mis-sorts is a long-lived + * table that resets on a cycle: that is a sequence of short lives, and belongs with the short-lived + * ones. Compare a table that evicts stale entries one at a time while the busy ones survive the + * cycle (evolving -- this class) against one that clears every entry each time it reports (resets + * -- {@code FlatHashtable}). * *

      This outer class is a pure namespace -- it can't be instantiated. The actual table types are - * {@link D1}, {@link D2}, and (for higher-arity callers) {@link Support}-driven custom tables. + * {@link D1}, {@link D2}, and (for higher-arity callers) custom tables driven by the static + * building blocks on this class (see {@link #create(Class, int)}, {@link + * #bucketFor(Hashtable.Entry[], long)}, {@link #insertHeadEntryAt(Hashtable.Entry[], int, + * Hashtable.Entry)}, and friends). */ public final class Hashtable { private Hashtable() {} @@ -37,7 +65,8 @@ private Hashtable() {} * *

      Subclasses add the actual key field(s) and a {@code matches(...)} method tailored to their * key arity. See {@link D1.Entry} and {@link D2.Entry}; for higher arities, client code can - * subclass this directly and use {@link Support} to drive the table mechanics. + * subclass this directly and drive the table with the static building blocks on {@link + * Hashtable}. */ public abstract static class Entry { public final long keyHash; @@ -47,11 +76,12 @@ protected Entry(long keyHash) { this.keyHash = keyHash; } - public final void setNext(TEntry next) { + public final void setNext(@Nullable TEntry next) { this.next = next; } @SuppressWarnings("unchecked") + @Nullable public final TEntry next() { return (TEntry) this.next; } @@ -68,8 +98,14 @@ public final TEntry next() { * Long>} and produces effectively zero GC pressure. * *

      Capacity is fixed at construction. The table does not resize, so the caller is responsible - * for choosing a capacity appropriate to the working set. Actual bucket-array length is rounded - * up to the next power of two. + * for choosing a capacity appropriate to the working set. Once {@link #size()} reaches that + * capacity, {@link #insert} returns {@code false} and {@link #tryGetOrCreateOrNull} returns + * {@code null} rather than adding more entries -- a lookup hit is still always returned even at + * capacity, the cap only blocks new entries. Want your own eviction policy instead of a hard cap? + * Drop down to the static building blocks and drive the bucket array yourself -- {@link + * Hashtable#createCapped(int)} hands you a spine and a {@link SizeManager} already matched to + * each other, and the manager evicts as well as counts. Actual bucket-array length is rounded up + * to the next power of two. * *

      Null keys are permitted; they collapse to a single bucket via the sentinel hash {@link * Long#MIN_VALUE} defined in {@link D1.Entry#hash}. @@ -94,17 +130,18 @@ public static final class D1> { public abstract static class Entry extends Hashtable.Entry { final K key; - protected Entry(K key) { + protected Entry(@Nullable K key) { super(hash(key)); this.key = key; } /** The key this entry was created with. */ + @Nullable public K key() { return this.key; } - public boolean matches(Object key) { + public boolean matches(@Nullable Object key) { return Objects.equals(this.key, key); } @@ -116,105 +153,284 @@ public boolean matches(Object key) { * [Integer.MIN_VALUE, Integer.MAX_VALUE]}; real-key collisions in chains 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(); } } - // Package-private so iterator tests in the same package can drive Support.bucketIterator and - // friends directly against the table's bucket array. + // Package-private so iterator tests in the same package can drive the Hashtable static + // building blocks directly against the table's bucket array. final Hashtable.Entry[] buckets; - private int size; + private final SizeManager sizeManager; - public D1(int capacity) { - this.buckets = Support.create(capacity); - this.size = 0; + private D1(int maxCapacity) { + // Bucket array gets load-factor headroom over the strict entry cap below, so chains stay + // short even when the table is full; see Hashtable#capacityFor. + this.buckets = Hashtable.create(capacityFor(maxCapacity)); + this.sizeManager = new SizeManager(maxCapacity); } + /** + * A capped single-key table: it holds at most {@code maxCapacity} live entries, after + * which {@link #insert} returns {@code false} and {@link #tryGetOrCreateOrNull} returns {@code + * null}. A lookup hit is still always returned at capacity -- the cap only blocks new entries. + * + *

      "Capped" names the promise, not the mechanism: the bucket array is sized once from {@code + * maxCapacity} via {@link Hashtable#capacityFor(int)} and never resized, but that is an + * implementation detail. What the caller is choosing here is a bounded entry count and, with + * it, a bounded footprint -- the posture an agent living in someone else's heap wants by + * default. Callers that need overflow to be absorbed rather than refused should pair a {@link + * SizeManager}'s eviction half over the static building blocks (see {@link + * Hashtable#createCapped(int)}) rather than reaching for an uncapped table. + * + *

      Pick {@code maxCapacity} in the right ballpark of what you actually expect to hold + * -- the bucket array is sized from it, so it is read as both the limit and a rough estimate. + * Nothing assumes you will reach the cap, but a cap set as a paranoid safety valve far above + * typical usage over-allocates the spine for a fill that never arrives. When the limit and the + * expectation genuinely differ by a lot, size the two independently with the low-level API: + * {@code Hashtable.create(capacityFor(expected))} paired with {@code new SizeManager(limit)}. + * + *

      {@code entryClass} is a type token only -- it pins the concrete entry type so the compiler + * infers both {@code K} and {@code TEntry} at the call site (e.g. {@code + * D1.createCapped(MyEntry.class, 64)}), keeping the factory symmetric with the rest of the + * collections family. Unlike {@link Hashtable#create(Class, int)} it is not reflectively + * allocated: {@code buckets} stays a plain {@code Hashtable.Entry[]} internally, matching the + * static building blocks ({@link Hashtable#bucketFor}, {@link Hashtable#insertHeadEntryFor}, + * etc.) that {@link #get}, {@link #insert}, and friends delegate to. + */ + @Nonnull + public static > D1 createCapped( + @Nonnull Class entryClass, int maxCapacity) { + return new D1<>(maxCapacity); + } + + /** + * Live entry count. Exact here, unlike {@link SizeManager#estimateSize()}: this class reserves + * and links within a single call, so a caller can never observe the reservation window. + */ public int size() { - return this.size; + return this.sizeManager.estimateSize(); + } + + /** {@code true} once {@link #size()} has reached this table's fixed capacity. */ + public boolean isFull() { + return this.sizeManager.isFull(); } - public TEntry get(K key) { + @Nullable + public TEntry get(@Nullable K key) { long keyHash = D1.Entry.hash(key); - for (TEntry te = Support.bucket(this.buckets, keyHash); te != null; te = te.next()) { - if (te.keyHash == keyHash && te.matches(key)) { - return te; + for (TEntry curEntry = bucketFor(this.buckets, keyHash); + curEntry != null; + curEntry = curEntry.next()) { + if (curEntry.keyHash == keyHash && curEntry.matches(key)) { + return curEntry; } } return null; } - public TEntry remove(K key) { + @Nullable + public TEntry remove(@Nullable K key) { + // Walks the chain directly rather than delegating to Hashtable#removeMatching: a + // `e -> e.matches(key)` predicate captures `key`, so it allocates a fresh Predicate on every + // call. This class ships context-passing forEach/drain overloads precisely so callers can + // avoid capturing lambdas -- the write paths follow the same discipline. Same loop shape as + // tryInsertOrReplace below. long keyHash = D1.Entry.hash(key); - - for (MutatingBucketIterator iter = - Support.mutatingBucketIterator(this.buckets, keyHash); + for (MutatingBucketIterator iter = mutatingBucketIterator(this.buckets, keyHash); iter.hasNext(); ) { TEntry curEntry = iter.next(); - if (curEntry.matches(key)) { iter.remove(); - this.size -= 1; + this.sizeManager.decrement(); return curEntry; } } - return null; } - public void insert(TEntry newEntry) { - Support.insertHeadEntry(this.buckets, newEntry.keyHash, newEntry); - this.size += 1; + /** + * Unconditionally adds {@code newEntry} ({@code true}), or {@code false} if the table is + * already at capacity. Caller-responsible: {@code newEntry}'s key must be absent, else it lands + * shadowed behind the existing entry. + */ + public boolean insert(@Nonnull TEntry newEntry) { + return insertHeadEntryFor(this.sizeManager, this.buckets, newEntry.keyHash, newEntry); } - public TEntry insertOrReplace(TEntry newEntry) { + /** + * Makes {@code newEntry} the entry for its key: replaces the existing entry for that key if one + * is present, otherwise inserts it fresh. Returns {@code false} only when the key is absent + * and the table is at capacity -- a replacement swaps one entry for another without + * growing {@link #size()}, so it always succeeds, even on a full table. + * + *

      Does not hand back the entry it displaced. Callers that need it can {@link #get} first; + * that is rare enough (the same way {@code Map.put}'s return value is rarely read) not to be + * worth the cost of the alternative, which was throwing {@link IllegalStateException} on + * refusal because {@code null} was already spoken for by "inserted fresh". Refusal at a cap is + * ordinary steady-state behaviour, not a programming error, and an exception would allocate a + * throwable plus stack trace exactly when the table is under the most pressure. + * + *

      Note this swaps the entry object. Where the goal is to change values on an entry + * that may or may not exist yet, prefer looking it up once and mutating in place -- that is the + * allocation-free path this class exists for. + */ + public boolean tryInsertOrReplace(@Nonnull TEntry newEntry) { for (MutatingBucketIterator iter = - Support.mutatingBucketIterator(this.buckets, newEntry.keyHash); + mutatingBucketIterator(this.buckets, newEntry.keyHash); iter.hasNext(); ) { TEntry curEntry = iter.next(); if (curEntry.matches(newEntry.key)) { iter.replace(newEntry); - return curEntry; + return true; } } - Support.insertHeadEntry(this.buckets, newEntry.keyHash, newEntry); - this.size += 1; - return null; + return insertHeadEntryFor(this.sizeManager, this.buckets, newEntry.keyHash, newEntry); } /** - * Returns the entry for {@code key}, building one via {@code creator} if absent. Computes the - * hash once and reuses it for both the lookup and (on miss) the insert -- avoids the - * double-hash that "{@code get}; if null then {@code insert}" would incur. + * Returns the entry for {@code key}, building one via {@code creator} if absent -- wrapped in a + * {@link Maybe} that is absent if the key is absent and the table is at capacity. A + * lookup hit is always returned even at capacity, so only the create half can fail. Check + * {@link #isFull()} beforehand if you want to distinguish "refused" from "created" without + * inspecting the result. + * + *

      Refusal is a designed steady state for a capped table, not an exceptional condition -- see + * {@link #createCapped}. Decide deliberately what a refused create should do (drop the sample, + * fall back, make room); silently ignoring an absent {@link Maybe} turns the cap into data loss + * you cannot see. + * + *

      Computes the hash once and reuses it for both the lookup and (on miss) the insert -- + * avoids the double-hash that "{@code get}; if null then {@code insert}" would incur. * *

      The {@code creator} is expected to build an entry whose {@code keyHash} equals {@link * Entry#hash(Object) D1.Entry.hash(key)} -- typically by passing {@code key} to a constructor * that calls {@code super(key)}. A mismatched hash will leave the new entry inserted at a * bucket that future {@link #get} calls won't probe. + * + *

      Exactly one {@link Maybe#of} call site, fed by delegating to {@link #tryGetOrCreateOrNull} + * -- see {@link Maybe}'s class javadoc for why that shape is required to stay allocation-free. + * Use {@link #tryGetOrCreateOrNull} directly only when a manual null check is genuinely more + * convenient than {@link Maybe#update}/{@link Maybe#getOrNull}. */ - public TEntry getOrCreate(K key, Function creator) { + @Nonnull + public Maybe tryGetOrCreate( + @Nullable K key, @Nonnull Function creator) { + return Maybe.of(tryGetOrCreateOrNull(key, creator)); + } + + /** + * Low-level, {@code null}-returning form of {@link #tryGetOrCreate}. Prefer the {@link Maybe} + * form above for new call sites; this one remains as an escape hatch for callers where the + * {@link Maybe} allocation-free contract doesn't fit (e.g. storing the result past the current + * stack frame) or that pre-date it. + */ + @Nullable + public TEntry tryGetOrCreateOrNull( + @Nullable K key, @Nonnull Function creator) { long keyHash = D1.Entry.hash(key); - for (TEntry te = Support.bucket(this.buckets, keyHash); te != null; te = te.next()) { - if (te.keyHash == keyHash && te.matches(key)) { - return te; + for (TEntry curEntry = bucketFor(this.buckets, keyHash); + curEntry != null; + curEntry = curEntry.next()) { + if (curEntry.keyHash == keyHash && curEntry.matches(key)) { + return curEntry; } } + // Deliberately isFull() -> create -> increment, rather than the one-call tracked + // insertHeadEntryFor(sizeManager, ...) that insert/tryInsertOrReplace use: `creator` runs + // between the check and the link and may throw, so a slot reserved up front could leak. See + // SizeManager#tryReserve. + if (this.sizeManager.isFull()) { + return null; + } TEntry newEntry = creator.apply(key); - Support.insertHeadEntry(this.buckets, newEntry.keyHash, newEntry); - this.size += 1; + insertHeadEntryFor(this.buckets, newEntry.keyHash, newEntry); + this.sizeManager.increment(); return newEntry; } - public void clear() { - Support.clear(this.buckets); - this.size = 0; + /** + * {@link #tryGetOrCreateOrNull} followed by {@code updater}, returning whether the update + * happened. + * + *

      Prefer this over the two-call form for the common read-modify-write shape -- a counter + * bump, a max, a timestamp refresh: + * + * 

      {@code
+     * table.tryGetOrUpdate(key, Counter::new, Counter::inc);
+     * }
      + * + *

      The two-call form leaves a {@code null} on the caller's happy path, and the {@code null} + * only ever appears once the table is at capacity -- so {@code + * tryGetOrCreateOrNull(...).inc()} reads fine, tests fine, and throws in production under + * cardinality pressure. Fusing the update keeps that reference inside the table: at capacity + * the update is skipped and {@code false} is returned, which a counter caller can safely ignore + * or check deliberately. + * + *

      No extra work versus doing it by hand -- the hash is still computed once, by the delegated + * {@link #tryGetOrCreateOrNull}. + */ + public boolean tryGetOrUpdate( + @Nullable K key, + @Nonnull Function creator, + @Nonnull Consumer updater) { + TEntry entry = tryGetOrCreateOrNull(key, creator); + if (entry == null) { + return false; + } + updater.accept(entry); + return true; } - public void forEach(Consumer consumer) { - Support.forEach(this.buckets, consumer); + /** + * Context-passing {@link #tryGetOrUpdate}, for updates that need a value the entry doesn't + * carry. {@code c -> c.add(n)} captures {@code n} and allocates a lambda per call; passing + * {@code n} as {@code context} against a non-capturing {@link BiConsumer} (typically a {@code + * static final}) does not. + */ + public boolean tryGetOrUpdate( + @Nullable K key, + @Nonnull Function creator, + C context, + @Nonnull BiConsumer updater) { + TEntry entry = tryGetOrCreateOrNull(key, creator); + if (entry == null) { + return false; + } + updater.accept(context, entry); + return true; + } + + /** + * Primitive-{@code long} {@link #tryGetOrUpdate}, for the accumulate-a-count shape: + * + * 

      {@code
+     * private static final ObjLongConsumer ADD = (c, n) -> c.count += n;
+     * table.tryGetOrUpdate(key, Counter::new, n, ADD);
+     * }
      + * + *

      The generic context overload would box {@code n} on every call; this one does not. Note + * the argument order is {@code (entry, value)} -- {@link ObjLongConsumer}'s, not the {@code + * (context, entry)} of the {@link BiConsumer} overload. + */ + public boolean tryGetOrUpdate( + @Nullable K key, + @Nonnull Function creator, + long context, + @Nonnull ObjLongConsumer updater) { + TEntry entry = tryGetOrCreateOrNull(key, creator); + if (entry == null) { + return false; + } + updater.accept(entry, context); + return true; + } + + public void forEach(@Nonnull Consumer consumer) { + Hashtable.forEach(this.buckets, consumer); } /** @@ -222,8 +438,30 @@ public void forEach(Consumer consumer) { * -- pass a non-capturing {@link BiConsumer} (typically a {@code static final}) plus whatever * side-band state it needs as {@code context}. */ - public void forEach(T context, BiConsumer consumer) { - Support.forEach(this.buckets, context, consumer); + public void forEach(C context, @Nonnull BiConsumer consumer) { + Hashtable.forEach(this.buckets, context, consumer); + } + + public void clear() { + Hashtable.clear(this.sizeManager, this.buckets); + } + + /** + * Removes every entry, passing each to {@code sink} as it is unlinked -- the read-and-reset + * primitive for flush/publish workflows (drain the table into a telemetry batch, an event + * emitter, etc.). Equivalent to {@link #forEach} then {@link #clear} in a single call. + */ + public void drain(@Nonnull Consumer sink) { + Hashtable.drain(this.sizeManager, this.buckets, sink); + } + + /** + * Context-passing {@link #drain(Consumer)}. Pass a non-capturing {@link BiConsumer} (typically + * a {@code static final}) plus the accumulator as {@code context} to avoid a capturing-lambda + * allocation. + */ + public void drain(C context, @Nonnull BiConsumer sink) { + Hashtable.drain(this.sizeManager, this.buckets, context, sink); } } @@ -233,12 +471,12 @@ public void forEach(T context, BiConsumer consume *

      The user supplies a {@link D2.Entry} subclass carrying both key parts and any value fields. * Compared to {@code HashMap} this avoids the per-lookup {@code Pair} (or record) * allocation: both key parts are passed directly through {@link #get}, {@link #remove}, {@link - * #insert}, and {@link #insertOrReplace}. Combined with in-place value mutation, this makes + * #insert}, and {@link #tryInsertOrReplace}. Combined with in-place value mutation, this makes * {@code D2} substantially less GC-intensive than the equivalent {@code HashMap} for * counter-style workloads. * - *

      Capacity is fixed at construction; the table does not resize. Actual bucket-array length is - * rounded up to the next power of two. + *

      Capacity is fixed at construction; the table does not resize. Same strict-cap semantics as + * {@link D1} once {@link #size()} reaches capacity. * *

      Key parts are combined into a 64-bit hash via {@link LongHashingUtils}; see {@link * D2.Entry#hash(Object, Object)}. @@ -264,23 +502,25 @@ public abstract static class Entry extends Hashtable.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; } /** The first key part this entry was created with. */ + @Nullable public K1 key1() { return this.key1; } /** The second key part this entry was created with. */ + @Nullable public K2 key2() { return this.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); } @@ -291,100 +531,198 @@ public boolean matches(K1 key1, K2 key2) { * combinations whose chained hash equals {@code hash(0, 0) = 0} or similar values. {@link * #matches(Object, Object)} resolves any such collision. */ - public static long hash(Object key1, Object key2) { + public static long hash(@Nullable Object key1, @Nullable Object key2) { return LongHashingUtils.hash(key1, key2); } } // Package-private to match D1.buckets -- available for iterator tests in the same package. final Hashtable.Entry[] buckets; - private int size; + private final SizeManager sizeManager; - public D2(int capacity) { - this.buckets = Support.create(capacity); - this.size = 0; + private D2(int maxCapacity) { + // Bucket array gets load-factor headroom over the strict entry cap below, so chains stay + // short even when the table is full; see Hashtable#capacityFor. + this.buckets = Hashtable.create(capacityFor(maxCapacity)); + this.sizeManager = new SizeManager(maxCapacity); + } + + /** + * Composite-key analogue of {@link D1#createCapped}: a capped table holding at most + * {@code maxCapacity} live entries, after which {@link #insert} returns {@code false} and + * {@link #tryGetOrCreateOrNull} returns {@code null}, with lookup hits still always returned. + * See {@link D1#createCapped} for what "capped" promises and why it is the default posture. + * + *

      {@code entryClass} is a type token only -- it pins the concrete entry type so the compiler + * infers {@code K1}, {@code K2}, and {@code TEntry} at the call site (e.g. {@code + * D2.createCapped(MyEntry.class, 64)}). Unlike {@link Hashtable#create(Class, int)} it is not + * reflectively allocated: {@code buckets} stays a plain {@code Hashtable.Entry[]} internally, + * matching the static building blocks that {@link #get}, {@link #insert}, and friends delegate + * to. + */ + @Nonnull + public static > D2 createCapped( + @Nonnull Class entryClass, int maxCapacity) { + return new D2<>(maxCapacity); } + /** + * Live entry count. Exact here, unlike {@link SizeManager#estimateSize()}: this class reserves + * and links within a single call, so a caller can never observe the reservation window. + */ public int size() { - return this.size; + return this.sizeManager.estimateSize(); + } + + /** {@code true} once {@link #size()} has reached this table's fixed capacity. */ + public boolean isFull() { + return this.sizeManager.isFull(); } - public TEntry get(K1 key1, K2 key2) { + @Nullable + public TEntry get(@Nullable K1 key1, @Nullable K2 key2) { long keyHash = D2.Entry.hash(key1, key2); - for (TEntry te = Support.bucket(this.buckets, keyHash); te != null; te = te.next()) { - if (te.keyHash == keyHash && te.matches(key1, key2)) { - return te; + for (TEntry curEntry = bucketFor(this.buckets, keyHash); + curEntry != null; + curEntry = curEntry.next()) { + if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { + return curEntry; } } return null; } - public TEntry remove(K1 key1, K2 key2) { + @Nullable + public TEntry remove(@Nullable K1 key1, @Nullable K2 key2) { + // Chain walked directly rather than via Hashtable#removeMatching -- see D1#remove for why a + // capturing predicate is avoided on this path. long keyHash = D2.Entry.hash(key1, key2); - - for (MutatingBucketIterator iter = - Support.mutatingBucketIterator(this.buckets, keyHash); + for (MutatingBucketIterator iter = mutatingBucketIterator(this.buckets, keyHash); iter.hasNext(); ) { TEntry curEntry = iter.next(); - if (curEntry.matches(key1, key2)) { iter.remove(); - this.size -= 1; + this.sizeManager.decrement(); return curEntry; } } - return null; } - public void insert(TEntry newEntry) { - Support.insertHeadEntry(this.buckets, newEntry.keyHash, newEntry); - this.size += 1; + /** Two-key analogue of {@link D1#insert}, with the same strict-cap refusal contract. */ + public boolean insert(@Nonnull TEntry newEntry) { + return insertHeadEntryFor(this.sizeManager, this.buckets, newEntry.keyHash, newEntry); } - public TEntry insertOrReplace(TEntry newEntry) { + /** Two-key analogue of {@link D1#tryInsertOrReplace}, with the same refusal contract. */ + public boolean tryInsertOrReplace(@Nonnull TEntry newEntry) { for (MutatingBucketIterator iter = - Support.mutatingBucketIterator(this.buckets, newEntry.keyHash); + mutatingBucketIterator(this.buckets, newEntry.keyHash); iter.hasNext(); ) { TEntry curEntry = iter.next(); if (curEntry.matches(newEntry.key1, newEntry.key2)) { iter.replace(newEntry); - return curEntry; + return true; } } - Support.insertHeadEntry(this.buckets, newEntry.keyHash, newEntry); - this.size += 1; - return null; + return insertHeadEntryFor(this.sizeManager, this.buckets, newEntry.keyHash, newEntry); } /** - * Two-key analogue of {@link D1#getOrCreate}. Computes the combined hash once and reuses it for - * both lookup and (on miss) insert. The {@code creator} is expected to build an entry whose - * {@code keyHash} equals {@link Entry#hash(Object, Object) D2.Entry.hash(key1, key2)}. + * Two-key analogue of {@link D1#tryGetOrCreate}: returns the entry for {@code (key1, key2)}, + * building one via {@code creator} if absent -- wrapped in a {@link Maybe} that is absent if + * the pair is absent and the table is at capacity. Refusal is a designed steady state + * rather than an exceptional one; see {@link D1#tryGetOrCreate} for the full contract and what + * to do about a refused create. + * + *

      Computes the combined hash once and reuses it for both lookup and (on miss) insert. The + * {@code creator} is expected to build an entry whose {@code keyHash} equals {@link + * Entry#hash(Object, Object) D2.Entry.hash(key1, key2)}. + * + *

      Delegates to {@link #tryGetOrCreateOrNull} as the sole {@link Maybe#of} call site. */ - public TEntry getOrCreate( - K1 key1, K2 key2, BiFunction creator) { + @Nonnull + public Maybe tryGetOrCreate( + @Nullable K1 key1, + @Nullable K2 key2, + @Nonnull BiFunction creator) { + return Maybe.of(tryGetOrCreateOrNull(key1, key2, creator)); + } + + /** + * Two-key analogue of {@link D1#tryGetOrCreateOrNull}: low-level, {@code null}-returning form + * of {@link #tryGetOrCreate}. Prefer the {@link Maybe} form above for new call sites. + */ + @Nullable + public TEntry tryGetOrCreateOrNull( + @Nullable K1 key1, + @Nullable K2 key2, + @Nonnull BiFunction creator) { long keyHash = D2.Entry.hash(key1, key2); - for (TEntry te = Support.bucket(this.buckets, keyHash); te != null; te = te.next()) { - if (te.keyHash == keyHash && te.matches(key1, key2)) { - return te; + for (TEntry curEntry = bucketFor(this.buckets, keyHash); + curEntry != null; + curEntry = curEntry.next()) { + if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { + return curEntry; } } + // Deliberately isFull() -> create -> increment, rather than the one-call tracked + // insertHeadEntryFor(sizeManager, ...) that insert/tryInsertOrReplace use: `creator` runs + // between the check and the link and may throw, so a slot reserved up front could leak. See + // SizeManager#tryReserve. + if (this.sizeManager.isFull()) { + return null; + } TEntry newEntry = creator.apply(key1, key2); - Support.insertHeadEntry(this.buckets, newEntry.keyHash, newEntry); - this.size += 1; + insertHeadEntryFor(this.buckets, newEntry.keyHash, newEntry); + this.sizeManager.increment(); return newEntry; } - public void clear() { - Support.clear(this.buckets); - this.size = 0; + /** + * Two-key analogue of {@link D1#tryGetOrUpdate(Object, Function, Consumer)}: applies {@code + * updater} to the entry for {@code (key1, key2)}, creating one if absent, and returns whether + * the update happened. Returns {@code false} without updating when the pair is absent and the + * table is at capacity. See the single-key form for why fusing the update is preferred over + * {@code tryGetOrCreateOrNull(...)} followed by a dereference. + */ + public boolean tryGetOrUpdate( + @Nullable K1 key1, + @Nullable K2 key2, + @Nonnull BiFunction creator, + @Nonnull Consumer updater) { + TEntry entry = tryGetOrCreateOrNull(key1, key2, creator); + if (entry == null) { + return false; + } + updater.accept(entry); + return true; + } + + /** + * Context-passing {@link #tryGetOrUpdate(Object, Object, BiFunction, Consumer)}, for updates + * that need a value the entry doesn't carry. Pass a non-capturing {@link BiConsumer} (typically + * a {@code static final}) plus its side-band state as {@code context} to avoid allocating a + * capturing lambda per call. + */ + public boolean tryGetOrUpdate( + @Nullable K1 key1, + @Nullable K2 key2, + @Nonnull BiFunction creator, + C context, + @Nonnull BiConsumer updater) { + TEntry entry = tryGetOrCreateOrNull(key1, key2, creator); + if (entry == null) { + return false; + } + updater.accept(context, entry); + return true; } - public void forEach(Consumer consumer) { - Support.forEach(this.buckets, consumer); + public void forEach(@Nonnull Consumer consumer) { + Hashtable.forEach(this.buckets, consumer); } /** @@ -392,198 +730,773 @@ public void forEach(Consumer consumer) { * -- pass a non-capturing {@link BiConsumer} (typically a {@code static final}) plus whatever * side-band state it needs as {@code context}. */ - public void forEach(T context, BiConsumer consumer) { - Support.forEach(this.buckets, context, consumer); + public void forEach(C context, @Nonnull BiConsumer consumer) { + Hashtable.forEach(this.buckets, context, consumer); + } + + public void clear() { + Hashtable.clear(this.sizeManager, this.buckets); + } + + /** + * Removes every entry, passing each to {@code sink} as it is unlinked -- the read-and-reset + * primitive for flush/publish workflows (drain the table into a telemetry batch, an event + * emitter, etc.). Equivalent to {@link #forEach} then {@link #clear} in a single call. + */ + public void drain(@Nonnull Consumer sink) { + Hashtable.drain(this.sizeManager, this.buckets, sink); + } + + /** + * Context-passing {@link #drain(Consumer)}. Pass a non-capturing {@link BiConsumer} (typically + * a {@code static final}) plus the accumulator as {@code context} to avoid a capturing-lambda + * allocation. + */ + public void drain(C context, @Nonnull BiConsumer sink) { + Hashtable.drain(this.sizeManager, this.buckets, context, sink); } } + // ============================================================================================ + // Static building blocks over a caller-owned bucket array. + // + // Use these to assemble a custom table (higher arity, primitive keys, extra value fields) when + // D1/D2 don't fit; D1/D2 delegate to them internally. The calling class owns the array and + // exposes whatever operations it needs. + // + // Not thread-safe: there is no locking here. Concurrent access, including mixing reads with + // writes, requires external synchronization. + // ============================================================================================ + + /** Upper bound on the bucket-array length returned by {@link #sizeFor(int)}. */ + static final int MAX_BUCKETS = 1 << 30; + /** - * Building blocks for hash-table operations. + * Allocates a fixed-size bucket array sized to hold {@code capacity} entries: {@code capacity} + * rounded up to the next power of two. * - *

      Used by {@link D1} and {@link D2}, and available to callers that want to assemble their own - * higher-arity table (3+ key parts) without re-implementing the bucket-array mechanics. The - * typical recipe: + *

      Erasure stops a caller writing {@code new TEntry[n]}, so {@code entryClass} is allocated + * reflectively via {@link Array#newInstance}. That buys a real {@code TEntry} component type + * rather than the base {@code Entry[]}: typed reads, real array-store checks, and a monomorphic + * element type for the JIT. The one reflective call happens at construction, off any hot path. + * Capacity is fixed; the table does not resize. Use {@link #create(int)} when the spine is driven + * purely through the static building blocks and the base component type is enough. * - *

        - *
      • Subclass {@link Hashtable.Entry} directly, adding the key fields and a {@code - * matches(...)} method of your chosen arity. - *
      • Allocate a backing array with {@link #create(int)} or {@link #create(int, float)} (the - * latter scales for a target load factor; see {@link #MAX_RATIO}). - *
      • Use {@link #bucketIndex(Object[], long)} for the bucket lookup, {@link - * #bucketIterator(Hashtable.Entry[], long)} for read-only chain walks, and {@link - * #mutatingBucketIterator(Hashtable.Entry[], long)} when you also need {@code remove} / - * {@code replace}. - *
      • Use {@link #insertHeadEntry(Hashtable.Entry[], int, Hashtable.Entry)} to splice a new - * entry as the head of a bucket chain. - *
      • Iterate every entry with {@link #forEach(Hashtable.Entry[], Consumer)} or its - * context-passing sibling. For full-table sweeps with {@code remove}, use {@link - * #mutatingTableIterator(Hashtable.Entry[])}. - *
      • Clear with {@link #clear(Hashtable.Entry[])}. - *
      + *

      {@code capacity} sizes the bucket array 1:1 (no headroom) -- chains stay a plain hash table + * at exactly this many entries. For load-factor headroom over a target cap on live entries (so + * chains stay short even as the table fills, the way {@link D1}/{@link D2}/{@link #createCapped} + * size themselves), pass {@link #capacityFor(int)} instead: {@code create(MyEntry.class, + * capacityFor(cardinalityLimit))}. + */ + @SuppressWarnings("unchecked") + @Nonnull + public static TEntry[] create( + @Nonnull Class entryClass, int capacity) { + return (TEntry[]) Array.newInstance(entryClass, sizeFor(capacity)); + } + + /** + * Untyped sibling of {@link #create(Class, int)}: allocates a bucket array of {@code buckets} + * rounded up to the next power of two, with the base {@code Hashtable.Entry[]} component type. + * + *

      Use this when the spine is driven purely through the static building blocks, which all take + * {@code Hashtable.Entry[]} -- that is what {@link D1}, {@link D2}, and {@link #createCapped} + * allocate internally. Prefer {@link #create(Class, int)} when you own the array and want a real + * {@code TEntry} component type (typed reads, array-store checks, a monomorphic element type for + * the JIT); prefer this one when a typed spine would only buy you covariant array-store checks on + * every insert. Capacity is fixed; the table does not resize. * - *

      All bucket arrays produced by {@code create} have a power-of-two length, so {@link - * #bucketIndex(Object[], long)} can use a bit mask. + *

      {@code buckets} is a bucket count, not an entry cap -- see {@link #capacityFor(int)} to + * derive one from a target cap on live entries. */ - public static final class Support { - /** - * Allocates a bucket array sized to hold {@code requestedSize} entries. Returned length is - * {@code requestedSize} rounded up to the next power of two (capped at {@link #MAX_BUCKETS}). - */ - public static final Hashtable.Entry[] create(int requestedSize) { - return new Entry[sizeFor(requestedSize)]; + @Nonnull + public static Hashtable.Entry[] create(int buckets) { + return new Hashtable.Entry[sizeFor(buckets)]; + } + + /** + * Balanced default load factor for a chained bucket array: at this target fill, chains from a + * well-spread hash stay short (average chain length {@code ~1/DEFAULT_LOAD_FACTOR}) without + * over-provisioning the array. Chaining tolerates a high target fill: past 1.0 it degrades + * gradually into longer chains rather than failing, so there is no cliff to stay clear of and no + * reason to over-allocate the spine. + */ + public static final float DEFAULT_LOAD_FACTOR = 0.75f; + + /** + * Bucket-array length for a strict cap of {@code cardinalityLimit} live entries at {@link + * #DEFAULT_LOAD_FACTOR}: infers a reasonable bucket count from the entry cap you actually care + * about, rather than making every caller redo the headroom math ({@link D1}, {@link D2}, and + * {@link #createCapped} all size themselves this way). Pair with a {@link SizeManager} of {@code + * cardinalityLimit} for the matching strict cap; this method only sizes the array. + */ + public static int capacityFor(int cardinalityLimit) { + return capacityFor(cardinalityLimit, DEFAULT_LOAD_FACTOR); + } + + /** + * {@link #capacityFor(int)} at an explicit {@code loadFactor} in {@code (0, 1)}: the bucket-array + * length for a strict cap of {@code cardinalityLimit} live entries, rounded up to a power of two + * via {@link #sizeFor(int)}. + */ + public static int capacityFor(int cardinalityLimit, float loadFactor) { + if (!(loadFactor > 0f && loadFactor < 1f)) { + throw new IllegalArgumentException("loadFactor must be in (0, 1): " + loadFactor); } + return sizeFor((int) (cardinalityLimit / loadFactor)); + } - /** - * Variant of {@link #create(int)} that scales the requested working-set size before sizing the - * bucket array. Pair with {@link #MAX_RATIO} to leave headroom over the working set for a - * desired load factor; the canonical call is {@code create(n, MAX_RATIO)}. - * - *

      The scaled size is truncated to {@code int} before going through {@link #sizeFor(int)}. - * Truncation rather than {@code ceil} is intentional: {@code sizeFor} rounds up to the next - * power of two anyway, so the fractional part would only matter when float fuzz pushes the - * result across a power-of-two boundary -- {@code ceil} would then double the array size for no - * reason (e.g. {@code 12 * 4/3 = 16.0...0005f -> ceil 17 -> sizeFor 32}). - */ - public static final Hashtable.Entry[] create(int requestedSize, float scale) { - return new Entry[sizeFor((int) (requestedSize * scale))]; + /** + * Rounds {@code requestedSize} up to the next power of two, capped at {@link #MAX_BUCKETS}, and + * returns the bucket-array length to allocate. Throws {@link IllegalArgumentException} for + * negative inputs or inputs above the cap. + */ + public static int sizeFor(int requestedSize) { + if (requestedSize < 0) { + throw new IllegalArgumentException("requestedSize must be non-negative: " + requestedSize); + } + if (requestedSize > MAX_BUCKETS) { + throw new IllegalArgumentException( + "requestedSize exceeds maximum bucket count (" + MAX_BUCKETS + "): " + requestedSize); + } + if (requestedSize <= 1) { + return 1; } + return Integer.highestOneBit(requestedSize - 1) << 1; + } - /** Upper bound on the bucket array length returned by {@link #sizeFor(int)}. */ - static final int MAX_BUCKETS = 1 << 30; + public static int bucketIndex(@Nonnull Object[] buckets, long keyHash) { + return (int) (keyHash & buckets.length - 1); + } - /** - * Inverse of a 75% load factor. Callers that size their bucket array from a target working-set - * size {@code n} should pass {@code create(n, MAX_RATIO)} to leave ~25% headroom in the array. - */ - public static final float MAX_RATIO = 4.0f / 3.0f; + /** + * Returns the head entry of the bucket that {@code keyHash} maps to, cast to the caller's + * concrete entry type. The unchecked cast lives here so the chain-walk loop at the call site + * doesn't need to thread a raw {@link Entry} variable through. + * + *

      Named {@code bucketFor} rather than {@code bucket}: there is no competing {@code int}-index + * overload today, but the {@code For} suffix marks "derives the index from a key hash" up front, + * so adding an index-taking sibling later cannot reintroduce the int-vs-long overload ambiguity + * described on {@link #insertHeadEntryFor(Hashtable.Entry[], long, Hashtable.Entry)}. + */ + @SuppressWarnings("unchecked") + @Nullable + public static TEntry bucketFor( + @Nonnull Hashtable.Entry[] buckets, long keyHash) { + return (TEntry) buckets[bucketIndex(buckets, keyHash)]; + } - /** - * Rounds {@code requestedSize} up to the next power of two, capped at {@link #MAX_BUCKETS}. - * Throws {@link IllegalArgumentException} for negative inputs or inputs above the cap. Returns - * the bucket-array length to allocate. - */ - static final int sizeFor(int requestedSize) { - if (requestedSize < 0) { - throw new IllegalArgumentException("requestedSize must be non-negative: " + requestedSize); - } - if (requestedSize > MAX_BUCKETS) { - throw new IllegalArgumentException( - "requestedSize exceeds maximum bucket count (" + MAX_BUCKETS + "): " + requestedSize); + /** + * Splices {@code entry} in as the new head of the chain at {@code bucketIndex}. Caller is + * responsible for size accounting -- this method only touches the chain pointers. + */ + public static void insertHeadEntryAt( + @Nonnull Hashtable.Entry[] buckets, int bucketIndex, @Nonnull Hashtable.Entry entry) { + assert entry.next() == null + : "Entry already linked -- inserting the same Entry instance twice corrupts the chain"; + entry.setNext(buckets[bucketIndex]); + buckets[bucketIndex] = entry; + } + + /** + * Convenience form of {@link #insertHeadEntryAt} that derives the bucket index from {@code + * keyHash}. Use this when the caller has the hash but not the index; if the index has already + * been computed for another reason, prefer {@link #insertHeadEntryAt} to avoid the redundant + * mask. + * + *

      Named distinctly from {@link #insertHeadEntryAt} rather than overloaded on {@code long} vs. + * {@code int}, because the overloaded form is a trap: a caller with a primitive {@code int}-typed + * key hash calling an overloaded {@code insertHeadEntry(buckets, intHash, entry)} would silently + * bind to the {@code int}-index overload instead of widening to this one, treating the raw hash + * as an array index. + */ + public static void insertHeadEntryFor( + @Nonnull Hashtable.Entry[] buckets, long keyHash, @Nonnull Hashtable.Entry entry) { + insertHeadEntryAt(buckets, bucketIndex(buckets, keyHash), entry); + } + + /** + * {@link #insertHeadEntryFor(Hashtable.Entry[], long, Hashtable.Entry)}, but folding in the + * strict-cap check that every unconditional insert needs: reserves a slot from {@code + * sizeManager} first, splicing {@code entry} in only if the reservation succeeds. Returns {@code + * false} (without touching {@code buckets}) once {@code sizeManager} is at capacity. Lets a + * composer working directly against the static building blocks (e.g. {@link D1#insert}, or a + * caller-owned table of higher key arity) get the same one-call insert-with-cap-check contract + * that {@link D1}/{@link D2} give their own callers. + * + *

      {@code sizeManager} leads, per this class's parameter order for the size-tracked statics: + * mutated bookkeeping, then the spine, then the key, then callbacks. Putting it first (rather + * than appending it) makes the tracked and untracked forms visibly different at the head of the + * call instead of differing only in a trailing argument -- forgetting the tracker leaks the cap + * silently, so the distinction should be hard to overlook at the call site and in review. + */ + public static boolean insertHeadEntryFor( + @Nonnull SizeManager sizeManager, + @Nonnull Hashtable.Entry[] buckets, + long keyHash, + @Nonnull Hashtable.Entry entry) { + if (!sizeManager.tryReserve()) { + return false; + } + insertHeadEntryFor(buckets, keyHash, entry); + return true; + } + + /** + * {@link #insertHeadEntryFor(SizeManager, Hashtable.Entry[], long, Hashtable.Entry)} over a + * {@link State}, which carries the spine and its manager together -- so there is no way to pass a + * manager that belongs to a different table, and {@code TEntry} is inferred rather than needing a + * witness at the call site. + */ + public static boolean insertHeadEntryFor( + @Nonnull State state, long keyHash, @Nonnull TEntry entry) { + return insertHeadEntryFor(state.sizeManager, state.buckets, keyHash, entry); + } + + /** + * Scans the bucket chain at {@code keyHash} for the first entry matching {@code matches}, unlinks + * it, decrements {@code sizeManager}, and returns it -- or returns {@code null} (leaving {@code + * buckets} and {@code sizeManager} untouched) if nothing in the chain matches. Mirrors {@link + * #insertHeadEntryFor(SizeManager, Hashtable.Entry[], long, Hashtable.Entry)} on the removal + * side: the one-call, size-tracked shape a composer driving the static building blocks directly + * can use instead of hand-rolling the mutating-iterator loop and remembering to decrement. + * + *

      {@code sizeManager} leads for the same reason it does on the insert side. + */ + @Nullable + public static TEntry removeMatching( + @Nonnull SizeManager sizeManager, + @Nonnull Hashtable.Entry[] buckets, + long keyHash, + @Nonnull Predicate matches) { + for (MutatingBucketIterator iter = mutatingBucketIterator(buckets, keyHash); + iter.hasNext(); ) { + TEntry curEntry = iter.next(); + if (matches.test(curEntry)) { + iter.remove(); + sizeManager.decrement(); + return curEntry; } - if (requestedSize <= 1) { - return 1; + } + return null; + } + + /** + * {@link #drain(Hashtable.Entry[], Consumer)} plus the matching bookkeeping: empties the table + * into {@code sink} and resets {@code sizeManager} to zero. Draining without resetting leaves the + * cap permanently consumed, so the two belong in one call rather than as a pair the caller has to + * remember -- same reasoning as {@link #clear(SizeManager, Hashtable.Entry[])}. + */ + public static void drain( + @Nonnull SizeManager sizeManager, + @Nonnull Hashtable.Entry[] buckets, + @Nonnull Consumer sink) { + Hashtable.drain(buckets, sink); + sizeManager.reset(); + } + + /** Context-passing form of {@link #drain(SizeManager, Hashtable.Entry[], Consumer)}. */ + public static void drain( + @Nonnull SizeManager sizeManager, + @Nonnull Hashtable.Entry[] buckets, + C context, + @Nonnull BiConsumer sink) { + Hashtable.drain(buckets, context, sink); + sizeManager.reset(); + } + + /** {@link #drain(SizeManager, Hashtable.Entry[], Consumer)} over a {@link State}. */ + public static void drain( + @Nonnull State state, @Nonnull Consumer sink) { + drain(state.sizeManager, state.buckets, sink); + } + + /** Context-passing form of {@link #drain(State, Consumer)}. */ + public static void drain( + @Nonnull State state, + C context, + @Nonnull BiConsumer sink) { + drain(state.sizeManager, state.buckets, context, sink); + } + + /** Live entries in {@code state}; see {@link SizeManager#estimateSize()} for why an estimate. */ + public static int estimateSize(@Nonnull State state) { + return state.sizeManager.estimateSize(); + } + + /** + * {@code true} when {@code state} appears to hold no entries. Derived from {@link + * SizeManager#estimateSize()} and inherits its imprecision -- an outstanding reservation reads as + * non-empty, and a link made without one can read as empty while the spine is not. Named for what + * it can honestly promise: use it to skip work that is merely wasted on an empty table, not to + * establish that there is nothing there. + */ + public static boolean isLikelyEmpty(@Nonnull State state) { + return state.sizeManager.estimateSize() == 0; + } + + /** + * Head entry of the bucket {@code keyHash} maps to in {@code state}, typed to the state's entry + * type so the chain walk at the call site needs no cast or witness. + */ + @Nullable + public static TEntry bucketFor( + @Nonnull State state, long keyHash) { + return bucketFor(state.buckets, keyHash); + } + + /** + * Splices {@code entry} in as the new head of its bucket without touching the count, + * because the caller already holds a reservation for it -- from {@link #tryReserveOrEvict} or a + * bare {@link SizeManager#tryReserve()}. Pairing those is the shape of a miss path that wants to + * refuse before it allocates: + * + * 

      {@code
+   * if (!tryReserveOrEvict(state, STALE)) {
+   *   return null;                       // refused -- no entry was built
+   * }
+   * insertReserved(state, keyHash, buildEntry());
+   * }
      + * + *

      Distinct from {@link #insertHeadEntryFor(State, long, Entry)}, which reserves as it inserts; + * calling that one here would count the entry twice. + */ + public static void insertReserved( + @Nonnull State state, long keyHash, @Nonnull TEntry entry) { + insertHeadEntryFor(state.buckets, keyHash, entry); + } + + /** {@link #forEach(Hashtable.Entry[], Consumer)} over a {@link State}. */ + public static void forEach( + @Nonnull State state, @Nonnull Consumer consumer) { + Hashtable.forEach(state.buckets, consumer); + } + + /** {@link #forEach(Hashtable.Entry[], Object, BiConsumer)} over a {@link State}. */ + public static void forEach( + @Nonnull State state, + C context, + @Nonnull BiConsumer consumer) { + Hashtable.forEach(state.buckets, context, consumer); + } + + /** + * Reserves a slot in {@code state} for a fresh insert, evicting one entry matching {@code + * evictable} if the table is full. {@code false} means full with nothing evictable -- the caller + * should drop the datum. The whole capacity decision of a self-evicting table's miss path in one + * call; pass a non-capturing {@code evictable} (typically a {@code static final}) to keep it + * allocation-free. + */ + public static boolean tryReserveOrEvict( + @Nonnull State state, @Nonnull Predicate evictable) { + return state.sizeManager.tryReserveOrEvict(state.buckets, evictable); + } + + /** + * Unlinks the first entry in {@code state} matching {@code evictable}, resuming from where the + * last eviction looked, and decrements the count. {@code null} if nothing matched anywhere. + */ + @Nullable + public static TEntry evictOne( + @Nonnull State state, @Nonnull Predicate evictable) { + return state.sizeManager.evictOne(state.buckets, evictable); + } + + /** + * Unlinks every entry in {@code state} matching {@code evictable}, decrementing per removal, and + * returns how many went. + */ + public static int evictAll( + @Nonnull State state, @Nonnull Predicate evictable) { + return state.sizeManager.evictAll(state.buckets, evictable); + } + + /** {@link #clear(SizeManager, Hashtable.Entry[])} over a {@link State}. */ + public static void clear(@Nonnull State state) { + clear(state.sizeManager, state.buckets); + } + + /** + * Walks every entry in {@code buckets} and invokes {@code consumer} on it. The unchecked cast to + * {@code TEntry} lives here (mirroring {@link Entry#next()}) so callers don't have to sprinkle it + * across their own forEach loops. + */ + @SuppressWarnings("unchecked") + public static void forEach( + @Nonnull Hashtable.Entry[] buckets, @Nonnull Consumer consumer) { + for (int i = 0; i < buckets.length; i++) { + for (Hashtable.Entry e = buckets[i]; e != null; e = e.next()) { + consumer.accept((TEntry) e); } - return Integer.highestOneBit(requestedSize - 1) << 1; } + } - public static final void clear(Hashtable.Entry[] buckets) { - Arrays.fill(buckets, null); + /** + * Context-passing variant of {@link #forEach(Hashtable.Entry[], Consumer)}. Pair a non-capturing + * {@link BiConsumer} (typically a {@code static final}) with side-band state passed as {@code + * context} to avoid a fresh-Consumer allocation each call. + */ + @SuppressWarnings("unchecked") + public static void forEach( + @Nonnull Hashtable.Entry[] buckets, + C context, + @Nonnull BiConsumer consumer) { + for (int i = 0; i < buckets.length; i++) { + for (Hashtable.Entry e = buckets[i]; e != null; e = e.next()) { + consumer.accept(context, (TEntry) e); + } } + } + + @Nonnull + public static BucketIterator bucketIterator( + @Nonnull Hashtable.Entry[] buckets, long keyHash) { + return new BucketIterator(buckets, keyHash); + } - public static final BucketIterator bucketIterator( - Hashtable.Entry[] buckets, long keyHash) { - return new BucketIterator(buckets, keyHash); + @Nonnull + public static + MutatingBucketIterator mutatingBucketIterator( + @Nonnull Hashtable.Entry[] buckets, long keyHash) { + return new MutatingBucketIterator(buckets, keyHash); + } + + /** + * Returns a {@link MutatingTableIterator} over every entry in {@code buckets}. Useful for sweeps + * -- eviction, expunge -- that aren't keyed to a specific hash. + */ + @Nonnull + public static + MutatingTableIterator mutatingTableIterator(@Nonnull Hashtable.Entry[] buckets) { + return new MutatingTableIterator(buckets, 0, buckets.length); + } + + /** + * Variant of {@link #mutatingTableIterator(Hashtable.Entry[])} that walks only the half-open + * bucket range {@code [startBucket, endBucket)}. Useful for resumable sweeps -- e.g. the + * cursor-based eviction in {@link SizeManager#evictOne} -- where one call drives {@code [cursor, + * length)} and a wrap-around call drives {@code [0, cursor)}. The iterator does not wrap + * around within a single instance; callers compose two iterators when wrap-around is desired. An + * empty range ({@code startBucket == endBucket}) produces an immediately exhausted iterator. + * + * @param startBucket inclusive lower bound; must be in {@code [0, buckets.length]}. + * @param endBucket exclusive upper bound; must be in {@code [startBucket, buckets.length]}. + */ + @Nonnull + public static + MutatingTableIterator mutatingTableIterator( + @Nonnull Hashtable.Entry[] buckets, int startBucket, int endBucket) { + return new MutatingTableIterator(buckets, startBucket, endBucket); + } + + /** + * {@link #removeMatching(SizeManager, Hashtable.Entry[], long, Predicate)} over a {@link State}. + * The predicate is typed to {@code TEntry}, so a caller matching on entry fields needs no cast. + */ + @Nullable + public static TEntry removeMatching( + @Nonnull State state, long keyHash, @Nonnull Predicate matches) { + return removeMatching(state.sizeManager, state.buckets, keyHash, matches); + } + + public static void clear(@Nonnull Hashtable.Entry[] buckets) { + Arrays.fill(buckets, null); + } + + /** + * {@link #clear(Hashtable.Entry[])} plus the matching bookkeeping: empties {@code buckets} and + * resets {@code sizeManager} to zero. Emptying a table without resetting its tracker leaves the + * cap permanently consumed, so the two belong in one call rather than as a pair a caller has to + * remember. + * + *

      {@code sizeManager} leads, per this class's parameter order for the size-tracked statics. + */ + public static void clear(@Nonnull SizeManager sizeManager, @Nonnull Hashtable.Entry[] buckets) { + clear(buckets); + sizeManager.reset(); + } + + /** + * Removes every entry, passing each removed entry to {@code sink} as it is unlinked -- the + * read-and-reset primitive for flush/publish workflows (drain the table into a telemetry batch, + * an event emitter, etc.). Equivalent to {@link #forEach} then {@link #clear}, offered as one + * call so composers don't have to spell out both steps. + */ + @SuppressWarnings("unchecked") + public static void drain( + @Nonnull Hashtable.Entry[] buckets, @Nonnull Consumer sink) { + for (int i = 0; i < buckets.length; i++) { + Entry entry = buckets[i]; + buckets[i] = null; + while (entry != null) { + // Unhook before handing over: a sink that retains one entry of a chain would otherwise pin + // the whole chain through `next`, including entries it chose not to keep. Read `next` + // first, since the sink may do anything with the entry once it has it. + Entry next = entry.next(); + entry.setNext(null); + sink.accept((TEntry) entry); + entry = next; + } } + } - public static final - MutatingBucketIterator mutatingBucketIterator( - Hashtable.Entry[] buckets, long keyHash) { - return new MutatingBucketIterator(buckets, keyHash); + /** + * Context-passing variant of {@link #drain(Hashtable.Entry[], Consumer)}. Pass a non-capturing + * {@link BiConsumer} (typically a {@code static final}) plus the accumulator as {@code context} + * (e.g. the target list or event builder) to avoid a capturing-lambda allocation. + */ + @SuppressWarnings("unchecked") + public static void drain( + @Nonnull Hashtable.Entry[] buckets, + C context, + @Nonnull BiConsumer sink) { + for (int i = 0; i < buckets.length; i++) { + Entry entry = buckets[i]; + buckets[i] = null; + while (entry != null) { + Entry next = entry.next(); + entry.setNext(null); + sink.accept(context, (TEntry) entry); + entry = next; + } } + } + + /** + * Manages a table's occupancy against a fixed cap -- both directions. Reserving a slot for an + * insert and evicting to make room are two halves of the same policy, so they live on one object: + * a caller never has to remember to decrement after unlinking, and there is no second object to + * wire up (or mis-wire) alongside the count. + * + *

      {@link D1} and {@link D2} use one internally for their strict entry-count cap; composers + * driving a {@code Hashtable.Entry[]} through the static building blocks can reuse it instead of + * hand-rolling the same increment/decrement/cap-check bookkeeping. A table that never evicts + * simply never calls the eviction half. + * + * 

      {@code
+   * // miss path of a capped, self-evicting table
+   * if (!sizeManager.tryReserveOrEvict(buckets, STALE)) {
+   *   return null;                       // full, and nothing was evictable -- drop the datum
+   * }
+   * insertHeadEntryFor(buckets, keyHash, newEntry);   // slot already reserved
+   * }
      + * + *

      Not thread-safe, matching the rest of this class. + */ + public static final class SizeManager { + private final int capacity; + private int size; /** - * Returns a {@link MutatingTableIterator} over every entry in {@code buckets}. Useful for - * sweeps -- eviction, expunge -- that aren't keyed to a specific hash. + * Bucket index the last eviction removed from. The next scan resumes here, so a sustained + * eviction stream doesn't repeatedly re-walk the same hot entries clustered near bucket 0. */ - public static final - MutatingTableIterator mutatingTableIterator(Hashtable.Entry[] buckets) { - return new MutatingTableIterator(buckets, 0, buckets.length); + private int cursor; + + public SizeManager(int capacity) { + this.capacity = capacity; } /** - * Variant of {@link #mutatingTableIterator(Hashtable.Entry[])} that walks only the half-open - * bucket range {@code [startBucket, endBucket)}. Useful for resumable sweeps -- e.g. cursor- - * based eviction in {@code AggregateTable} -- where one call drives {@code [cursor, length)} - * and a wrap-around call drives {@code [0, cursor)}. The iterator does not wrap around - * within a single instance; callers compose two iterators when wrap-around is desired. An empty - * range ({@code startBucket == endBucket}) produces an immediately exhausted iterator. + * Live entries, as far as this manager knows -- an estimate, not a census. A reservation taken + * by {@link #tryReserve()} or {@link #tryReserveOrEvict} counts immediately, so between + * reserving and linking the figure runs one high; and {@link Hashtable#insertReserved} trusts + * the caller to have reserved, so a link without one leaves it low. The manager counts what it + * is told, and cannot audit the spine to check. * - * @param startBucket inclusive lower bound; must be in {@code [0, buckets.length]}. - * @param endBucket exclusive upper bound; must be in {@code [startBucket, buckets.length]}. + *

      Wrappers that never expose the reservation window -- {@link D1} and {@link D2}, which + * reserve and link inside a single call -- can and do present this as an exact {@code size()}. */ - public static final - MutatingTableIterator mutatingTableIterator( - Hashtable.Entry[] buckets, int startBucket, int endBucket) { - return new MutatingTableIterator(buckets, startBucket, endBucket); + public int estimateSize() { + return this.size; + } + + public int capacity() { + return this.capacity; } - public static final int bucketIndex(Object[] buckets, long keyHash) { - return (int) (keyHash & buckets.length - 1); + /** {@code true} once {@link #size()} has reached {@link #capacity()}. */ + public boolean isFull() { + return this.size >= this.capacity; } /** - * Splices {@code entry} in as the new head of the chain at {@code bucketIndex}. Caller is - * responsible for size accounting -- this method only touches the chain pointers. + * Reserves a slot for a fresh insert: increments and returns {@code true}, or leaves the count + * unchanged and returns {@code false} if already at capacity. Use this when the entry to link + * is already fully built (nothing between the check and the increment can fail). When building + * the entry is itself fallible, check {@link #isFull()} first, do the fallible work, then call + * {@link #increment()} only once linking actually succeeds. + * + *

      Returning {@code false} is not a final refusal -- it is the caller's cue to either refuse + * the insert or make room. {@link #tryReserveOrEvict} folds those two steps into one call. */ - public static final void insertHeadEntry( - Hashtable.Entry[] buckets, int bucketIndex, Hashtable.Entry entry) { - entry.setNext(buckets[bucketIndex]); - buckets[bucketIndex] = entry; + public boolean tryReserve() { + if (isFull()) { + return false; + } + this.size += 1; + return true; } /** - * Convenience overload of {@link #insertHeadEntry(Hashtable.Entry[], int, Hashtable.Entry)} - * that derives the bucket index from {@code keyHash}. Use this when the caller has the hash but - * not the index; if the index has already been computed for another reason, prefer the - * int-taking overload to avoid the redundant mask. + * {@link #tryReserve()}, falling back to evicting one entry matching {@code evictable} when the + * table is full. Returns {@code true} with a slot reserved, or {@code false} if the table was + * full and nothing was evictable -- in which case {@code buckets} is untouched and the caller + * should drop the datum. + * + *

      The whole capacity decision of a self-evicting table's miss path, in one call. Pass a + * non-capturing {@code evictable} (typically a {@code static final}) to keep it + * allocation-free. */ - public static final void insertHeadEntry( - Hashtable.Entry[] buckets, long keyHash, Hashtable.Entry entry) { - insertHeadEntry(buckets, bucketIndex(buckets, keyHash), entry); + public boolean tryReserveOrEvict( + @Nonnull Hashtable.Entry[] buckets, @Nonnull Predicate evictable) { + if (tryReserve()) { + return true; + } + if (evictOne(buckets, evictable) == null) { + return false; + } + // evictOne decremented; the slot it freed is ours. + this.size += 1; + return true; + } + + /** Call after successfully linking a new entry. */ + public void increment() { + this.size += 1; + } + + /** Call after successfully unlinking an entry. */ + public void decrement() { + this.size -= 1; + } + + /** Zeroes both the live count and the eviction scan position. */ + public void reset() { + this.size = 0; + this.cursor = 0; } /** - * Returns the head entry of the bucket that {@code keyHash} maps to, cast to the caller's - * concrete entry type. The unchecked cast lives here so the chain-walk loop at the call site - * doesn't need to thread a raw {@link Entry} variable through. + * Scans {@code buckets} for the first entry matching {@code evictable}, starting where the last + * eviction left off and wrapping around if needed. Unlinks and returns the evicted entry, + * decrementing the count; returns {@code null} (count untouched) if nothing matched anywhere. + * + *

      Resuming from the previous position is what keeps a sustained eviction stream amortized: + * the worst case for a single call is still O(N) when nearly every entry is hot, but N + * successful evictions never re-scan the hot prefix more than twice. + * + *

      That amortization covers successes only. A call that matches nothing has, by + * definition, tested every live entry -- so a table that is full and entirely hot pays a full + * pass per attempt. The cursor still steps on, so repeated refusals at least start from a + * different bucket rather than re-testing in identical order, but the per-attempt cost does not + * shrink. Size the cap to the steady-state working set so this stays the rare path, and keep + * {@code evictable} cheap -- it is called once per live entry on every refusal. */ @SuppressWarnings("unchecked") - public static final TEntry bucket( - Hashtable.Entry[] buckets, long keyHash) { - return (TEntry) buckets[bucketIndex(buckets, keyHash)]; + @Nullable + public TEntry evictOne( + @Nonnull Hashtable.Entry[] buckets, @Nonnull Predicate evictable) { + Entry evicted = evictOneInRange(buckets, evictable, this.cursor, buckets.length); + if (evicted == null && this.cursor != 0) { + evicted = evictOneInRange(buckets, evictable, 0, this.cursor); + } + if (evicted != null) { + this.size -= 1; + return (TEntry) evicted; + } + // Nothing matched anywhere. Step the cursor on regardless, so a table that is full of hot + // entries doesn't retry from the same origin every time -- successive refusals sweep a + // different starting bucket instead of re-testing the same entries in the same order. + // (buckets.length is a power of two, so the mask wraps.) + this.cursor = (this.cursor + 1) & (buckets.length - 1); + return null; } - /** - * Walks every entry in {@code buckets} and invokes {@code consumer} on it. The unchecked cast - * to {@code TEntry} lives here (mirroring {@link Entry#next()}) so callers don't have to - * sprinkle it across their own forEach loops. - */ @SuppressWarnings("unchecked") - public static final void forEach( - Hashtable.Entry[] buckets, Consumer consumer) { - for (int i = 0; i < buckets.length; i++) { - for (Hashtable.Entry e = buckets[i]; e != null; e = e.next()) { - consumer.accept((TEntry) e); + @Nullable + private Entry evictOneInRange( + @Nonnull Hashtable.Entry[] buckets, + @Nonnull Predicate evictable, + int startBucket, + int endBucket) { + MutatingTableIterator iter = mutatingTableIterator(buckets, startBucket, endBucket); + while (iter.hasNext()) { + Entry candidate = iter.next(); + if (evictable.test((TEntry) candidate)) { + int bucket = iter.currentBucket(); + iter.remove(); + this.cursor = bucket; + return candidate; } } + return null; } /** - * Context-passing variant of {@link #forEach(Hashtable.Entry[], Consumer)}. Pair a - * non-capturing {@link BiConsumer} (typically a {@code static final}) with side-band state - * passed as {@code context} to avoid a fresh-Consumer allocation each call. + * Unlinks every entry matching {@code evictable} in one full pass, decrementing the count for + * each, and returns how many were removed. Resets the scan position, since a full pass leaves + * nothing later to resume from. + * + *

      Named {@code evictAll} rather than {@code drain} to keep it distinct from {@link + * Hashtable#drain(Hashtable.Entry[], Consumer)}, which empties the whole table into a sink. + * This one removes only what matches, and hands back a count rather than the entries. */ @SuppressWarnings("unchecked") - public static final void forEach( - Hashtable.Entry[] buckets, T context, BiConsumer consumer) { - for (int i = 0; i < buckets.length; i++) { - for (Hashtable.Entry e = buckets[i]; e != null; e = e.next()) { - consumer.accept(context, (TEntry) e); + public int evictAll( + @Nonnull Hashtable.Entry[] buckets, @Nonnull Predicate evictable) { + int count = 0; + MutatingTableIterator iter = mutatingTableIterator(buckets); + while (iter.hasNext()) { + Entry candidate = iter.next(); + if (evictable.test((TEntry) candidate)) { + iter.remove(); + // Decrement per removal rather than subtracting `count` after the loop: if `evictable` + // throws part way through, the entries unlinked so far are already gone from the chains, + // and a deferred subtraction would never run -- leaving the count permanently high. + this.size -= 1; + count++; } } + this.cursor = 0; + return count; + } + } + + /** + * The mutable state of a caller-driven table: a bucket array and the {@link SizeManager} sized + * and matched to it. Both halves are stateful and neither is much use without the other, which is + * what the name is getting at -- the spine holds the entries, the manager holds how many there + * are and where the last eviction looked. + * + *

      Hold this, rather than unpacking it. Keeping one field instead of two is not just + * tidier: an array and a manager stored separately can drift apart, which is the mistake this + * type exists to prevent. Composers reach through it -- {@code state.buckets}, {@code + * state.sizeManager} -- when calling the static building blocks. + * + *

      Same headroom idiom as {@link D1}/{@link D2}: {@code maxCapacity} is the strict cap on live + * entries, and the backing array is sized with load-factor headroom over it. + */ + public static final class State { + public final Hashtable.Entry[] buckets; + public final SizeManager sizeManager; + + private State(Hashtable.Entry[] buckets, int maxCapacity) { + this.buckets = buckets; + this.sizeManager = new SizeManager(maxCapacity); } } + /** + * Creates a {@link State}: a bucket array sized with load-factor headroom over {@code + * maxCapacity}, paired with a {@link SizeManager} capped at the strict {@code maxCapacity}. + */ + @Nonnull + public static State createCapped(int maxCapacity) { + Hashtable.Entry[] buckets = create(capacityFor(maxCapacity)); + return new State<>(buckets, maxCapacity); + } + /** * Read-only iterator over entries in a single bucket whose {@code keyHash} matches a specific * search hash. Cheaper than {@link MutatingBucketIterator} because it does not track the @@ -599,9 +1512,9 @@ public static final class BucketIterator implements Iterat private final long keyHash; private Hashtable.Entry nextEntry; - BucketIterator(Hashtable.Entry[] buckets, long keyHash) { + BucketIterator(@Nonnull Hashtable.Entry[] buckets, long keyHash) { this.keyHash = keyHash; - Hashtable.Entry cur = buckets[Support.bucketIndex(buckets, keyHash)]; + Hashtable.Entry cur = buckets[Hashtable.bucketIndex(buckets, keyHash)]; while (cur != null && cur.keyHash != keyHash) { cur = cur.next(); } @@ -615,6 +1528,7 @@ public boolean hasNext() { @Override @SuppressWarnings("unchecked") + @Nonnull public TEntry next() { Hashtable.Entry cur = this.nextEntry; if (cur == null) { @@ -661,11 +1575,11 @@ public static final class MutatingBucketIterator /** The next entry to be returned by next */ private Hashtable.Entry nextEntry; - MutatingBucketIterator(Hashtable.Entry[] buckets, long keyHash) { + MutatingBucketIterator(@Nonnull Hashtable.Entry[] buckets, long keyHash) { this.buckets = buckets; this.keyHash = keyHash; - int bucketIndex = Support.bucketIndex(buckets, keyHash); + int bucketIndex = Hashtable.bucketIndex(buckets, keyHash); Hashtable.Entry headEntry = this.buckets[bucketIndex]; if (headEntry == null) { this.nextEntry = null; @@ -695,6 +1609,7 @@ public boolean hasNext() { @Override @SuppressWarnings("unchecked") + @Nonnull public TEntry next() { Hashtable.Entry curEntry = this.nextEntry; if (curEntry == null) { @@ -739,7 +1654,7 @@ public void remove() { this.curEntry = null; } - public void replace(TEntry replacementEntry) { + public void replace(@Nonnull TEntry replacementEntry) { Hashtable.Entry oldCurEntry = this.curEntry; if (oldCurEntry == null) { throw new IllegalStateException(); @@ -759,10 +1674,10 @@ public void replace(TEntry replacementEntry) { this.curEntry = replacementEntry; } - void setPrevNext(Hashtable.Entry nextEntry) { + void setPrevNext(@Nullable Hashtable.Entry nextEntry) { if (this.curPrevEntry == null) { Hashtable.Entry[] buckets = this.buckets; - buckets[Support.bucketIndex(buckets, this.keyHash)] = nextEntry; + buckets[Hashtable.bucketIndex(buckets, this.keyHash)] = nextEntry; } else { this.curPrevEntry.setNext(nextEntry); } @@ -816,7 +1731,7 @@ public static final class MutatingTableIterator */ private Hashtable.Entry curEntry; - MutatingTableIterator(Hashtable.Entry[] buckets, int startBucket, int endBucket) { + MutatingTableIterator(@Nonnull Hashtable.Entry[] buckets, int startBucket, int endBucket) { this.buckets = buckets; if (startBucket < 0 || startBucket > buckets.length) { throw new IndexOutOfBoundsException( @@ -853,6 +1768,7 @@ public boolean hasNext() { @Override @SuppressWarnings("unchecked") + @Nonnull public TEntry next() { Hashtable.Entry e = this.nextEntry; if (e == null) { diff --git a/internal-api/src/main/java/datadog/trace/util/Maybe.java b/internal-api/src/main/java/datadog/trace/util/Maybe.java new file mode 100644 index 00000000000..a3e986174cf --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/Maybe.java @@ -0,0 +1,155 @@ +package datadog.trace.util; + +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.ObjDoubleConsumer; +import java.util.function.ObjIntConsumer; +import java.util.function.ObjLongConsumer; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * Candidate return shape for fallible operations (e.g. a table's capacity-refusing {@code + * tryGetOrCreate}), evaluating whether it can be allocation-free under escape analysis. + * + *

      Deliberately shaped against the {@code Optional}-style merge-with-singleton pitfall: {@link + * #of} is the only allocation site, always allocates (never returns a shared instance), and holds a + * plain nullable field. See {@code EscapeShapeBenchmark}'s {@code phiWithStatic} arm for why an + * {@code EMPTY} singleton would cost 8 B/op on every JDK measured, including 25. + * + *

      This shape -- one allocation site, a plain nullable field, no singleton merge -- scalar- + * replaces on ordinary escape analysis, on JDK 8/11/17/25, no JDK-21+ {@code + * ReduceAllocationMerges} needed (see {@code EscapeShapeBenchmark}). The discipline required of a + * caller is that the wrapping method itself construct a {@code Maybe} at exactly one call site (fed + * by a plain nullable local merged through ordinary branches, or by delegating to an + * already-nullable-returning method) rather than once per {@code return} statement -- multiple + * construction sites inline into a multi-producer phi that fails scalar replacement on JDK + * 8/11/17/21 (measured 16 B/op, {@code MaybeUsagePatternsBenchmark#badMultiConstructionSite}) once + * the refusal branch is reachable. On JDK 25, {@code ReduceAllocationMerges} collapses this + * specific shape -- two branches allocating the same final type with identical field layout -- back + * down to 0 B/op; do not rely on that JDK-25-only behavior, since it is exactly the kind of + * EA-dependent elision that can regress silently the moment the two branches stop being trivially + * mergeable (e.g. one branch gains extra state). See {@code EscapeShapeBenchmark}'s {@code + * phiOfTwoAllocations} arm, which uses two distinct interface implementations rather than one + * concrete type and therefore fails to scalar-replace on every JDK including 25 -- a different, + * stronger failure mode than the one demonstrated here. + */ +public final class Maybe { + @Nullable private final T value; + + private Maybe(@Nullable T value) { + this.value = value; + } + + @Nonnull + public static Maybe of(@Nullable T value) { + return new Maybe<>(value); + } + + /** + * Convenience form for the common shape {@code Maybe.of(receiver.someNullableMethod(args))}: + * {@code Maybe.of(receiver, r -> r.someNullableMethod(args))}. Useful when {@code receiver} would + * otherwise have to be re-evaluated or named twice at the call site. + * + *

      Unlike the single-arg {@link #of}, {@code fn} here is typically a capturing lambda + * -- it closes over whatever local arguments the caller's method has in scope, so a fresh lambda + * instance is created on every invocation (capturing lambdas are never cached the way a + * non-capturing lambda's singleton instance commonly is) -- which makes it a second heap-object + * candidate distinct from the {@code Maybe} itself. That freshly-allocated capturing lambda still + * scalar-replaces as reliably as a plain delegating method call does, for the shape actually + * measured (JDK 8/11/17/25): a monomorphic receiver and a {@code fn} that is applied exactly once + * and does not itself escape (e.g. by being stored or passed further). If {@code fn} itself + * captures something that must be freshly allocated per call (e.g. a non-singleton creator), that + * allocation is real regardless of what happens to the lambda wrapping it. + */ + @Nonnull + public static Maybe of(R receiver, @Nonnull Function fn) { + return new Maybe<>(fn.apply(receiver)); + } + + public boolean isPresent() { + return value != null; + } + + /** + * Raw accessor -- named to make the null case unmissable at the call site, rather than {@code + * orElse}/{@code get}, neither of which says so on its own. + */ + @Nullable + public T getOrNull() { + return value; + } + + /** + * Primary intended usage: a guard in front of mutation, e.g. {@code table.tryGetOrCreate(key, + * FooEntry::new).update(FooEntry::inc)}. No-op if the operation was refused (table full) rather + * than throwing or requiring the caller to branch on {@link #isPresent()} first. + */ + public void update(Consumer mutator) { + if (value != null) { + mutator.accept(value); + } + } + + /** + * Generic-context form of {@link #update(Consumer)}, for callers that already have a reusable, + * non-capturing {@code BiConsumer} (typically a {@code static final}) plus whatever context it + * needs -- {@code (value, context)} to stay consistent with the primitive-context overloads + * below, at the cost of departing from {@code Hashtable#forEach}'s {@code (context, entry)} + * convention. + */ + public void update(C context, BiConsumer mutator) { + if (value != null) { + mutator.accept(value, context); + } + } + + /** + * Primitive-context form of {@link #update(Consumer)}, for the common case where the mutation + * needs one caller-supplied number (e.g. a duration or count) and boxing it into a captured + * {@code Long}/generic-context object would be the actual per-call allocation. This exists so a + * table wrapping a fallible lookup in {@code Maybe} pays for this shape once, here, instead of + * once per mutator-flavor per table type -- see {@code Hashtable#tryGetOrUpdate}'s {@code + * ObjLongConsumer} overload for the caller-side problem this replaces. + * + *

      Deliberately the only primitive-context overload of {@code update}. An {@code + * int}/{@code boolean} sibling was tried and reverted: Java's overload resolution can pick + * cleanly between a primitive overload and the generic {@link #update(Object, BiConsumer)} form + * for a reference-typed argument (boxing is only considered once no non-boxing candidate + * applies), but that guarantee does not extend to a second primitive overload -- {@code update(1, + * lambda)} is ambiguous between {@code int} and {@code long} even with no {@code double} overload + * in the picture, because {@link ObjIntConsumer} and {@link ObjLongConsumer} are unrelated + * interfaces and JLS 15.12.2.5's most-specific-method rule requires every parameter position to + * agree, not just the numeric one. Confirmed by direct compilation, not just JLS reading: an + * inline lambda call breaks as soon as a second primitive overload exists. A plain {@code int} + * argument still widens to {@code long} for free at this single overload -- callers are not + * required to have a {@code long} in hand. {@code double} context is rare enough not to bother + * keeping pretty -- see {@link #updateDouble} for that case, given its own name to sidestep the + * ambiguity rather than trying to squeeze it into an overload. + */ + public void update(long context, ObjLongConsumer mutator) { + if (value != null) { + mutator.accept(value, context); + } + } + + /** + * {@code double}-context sibling of {@link #update(long, ObjLongConsumer)}, given a distinct name + * rather than a second primitive overload -- see that method's javadoc for why overloading {@code + * update} a second time breaks inline-lambda call sites. + */ + public void updateDouble(double context, ObjDoubleConsumer mutator) { + if (value != null) { + mutator.accept(value, context); + } + } + + public void ifPresentOrElse(Consumer action, Runnable emptyAction) { + if (value != null) { + action.accept(value); + } else { + emptyAction.run(); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/StringIndex.java b/internal-api/src/main/java/datadog/trace/util/StringIndex.java index 868558e0c38..66644e0fecf 100644 --- a/internal-api/src/main/java/datadog/trace/util/StringIndex.java +++ b/internal-api/src/main/java/datadog/trace/util/StringIndex.java @@ -74,7 +74,7 @@ public int indexOf(String name) { } public boolean contains(String name) { - return indexOf(name) >= 0; + return EmbeddingSupport.indexOf(this.hashes, this.names, name) >= 0; } /** Table size — allocate parallel payload arrays of this length. */ @@ -288,8 +288,8 @@ public static long[] mapLongValues(String[] names, ToLongFunction fn) { */ static int put(int[] hashes, String[] names, String name, int h) { final int mask = hashes.length - 1; - int i = h & mask; - for (int probes = 0; probes <= mask; probes++, i = (i + 1) & mask) { + for (int probes = 0; probes <= mask; probes++) { + int i = (h + probes) & mask; if (hashes[i] == 0) { hashes[i] = h; names[i] = name; @@ -313,8 +313,8 @@ static int put(int[] hashes, String[] names, String name, int h) { */ public static int indexOf(int[] hashes, String[] names, String name, int h) { final int mask = hashes.length - 1; - int i = h & mask; - for (int probes = 0; probes <= mask; probes++, i = (i + 1) & mask) { + for (int probes = 0; probes <= mask; probes++) { + int i = (h + probes) & mask; int sh = hashes[i]; if (sh == 0) { return -1; @@ -334,6 +334,11 @@ public static int indexOf(int[] hashes, String[] names, String name) { return indexOf(hashes, names, name, hash(name)); } + /** {@code indexOf(hashes, names, name) >= 0}. Mirrors {@link StringIndex#contains}. */ + public static boolean contains(int[] hashes, String[] names, String name) { + return indexOf(hashes, names, name) >= 0; + } + /** Number of slots — the length to size parallel payload arrays to. */ public static int numSlots(int[] hashes) { return hashes.length; diff --git a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java index 4fc6838974b..56594c292fc 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java @@ -188,7 +188,7 @@ void getOrCreateOnMissBuildsEntryViaCreator() { FlatHashtable.D1 table = growable(8); int[] createCount = {0}; StringIntEntry created = - table.getOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -209,7 +209,7 @@ void getOrCreateOnHitSkipsCreator() { table.insert(seeded); int[] createCount = {0}; StringIntEntry got = - table.getOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -237,7 +237,7 @@ void growableGrowsPastInitialCapacity() { 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)); + StringIntEntry e = table.tryGetOrCreateOrNull("k" + i, k -> new StringIntEntry(k, 0)); assertNotNull(e); } assertEquals(50, table.size()); @@ -246,15 +246,36 @@ void growableGetOrCreateNeverReturnsNull() { @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))); + assertNotNull(table.tryGetOrCreateOrNull("a", k -> new StringIntEntry(k, 1))); + assertNotNull(table.tryGetOrCreateOrNull("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))); + assertNull(table.tryGetOrCreateOrNull("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))); + assertSame(a, table.tryGetOrCreateOrNull("a", k -> new StringIntEntry(k, 99))); + } + + @Test + void fixedGetOrCreateAsMaybeIsAbsentWhenFullButStillReturnsHits() { + FlatHashtable.D1 table = fixed(2); + table.tryGetOrCreate("a", k -> new StringIntEntry(k, 1)); + table.tryGetOrCreate("b", k -> new StringIntEntry(k, 2)); + + assertFalse(table.tryGetOrCreate("c", k -> new StringIntEntry(k, 3)).isPresent()); + + Maybe hit = table.tryGetOrCreate("a", k -> new StringIntEntry(k, 99)); + assertEquals(1, hit.getOrNull().value, "existing entry is still returned even at capacity"); + } + + @Test + void growableGetOrCreateAsMaybeIsAlwaysPresent() { + FlatHashtable.D1 table = growable(1); + for (int i = 0; i < 50; i++) { + assertTrue(table.tryGetOrCreate("k" + i, k -> new StringIntEntry(k, 0)).isPresent()); + } + assertEquals(50, table.size()); } @Test 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 5d19af4fecf..948501185cd 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD2Test.java @@ -167,7 +167,7 @@ void getOrCreateOnMissBuildsEntryViaCreator() { FlatHashtable.D2 table = growable(8); int[] createCount = {0}; PairEntry created = - table.getOrCreate( + table.tryGetOrCreateOrNull( "foo", 1, (k1, k2) -> { @@ -190,7 +190,7 @@ void getOrCreateOnHitSkipsCreator() { table.insert(seeded); int[] createCount = {0}; PairEntry got = - table.getOrCreate( + table.tryGetOrCreateOrNull( "foo", 1, (k1, k2) -> { @@ -217,13 +217,35 @@ void growableGrowsPastInitialCapacity() { @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))); + assertNotNull(table.tryGetOrCreateOrNull("a", 1, (k1, k2) -> new PairEntry(k1, k2, 1))); + assertNotNull(table.tryGetOrCreateOrNull("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))); + assertNull(table.tryGetOrCreateOrNull("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))); + assertSame(a, table.tryGetOrCreateOrNull("a", 1, (k1, k2) -> new PairEntry(k1, k2, 99))); + } + + @Test + void fixedGetOrCreateAsMaybeIsAbsentWhenFullButStillReturnsHits() { + FlatHashtable.D2 table = fixed(2); + table.tryGetOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 1)); + table.tryGetOrCreate("b", 2, (k1, k2) -> new PairEntry(k1, k2, 2)); + + assertFalse(table.tryGetOrCreate("c", 3, (k1, k2) -> new PairEntry(k1, k2, 3)).isPresent()); + + Maybe hit = table.tryGetOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 99)); + assertEquals(1, hit.getOrNull().value, "existing entry is still returned even at capacity"); + } + + @Test + void growableGetOrCreateAsMaybeIsAlwaysPresent() { + FlatHashtable.D2 table = growable(1); + for (int i = 0; i < 50; i++) { + Maybe maybe = table.tryGetOrCreate("k", i, (k1, k2) -> new PairEntry(k1, k2, k2)); + assertTrue(maybe.isPresent()); + } + assertEquals(50, table.size()); } @Test @@ -258,7 +280,7 @@ void hashCollisionsResolveByKeyEquality() { 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)); + PairEntry e = table.tryGetOrCreateOrNull("k", i, (k1, k2) -> new PairEntry(k1, k2, k2)); assertNotNull(e); } assertEquals(50, table.size()); 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 b1099210611..0ecf3712d3f 100644 --- a/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/FlatHashtableTest.java @@ -185,10 +185,10 @@ void create_allocatesTypedTableOfCapacity() { @Test void getOrCreate_insertsOnceAndReturnsTheExistingEntry() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - TestEntry first = FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); + TestEntry first = FlatHashtable.tryGetOrCreate(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", TestEntryStrategy.INSTANCE, CREATE)); + assertSame(first, FlatHashtable.tryGetOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE)); assertSame(first, FlatHashtable.get(table, "a", TestEntryStrategy.INSTANCE)); } @@ -196,7 +196,7 @@ void getOrCreate_insertsOnceAndReturnsTheExistingEntry() { void get_returnsNullForAbsentKey() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); assertNull(FlatHashtable.get(table, "missing", TestEntryStrategy.INSTANCE)); - FlatHashtable.getOrCreate(table, "present", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "present", TestEntryStrategy.INSTANCE, CREATE); assertNull(FlatHashtable.get(table, "still-missing", TestEntryStrategy.INSTANCE)); } @@ -204,14 +204,16 @@ void get_returnsNullForAbsentKey() { void getOrCreate_returnsNullWhenTableIsFull() { // capacityFor(1) == 2 slots. TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); - assertTrue(FlatHashtable.getOrCreate(table, "k0", TestEntryStrategy.INSTANCE, CREATE) != null); - assertTrue(FlatHashtable.getOrCreate(table, "k1", TestEntryStrategy.INSTANCE, CREATE) != null); + assertTrue( + FlatHashtable.tryGetOrCreate(table, "k0", TestEntryStrategy.INSTANCE, CREATE) != null); + assertTrue( + FlatHashtable.tryGetOrCreate(table, "k1", TestEntryStrategy.INSTANCE, CREATE) != null); // Both slots occupied by distinct keys -> a third distinct key finds no room. - assertNull(FlatHashtable.getOrCreate(table, "k2", TestEntryStrategy.INSTANCE, CREATE)); + assertNull(FlatHashtable.tryGetOrCreate(table, "k2", TestEntryStrategy.INSTANCE, CREATE)); // ...but an existing key still resolves even when full. assertSame( FlatHashtable.get(table, "k0", TestEntryStrategy.INSTANCE), - FlatHashtable.getOrCreate(table, "k0", TestEntryStrategy.INSTANCE, CREATE)); + FlatHashtable.tryGetOrCreate(table, "k0", TestEntryStrategy.INSTANCE, CREATE)); } @Test @@ -225,11 +227,11 @@ void hashKey_isStableForEqualKeys() { 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", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry a = FlatHashtable.tryGetOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); // slot 0 taken -> 1 - TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.tryGetOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); // -> slot 2 - TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry c = FlatHashtable.tryGetOrCreate(table, "c", TestCollidingStrategy.INSTANCE, CREATE); assertNotSame(a, b); assertNotSame(b, c); @@ -240,7 +242,7 @@ void collision_probesPastOccupiedSlots_andResolvesEach() { assertSame(c, FlatHashtable.get(table, "c", TestCollidingStrategy.INSTANCE)); // existing colliding key: found after probing, no new entry minted - assertSame(b, FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE)); + assertSame(b, FlatHashtable.tryGetOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE)); // absent key: probe past the 3 occupied slots, hit an empty slot -> null assertNull(FlatHashtable.get(table, "absent", TestCollidingStrategy.INSTANCE)); @@ -251,9 +253,9 @@ 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", TestLastSlotStrategy.INSTANCE, CREATE); + TestEntry k0 = FlatHashtable.tryGetOrCreate(table, "k0", TestLastSlotStrategy.INSTANCE, CREATE); // taken -> wraps to 0 - TestEntry k1 = FlatHashtable.getOrCreate(table, "k1", TestLastSlotStrategy.INSTANCE, CREATE); + TestEntry k1 = FlatHashtable.tryGetOrCreate(table, "k1", TestLastSlotStrategy.INSTANCE, CREATE); assertNotSame(k0, k1); assertSame(k0, FlatHashtable.get(table, "k0", TestLastSlotStrategy.INSTANCE)); @@ -264,9 +266,9 @@ void collision_probeWrapsAroundToFront() { @Test void get_returnsNullWhenTableFullAndKeyAbsent() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 1); // 2 slots - FlatHashtable.getOrCreate(table, "k0", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "k0", TestCollidingStrategy.INSTANCE, CREATE); // fills slots 0 and 1 - FlatHashtable.getOrCreate(table, "k1", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(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", TestCollidingStrategy.INSTANCE)); @@ -302,9 +304,9 @@ void insert_returnsFalseWhenFull() { @Test void forEach_visitsEveryEntry() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "b", TestEntryStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "c", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "b", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "c", TestEntryStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); FlatHashtable.forEach(table, e -> seen.add(e.key)); @@ -314,8 +316,8 @@ void forEach_visitsEveryEntry() { @Test void forEach_contextVariant_passesContextWithoutCapture() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 8); - FlatHashtable.getOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "b", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "a", TestEntryStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "b", TestEntryStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); FlatHashtable.forEach(table, seen, (ctx, e) -> ctx.add(e.key)); @@ -325,9 +327,9 @@ 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", TestCollidingStrategy.INSTANCE, CREATE); - TestEntry b = FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); - TestEntry c = FlatHashtable.getOrCreate(table, "c", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry a = FlatHashtable.tryGetOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.tryGetOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry c = FlatHashtable.tryGetOrCreate(table, "c", TestCollidingStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); Iterator it = FlatHashtable.iterator(table, 0, TestCollidingStrategy.INSTANCE); @@ -340,8 +342,8 @@ void iterator_yieldsEveryEntrySharingTheHash() { @Test void iterator_filtersOutEntriesWithADifferentHash() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); // entries at slot 0, hashOf == 0 - FlatHashtable.getOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); - FlatHashtable.getOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(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); @@ -363,8 +365,8 @@ 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); + TestEntry a = FlatHashtable.tryGetOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + TestEntry b = FlatHashtable.tryGetOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); Set seen = new HashSet<>(); Iterator it = FlatHashtable.iterator(table, 0, TestCollidingStrategy.INSTANCE); @@ -379,8 +381,8 @@ 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); + FlatHashtable.tryGetOrCreate(table, "a", TestCollidingStrategy.INSTANCE, CREATE); + FlatHashtable.tryGetOrCreate(table, "b", TestCollidingStrategy.INSTANCE, CREATE); Iterator it = FlatHashtable.iterator(table, 5, TestCollidingStrategy.INSTANCE); assertFalse(it.hasNext()); @@ -543,7 +545,7 @@ void entryIterator_emptyRunHasNoNext() { void caseInsensitiveStrategy_matchesRegardlessOfCase() { TestEntry[] table = FlatHashtable.create(TestEntry.class, 4); TestEntry stored = - FlatHashtable.getOrCreate( + FlatHashtable.tryGetOrCreate( table, "Content-Type", TestCaseInsensitiveStrategy.INSTANCE, CREATE); // Look-ups in any case resolve to the same stored entry, allocation-free. @@ -553,10 +555,10 @@ void caseInsensitiveStrategy_matchesRegardlessOfCase() { stored, FlatHashtable.get(table, "CONTENT-TYPE", TestCaseInsensitiveStrategy.INSTANCE)); assertSame( stored, FlatHashtable.get(table, "cOnTeNt-TyPe", TestCaseInsensitiveStrategy.INSTANCE)); - // getOrCreate with a differently-cased key does not mint a second entry. + // tryGetOrCreate with a differently-cased key does not mint a second entry. assertSame( stored, - FlatHashtable.getOrCreate( + FlatHashtable.tryGetOrCreate( table, "CONTENT-TYPE", TestCaseInsensitiveStrategy.INSTANCE, CREATE)); assertNull(FlatHashtable.get(table, "content-length", TestCaseInsensitiveStrategy.INSTANCE)); } @@ -575,7 +577,7 @@ void caseInsensitiveStrategy_doesNotFalseMissOnSupplementaryCasePair() { 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); + FlatHashtable.tryGetOrCreate(table, s1, TestCaseInsensitiveStrategy.INSTANCE, CREATE); if (s1.equalsIgnoreCase(s2)) { assertSame(stored, FlatHashtable.get(table, s2, TestCaseInsensitiveStrategy.INSTANCE)); diff --git a/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java index a3cd4c25247..9905642fa92 100644 --- a/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java +++ b/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java @@ -4,26 +4,29 @@ import static datadog.trace.util.HashtableTestEntries.CollidingKeyEntry; import static datadog.trace.util.HashtableTestEntries.StringIntEntry; 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 java.util.function.ObjLongConsumer; import org.junit.jupiter.api.Test; class HashtableD1Test { @Test void emptyTableLookupReturnsNull() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); assertNull(table.get("missing")); assertEquals(0, table.size()); } @Test void insertedEntryIsRetrievable() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); StringIntEntry e = new StringIntEntry("foo", 1); table.insert(e); assertEquals(1, table.size()); @@ -38,7 +41,8 @@ void keyExposesTheConstructionKey() { @Test void multipleInsertsRetrievableSeparately() { - Hashtable.D1 table = new Hashtable.D1<>(16); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 16); StringIntEntry a = new StringIntEntry("alpha", 1); StringIntEntry b = new StringIntEntry("beta", 2); StringIntEntry c = new StringIntEntry("gamma", 3); @@ -53,7 +57,7 @@ void multipleInsertsRetrievableSeparately() { @Test void inPlaceMutationVisibleViaSubsequentGet() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("counter", 0)); for (int i = 0; i < 10; i++) { StringIntEntry e = table.get("counter"); @@ -64,7 +68,7 @@ void inPlaceMutationVisibleViaSubsequentGet() { @Test void removeUnlinksEntryAndDecrementsSize() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("a", 1)); table.insert(new StringIntEntry("b", 2)); assertEquals(2, table.size()); @@ -79,28 +83,28 @@ void removeUnlinksEntryAndDecrementsSize() { @Test void removeNonexistentReturnsNullAndDoesNotChangeSize() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("a", 1)); assertNull(table.remove("nope")); assertEquals(1, table.size()); } @Test - void insertOrReplaceReturnsPriorEntryOrNullOnInsert() { - Hashtable.D1 table = new Hashtable.D1<>(8); + void tryInsertOrReplaceInsertsThenReplacesWithoutGrowing() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); StringIntEntry first = new StringIntEntry("k", 1); - assertNull(table.insertOrReplace(first), "fresh insert returns null"); + assertTrue(table.tryInsertOrReplace(first), "fresh insert accepted"); assertEquals(1, table.size()); StringIntEntry second = new StringIntEntry("k", 2); - assertSame(first, table.insertOrReplace(second), "replace returns the prior entry"); - assertEquals(1, table.size()); + assertTrue(table.tryInsertOrReplace(second), "replace accepted"); + assertEquals(1, table.size(), "replacing an existing key does not grow the table"); assertSame(second, table.get("k"), "new entry visible after replace"); } @Test void clearEmptiesTheTable() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("a", 1)); table.insert(new StringIntEntry("b", 2)); table.clear(); @@ -113,7 +117,7 @@ void clearEmptiesTheTable() { @Test void forEachVisitsEveryInsertedEntry() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("a", 1)); table.insert(new StringIntEntry("b", 2)); table.insert(new StringIntEntry("c", 3)); @@ -127,7 +131,7 @@ void forEachVisitsEveryInsertedEntry() { @Test void forEachWithContextPassesContextToConsumer() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("a", 10)); table.insert(new StringIntEntry("b", 20)); table.insert(new StringIntEntry("c", 30)); @@ -141,7 +145,7 @@ void forEachWithContextPassesContextToConsumer() { @Test void forEachWithContextOnEmptyTableDoesNothing() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); Map seen = new HashMap<>(); table.forEach(seen, (ctx, e) -> ctx.put(e.key, e.value)); assertEquals(0, seen.size()); @@ -149,7 +153,7 @@ void forEachWithContextOnEmptyTableDoesNothing() { @Test void nullKeyIsPermittedAndDistinctFromAbsent() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); assertNull(table.get(null)); StringIntEntry nullKeyed = new StringIntEntry(null, 7); table.insert(nullKeyed); @@ -163,7 +167,8 @@ void nullKeyIsPermittedAndDistinctFromAbsent() { void hashCollisionsResolveByEquality() { // Force two distinct keys with the same hashCode -- the chain must still distinguish them // via matches(). - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); CollidingKeyEntry e1 = new CollidingKeyEntry(k1, 100); @@ -177,7 +182,8 @@ void hashCollisionsResolveByEquality() { @Test void hashCollisionsThenRemoveLeavesOtherIntact() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); CollidingKey k3 = new CollidingKey("third", 17); @@ -193,10 +199,10 @@ void hashCollisionsThenRemoveLeavesOtherIntact() { @Test void getOrCreateOnMissBuildsEntryViaCreator() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); int[] createCount = {0}; StringIntEntry created = - table.getOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -212,12 +218,12 @@ void getOrCreateOnMissBuildsEntryViaCreator() { @Test void getOrCreateOnHitSkipsCreator() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); StringIntEntry seeded = new StringIntEntry("foo", 1); table.insert(seeded); int[] createCount = {0}; StringIntEntry got = - table.getOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -230,12 +236,243 @@ void getOrCreateOnHitSkipsCreator() { @Test void getOrCreateNullKeyIsPermitted() { - Hashtable.D1 table = new Hashtable.D1<>(8); - StringIntEntry created = table.getOrCreate(null, k -> new StringIntEntry(k, 7)); + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + StringIntEntry created = table.tryGetOrCreateOrNull(null, k -> new StringIntEntry(k, 7)); assertNotNull(created); assertNull(created.key); assertEquals(7, created.value); - assertSame(created, table.getOrCreate(null, k -> new StringIntEntry(k, 999))); + assertSame(created, table.tryGetOrCreateOrNull(null, k -> new StringIntEntry(k, 999))); + assertEquals(1, table.size()); + } + + @Test + void getOrCreateAsMaybeOnMissBuildsEntryViaCreator() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + Maybe maybe = table.tryGetOrCreate("foo", k -> new StringIntEntry(k, 42)); + assertTrue(maybe.isPresent()); + assertEquals(42, maybe.getOrNull().value); + assertSame(table.get("foo"), maybe.getOrNull()); + } + + @Test + void getOrCreateAsMaybeReturnsAbsentOnceAtCapacityButStillReturnsHits() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 2); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + + assertFalse(table.tryGetOrCreate("c", k -> new StringIntEntry(k, 3)).isPresent()); + assertEquals(2, table.size()); + + Maybe hit = table.tryGetOrCreate("a", k -> new StringIntEntry(k, 999)); + assertEquals(1, hit.getOrNull().value, "existing entry is still returned even at capacity"); + } + + @Test + void getOrCreateAsMaybeUpdateAppliesOnlyWhenPresent() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 1); + table.insert(new StringIntEntry("a", 1)); + + ObjLongConsumer add = (e, n) -> e.value += n; + table.tryGetOrCreate("a", k -> new StringIntEntry(k, 0)).update(5L, add); + assertEquals(6, table.get("a").value); + + table.tryGetOrCreate("b", k -> new StringIntEntry(k, 0)).update(5L, add); + assertNull(table.get("b"), "refused create at capacity leaves nothing to update"); + } + + @Test + void insertReturnsFalseOnceAtCapacity() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 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()); + assertNull(table.get("c")); + } + + @Test + void getOrCreateReturnsNullOnceAtCapacityButStillReturnsHits() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 2); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + + assertNull(table.tryGetOrCreateOrNull("c", k -> new StringIntEntry(k, 3))); + assertEquals(2, table.size()); + + StringIntEntry hit = table.tryGetOrCreateOrNull("a", k -> new StringIntEntry(k, 999)); + assertEquals(1, hit.value, "existing entry is still returned even at capacity"); + } + + @Test + void tryInsertOrReplaceStillReplacesAtCapacityButRefusesFreshInsert() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 2); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + + StringIntEntry replacement = new StringIntEntry("a", 99); + assertTrue( + table.tryInsertOrReplace(replacement), "replacing an existing key succeeds when full"); + assertSame(replacement, table.get("a")); + assertEquals(2, table.size()); + + assertFalse( + table.tryInsertOrReplace(new StringIntEntry("c", 3)), + "a fresh insert is refused, not thrown"); + assertEquals(2, table.size()); + assertNull(table.get("c")); + } + + @Test + void isFullReflectsCapacity() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 2); + assertFalse(table.isFull()); + table.insert(new StringIntEntry("a", 1)); + assertFalse(table.isFull()); + table.insert(new StringIntEntry("b", 2)); + assertTrue(table.isFull()); + table.remove("a"); + assertFalse(table.isFull()); + } + + @Test + void drainVisitsEveryEntryThenEmptiesTable() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + Map drained = new HashMap<>(); + + table.drain(e -> drained.put(e.key, e.value)); + + assertEquals(2, drained.size()); + assertEquals(1, drained.get("a")); + assertEquals(2, drained.get("b")); + assertEquals(0, table.size()); + assertNull(table.get("a")); + assertNull(table.get("b")); + + // Table is reusable after drain. + table.insert(new StringIntEntry("c", 3)); assertEquals(1, table.size()); + assertEquals(3, table.get("c").value); } + + @Test + void drainWithContextPassesContextToSink() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + Map drained = new HashMap<>(); + + table.drain(drained, (ctx, e) -> ctx.put(e.key, e.value)); + + assertEquals(2, drained.size()); + assertEquals(0, table.size()); + } + + @Test + void drainOnEmptyTableDoesNothing() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + Map drained = new HashMap<>(); + table.drain(e -> drained.put(e.key, e.value)); + assertEquals(0, drained.size()); + assertEquals(0, table.size()); + } + + @Test + void tryGetOrUpdateCreatesThenAppliesUpdater() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + assertTrue(table.tryGetOrUpdate("a", k -> new StringIntEntry(k, 0), e -> e.value += 5)); + assertEquals(1, table.size()); + assertEquals(5, table.get("a").value); + } + + @Test + void tryGetOrUpdateUpdatesExistingEntryInPlace() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + table.insert(new StringIntEntry("a", 10)); + StringIntEntry existing = table.get("a"); + assertTrue(table.tryGetOrUpdate("a", k -> new StringIntEntry(k, 0), e -> e.value += 5)); + assertEquals(1, table.size()); + assertEquals(15, existing.value); + assertSame(existing, table.get("a")); + } + + @Test + void tryGetOrUpdateReturnsFalseAtCapacityWithoutUpdating() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 2); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + boolean[] updaterRan = {false}; + assertFalse( + table.tryGetOrUpdate( + "c", + k -> new StringIntEntry(k, 0), + e -> { + updaterRan[0] = true; + })); + assertFalse(updaterRan[0], "updater must not run when the create is refused"); + assertEquals(2, table.size()); + assertNull(table.get("c")); + } + + @Test + void tryGetOrUpdateAtCapacityStillUpdatesAnExistingKey() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 2); + table.insert(new StringIntEntry("a", 1)); + table.insert(new StringIntEntry("b", 2)); + assertTrue(table.tryGetOrUpdate("a", k -> new StringIntEntry(k, 0), e -> e.value += 7)); + assertEquals(8, table.get("a").value); + } + + @Test + void tryGetOrUpdateWithContextPassesContextToUpdater() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + // Boxed on purpose: an int literal would bind to the primitive-long overload instead. + assertTrue( + table.tryGetOrUpdate( + "a", k -> new StringIntEntry(k, 0), Integer.valueOf(4), (n, e) -> e.value += n)); + assertTrue( + table.tryGetOrUpdate( + "a", k -> new StringIntEntry(k, 0), Integer.valueOf(6), (n, e) -> e.value += n)); + assertEquals(1, table.size()); + assertEquals(10, table.get("a").value); + } + + @Test + void tryGetOrUpdateWithContextReturnsFalseAtCapacity() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 1); + table.insert(new StringIntEntry("a", 1)); + assertFalse( + table.tryGetOrUpdate( + "b", k -> new StringIntEntry(k, 0), Integer.valueOf(4), (n, e) -> e.value += n)); + assertEquals(1, table.size()); + } + + @Test + void tryGetOrUpdateWithLongContextCreatesThenAccumulates() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); + assertTrue(table.tryGetOrUpdate("a", k -> new StringIntEntry(k, 0), 4L, ADD_LONG)); + assertTrue(table.tryGetOrUpdate("a", k -> new StringIntEntry(k, 0), 6L, ADD_LONG)); + assertEquals(1, table.size()); + assertEquals(10, table.get("a").value); + } + + @Test + void tryGetOrUpdateWithLongContextReturnsFalseAtCapacityWithoutUpdating() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 1); + table.insert(new StringIntEntry("a", 1)); + assertFalse(table.tryGetOrUpdate("b", k -> new StringIntEntry(k, 0), 4L, ADD_LONG)); + assertEquals(1, table.size()); + assertEquals(1, table.get("a").value); + } + + @Test + void tryGetOrUpdateWithLongContextAtCapacityStillUpdatesAnExistingKey() { + Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 1); + table.insert(new StringIntEntry("a", 1)); + assertTrue(table.tryGetOrUpdate("a", k -> new StringIntEntry(k, 0), 4L, ADD_LONG)); + assertEquals(5, table.get("a").value); + } + + private static final ObjLongConsumer ADD_LONG = (e, n) -> e.value += (int) n; } diff --git a/internal-api/src/test/java/datadog/trace/util/HashtableD2Test.java b/internal-api/src/test/java/datadog/trace/util/HashtableD2Test.java index fb621f89482..a7378f55904 100644 --- a/internal-api/src/test/java/datadog/trace/util/HashtableD2Test.java +++ b/internal-api/src/test/java/datadog/trace/util/HashtableD2Test.java @@ -15,7 +15,7 @@ class HashtableD2Test { @Test void pairKeysParticipateInIdentity() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); PairEntry ab = new PairEntry("a", 1, 100); PairEntry ac = new PairEntry("a", 2, 200); PairEntry bb = new PairEntry("b", 1, 300); @@ -31,7 +31,7 @@ void pairKeysParticipateInIdentity() { @Test void removePairUnlinks() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); PairEntry ab = new PairEntry("a", 1, 100); PairEntry ac = new PairEntry("a", 2, 200); table.insert(ab); @@ -43,21 +43,22 @@ void removePairUnlinks() { } @Test - void insertOrReplaceMatchesOnBothKeys() { - Hashtable.D2 table = new Hashtable.D2<>(8); + void tryInsertOrReplaceMatchesOnBothKeys() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); PairEntry first = new PairEntry("k", 7, 1); - assertNull(table.insertOrReplace(first)); + assertTrue(table.tryInsertOrReplace(first)); PairEntry second = new PairEntry("k", 7, 2); - assertSame(first, table.insertOrReplace(second)); + assertTrue(table.tryInsertOrReplace(second)); + assertSame(second, table.get("k", 7), "same key pair replaced in place"); // Different second-key: should insert new, not replace PairEntry third = new PairEntry("k", 8, 3); - assertNull(table.insertOrReplace(third)); + assertTrue(table.tryInsertOrReplace(third)); assertEquals(2, table.size()); } @Test void forEachVisitsBothPairs() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); table.insert(new PairEntry("a", 1, 100)); table.insert(new PairEntry("b", 2, 200)); Set seen = new HashSet<>(); @@ -69,7 +70,7 @@ void forEachVisitsBothPairs() { @Test void forEachWithContextPassesContextToConsumer() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); table.insert(new PairEntry("a", 1, 100)); table.insert(new PairEntry("b", 2, 200)); Set seen = new HashSet<>(); @@ -81,10 +82,10 @@ void forEachWithContextPassesContextToConsumer() { @Test void getOrCreateOnMissBuildsEntryViaCreator() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); int[] createCount = {0}; PairEntry created = - table.getOrCreate( + table.tryGetOrCreateOrNull( "a", 1, (k1, k2) -> { @@ -102,12 +103,12 @@ void getOrCreateOnMissBuildsEntryViaCreator() { @Test void getOrCreateOnHitSkipsCreator() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); PairEntry seeded = new PairEntry("a", 1, 100); table.insert(seeded); int[] createCount = {0}; PairEntry got = - table.getOrCreate( + table.tryGetOrCreateOrNull( "a", 1, (k1, k2) -> { @@ -160,7 +161,7 @@ void entryHashDiffersForDifferentKeys() { @Test void removeReturnsNullForMissingKey() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); table.insert(new PairEntry("a", 1, 100)); assertNull(table.remove("a", 2)); @@ -170,7 +171,7 @@ void removeReturnsNullForMissingKey() { @Test void clearEmptiesTable() { - Hashtable.D2 table = new Hashtable.D2<>(8); + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); table.insert(new PairEntry("a", 1, 100)); table.insert(new PairEntry("b", 2, 200)); assertEquals(2, table.size()); @@ -182,6 +183,175 @@ void clearEmptiesTable() { assertNull(table.get("b", 2)); } + @Test + void insertReturnsFalseOnceAtCapacity() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 2); + assertTrue(table.insert(new PairEntry("a", 1, 100))); + assertTrue(table.insert(new PairEntry("b", 2, 200))); + assertFalse(table.insert(new PairEntry("c", 3, 300))); + assertEquals(2, table.size()); + assertNull(table.get("c", 3)); + } + + @Test + void getOrCreateReturnsNullOnceAtCapacityButStillReturnsHits() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 2); + table.insert(new PairEntry("a", 1, 100)); + table.insert(new PairEntry("b", 2, 200)); + + assertNull(table.tryGetOrCreateOrNull("c", 3, (k1, k2) -> new PairEntry(k1, k2, 300))); + assertEquals(2, table.size()); + + PairEntry hit = table.tryGetOrCreateOrNull("a", 1, (k1, k2) -> new PairEntry(k1, k2, 999)); + assertEquals(100, hit.value, "existing entry is still returned even at capacity"); + } + + @Test + void getOrCreateAsMaybeReturnsAbsentOnceAtCapacityButStillReturnsHits() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 2); + table.insert(new PairEntry("a", 1, 100)); + table.insert(new PairEntry("b", 2, 200)); + + assertFalse(table.tryGetOrCreate("c", 3, (k1, k2) -> new PairEntry(k1, k2, 300)).isPresent()); + assertEquals(2, table.size()); + + Maybe hit = table.tryGetOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 999)); + assertEquals(100, hit.getOrNull().value, "existing entry is still returned even at capacity"); + } + + @Test + void tryInsertOrReplaceStillReplacesAtCapacityButRefusesFreshInsert() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 2); + table.insert(new PairEntry("a", 1, 100)); + table.insert(new PairEntry("b", 2, 200)); + + PairEntry replacement = new PairEntry("a", 1, 999); + assertTrue( + table.tryInsertOrReplace(replacement), "replacing an existing key succeeds when full"); + assertSame(replacement, table.get("a", 1)); + assertEquals(2, table.size()); + + assertFalse( + table.tryInsertOrReplace(new PairEntry("c", 3, 300)), + "a fresh insert is refused, not thrown"); + assertEquals(2, table.size()); + assertNull(table.get("c", 3)); + } + + @Test + void isFullReflectsCapacity() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 2); + assertFalse(table.isFull()); + table.insert(new PairEntry("a", 1, 100)); + assertFalse(table.isFull()); + table.insert(new PairEntry("b", 2, 200)); + assertTrue(table.isFull()); + table.remove("a", 1); + assertFalse(table.isFull()); + } + + @Test + void drainVisitsEveryEntryThenEmptiesTable() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); + table.insert(new PairEntry("a", 1, 100)); + table.insert(new PairEntry("b", 2, 200)); + Set drained = new HashSet<>(); + + table.drain(e -> drained.add(e.key1 + ":" + e.key2)); + + assertEquals(2, drained.size()); + assertTrue(drained.contains("a:1")); + assertTrue(drained.contains("b:2")); + assertEquals(0, table.size()); + assertNull(table.get("a", 1)); + assertNull(table.get("b", 2)); + } + + @Test + void drainWithContextPassesContextToSink() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); + table.insert(new PairEntry("a", 1, 100)); + table.insert(new PairEntry("b", 2, 200)); + Set drained = new HashSet<>(); + + table.drain(drained, (ctx, e) -> ctx.add(e.key1 + ":" + e.key2)); + + assertEquals(2, drained.size()); + assertEquals(0, table.size()); + } + + @Test + void drainOnEmptyTableDoesNothing() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); + Set drained = new HashSet<>(); + table.drain(e -> drained.add(e.key1 + ":" + e.key2)); + assertEquals(0, drained.size()); + assertEquals(0, table.size()); + } + + @Test + void tryGetOrUpdateCreatesThenAppliesUpdater() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); + assertTrue( + table.tryGetOrUpdate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 0), e -> e.value += 5)); + assertEquals(1, table.size()); + assertEquals(5, table.get("a", 1).value); + } + + @Test + void tryGetOrUpdateUpdatesExistingEntryInPlace() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); + table.insert(new PairEntry("a", 1, 10)); + PairEntry existing = table.get("a", 1); + assertTrue( + table.tryGetOrUpdate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 0), e -> e.value += 5)); + assertEquals(1, table.size()); + assertEquals(15, existing.value); + assertSame(existing, table.get("a", 1)); + } + + @Test + void tryGetOrUpdateReturnsFalseAtCapacityWithoutUpdating() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 2); + table.insert(new PairEntry("a", 1, 1)); + table.insert(new PairEntry("b", 2, 2)); + boolean[] updaterRan = {false}; + assertFalse( + table.tryGetOrUpdate( + "c", + 3, + (k1, k2) -> new PairEntry(k1, k2, 0), + e -> { + updaterRan[0] = true; + })); + assertFalse(updaterRan[0], "updater must not run when the create is refused"); + assertEquals(2, table.size()); + assertNull(table.get("c", 3)); + } + + @Test + void tryGetOrUpdateWithContextPassesContextToUpdater() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); + assertTrue( + table.tryGetOrUpdate( + "a", 1, (k1, k2) -> new PairEntry(k1, k2, 0), 4, (n, e) -> e.value += n)); + assertTrue( + table.tryGetOrUpdate( + "a", 1, (k1, k2) -> new PairEntry(k1, k2, 0), 6, (n, e) -> e.value += n)); + assertEquals(1, table.size()); + assertEquals(10, table.get("a", 1).value); + } + + @Test + void tryGetOrUpdateWithContextReturnsFalseAtCapacity() { + Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 1); + table.insert(new PairEntry("a", 1, 1)); + assertFalse( + table.tryGetOrUpdate( + "b", 2, (k1, k2) -> new PairEntry(k1, k2, 0), 4, (n, e) -> e.value += n)); + assertEquals(1, table.size()); + } + private static final class PairEntry extends Hashtable.D2.Entry { int value; diff --git a/internal-api/src/test/java/datadog/trace/util/HashtableTest.java b/internal-api/src/test/java/datadog/trace/util/HashtableTest.java index 953453ca3aa..04b579a3c7f 100644 --- a/internal-api/src/test/java/datadog/trace/util/HashtableTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HashtableTest.java @@ -14,8 +14,9 @@ import datadog.trace.util.Hashtable.BucketIterator; import datadog.trace.util.Hashtable.MutatingBucketIterator; import datadog.trace.util.Hashtable.MutatingTableIterator; -import datadog.trace.util.Hashtable.Support; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.NoSuchElementException; import java.util.Set; import org.junit.jupiter.api.Nested; @@ -23,16 +24,16 @@ class HashtableTest { - // ============ Support ============ + // ============ Static building blocks ============ @Nested - class SupportTests { + class StaticBuildingBlockTests { @Test void createRoundsCapacityUpToPowerOfTwo() { // The Hashtable.D1 / D2 size() reflects entries, but the bucket array length is // a power of two >= requestedCapacity. We can verify indirectly via bucketIndex masking. - Hashtable.Entry[] buckets = Support.create(5); + Hashtable.Entry[] buckets = Hashtable.create(StringIntEntry.class, 5); // Length must be a power of two >= 5 int len = buckets.length; assertTrue(len >= 5); @@ -41,77 +42,159 @@ void createRoundsCapacityUpToPowerOfTwo() { @Test void sizeForReturnsAtLeastOne() { - assertEquals(1, Support.sizeFor(0)); - assertEquals(1, Support.sizeFor(1)); + assertEquals(1, Hashtable.sizeFor(0)); + assertEquals(1, Hashtable.sizeFor(1)); } @Test void sizeForRoundsUpToPowerOfTwo() { - assertEquals(2, Support.sizeFor(2)); - assertEquals(4, Support.sizeFor(3)); - assertEquals(4, Support.sizeFor(4)); - assertEquals(8, Support.sizeFor(5)); - assertEquals(1 << 30, Support.sizeFor(1 << 30)); + assertEquals(2, Hashtable.sizeFor(2)); + assertEquals(4, Hashtable.sizeFor(3)); + assertEquals(4, Hashtable.sizeFor(4)); + assertEquals(8, Hashtable.sizeFor(5)); + assertEquals(1 << 30, Hashtable.sizeFor(1 << 30)); } @Test void sizeForRejectsCapacityAboveMax() { - assertThrows(IllegalArgumentException.class, () -> Support.sizeFor((1 << 30) + 1)); - assertThrows(IllegalArgumentException.class, () -> Support.sizeFor(Integer.MAX_VALUE)); + assertThrows(IllegalArgumentException.class, () -> Hashtable.sizeFor((1 << 30) + 1)); + assertThrows(IllegalArgumentException.class, () -> Hashtable.sizeFor(Integer.MAX_VALUE)); } @Test void sizeForRejectsNegativeCapacity() { - assertThrows(IllegalArgumentException.class, () -> Support.sizeFor(-1)); - assertThrows(IllegalArgumentException.class, () -> Support.sizeFor(Integer.MIN_VALUE)); + assertThrows(IllegalArgumentException.class, () -> Hashtable.sizeFor(-1)); + assertThrows(IllegalArgumentException.class, () -> Hashtable.sizeFor(Integer.MIN_VALUE)); + } + + @Test + void capacityForAppliesDefaultLoadFactorHeadroom() { + // 12 / 0.75 = 16 -> already a power of two. + assertEquals(16, Hashtable.capacityFor(12)); + // 5 / 0.75 = 6.67 -> truncated to 6 -> sizeFor rounds up to 8. + assertEquals(8, Hashtable.capacityFor(5)); + } + + @Test + void capacityForMatchesDefaultLoadFactorConstant() { + assertEquals(0.75f, Hashtable.DEFAULT_LOAD_FACTOR); + assertEquals( + Hashtable.capacityFor(20), Hashtable.capacityFor(20, Hashtable.DEFAULT_LOAD_FACTOR)); + } + + @Test + void capacityForAtExplicitLoadFactor() { + // 10 / 0.5 = 20 -> sizeFor rounds up to 32. + assertEquals(32, Hashtable.capacityFor(10, 0.5f)); + } + + @Test + void capacityForRejectsLoadFactorOutOfRange() { + assertThrows(IllegalArgumentException.class, () -> Hashtable.capacityFor(10, 0f)); + assertThrows(IllegalArgumentException.class, () -> Hashtable.capacityFor(10, 1f)); + assertThrows(IllegalArgumentException.class, () -> Hashtable.capacityFor(10, -0.5f)); + } + + // removeMatching and the size-tracked insertHeadEntryFor are blessed building blocks for + // external composers (e.g. client-side stats) rather than something D1/D2 delegate to, so they + // are covered directly here. + + @Test + void insertHeadEntryForTracksSizeAndRefusesAtCapacity() { + Hashtable.Entry[] buckets = Hashtable.create(2); + Hashtable.SizeManager size = new Hashtable.SizeManager(2); + + StringIntEntry a = new StringIntEntry("a", 1); + StringIntEntry b = new StringIntEntry("b", 2); + StringIntEntry c = new StringIntEntry("c", 3); + + assertTrue(Hashtable.insertHeadEntryFor(size, buckets, a.keyHash, a)); + assertTrue(Hashtable.insertHeadEntryFor(size, buckets, b.keyHash, b)); + assertEquals(2, size.estimateSize()); + + assertFalse( + Hashtable.insertHeadEntryFor(size, buckets, c.keyHash, c), + "refused once the tracker is at capacity"); + assertEquals(2, size.estimateSize(), "a refused insert must not consume a slot"); + } + + @Test + void removeMatchingUnlinksAndDecrements() { + Hashtable.Entry[] buckets = Hashtable.create(8); + Hashtable.SizeManager size = new Hashtable.SizeManager(8); + StringIntEntry a = new StringIntEntry("a", 1); + Hashtable.insertHeadEntryFor(size, buckets, a.keyHash, a); + + StringIntEntry removed = + Hashtable.removeMatching(size, buckets, a.keyHash, e -> e.matches("a")); + + assertSame(a, removed); + assertEquals(0, size.estimateSize()); + assertNull(Hashtable.bucketFor(buckets, a.keyHash)); + } + + @Test + void removeMatchingReturnsNullAndLeavesStateWhenNothingMatches() { + Hashtable.Entry[] buckets = Hashtable.create(8); + Hashtable.SizeManager size = new Hashtable.SizeManager(8); + StringIntEntry a = new StringIntEntry("a", 1); + Hashtable.insertHeadEntryFor(size, buckets, a.keyHash, a); + + assertNull( + Hashtable.removeMatching( + size, buckets, a.keyHash, e -> e.matches("nope"))); + assertEquals(1, size.estimateSize(), "a non-matching scan must not decrement"); + assertSame(a, Hashtable.bucketFor(buckets, a.keyHash)); } @Test void bucketIndexIsBoundedByArrayLength() { - Hashtable.Entry[] buckets = Support.create(16); + Hashtable.Entry[] buckets = Hashtable.create(StringIntEntry.class, 16); for (long h : new long[] {0L, 1L, -1L, Long.MIN_VALUE, Long.MAX_VALUE, 12345L}) { - int idx = Support.bucketIndex(buckets, h); + int idx = Hashtable.bucketIndex(buckets, h); assertTrue(idx >= 0 && idx < buckets.length, "bucketIndex out of range for hash " + h); } } @Test void clearNullsAllBuckets() { - Hashtable.Entry[] buckets = Support.create(4); + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; buckets[0] = new StringIntEntry("x", 1); buckets[1] = new StringIntEntry("y", 2); - Support.clear(buckets); + Hashtable.clear(buckets); for (Hashtable.Entry b : buckets) { assertNull(b); } } @Test - void maxRatioScalesTargetForLoadFactor() { - // 75% load factor => bucket array sized at requestedSize * 4/3, rounded up to power of 2. - // 12 * (4/3) = 16 entries, rounded up to power-of-2 length = 16. - assertEquals(4.0f / 3.0f, Support.MAX_RATIO); - Hashtable.Entry[] buckets = Support.create(12, Support.MAX_RATIO); - assertEquals(16, buckets.length); - } - - @Test - void createWithScaleRoundsUpToPowerOfTwo() { - // 7 * 1.5 = 10.5 -> (int) 10 -> sizeFor rounds up to next power-of-two = 16 - Hashtable.Entry[] buckets = Support.create(7, 1.5f); - assertEquals(16, buckets.length); + void drainVisitsEveryEntryThenClears() { + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; + buckets[0] = new StringIntEntry("x", 1); + buckets[1] = new StringIntEntry("y", 2); + Set drained = new HashSet<>(); + Hashtable.drain(buckets, e -> drained.add(e.key)); + assertEquals(2, drained.size()); + assertTrue(drained.contains("x")); + assertTrue(drained.contains("y")); + for (Hashtable.Entry b : buckets) { + assertNull(b); + } } @Test void insertHeadEntrySplicesAsNewHead() { - Hashtable.Entry[] buckets = Support.create(4); + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; StringIntEntry a = new StringIntEntry("a", 1); StringIntEntry b = new StringIntEntry("b", 2); - Support.insertHeadEntry(buckets, 0, a); + Hashtable.insertHeadEntryAt(buckets, 0, a); assertSame(a, buckets[0]); assertNull(a.next()); - Support.insertHeadEntry(buckets, 0, b); + Hashtable.insertHeadEntryAt(buckets, 0, b); assertSame(b, buckets[0]); assertSame(a, b.next()); assertNull(a.next()); @@ -126,9 +209,10 @@ class BucketIteratorTests { @Test void walksOnlyMatchingHash() { // Build a bucket array with two entries that share a bucket but have different hashes. - // Use Hashtable.D1 to seed; then call Support.bucketIterator directly with the matching + // Use Hashtable.D1 to seed; then call Hashtable.bucketIterator directly with the matching // hash and verify it only returns the matching entry. - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); CollidingKey k3 = new CollidingKey("third", 17); @@ -136,7 +220,7 @@ void walksOnlyMatchingHash() { table.insert(new CollidingKeyEntry(k2, 2)); table.insert(new CollidingKeyEntry(k3, 3)); // All three share the same hash (17), so a bucket iterator over hash=17 yields all three. - BucketIterator it = Support.bucketIterator(table.buckets, 17L); + BucketIterator it = Hashtable.bucketIterator(table.buckets, 17L); int count = 0; while (it.hasNext()) { assertNotNull(it.next()); @@ -147,10 +231,11 @@ void walksOnlyMatchingHash() { @Test void exhaustedIteratorThrowsNoSuchElement() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 4); table.insert(new StringIntEntry("only", 1)); long h = Hashtable.D1.Entry.hash("only"); - BucketIterator it = Support.bucketIterator(table.buckets, h); + BucketIterator it = Hashtable.bucketIterator(table.buckets, h); it.next(); assertFalse(it.hasNext()); assertThrows(NoSuchElementException.class, it::next); @@ -165,7 +250,8 @@ class MutatingBucketIteratorTests { @Test void removeFromHeadOfChainUnlinks() { // Make three entries with the same hash so they chain in one bucket - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); CollidingKey k3 = new CollidingKey("third", 17); @@ -174,7 +260,7 @@ void removeFromHeadOfChainUnlinks() { table.insert(new CollidingKeyEntry(k3, 3)); MutatingBucketIterator it = - Support.mutatingBucketIterator(table.buckets, 17L); + Hashtable.mutatingBucketIterator(table.buckets, 17L); it.next(); // first match (head of chain in insertion-reverse order) it.remove(); // Two should remain @@ -198,7 +284,8 @@ void removeFromHeadOfChainUnlinks() { @Test void replaceSwapsEntryAndPreservesChain() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); CollidingKeyEntry e1 = new CollidingKeyEntry(k1, 1); @@ -207,7 +294,7 @@ void replaceSwapsEntryAndPreservesChain() { table.insert(e2); MutatingBucketIterator it = - Support.mutatingBucketIterator(table.buckets, 17L); + Hashtable.mutatingBucketIterator(table.buckets, 17L); CollidingKeyEntry first = it.next(); CollidingKeyEntry replacement = new CollidingKeyEntry(first.key, 999); it.replace(replacement); @@ -220,10 +307,11 @@ void replaceSwapsEntryAndPreservesChain() { @Test void removeWithoutNextThrows() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 4); table.insert(new StringIntEntry("a", 1)); MutatingBucketIterator it = - Support.mutatingBucketIterator(table.buckets, Hashtable.D1.Entry.hash("a")); + Hashtable.mutatingBucketIterator(table.buckets, Hashtable.D1.Entry.hash("a")); assertThrows(IllegalStateException.class, it::remove); } } @@ -235,13 +323,15 @@ class MutatingTableIteratorTests { @Test void walksEveryEntryAcrossBuckets() { - Hashtable.D1 table = new Hashtable.D1<>(16); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 16); table.insert(new StringIntEntry("a", 1)); table.insert(new StringIntEntry("b", 2)); table.insert(new StringIntEntry("c", 3)); Set seen = new HashSet<>(); - for (MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + for (MutatingTableIterator it = + Hashtable.mutatingTableIterator(table.buckets); it.hasNext(); ) { seen.add(it.next().key); } @@ -253,22 +343,24 @@ void walksEveryEntryAcrossBuckets() { @Test void emptyTableIteratorIsExhausted() { - Hashtable.D1 table = new Hashtable.D1<>(8); - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 8); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); assertFalse(it.hasNext()); assertThrows(NoSuchElementException.class, it::next); } @Test void removeUnlinksBucketHead() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); table.insert(new CollidingKeyEntry(k1, 1)); table.insert(new CollidingKeyEntry(k2, 2)); // The head of the chain is whichever was inserted last (insert prepends). - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); CollidingKeyEntry head = it.next(); it.remove(); @@ -280,7 +372,8 @@ void removeUnlinksBucketHead() { @Test void removeUnlinksMidChainEntry() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 4); CollidingKey k1 = new CollidingKey("first", 17); CollidingKey k2 = new CollidingKey("second", 17); CollidingKey k3 = new CollidingKey("third", 17); @@ -289,7 +382,7 @@ void removeUnlinksMidChainEntry() { table.insert(new CollidingKeyEntry(k3, 3)); // Walk to the second entry, remove it. - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); it.next(); CollidingKeyEntry victim = it.next(); it.remove(); @@ -315,12 +408,13 @@ void removeSkipsOverEmptyBuckets() { // Three distinct keys that land in different buckets (low entry count vs large bucket array // makes empty buckets between them very likely). Verify the iterator skips empties cleanly // after a remove. - Hashtable.D1 table = new Hashtable.D1<>(64); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 64); table.insert(new StringIntEntry("alpha", 1)); table.insert(new StringIntEntry("beta", 2)); table.insert(new StringIntEntry("gamma", 3)); - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); it.next(); it.remove(); int remaining = 0; @@ -333,18 +427,20 @@ void removeSkipsOverEmptyBuckets() { @Test void removeWithoutNextThrows() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 4); table.insert(new StringIntEntry("a", 1)); - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); assertThrows(IllegalStateException.class, it::remove); } @Test void removeTwiceWithoutInterveningNextThrows() { - Hashtable.D1 table = new Hashtable.D1<>(4); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 4); table.insert(new StringIntEntry("a", 1)); table.insert(new StringIntEntry("b", 2)); - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); it.next(); it.remove(); assertThrows(IllegalStateException.class, it::remove); @@ -355,14 +451,15 @@ void halfOpenRangeOmitsBucketsOutsideTheRange() { // CollidingKey lets us pin entries to specific buckets via controlled hashCode. 16-slot // table -> bucketIndex = hash & 15. Place entries in buckets 0, 5, and 10; iterate // [5, 10) -- should see only bucket 5. - Hashtable.D1 table = new Hashtable.D1<>(16); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 16); table.insert(new CollidingKeyEntry(new CollidingKey("b0", 0), 1)); table.insert(new CollidingKeyEntry(new CollidingKey("b5", 5), 2)); table.insert(new CollidingKeyEntry(new CollidingKey("b10", 10), 3)); Set seen = new HashSet<>(); for (MutatingTableIterator it = - Support.mutatingTableIterator(table.buckets, 5, 10); + Hashtable.mutatingTableIterator(table.buckets, 5, 10); it.hasNext(); ) { seen.add(it.next().key.label); } @@ -374,25 +471,28 @@ void halfOpenRangeOmitsBucketsOutsideTheRange() { void emptyHalfOpenRangeIsExhausted() { // start == end -> immediately-exhausted iterator. Important: this is the wrap-around // pass [0, cursor) when cursor == 0 in resumable sweeps. - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 8); table.insert(new StringIntEntry("a", 1)); - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets, 0, 0); + MutatingTableIterator it = + Hashtable.mutatingTableIterator(table.buckets, 0, 0); assertFalse(it.hasNext()); } @Test void rangeBoundsOutOfOrderThrows() { - Hashtable.D1 table = new Hashtable.D1<>(8); + Hashtable.D1 table = + Hashtable.D1.createCapped(StringIntEntry.class, 8); assertThrows( IndexOutOfBoundsException.class, - () -> Support.mutatingTableIterator(table.buckets, -1, 4)); + () -> Hashtable.mutatingTableIterator(table.buckets, -1, 4)); assertThrows( IndexOutOfBoundsException.class, - () -> Support.mutatingTableIterator(table.buckets, 4, 2)); // end < start + () -> Hashtable.mutatingTableIterator(table.buckets, 4, 2)); // end < start assertThrows( IndexOutOfBoundsException.class, () -> - Support.mutatingTableIterator( + Hashtable.mutatingTableIterator( table.buckets, 0, table.buckets.length + 1)); // end > len } @@ -400,13 +500,289 @@ void rangeBoundsOutOfOrderThrows() { void currentBucketReportsLandingIndex() { // Pin one entry to a known bucket and check currentBucket() after next() reports that // bucket. Before any next() (or after remove()), currentBucket() returns -1. - Hashtable.D1 table = new Hashtable.D1<>(16); + Hashtable.D1 table = + Hashtable.D1.createCapped(CollidingKeyEntry.class, 16); table.insert(new CollidingKeyEntry(new CollidingKey("b3", 3), 1)); - MutatingTableIterator it = Support.mutatingTableIterator(table.buckets); + MutatingTableIterator it = Hashtable.mutatingTableIterator(table.buckets); assertEquals(-1, it.currentBucket(), "before any next() currentBucket should be -1"); it.next(); assertEquals(3, it.currentBucket(), "currentBucket should report the entry's bucket"); } } + + // ============ Eviction (SizeManager) ============ + + @Nested + class EvictionTests { + + @Test + void evictOneRemovesFirstMatchAndAdvancesCursor() { + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; + buckets[0] = new StringIntEntry("a", 1); + buckets[1] = new StringIntEntry("b", 2); + StringIntEntry evicted = Hashtable.evictOne(table, e -> e.value == 2); + + assertEquals("b", evicted.key); + assertNull(buckets[1]); + assertNotNull(buckets[0]); + } + + @Test + void tryReserveOrEvictReservesWhileRoomRemains() { + Hashtable.State table = Hashtable.createCapped(2); + + assertTrue(Hashtable.tryReserveOrEvict(table, e -> e.value == 0)); + assertTrue(Hashtable.tryReserveOrEvict(table, e -> e.value == 0)); + assertEquals(2, table.sizeManager.estimateSize()); + } + + @Test + void tryReserveOrEvictMakesRoomWhenFull() { + Hashtable.State table = Hashtable.createCapped(2); + StringIntEntry stale = new StringIntEntry("stale", 0); + StringIntEntry hot = new StringIntEntry("hot", 1); + assertTrue(Hashtable.insertHeadEntryFor(table, stale.keyHash, stale)); + assertTrue(Hashtable.insertHeadEntryFor(table, hot.keyHash, hot)); + assertTrue(table.sizeManager.isFull()); + + // Full, but one entry is evictable -- the slot it frees becomes the reservation. + assertTrue(Hashtable.tryReserveOrEvict(table, e -> e.value == 0)); + assertEquals(2, table.sizeManager.estimateSize(), "one out, one reserved"); + Set remaining = new HashSet<>(); + Hashtable.forEach(table.buckets, e -> remaining.add(e.key)); + assertFalse(remaining.contains("stale"), "the evictable entry is gone"); + assertTrue(remaining.contains("hot"), "the hot entry survived"); + } + + @Test + void tryReserveOrEvictRefusesWhenFullAndNothingEvictable() { + Hashtable.State table = Hashtable.createCapped(1); + StringIntEntry hot = new StringIntEntry("hot", 1); + assertTrue(Hashtable.insertHeadEntryFor(table, hot.keyHash, hot)); + + assertFalse(Hashtable.tryReserveOrEvict(table, e -> e.value == 0)); + assertEquals(1, table.sizeManager.estimateSize(), "a refused reservation consumes nothing"); + Set remaining = new HashSet<>(); + Hashtable.forEach(table.buckets, e -> remaining.add(e.key)); + assertTrue(remaining.contains("hot"), "nothing was evicted"); + } + + @Test + void removeMatchingOverStateNeedsNoTypeWitness() { + Hashtable.State table = Hashtable.createCapped(4); + StringIntEntry a = new StringIntEntry("a", 1); + Hashtable.insertHeadEntryFor(table, a.keyHash, a); + + StringIntEntry removed = Hashtable.removeMatching(table, a.keyHash, e -> e.matches("a")); + + assertSame(a, removed); + assertEquals(0, table.sizeManager.estimateSize()); + } + + @Test + void stateAccessorsAndInsertReservedRoundTrip() { + Hashtable.State table = Hashtable.createCapped(4); + assertTrue(Hashtable.isLikelyEmpty(table)); + assertEquals(0, Hashtable.estimateSize(table)); + + // Reserve first, build second -- a refused reservation must cost no allocation. + assertTrue(Hashtable.tryReserveOrEvict(table, e -> e.value == 0)); + StringIntEntry a = new StringIntEntry("a", 1); + Hashtable.insertReserved(table, a.keyHash, a); + + assertEquals( + 1, Hashtable.estimateSize(table), "insertReserved must not count the entry twice"); + assertFalse(Hashtable.isLikelyEmpty(table)); + assertSame(a, Hashtable.bucketFor(table, a.keyHash), "typed, no witness needed"); + + Set seen = new HashSet<>(); + Hashtable.forEach(table, e -> seen.add(e.key)); + assertEquals(1, seen.size()); + assertTrue(seen.contains("a")); + + Set viaContext = new HashSet<>(); + Hashtable.forEach(table, viaContext, (ctx, e) -> ctx.add(e.key)); + assertTrue(viaContext.contains("a")); + } + + @Test + void clearOverStateEmptiesSpineAndResetsCount() { + Hashtable.State table = Hashtable.createCapped(4); + StringIntEntry a = new StringIntEntry("a", 1); + Hashtable.insertHeadEntryFor(table, a.keyHash, a); + assertEquals(1, table.sizeManager.estimateSize()); + + Hashtable.clear(table); + + assertEquals(0, table.sizeManager.estimateSize()); + assertNull(table.buckets[Hashtable.bucketIndex(table.buckets, a.keyHash)]); + } + + @Test + void evictOneReturnsNullWhenNothingMatches() { + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; + buckets[0] = new StringIntEntry("a", 1); + assertNull(Hashtable.evictOne(table, e -> e.value == 999)); + assertNotNull(buckets[0]); + } + + @Test + void evictAllKeepsCountConsistentWhenThePredicateThrows() { + Hashtable.State table = Hashtable.createCapped(8); + for (int i = 0; i < 4; i++) { + StringIntEntry e = new StringIntEntry("k" + i, i); + assertTrue(Hashtable.insertHeadEntryFor(table, e.keyHash, e)); + } + assertEquals(4, Hashtable.estimateSize(table)); + + // Removes some entries, then blows up. The count must reflect what actually left the table. + assertThrows( + IllegalStateException.class, + () -> + Hashtable.evictAll( + table, + e -> { + if (e.value == 3) { + throw new IllegalStateException("boom"); + } + return true; + })); + + int counted = Hashtable.estimateSize(table); + Set actuallyThere = new HashSet<>(); + Hashtable.forEach(table, e -> actuallyThere.add(e.key)); + assertEquals( + actuallyThere.size(), counted, "count must match the spine after a partial evictAll"); + } + + @Test + void drainDetachesEntriesSoASinkCannotPinTheChain() { + // Two entries forced into one bucket, so the drained pair is chained. + Hashtable.State table = Hashtable.createCapped(4); + CollidingKeyEntry first = new CollidingKeyEntry(new CollidingKey("first", 17), 1); + CollidingKeyEntry second = new CollidingKeyEntry(new CollidingKey("second", 17), 2); + assertTrue(Hashtable.insertHeadEntryFor(table, first.keyHash, first)); + assertTrue(Hashtable.insertHeadEntryFor(table, second.keyHash, second)); + + List drained = new ArrayList<>(); + Hashtable.drain(table, drained::add); + + assertEquals(2, drained.size()); + assertEquals(0, Hashtable.estimateSize(table), "drain resets the tracked count"); + for (CollidingKeyEntry e : drained) { + assertNull(e.next(), "a retained entry must not pin the rest of its chain"); + } + } + + @Test + void evictOneAdvancesCursorEvenWhenNothingMatches() { + Hashtable.State table = Hashtable.createCapped(4); + StringIntEntry a = new StringIntEntry("a", 1); + assertTrue(Hashtable.insertHeadEntryFor(table, a.keyHash, a)); + + // Nothing is evictable, so the scan fails -- but the cursor must still move on. + assertNull(Hashtable.evictOne(table, e -> e.value == 999)); + assertEquals(1, Hashtable.estimateSize(table), "a failed scan evicts nothing"); + + // The cursor has stepped past where the entry sits, so finding it again needs the + // wrap-around pass; that it is still found proves the step did not strand it. + StringIntEntry evicted = Hashtable.evictOne(table, e -> e.value == 1); + assertEquals("a", evicted.key); + assertEquals(0, Hashtable.estimateSize(table)); + } + + @Test + void evictOneWrapsAroundToStartOfTable() { + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; + buckets[0] = new StringIntEntry("a", 1); + buckets[3] = new StringIntEntry("d", 4); + // First eviction matches bucket 3, advancing the cursor there. + StringIntEntry first = Hashtable.evictOne(table, e -> e.value == 4); + assertEquals("d", first.key); + + // Only remaining candidate is bucket 0, before the cursor -- requires wrap-around. + StringIntEntry second = Hashtable.evictOne(table, e -> e.value == 1); + assertEquals("a", second.key); + } + + @Test + void drainRemovesAllMatchesAndResetsCursor() { + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; + buckets[0] = new StringIntEntry("a", 1); + buckets[1] = new StringIntEntry("b", 2); + buckets[2] = new StringIntEntry("c", 3); + Hashtable.evictOne(table, e -> e.value == 3); + + int removed = Hashtable.evictAll(table, e -> e.value < 3); + + assertEquals(2, removed); + assertNull(buckets[0]); + assertNull(buckets[1]); + + // drain resets the cursor to the start, so a fresh scan finds bucket 0 without wrapping. + buckets[0] = new StringIntEntry("a2", 1); + StringIntEntry evicted = Hashtable.evictOne(table, e -> e.value == 1); + assertEquals("a2", evicted.key); + } + + @Test + void resetZeroesCursor() { + Hashtable.State table = Hashtable.createCapped(4); + Hashtable.Entry[] buckets = table.buckets; + buckets[3] = new StringIntEntry("d", 4); + Hashtable.evictOne(table, e -> e.value == 4); + + table.sizeManager.reset(); + + buckets[0] = new StringIntEntry("a", 1); + StringIntEntry evicted = Hashtable.evictOne(table, e -> e.value == 1); + assertEquals("a", evicted.key); + } + } + + // ============ Table ============ + + @Nested + class StateTests { + + @Test + void createTableSizesBucketsWithHeadroomAndCapsSize() { + Hashtable.State table = Hashtable.createCapped(4); + + int len = table.buckets.length; + assertTrue(len >= 4, "backing array must have load-factor headroom over capacity"); + assertEquals(0, len & (len - 1), "length must be a power of two"); + assertNotNull(table.sizeManager); + assertNotNull(table.sizeManager); + assertEquals(4, table.sizeManager.capacity()); + assertFalse(table.sizeManager.isFull()); + } + + @Test + void tableSizeTrackerRespectsCapacity() { + Hashtable.State table = Hashtable.createCapped(1); + + assertTrue(table.sizeManager.tryReserve()); + assertTrue(table.sizeManager.isFull()); + assertFalse(table.sizeManager.tryReserve()); + } + + @Test + void tableSizeManagerOperatesOnItsOwnBuckets() { + Hashtable.State table = Hashtable.createCapped(4); + table.buckets[0] = new StringIntEntry("a", 1); + + StringIntEntry evicted = + (StringIntEntry) + table.sizeManager.evictOne(table.buckets, e -> ((StringIntEntry) e).value == 1); + + assertEquals("a", evicted.key); + assertNull(table.buckets[0]); + } + } } diff --git a/internal-api/src/test/java/datadog/trace/util/MaybeTest.java b/internal-api/src/test/java/datadog/trace/util/MaybeTest.java new file mode 100644 index 00000000000..2420d530bbe --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/MaybeTest.java @@ -0,0 +1,114 @@ +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +public class MaybeTest { + + static final class Widget { + long count; + double total; + } + + @Test + public void ofPresent() { + Maybe maybe = Maybe.of("value"); + assertTrue(maybe.isPresent()); + assertEquals("value", maybe.getOrNull()); + } + + @Test + public void ofAbsent() { + Maybe maybe = Maybe.of(null); + assertFalse(maybe.isPresent()); + assertNull(maybe.getOrNull()); + } + + @Test + public void ofReceiverFunctionPresent() { + Maybe maybe = Maybe.of("value", String::length); + assertTrue(maybe.isPresent()); + assertEquals(5, maybe.getOrNull()); + } + + @Test + public void ofReceiverFunctionAbsent() { + Maybe maybe = Maybe.of("value", r -> null); + assertFalse(maybe.isPresent()); + assertNull(maybe.getOrNull()); + } + + @Test + public void updateConsumerRunsWhenPresent() { + Widget w = new Widget(); + Maybe.of(w).update(widget -> widget.count = 42); + assertEquals(42, w.count); + } + + @Test + public void updateConsumerNoOpWhenAbsent() { + Maybe maybe = Maybe.of(null); + maybe.update(widget -> widget.count = 42); + assertFalse(maybe.isPresent()); + } + + @Test + public void updateBiConsumerRunsWhenPresent() { + Widget w = new Widget(); + Maybe.of(w).update("context", (widget, ctx) -> widget.count = ctx.length()); + assertEquals(7, w.count); + } + + @Test + public void updateBiConsumerNoOpWhenAbsent() { + Maybe maybe = Maybe.of(null); + maybe.update("context", (widget, ctx) -> widget.count = ctx.length()); + assertFalse(maybe.isPresent()); + } + + @Test + public void updateLongRunsWhenPresent() { + Widget w = new Widget(); + Maybe.of(w).update(5L, (widget, delta) -> widget.count += delta); + assertEquals(5, w.count); + } + + @Test + public void updateLongNoOpWhenAbsent() { + Maybe maybe = Maybe.of(null); + maybe.update(5L, (widget, delta) -> widget.count += delta); + assertFalse(maybe.isPresent()); + } + + @Test + public void updateDoubleRunsWhenPresent() { + Widget w = new Widget(); + Maybe.of(w).updateDouble(2.5, (widget, delta) -> widget.total += delta); + assertEquals(2.5, w.total); + } + + @Test + public void updateDoubleNoOpWhenAbsent() { + Maybe maybe = Maybe.of(null); + maybe.updateDouble(2.5, (widget, delta) -> widget.total += delta); + assertFalse(maybe.isPresent()); + } + + @Test + public void ifPresentOrElseRunsActionWhenPresent() { + StringBuilder sb = new StringBuilder(); + Maybe.of("value").ifPresentOrElse(sb::append, () -> sb.append("empty")); + assertEquals("value", sb.toString()); + } + + @Test + public void ifPresentOrElseRunsEmptyActionWhenAbsent() { + StringBuilder sb = new StringBuilder(); + Maybe.of(null).ifPresentOrElse(sb::append, () -> sb.append("empty")); + assertEquals("empty", sb.toString()); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java index 46fa68d040e..966d0d0d4ae 100644 --- a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java +++ b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java @@ -81,6 +81,16 @@ void support_create_then_indexOf() { assertEquals(-1, EmbeddingSupport.indexOf(d.hashes, d.names, "q")); } + @Test + void support_contains_internedAndCopy_andMiss() { + Data d = EmbeddingSupport.create("foo", "bar", "baz"); + + assertTrue( + EmbeddingSupport.contains(d.hashes, d.names, "foo")); // interned literal -> == fast path + assertTrue(EmbeddingSupport.contains(d.hashes, d.names, new String("bar"))); // non-interned + assertFalse(EmbeddingSupport.contains(d.hashes, d.names, "nope")); + } + /** Controlled hashes force collision, linear-probe wraparound, and the already-present path. */ @Test void put_and_indexOf_collisionAndWraparound() {