From d53bc7852b4292ba2ef948760c2ad7fdecc00243 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 26 Aug 2026 18:09:40 -0400 Subject: [PATCH 1/8] Migrate AggregateTable to the blessed Hashtable API Replaces the deprecated Support facade, and the hand-rolled bookkeeping, with Hashtable.State: - four fields (buckets, maxAggregates, size, evictCursor) become one Hashtable.State - evictOneStale's cursor-resumed two-pass scan -- the [cursor, length) then [0, cursor) walk, plus its helper, ~25 lines -- disappears into Hashtable.tryReserveOrEvict, which reserves a slot and only evicts if the table is actually full - expungeStaleAggregates' manual iterator loop becomes evictAll - clear stops pairing three resets by hand - the stale test is a static final Predicate, so eviction allocates no lambda and needs no cast Behaviour is unchanged: same cap, same evict-a-stale-entry-or-drop policy on the miss path, same amortized resumable scan -- that scan just lives in the primitive now instead of here. Co-Authored-By: Claude Opus 5 (1M context) --- .../trace/common/metrics/AggregateTable.java | 82 ++++++------------- 1 file changed, 25 insertions(+), 57 deletions(-) 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..57297f35ec3 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,9 +2,9 @@ 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; +import java.util.function.Predicate; /** * The {@link AggregateEntry} store of the consuming aggregator thread, keyed on the canonical @@ -25,17 +25,20 @@ */ final class AggregateTable { - private final Hashtable.Entry[] buckets; - private final int maxAggregates; - private final AggregateEntry.Canonical canonical; - private int size; + /** + * Stale means "not used in this reporting cycle". Held as a {@code static final} so it is a + * non-capturing singleton rather than a fresh lambda per eviction. + */ + private static final Predicate STALE = entry -> entry.getHitCount() == 0; /** - * 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}. + * Bucket spine plus the manager that keeps it within {@code maxAggregates} -- the manager also + * owns the resumable eviction scan, so consecutive evictions don't re-walk the same hot entries + * clustered near bucket 0. */ - private int evictCursor; + private final Hashtable.State state; + + private final AggregateEntry.Canonical canonical; AggregateTable(int maxAggregates) { this(maxAggregates, AdditionalTagsSchema.EMPTY); @@ -47,8 +50,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); } @@ -57,11 +59,11 @@ void resetCoreHandlers(HealthMetrics healthMetrics, CardinalityLimitReporter rep } int size() { - return size; + return state.sizeManager.size(); } boolean isEmpty() { - return size == 0; + return state.sizeManager.size() == 0; } /** @@ -72,20 +74,20 @@ boolean isEmpty() { 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.buckets, 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, STALE)) { return null; } AggregateEntry entry = canonical.createEntry(); - Hashtable.Support.insertHeadEntry(buckets, keyHash, entry); - size++; + Hashtable.insertHeadEntryFor(state.buckets, keyHash, entry); return entry; } @@ -106,32 +108,8 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { * 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.buckets, consumer); } /** @@ -139,26 +117,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.buckets, 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, STALE); } void clear() { - Hashtable.Support.clear(buckets); - size = 0; - evictCursor = 0; + Hashtable.clear(state); } } From 9ce641f100b567c246f76a8fb37316497fa8cf29 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 26 Aug 2026 18:51:05 -0400 Subject: [PATCH 2/8] Encapsulate the staleness rule as AggregateEntry.isStale The eviction predicate spelled the rule out as hitCount == 0, so the table had to know how staleness is defined. Moving it onto the entry leaves the call site reading AggregateEntry::isStale. That is an unbound instance-method reference, so it still coerces to Predicate and is still non-capturing -- LambdaMetafactory links it to one cached instance, same as the lambda it replaces. Co-Authored-By: Claude Opus 5 (1M context) --- .../datadog/trace/common/metrics/AggregateEntry.java | 11 +++++++++++ .../datadog/trace/common/metrics/AggregateTable.java | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) 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..646725636ec 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,17 @@ 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. Encapsulates the staleness rule on the entry so + * the table doesn't have to know it is spelled {@code hitCount == 0}, and reads as {@code + * AggregateEntry::isStale} at an eviction call site -- an unbound method reference, so it is + * non-capturing and costs no allocation. + */ + 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 57297f35ec3..cac8d539655 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 @@ -29,7 +29,7 @@ final class AggregateTable { * Stale means "not used in this reporting cycle". Held as a {@code static final} so it is a * non-capturing singleton rather than a fresh lambda per eviction. */ - private static final Predicate STALE = entry -> entry.getHitCount() == 0; + private static final Predicate STALE = AggregateEntry::isStale; /** * Bucket spine plus the manager that keeps it within {@code maxAggregates} -- the manager also From 7b779685d5416b555d9fc69e2e1ff1e8921dc99b Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 26 Aug 2026 18:57:01 -0400 Subject: [PATCH 3/8] Reach State only through the statics in AggregateTable Addresses the review comments on #12312, with the API additions made in #12101 and percolated here: state.sizeManager.size() -> Hashtable.size(state) ... == 0 -> Hashtable.isEmpty(state) bucketFor(state.buckets, hash) -> bucketFor(state, hash) insertHeadEntryFor(state.buckets, ...) -> insertReserved(state, ...) forEach(state.buckets, ...) -> forEach(state, ...) No reference to state.buckets or state.sizeManager remains -- what State holds is now its own business. Also drops the field comment that re-documented the eviction cursor living inside it. Co-Authored-By: Claude Opus 5 (1M context) --- .../trace/common/metrics/AggregateTable.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) 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 cac8d539655..5eb5ac9269a 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 @@ -31,11 +31,6 @@ final class AggregateTable { */ private static final Predicate STALE = AggregateEntry::isStale; - /** - * Bucket spine plus the manager that keeps it within {@code maxAggregates} -- the manager also - * owns the resumable eviction scan, so consecutive evictions don't re-walk the same hot entries - * clustered near bucket 0. - */ private final Hashtable.State state; private final AggregateEntry.Canonical canonical; @@ -59,11 +54,11 @@ void resetCoreHandlers(HealthMetrics healthMetrics, CardinalityLimitReporter rep } int size() { - return state.sizeManager.size(); + return Hashtable.size(state); } boolean isEmpty() { - return state.sizeManager.size() == 0; + return Hashtable.isEmpty(state); } /** @@ -74,7 +69,7 @@ boolean isEmpty() { AggregateEntry findOrInsert(SpanSnapshot snapshot) { canonical.populateFrom(snapshot); long keyHash = canonical.keyHash; - for (AggregateEntry candidate = Hashtable.bucketFor(state.buckets, keyHash); + for (AggregateEntry candidate = Hashtable.bucketFor(state, keyHash); candidate != null; candidate = candidate.next()) { if (candidate.keyHash == keyHash && canonical.matches(candidate)) { @@ -87,7 +82,7 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { return null; } AggregateEntry entry = canonical.createEntry(); - Hashtable.insertHeadEntryFor(state.buckets, keyHash, entry); + Hashtable.insertReserved(state, keyHash, entry); return entry; } @@ -109,7 +104,7 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { * backstop. */ void forEach(Consumer consumer) { - Hashtable.forEach(state.buckets, consumer); + Hashtable.forEach(state, consumer); } /** @@ -118,7 +113,7 @@ void forEach(Consumer consumer) { * plus whatever side-band state it needs as {@code context}. */ void forEach(C context, BiConsumer consumer) { - Hashtable.forEach(state.buckets, context, consumer); + Hashtable.forEach(state, context, consumer); } /** Removes entries whose {@code getHitCount() == 0}. */ From d3eab278abf0141449fdae32c3752464ccb1d847 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 26 Aug 2026 20:07:28 -0400 Subject: [PATCH 4/8] Follow the estimateSize/isLikelyEmpty rename in AggregateTable Notes why AggregateTable.size() stays exact despite delegating to an estimate: findOrInsert reserves and links without yielding, so the reservation window is never observable from outside this class. Co-Authored-By: Claude Opus 5 (1M context) --- .../datadog/trace/common/metrics/AggregateTable.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 5eb5ac9269a..2e6a959f139 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 @@ -53,12 +53,17 @@ 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 Hashtable.size(state); + return Hashtable.estimateSize(state); } boolean isEmpty() { - return Hashtable.isEmpty(state); + return Hashtable.isLikelyEmpty(state); } /** From 9be03b39f7e02b1f0a1f8b1089692059ea9a7e3c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 26 Aug 2026 20:21:26 -0400 Subject: [PATCH 5/8] Rehome the eviction rationale after deleting evictOneStale Deleting evictOneStale left its javadoc behind, where it silently attached to forEach -- so forEach claimed to unlink stale entries and linked #evictCursor, a field that no longer exists. The mechanical half of that text (cursor-resumed two-pass scan, its amortization) now belongs to Hashtable.tryReserveOrEvict, so it goes. The domain half is knowledge this class still owns and nothing else records: why a full table drops the new key instead of evicting an established one, and why cardinality limiting reduces but does not eliminate eviction. That moves onto findOrInsert, where the decision is actually made. Co-Authored-By: Claude Opus 5 (1M context) --- .../trace/common/metrics/AggregateTable.java | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) 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 2e6a959f139..dd03187ad55 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 @@ -69,7 +69,19 @@ boolean isEmpty() { /** * 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 #STALE}. */ AggregateEntry findOrInsert(SpanSnapshot snapshot) { canonical.populateFrom(snapshot); @@ -91,23 +103,6 @@ AggregateEntry findOrInsert(SpanSnapshot snapshot) { 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. - */ void forEach(Consumer consumer) { Hashtable.forEach(state, consumer); } From 8435b639f54331690e7a81e25fc2c6bd35675816 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 26 Aug 2026 20:28:57 -0400 Subject: [PATCH 6/8] Delete the deprecated Hashtable.Support facade Nothing references it any more: this PR moved the last production caller (AggregateTable) onto the blessed statics, and the facade held no logic of its own -- every member was a one-line delegate. Removes 174 lines from Hashtable and the 135-line DeprecatedSupportTests group, most of which asserted only that a one-liner forwards. The two members that did have unique behaviour, create(int, float) and MAX_RATIO, are covered by capacityFor(int, float), which has its own tests. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/datadog/trace/util/Hashtable.java | 174 ------------------ .../datadog/trace/util/HashtableTest.java | 138 -------------- 2 files changed, 312 deletions(-) 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 537f2a4c5b3..a1a0505f893 100644 --- a/internal-api/src/main/java/datadog/trace/util/Hashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/Hashtable.java @@ -1347,180 +1347,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/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 From 7dcce97820d8e516bd1cb8183dd2e5b406d2e9f7 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 27 Aug 2026 13:09:20 -0400 Subject: [PATCH 7/8] Fuse the CardinalityLimitReporter counter bump into tryGetOrUpdate Removes the nullable that only ever appears once the tag table is at capacity -- the shape most likely to ship as a rare production NPE. The primitive-long overload keeps record() allocation-free. Co-Authored-By: Claude Opus 5 --- .../common/metrics/CardinalityLimitReporter.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) 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..ee16129a0a8 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 @@ -4,6 +4,7 @@ import datadog.logging.RatelimitedLogger; import datadog.trace.util.Hashtable; +import java.util.function.ObjLongConsumer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -55,13 +56,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 {@code false} return -- 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.tryGetOrUpdate(tag, TagBlockEntry::new, count, ADD_BLOCKED); } } @@ -100,6 +103,9 @@ private String summarize() { /** * Single-key counter entry: the tag name (via {@link #key()}) plus its in-place-mutated count. */ + /** Non-capturing, so {@link #record} allocates nothing per call. */ + private static final ObjLongConsumer ADD_BLOCKED = (entry, n) -> entry.count += n; + private static final class TagBlockEntry extends Hashtable.D1.Entry { long count; From 6bc66323027877c43ec75302428d9474b48e64b0 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 27 Aug 2026 13:09:22 -0400 Subject: [PATCH 8/8] Rewrap an AggregateTable javadoc paragraph per spotless Co-Authored-By: Claude Opus 5 --- .../java/datadog/trace/common/metrics/AggregateTable.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 dd03187ad55..d7d726935f3 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 @@ -69,10 +69,10 @@ boolean isEmpty() { /** * 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 (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. + * 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