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 b120cb8b915..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 @@ -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,15 @@ */ final class AggregateTable { - private final Hashtable.Entry[] buckets; - private final int maxAggregates; - private final AggregateEntry.Canonical canonical; - private int size; - /** - * 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}. + * 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 int evictCursor; + private static final Predicate STALE = AggregateEntry::isStale; + + private final Hashtable.State state; + + private final AggregateEntry.Canonical canonical; AggregateTable(int maxAggregates) { this(maxAggregates, AdditionalTagsSchema.EMPTY); @@ -47,8 +45,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 +53,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 #STALE}. */ 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, STALE)) { 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 +112,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, STALE); } 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..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; 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