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 fc64b9015d7..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 @@ -55,13 +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) { - TagBlockEntry entry = blockedByTag.tryGetOrCreate(tag, TagBlockEntry::new); - if (entry != null) { - entry.count += count; - } + blockedByTag.tryGetOrCreate(tag, TagBlockEntry::new).update(count, TagBlockEntry::inc); } } @@ -106,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/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/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: + * + *

+ */ +@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 f8f6731d9a7..6b78c1c4539 100644 --- a/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/FlatHashtable.java @@ -123,12 +123,12 @@ protected Entry(long hash) { * *

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 - * #tryGetOrCreate} caps and returns {@code null} (the caller supplies the overflow default). - * {@link #createGrowable} trades that for {@code HashMap}-like ergonomics — its {@code + * #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 tryGetOrCreate} 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 + * 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. * @@ -197,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 #tryGetOrCreate} returns {@code null}). + * #DEFAULT_LOAD_FACTOR}, then capping ({@link #tryGetOrCreateOrNull} returns {@code null}). */ @Nonnull public static > D1 createFixed( @@ -257,18 +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. The name has to serve both postures, since the posture is chosen per instance at the - * factory while the method name is per class, and the two mistakes are not symmetric: - * under-promising refusal costs an NPE at the cap, over-promising it costs a redundant null - * check. So it errs toward {@code try}. + * 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 tryGetOrCreate(@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; @@ -393,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 #tryGetOrCreate} returns {@code null}). + * #DEFAULT_LOAD_FACTOR}, then capping ({@link #tryGetOrCreateOrNull} returns {@code null}). */ @Nonnull public static > D2 createFixed( @@ -452,11 +467,25 @@ public TEntry get(@Nullable K1 key1, @Nullable K2 key2) { } /** - * Two-key analogue of {@link D1#tryGetOrCreate}: 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 tryGetOrCreate( + public TEntry tryGetOrCreateOrNull( @Nullable K1 key1, @Nullable K2 key2, @Nonnull CreateStrategy2 createStrat) { 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 3692a365f76..81e5daa7a0f 100644 --- a/internal-api/src/main/java/datadog/trace/util/Hashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/Hashtable.java @@ -99,8 +99,8 @@ public final TEntry next() { * *

Capacity is fixed at construction. The table does not resize, so the caller is responsible * for choosing a capacity appropriate to the working set. Once {@link #size()} reaches that - * capacity, {@link #insert} returns {@code false} and {@link #tryGetOrCreate} returns {@code - * null} rather than adding more entries -- a lookup hit is still always returned even at + * 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 @@ -172,8 +172,8 @@ private D1(int maxCapacity) { /** * A capped single-key table: it holds at most {@code maxCapacity} live entries, after - * which {@link #insert} returns {@code false} and {@link #tryGetOrCreate} returns {@code null}. - * A lookup hit is still always returned at capacity -- the cap only blocks new entries. + * 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 @@ -292,17 +292,16 @@ public boolean tryInsertOrReplace(@Nonnull TEntry newEntry) { } /** - * Returns the entry for {@code key}, building one via {@code creator} if absent -- or {@code - * null} if the key is absent and the table is at capacity. This method can refuse: - * despite the name it is not total, and a caller that dereferences the result without a null - * check will NPE the first time the cap is reached. 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. + * 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 the {@code null} turns the cap into data loss you - * cannot see. + * 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. @@ -311,9 +310,26 @@ public boolean tryInsertOrReplace(@Nonnull TEntry newEntry) { * 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}. + */ + @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 tryGetOrCreate( + public TEntry tryGetOrCreateOrNull( @Nullable K key, @Nonnull Function creator) { long keyHash = D1.Entry.hash(key); for (TEntry curEntry = bucketFor(this.buckets, keyHash); @@ -337,7 +353,8 @@ public TEntry tryGetOrCreate( } /** - * {@link #tryGetOrCreate} followed by {@code updater}, returning whether the update happened. + * {@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: @@ -348,19 +365,19 @@ public TEntry tryGetOrCreate( * *

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 - * tryGetOrCreate(...).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. + * 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 #tryGetOrCreate}. + * {@link #tryGetOrCreateOrNull}. */ public boolean tryGetOrUpdate( @Nullable K key, @Nonnull Function creator, @Nonnull Consumer updater) { - TEntry entry = tryGetOrCreate(key, creator); + TEntry entry = tryGetOrCreateOrNull(key, creator); if (entry == null) { return false; } @@ -379,7 +396,7 @@ public boolean tryGetOrUpdate( @Nonnull Function creator, C context, @Nonnull BiConsumer updater) { - TEntry entry = tryGetOrCreate(key, creator); + TEntry entry = tryGetOrCreateOrNull(key, creator); if (entry == null) { return false; } @@ -404,7 +421,7 @@ public boolean tryGetOrUpdate( @Nonnull Function creator, long context, @Nonnull ObjLongConsumer updater) { - TEntry entry = tryGetOrCreate(key, creator); + TEntry entry = tryGetOrCreateOrNull(key, creator); if (entry == null) { return false; } @@ -533,8 +550,8 @@ private D2(int 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 #tryGetOrCreate} returns {@code null}, with lookup hits still always returned. See - * {@link D1#createCapped} for what "capped" promises and why it is the default posture. + * {@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 @@ -615,17 +632,31 @@ public boolean tryInsertOrReplace(@Nonnull TEntry newEntry) { /** * Two-key analogue of {@link D1#tryGetOrCreate}: returns the entry for {@code (key1, key2)}, - * building one via {@code creator} if absent -- or {@code null} if the pair is absent and the - * table is at capacity. Like the single-key form it is not total despite the name, and - * 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. + * 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. + */ + @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 tryGetOrCreate( + public TEntry tryGetOrCreateOrNull( @Nullable K1 key1, @Nullable K2 key2, @Nonnull BiFunction creator) { @@ -655,14 +686,14 @@ public TEntry tryGetOrCreate( * 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 tryGetOrCreate(...)} followed by a dereference. + * {@code tryGetOrCreateOrNull(...)} followed by a dereference. */ public boolean tryGetOrUpdate( @Nullable K1 key1, @Nullable K2 key2, @Nonnull BiFunction creator, @Nonnull Consumer updater) { - TEntry entry = tryGetOrCreate(key1, key2, creator); + TEntry entry = tryGetOrCreateOrNull(key1, key2, creator); if (entry == null) { return false; } @@ -682,7 +713,7 @@ public boolean tryGetOrUpdate( @Nonnull BiFunction creator, C context, @Nonnull BiConsumer updater) { - TEntry entry = tryGetOrCreate(key1, key2, creator); + TEntry entry = tryGetOrCreateOrNull(key1, key2, creator); if (entry == null) { return false; } @@ -1466,180 +1497,6 @@ public static State createCapped(int maxCapacity) return new State<>(buckets, maxCapacity); } - /** - * Deprecated facade over the static building blocks that are now methods on {@link Hashtable} - * itself. Every member here delegates to its {@code Hashtable.*} counterpart -- no real logic - * lives in this class, so it can be deleted outright once the last caller migrates. - * - *

Retained only for source compatibility with existing callers. New code should call the - * {@code Hashtable.*} statics directly. - * - * @deprecated use the static building blocks on {@link Hashtable} directly. - */ - @Deprecated - public static final class Support { - private Support() {} - - /** - * @deprecated use {@link Hashtable#create(int)} (or {@link Hashtable#create(Class, int)} for a - * typed spine). - */ - @Deprecated - @Nonnull - public static Hashtable.Entry[] create(int requestedSize) { - return Hashtable.create(requestedSize); - } - - /** - * 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 - * Hashtable#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}). - * - * @deprecated use {@link Hashtable#capacityFor(int)} (or {@link Hashtable#capacityFor(int, - * float)} for a load factor other than {@link Hashtable#DEFAULT_LOAD_FACTOR}), then {@link - * Hashtable#create(Class, int)} with the result. - */ - @Deprecated - @Nonnull - public static Hashtable.Entry[] create(int requestedSize, float scale) { - // Deliberately multiplies by `scale` rather than routing through - // Hashtable#capacityFor(int, float), which divides by a load factor: `n * MAX_RATIO` and - // `n / DEFAULT_LOAD_FACTOR` are not bit-identical in float, and this deprecated path keeps - // its exact legacy sizing. Only the allocation itself is inverted onto the blessed API. - return Hashtable.create((int) (requestedSize * scale)); - } - - /** - * 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. - * - * @deprecated equivalent to {@code 1f / Hashtable#DEFAULT_LOAD_FACTOR}; prefer {@link - * Hashtable#capacityFor(int)}, which applies that load factor directly. - */ - @Deprecated public static final float MAX_RATIO = 1.0f / Hashtable.DEFAULT_LOAD_FACTOR; - - /** - * @deprecated use {@link Hashtable#sizeFor(int)}. - */ - @Deprecated - static int sizeFor(int requestedSize) { - return Hashtable.sizeFor(requestedSize); - } - - /** - * @deprecated use {@link Hashtable#clear(Hashtable.Entry[])}. - */ - @Deprecated - public static void clear(@Nonnull Hashtable.Entry[] buckets) { - Hashtable.clear(buckets); - } - - /** - * @deprecated use {@link Hashtable#bucketIterator(Hashtable.Entry[], long)}. - */ - @Deprecated - @Nonnull - public static BucketIterator bucketIterator( - @Nonnull Hashtable.Entry[] buckets, long keyHash) { - return Hashtable.bucketIterator(buckets, keyHash); - } - - /** - * @deprecated use {@link Hashtable#mutatingBucketIterator(Hashtable.Entry[], long)}. - */ - @Deprecated - @Nonnull - public static - MutatingBucketIterator mutatingBucketIterator( - @Nonnull Hashtable.Entry[] buckets, long keyHash) { - return Hashtable.mutatingBucketIterator(buckets, keyHash); - } - - /** - * @deprecated use {@link Hashtable#mutatingTableIterator(Hashtable.Entry[])}. - */ - @Deprecated - @Nonnull - public static - MutatingTableIterator mutatingTableIterator(@Nonnull Hashtable.Entry[] buckets) { - return Hashtable.mutatingTableIterator(buckets); - } - - /** - * @deprecated use {@link Hashtable#mutatingTableIterator(Hashtable.Entry[], int, int)}. - */ - @Deprecated - @Nonnull - public static - MutatingTableIterator mutatingTableIterator( - @Nonnull Hashtable.Entry[] buckets, int startBucket, int endBucket) { - return Hashtable.mutatingTableIterator(buckets, startBucket, endBucket); - } - - /** - * @deprecated use {@link Hashtable#bucketIndex(Object[], long)}. - */ - @Deprecated - public static int bucketIndex(@Nonnull Object[] buckets, long keyHash) { - return Hashtable.bucketIndex(buckets, keyHash); - } - - /** - * @deprecated use {@link Hashtable#insertHeadEntryAt(Hashtable.Entry[], int, Hashtable.Entry)}. - */ - @Deprecated - public static void insertHeadEntry( - @Nonnull Hashtable.Entry[] buckets, int bucketIndex, @Nonnull Hashtable.Entry entry) { - Hashtable.insertHeadEntryAt(buckets, bucketIndex, entry); - } - - /** - * @deprecated use {@link Hashtable#insertHeadEntryFor(Hashtable.Entry[], long, - * Hashtable.Entry)}. - */ - @Deprecated - public static void insertHeadEntry( - @Nonnull Hashtable.Entry[] buckets, long keyHash, @Nonnull Hashtable.Entry entry) { - Hashtable.insertHeadEntryFor(buckets, keyHash, entry); - } - - /** - * @deprecated use {@link Hashtable#bucketFor(Hashtable.Entry[], long)}. - */ - @Deprecated - @Nullable - public static TEntry bucket( - @Nonnull Hashtable.Entry[] buckets, long keyHash) { - return Hashtable.bucketFor(buckets, keyHash); - } - - /** - * @deprecated use {@link Hashtable#forEach(Hashtable.Entry[], Consumer)}. - */ - @Deprecated - public static void forEach( - @Nonnull Hashtable.Entry[] buckets, @Nonnull Consumer consumer) { - Hashtable.forEach(buckets, consumer); - } - - /** - * @deprecated use {@link Hashtable#forEach(Hashtable.Entry[], Object, BiConsumer)}. - */ - @Deprecated - public static void forEach( - @Nonnull Hashtable.Entry[] buckets, - C context, - @Nonnull BiConsumer consumer) { - Hashtable.forEach(buckets, context, consumer); - } - } - /** * 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 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/test/java/datadog/trace/util/FlatHashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/FlatHashtableD1Test.java index e51462451aa..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.tryGetOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -209,7 +209,7 @@ void getOrCreateOnHitSkipsCreator() { table.insert(seeded); int[] createCount = {0}; StringIntEntry got = - table.tryGetOrCreate( + 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.tryGetOrCreate("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.tryGetOrCreate("a", k -> new StringIntEntry(k, 1))); - assertNotNull(table.tryGetOrCreate("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.tryGetOrCreate("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.tryGetOrCreate("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 0900617035e..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.tryGetOrCreate( + table.tryGetOrCreateOrNull( "foo", 1, (k1, k2) -> { @@ -190,7 +190,7 @@ void getOrCreateOnHitSkipsCreator() { table.insert(seeded); int[] createCount = {0}; PairEntry got = - table.tryGetOrCreate( + table.tryGetOrCreateOrNull( "foo", 1, (k1, k2) -> { @@ -217,13 +217,35 @@ void growableGrowsPastInitialCapacity() { @Test void fixedGetOrCreateCapsWhenFull() { FlatHashtable.D2 table = fixed(2); - assertNotNull(table.tryGetOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 1))); - assertNotNull(table.tryGetOrCreate("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.tryGetOrCreate("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.tryGetOrCreate("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.tryGetOrCreate("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/HashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java index 2c1a28b4bf4..9905642fa92 100644 --- a/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java +++ b/internal-api/src/test/java/datadog/trace/util/HashtableD1Test.java @@ -202,7 +202,7 @@ void getOrCreateOnMissBuildsEntryViaCreator() { Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); int[] createCount = {0}; StringIntEntry created = - table.tryGetOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -223,7 +223,7 @@ void getOrCreateOnHitSkipsCreator() { table.insert(seeded); int[] createCount = {0}; StringIntEntry got = - table.tryGetOrCreate( + table.tryGetOrCreateOrNull( "foo", k -> { createCount[0]++; @@ -237,14 +237,49 @@ void getOrCreateOnHitSkipsCreator() { @Test void getOrCreateNullKeyIsPermitted() { Hashtable.D1 table = Hashtable.D1.createCapped(StringIntEntry.class, 8); - StringIntEntry created = table.tryGetOrCreate(null, k -> new StringIntEntry(k, 7)); + StringIntEntry created = table.tryGetOrCreateOrNull(null, k -> new StringIntEntry(k, 7)); assertNotNull(created); assertNull(created.key); assertEquals(7, created.value); - assertSame(created, table.tryGetOrCreate(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); @@ -261,10 +296,10 @@ void getOrCreateReturnsNullOnceAtCapacityButStillReturnsHits() { table.insert(new StringIntEntry("a", 1)); table.insert(new StringIntEntry("b", 2)); - assertNull(table.tryGetOrCreate("c", k -> new StringIntEntry(k, 3))); + assertNull(table.tryGetOrCreateOrNull("c", k -> new StringIntEntry(k, 3))); assertEquals(2, table.size()); - StringIntEntry hit = table.tryGetOrCreate("a", k -> new StringIntEntry(k, 999)); + StringIntEntry hit = table.tryGetOrCreateOrNull("a", k -> new StringIntEntry(k, 999)); assertEquals(1, hit.value, "existing entry is still returned even at capacity"); } 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 16739c4a9a5..a7378f55904 100644 --- a/internal-api/src/test/java/datadog/trace/util/HashtableD2Test.java +++ b/internal-api/src/test/java/datadog/trace/util/HashtableD2Test.java @@ -85,7 +85,7 @@ void getOrCreateOnMissBuildsEntryViaCreator() { Hashtable.D2 table = Hashtable.D2.createCapped(PairEntry.class, 8); int[] createCount = {0}; PairEntry created = - table.tryGetOrCreate( + table.tryGetOrCreateOrNull( "a", 1, (k1, k2) -> { @@ -108,7 +108,7 @@ void getOrCreateOnHitSkipsCreator() { table.insert(seeded); int[] createCount = {0}; PairEntry got = - table.tryGetOrCreate( + table.tryGetOrCreateOrNull( "a", 1, (k1, k2) -> { @@ -199,13 +199,26 @@ void getOrCreateReturnsNullOnceAtCapacityButStillReturnsHits() { table.insert(new PairEntry("a", 1, 100)); table.insert(new PairEntry("b", 2, 200)); - assertNull(table.tryGetOrCreate("c", 3, (k1, k2) -> new PairEntry(k1, k2, 300))); + assertNull(table.tryGetOrCreateOrNull("c", 3, (k1, k2) -> new PairEntry(k1, k2, 300))); assertEquals(2, table.size()); - PairEntry hit = table.tryGetOrCreate("a", 1, (k1, k2) -> new PairEntry(k1, k2, 999)); + 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); 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 91bead646fd..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,7 +14,6 @@ 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; @@ -202,143 +201,6 @@ void insertHeadEntrySplicesAsNewHead() { } } - // ============ Deprecated Support facade ============ - - /** - * The scaled {@code create(int, float)} factory and {@code MAX_RATIO} are deprecated-only: they - * have no blessed equivalent on {@link Hashtable} but remain in use by client-side statistics, so - * they keep dedicated coverage here. - */ - @Nested - @SuppressWarnings("deprecation") - class DeprecatedSupportTests { - - @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); - } - - @Test - void createWithoutScaleDelegatesToHashtableSizeFor() { - Hashtable.Entry[] buckets = Support.create(5); - assertEquals(Hashtable.create(StringIntEntry.class, 5).length, buckets.length); - } - - @Test - void clearDelegatesToHashtableClear() { - Hashtable.Entry[] buckets = Support.create(4); - buckets[0] = new StringIntEntry("a", 1); - Support.clear(buckets); - for (Hashtable.Entry b : buckets) { - assertNull(b); - } - } - - @Test - void bucketIndexDelegatesToHashtableBucketIndex() { - Hashtable.Entry[] buckets = Support.create(4); - long hash = StringIntEntry.hash("a"); - assertEquals(Hashtable.bucketIndex(buckets, hash), Support.bucketIndex(buckets, hash)); - } - - @Test - void insertHeadEntryByIndexDelegatesToHashtableInsertHeadEntryAt() { - Hashtable.Entry[] buckets = Support.create(4); - StringIntEntry entry = new StringIntEntry("a", 1); - Support.insertHeadEntry(buckets, 0, entry); - assertSame(entry, buckets[0]); - } - - @Test - void insertHeadEntryByHashDelegatesToHashtableInsertHeadEntryFor() { - Hashtable.Entry[] buckets = Support.create(4); - StringIntEntry entry = new StringIntEntry("a", 1); - Support.insertHeadEntry(buckets, entry.keyHash, entry); - assertSame(entry, Support.bucket(buckets, entry.keyHash)); - } - - @Test - void bucketDelegatesToHashtableBucketFor() { - Hashtable.Entry[] buckets = Support.create(4); - StringIntEntry entry = new StringIntEntry("a", 1); - Support.insertHeadEntry(buckets, entry.keyHash, entry); - assertSame(entry, Support.bucket(buckets, entry.keyHash)); - } - - @Test - void bucketIteratorDelegatesToHashtableBucketIterator() { - Hashtable.Entry[] buckets = Support.create(4); - StringIntEntry entry = new StringIntEntry("a", 1); - Support.insertHeadEntry(buckets, entry.keyHash, entry); - BucketIterator it = Support.bucketIterator(buckets, entry.keyHash); - assertTrue(it.hasNext()); - assertSame(entry, it.next()); - } - - @Test - void mutatingBucketIteratorDelegatesToHashtableMutatingBucketIterator() { - Hashtable.Entry[] buckets = Support.create(4); - StringIntEntry entry = new StringIntEntry("a", 1); - Support.insertHeadEntry(buckets, entry.keyHash, entry); - MutatingBucketIterator it = - Support.mutatingBucketIterator(buckets, entry.keyHash); - assertTrue(it.hasNext()); - assertSame(entry, it.next()); - it.remove(); - assertNull(Support.bucket(buckets, entry.keyHash)); - } - - @Test - void mutatingTableIteratorOverFullTableDelegatesToHashtable() { - Hashtable.Entry[] buckets = Support.create(4); - buckets[0] = new StringIntEntry("a", 1); - MutatingTableIterator it = Support.mutatingTableIterator(buckets); - assertTrue(it.hasNext()); - assertEquals("a", it.next().key); - } - - @Test - void mutatingTableIteratorOverRangeDelegatesToHashtable() { - Hashtable.Entry[] buckets = Support.create(4); - buckets[0] = new StringIntEntry("a", 1); - buckets[2] = new StringIntEntry("b", 2); - MutatingTableIterator it = Support.mutatingTableIterator(buckets, 0, 2); - assertTrue(it.hasNext()); - assertEquals("a", it.next().key); - assertFalse(it.hasNext(), "range end is exclusive"); - } - - @Test - void forEachDelegatesToHashtableForEach() { - Hashtable.Entry[] buckets = Support.create(4); - buckets[0] = new StringIntEntry("a", 1); - buckets[1] = new StringIntEntry("b", 2); - Set seen = new HashSet<>(); - Support.forEach(buckets, e -> seen.add(e.key)); - assertEquals(2, seen.size()); - } - - @Test - void forEachWithContextDelegatesToHashtableForEach() { - Hashtable.Entry[] buckets = Support.create(4); - buckets[0] = new StringIntEntry("a", 1); - Set seen = new HashSet<>(); - Support., StringIntEntry>forEach(buckets, seen, (ctx, e) -> ctx.add(e.key)); - assertEquals(1, seen.size()); - } - } - // ============ BucketIterator ============ @Nested 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()); + } +}