diff --git a/internal-api/src/jmh/java/datadog/trace/api/telemetry/LogCollectorBenchmark.java b/internal-api/src/jmh/java/datadog/trace/api/telemetry/LogCollectorBenchmark.java index 81d2a9180b6..3754ee33fde 100644 --- a/internal-api/src/jmh/java/datadog/trace/api/telemetry/LogCollectorBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/api/telemetry/LogCollectorBenchmark.java @@ -60,4 +60,47 @@ public void unsupportedOperationException(CollectorState state) { static void unsupportedOperation() { throw new UnsupportedOperationException(); } + + /** + * Exercises the near-capacity path the other benchmarks skip: capacity is well below the number + * of distinct keys in play, so once warmed up the table stays full and most calls miss {@code + * find()}'s lock-free scan and fall through to {@code tryReserve} -- including its locked recheck + * for a concurrent duplicate. {@link #duplicateWithoutException} and friends only ever hit the + * lock-free fast path, so they don't touch that code at all. + */ + @State(Scope.Benchmark) + public static class ContendedCollectorState { + static final int N_KEYS = 32; + static final String[] MESSAGES = new String[N_KEYS]; + + static { + for (int i = 0; i < N_KEYS; i++) { + MESSAGES[i] = "message-" + i; + } + } + + LogCollector collector; + + @Setup(Level.Iteration) + public void setup() { + // Capacity well below N_KEYS keeps the table full/near-full once warmed up. + collector = new LogCollector(8); + } + } + + @State(Scope.Thread) + public static class KeyCursorState { + int cursor; + + int next() { + int i = cursor; + cursor = (i + 1) % ContendedCollectorState.N_KEYS; + return i; + } + } + + @Benchmark + public void variedKeysNearCapacity(ContendedCollectorState state, KeyCursorState cursor) { + state.collector.addLogMessage("error", ContendedCollectorState.MESSAGES[cursor.next()], null); + } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/ConcurrentHashtableDrainBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ConcurrentHashtableDrainBenchmark.java new file mode 100644 index 00000000000..97ee0914e34 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/ConcurrentHashtableDrainBenchmark.java @@ -0,0 +1,173 @@ +package datadog.trace.util; + +import static java.util.concurrent.TimeUnit.MICROSECONDS; + +import java.util.concurrent.atomic.AtomicReferenceArray; +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Consumer; +import javax.annotation.Nonnull; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Group; +import org.openjdk.jmh.annotations.GroupThreads; +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.Warmup; + +/** + * Compares writer throughput against a continuously draining table under two locking schemes: + * + *
Both variants release capacity per-entry (one {@link + * ConcurrentHashtable.SizeManager#decrement()} per drained entry, mirroring current production + * behavior) so the only variable under test is lock granularity, not the capacity-release change + * that landed alongside it. + * + *
The writer mirrors {@code LogCollector.addLogMessage}'s shape: a lock-free scan over a small, + * rotating key set ({@link #N_KEYS} distinct log groups) that hits on the fast path once a key has + * been inserted, falling back to {@link ConcurrentHashtable#tryReserve} (which holds the table lock + * for the reservation's lifetime) only on a miss. Because {@code drainer} periodically empties the + * table, writers keep taking the slow, lock-holding path throughout the run instead of settling + * into steady-state lock-free hits -- this keeps occupancy well under capacity so writers aren't + * dominated by the lock-free {@code isFull()} fast-reject once full, and the throughput actually + * reflects contention for the table lock against an in-progress drain. + * + *
{@code
+ * ./gradlew :internal-api:jmh -Pjmh.includes=ConcurrentHashtableDrainBenchmark -Pjmh.forks=1
+ * }
+ */
+@Fork(2)
+@Warmup(iterations = 2)
+@Measurement(iterations = 3)
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(MICROSECONDS)
+public class ConcurrentHashtableDrainBenchmark {
+
+ static final int N_KEYS = 32;
+ static final int CAPACITY = 64;
+
+ static final long[] KEY_HASHES = new long[N_KEYS];
+
+ static {
+ for (int i = 0; i < N_KEYS; ++i) {
+ KEY_HASHES[i] = LongHashingUtils.hash("key-" + i);
+ }
+ }
+
+ static final class DrainEntry extends ConcurrentHashtable.EntryRun with {@code -Pjmh.profilers=gc} to confirm the {@link Iterable}/{@link java.util.Iterator} + * allocated per call (the anonymous instances returned by {@link + * ConcurrentHashtable#hashIterable}/{@link ConcurrentHashtable#hashIterator}) are scalar-replaced + * away by escape analysis rather than landing on the heap: + * + *
{@code
+ * ./gradlew :internal-api:jmh -Pjmh.includes=ConcurrentHashtableFindBenchmark -Pjmh.profilers=gc -Pjmh.forks=1
+ * }
+ */
+@Fork(2)
+@Warmup(iterations = 2)
+@Measurement(iterations = 3)
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(MICROSECONDS)
+@Threads(8)
+public class ConcurrentHashtableFindBenchmark {
+
+ static final int N_KEYS = 64;
+ static final int CAPACITY = 128;
+
+ static final long[] KEY_HASHES = new long[N_KEYS];
+
+ static {
+ for (int i = 0; i < N_KEYS; ++i) {
+ KEY_HASHES[i] = LongHashingUtils.hash("key-" + i);
+ }
+ }
+
+ /** Mirrors {@code LogCollector.RawLogMessage}: a keyHash plus a payload compared on match. */
+ static final class FindEntry extends ConcurrentHashtable.EntryNote, throwables are matched by identity or by class and stack trace. * *
The bucket chain supports lock-free reads. A caller that inserts after a miss must repeat
* the search under the table write lock.
*
- * @param bucketIndex bucket selected for {@code keyHash}
* @param keyHash precomputed hash of the level, message, and throwable class
* @param logLevel log level to match
* @param message message to match
@@ -150,19 +125,10 @@ public Collection The self-bound type parameter ({@code TEntry extends Entry Deliberately parameterized on {@code K} alone, not self-bound on the concrete subclass:
+ * {@link D1} stores and links entries internally as {@code Entry The drain holds the table write lock while detaching buckets and invoking the consumer.
- * For each removed entry, the consumer is invoked synchronously after its bucket is detached.
- * Capacity is released only after all invocations return. The consumer should be quick and must
- * not throw; failures are not rolled back.
+ * The lock is acquired and released bucket by bucket rather than held for the whole sweep,
+ * so a table-level operation (an insert, a reservation, an eviction) can interleave between
+ * buckets instead of waiting out the entire drain. This means the drain is no longer an atomic
+ * snapshot: an entry inserted into a not-yet-visited bucket while the drain is in progress is
+ * swept up too, and each bucket's capacity slot is released immediately before its consumer
+ * invocation runs, rather than once for the whole sweep at the end. The consumer should be
+ * quick and must not throw; entries not yet reached are left in the table with their capacity
+ * still counted, but entries already detached keep their capacity released regardless.
*
* Use {@link #drain(Object, BiConsumer)} to avoid a capturing lambda.
*
* @param drainedEntryConsumer action invoked for each removed entry
*/
- public void drain(@Nonnull Consumer super TEntry> drainedEntryConsumer) {
- ConcurrentHashtable.drain(state, drainedEntryConsumer);
+ public void drain(@Strategy @Nonnull Consumer super TEntry> drainedEntryConsumer) {
+ ConcurrentHashtable.drain(state, castConsumer(drainedEntryConsumer));
}
/**
- * Context-passing {@link #drain(Consumer)}. The drain holds the table write lock while
- * detaching buckets and invoking {@code drainedEntryConsumer}. For each removed entry, the
- * consumer is invoked synchronously after its bucket is detached. Capacity is released only
- * after all invocations return.
+ * Context-passing {@link #drain(Consumer)}. The lock is acquired and released bucket by bucket
+ * rather than held for the whole sweep, so a table-level operation (an insert, a reservation,
+ * an eviction) can interleave between buckets instead of waiting out the entire drain. This
+ * means the drain is no longer an atomic snapshot: an entry inserted into a not-yet-visited
+ * bucket while the drain is in progress is swept up too, and each bucket's capacity slot is
+ * released immediately before its consumer invocation runs, rather than once for the whole
+ * sweep at the end.
*
* Pass a non-capturing {@link BiConsumer} (typically a {@code static final}) plus the
* accumulator as {@code context} (e.g. the target list or event builder) to avoid a
@@ -340,8 +430,8 @@ public void drain(@Nonnull Consumer super TEntry> drainedEntryConsumer) {
* @param drainedEntryConsumer action invoked with the context and each removed entry
*/
public Deliberately parameterized on {@code K1}/{@code K2} alone, not self-bound on the concrete
+ * subclass -- see {@link D1.Entry} for why.
*
* @param The {@code creator} should build an entry whose {@code keyHash} equals {@link
- * D2.Entry#hash(Object, Object) D2.Entry.hash(key1, key2)}.
*/
@Nonnull
public Maybe The drain holds the table write lock while detaching buckets and invoking the consumer.
- * For each removed entry, the consumer is invoked synchronously after its bucket is detached.
- * Capacity is released only after all invocations return. The consumer should be quick and must
- * not throw; failures are not rolled back.
+ * The lock is acquired and released bucket by bucket rather than held for the whole sweep,
+ * so a table-level operation (an insert, a reservation, an eviction) can interleave between
+ * buckets instead of waiting out the entire drain. This means the drain is no longer an atomic
+ * snapshot: an entry inserted into a not-yet-visited bucket while the drain is in progress is
+ * swept up too, and each bucket's capacity slot is released immediately before its consumer
+ * invocation runs, rather than once for the whole sweep at the end. The consumer should be
+ * quick and must not throw; entries not yet reached are left in the table with their capacity
+ * still counted, but entries already detached keep their capacity released regardless.
*
* Use {@link #drain(Object, BiConsumer)} to avoid a capturing lambda.
*
* @param drainedEntryConsumer action invoked for each removed entry
*/
- public void drain(@Nonnull Consumer super TEntry> drainedEntryConsumer) {
- ConcurrentHashtable.drain(state, drainedEntryConsumer);
+ public void drain(@Strategy @Nonnull Consumer super TEntry> drainedEntryConsumer) {
+ ConcurrentHashtable.drain(state, castConsumer(drainedEntryConsumer));
}
/**
- * Context-passing {@link #drain(Consumer)}. The drain holds the table write lock while
- * detaching buckets and invoking {@code drainedEntryConsumer}. For each removed entry, the
- * consumer is invoked synchronously after its bucket is detached. Capacity is released only
- * after all invocations return.
+ * Context-passing {@link #drain(Consumer)}. The lock is acquired and released bucket by bucket
+ * rather than held for the whole sweep, so a table-level operation (an insert, a reservation,
+ * an eviction) can interleave between buckets instead of waiting out the entire drain. This
+ * means the drain is no longer an atomic snapshot: an entry inserted into a not-yet-visited
+ * bucket while the drain is in progress is swept up too, and each bucket's capacity slot is
+ * released immediately before its consumer invocation runs, rather than once for the whole
+ * sweep at the end.
*
* Pass a non-capturing {@link BiConsumer} (typically a {@code static final}) plus the
* accumulator as {@code context} (e.g. the target list or event builder) to avoid a
@@ -627,8 +779,8 @@ public void drain(@Nonnull Consumer super TEntry> drainedEntryConsumer) {
* @param drainedEntryConsumer action invoked with the context and each removed entry
*/
public Build the entry before reserving: there is no cancellation operation, so abandoning a
- * successful reservation permanently consumes capacity.
+ * Prefer {@link #cancelReservation()} plus deferred entry construction (see {@link
+ * ConcurrentHashtable#reserve}) over abandoning a reservation outright: this method by itself
+ * still has no way to give back a slot once claimed.
*/
public boolean tryReserve() {
if (size.incrementAndGet() > capacity) {
@@ -704,6 +858,27 @@ public boolean tryReserve() {
return true;
}
+ /**
+ * Gives back a slot claimed by {@link #tryReserve()} that was never filled — e.g. a concurrent
+ * match was found under the write lock instead of inserting. Lock-free, symmetric with {@link
+ * #decrement()}.
+ *
+ * Caller must call this at most once per successful {@link #tryReserve()}; double-cancelling
+ * corrupts the count the same way double-incrementing would. {@link Reservation#close()}
+ * handles this bookkeeping automatically and should be preferred over calling this directly.
+ *
+ * Under heavy contention on the same logical duplicate, multiple threads can each reserve a
+ * slot for what turns out to be the same entry before any of them cancels, transiently
+ * inflating {@code size} above the table's true occupancy. This can cause an unrelated,
+ * genuinely distinct concurrent insert to see the table as full when it isn't, until the losing
+ * reservations cancel. The effect is self-correcting (bounded by in-flight reservations, not
+ * sustained) and considered an acceptable tradeoff for tables expecting bursts of identical
+ * inserts (e.g. deduplication).
+ */
+ public void cancelReservation() {
+ size.decrementAndGet();
+ }
+
/**
* Reserves one slot, evicting an entry matching {@code evictable} when the table is full.
* Returns {@code false} without changing the table when no entry can be evicted.
@@ -711,10 +886,10 @@ public boolean tryReserve() {
* The reservation survives concurrent drain and clear operations. The caller must fill it;
* an abandoned reservation permanently consumes capacity.
*/
- @GuardedBy("getTableWriteLock(buckets)")
- public This operation may inspect every live entry while holding the table write lock, so the
* predicate should be quick.
*/
- @GuardedBy("getTableWriteLock(buckets)")
+ @GuardedBy("the corresponding State's getTableWriteLock(state)")
@Nullable
- public {@code sizeManager} is intentionally package-private: callers outside this class must go
+ * through the {@code State}-accepting static helpers ({@link #estimateSize}, {@link #isFull},
+ * {@link #tryReserve}, {@link #tryReserveOrEvict}, {@link #evictOne}, {@link #evictAll}) rather
+ * than reach into the manager directly.
*/
- public static final class State Complete it with {@link #insertReserved}, or prefer {@link #tryReserve} for a higher-level,
+ * auto-cancelling handle that also defers entry construction until the reservation succeeds.
+ */
+ public static Run a lock-free scan first (see {@link #bucketFor}/{@link #bucketAt}) and only call this
+ * once that scan has missed — a successful reservation isn't required for correctness (the
+ * reservation itself, and the locked comparison inside {@link Reservation#tryGetOrInsertOrNull},
+ * are the source of truth), it just avoids paying for a lock and a factory call when a hit was
+ * already visible lock-free.
+ *
+ * Checks {@link SizeManager#isFull()} lock-free first. If the table looks full, {@code
+ * keyHash}'s bucket is checked too: if it's empty, nothing could possibly match this key, so this
+ * returns a definitively empty reservation without ever taking the write lock. Otherwise it
+ * acquires the write lock, re-checks (the lock-free peek may be stale), and either reserves the
+ * slot -- returning a real reservation that holds the lock open across this call, to be
+ * released later by {@link Reservation#finish}/{@link Reservation#close} -- or, if the table is
+ * genuinely still full, keeps the lock open anyway (rather than releasing it immediately) so
+ * {@link Reservation#finish} can still scan {@code keyHash}'s bucket the next time the caller
+ * supplies a candidate entry. That's what closes the race where a concurrent insert for the same
+ * logical duplicate lands in the exact window between this caller's own lock-free scan and its
+ * reservation attempt: without the lock held open here, that concurrent duplicate would go
+ * uncounted. Holding the lock for the reservation's whole lifetime (rather than just across this
+ * call, the way {@link SizeManager#tryReserve()} does) is also what lets {@link
+ * Reservation#tryGetOrInsertOrNull} skip a second, separately-locked comparison pass: the
+ * reserve-then-insert-or-discard sequence is one uninterrupted critical section, so no concurrent
+ * reservation for the same logical duplicate can slip in between.
+ *
+ * Always returns a non-null handle — even when the table is full — so the caller must check
+ * {@link Reservation#isReserved()} (or simply call {@link Reservation#tryGetOrInsertOrNull},
+ * which returns {@code null} on an absent reservation) rather than assume every reservation is
+ * real. A reservation can also report {@link Reservation#isReserved()} {@code true} while still
+ * having no slot to insert into (the lock-held-but-full case above); either way, {@code
+ * tryGetOrInsertOrNull}/{@code tryGetOrInsert} return the concurrent match found under the lock,
+ * or {@code null} -- never a newly linked entry when no slot was claimed.
+ *
+ * @param keyHash hash of the key the caller is about to look up or insert
+ */
+ @Nonnull
+ public static Overloaded up to 4 key components ({@link #tryGetOrInsertOrNull(Object, Function)} through
+ * {@link #tryGetOrInsertOrNull(Object, Object, Object, Object, Function4)}) so a non-capturing
+ * method reference can build the entry directly from its natural constructor arguments, without
+ * an intermediate holder object or a capturing lambda.
+ *
+ * A real (non-empty) reservation holds the table's write lock from the moment {@link
+ * #tryReserve} returns until {@link #finish} or {@link #close} releases it -- unlike a
+ * lexically-scoped {@code synchronized} block, whose acquisition and release can't span two
+ * separate calls. Keep a reservation's lifetime short: nothing else can write to the table while
+ * one is open, and (having deliberately dropped the CAS-based fast path that would let two
+ * threads race for the same slot) nothing else can even reserve.
+ *
+ * @param Building the entry here, after the reservation already succeeded, keeps the write lock's
+ * critical section limited to the comparison/link/discard decision rather than whatever
+ * construction cost {@code factory} pays. See {@link ConcurrentHashtable.Entry#matches} — the
+ * under-lock comparison is entry-to-entry, so it needs {@code newEntry} already built.
+ */
+ @StrategyConsumer
+ @Nullable
+ public TEntry tryGetOrInsertOrNull(
+ A a, B b, @Strategy @Nonnull BiFunction super A, ? super B, ? extends TEntry> factory) {
+ return state == null ? null : finish(factory.apply(a, b));
+ }
+
+ /**
+ * Three key components; see {@link #tryGetOrInsertOrNull(Object, Object, BiFunction)} for the
+ * general contract.
+ */
+ @StrategyConsumer
+ @Nullable
+ public TEntry tryGetOrInsertOrNull(
+ A a,
+ B b,
+ C c,
+ @Strategy @Nonnull Function3 super A, ? super B, ? super C, ? extends TEntry> factory) {
+ return state == null ? null : finish(factory.apply(a, b, c));
+ }
+
+ /** Four key components; see {@link #tryGetOrInsertOrNull(Object, Object, BiFunction)}. */
+ @StrategyConsumer
+ @Nullable
+ public TEntry tryGetOrInsertOrNull(
+ A a,
+ B b,
+ C c,
+ D d,
+ @Strategy @Nonnull
+ Function4 super A, ? super B, ? super C, ? super D, ? extends TEntry> factory) {
+ return state == null ? null : finish(factory.apply(a, b, c, d));
+ }
+
+ /** {@link Maybe}-wrapping counterpart of {@link #tryGetOrInsertOrNull(Entry)}. */
+ @Nonnull
+ public Maybe Always scans for a match first, whether or not a slot was actually claimed: a {@link
+ * #tryReserve} reservation with no slot still holds the lock specifically so this scan can run.
+ * Only links {@code newEntry} when a slot was claimed; otherwise a miss here means there really
+ * is nothing to return, so this returns {@code null}.
+ */
+ @Nullable
+ private TEntry finish(@Nonnull TEntry newEntry) {
+ int index = bucketIndex(state.buckets, newEntry.keyHash);
+ for (TEntry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) {
+ if (curEntry.keyHash == newEntry.keyHash && curEntry.matches(newEntry)) {
+ return curEntry;
+ }
+ }
+ if (!slotClaimed) {
+ return null;
+ }
+ insertHeadEntryAt(state, index, newEntry);
+ consumed = true;
+ return newEntry;
+ }
+
+ /**
+ * Releases the write lock {@link #tryReserve} acquired, giving back a claimed slot first if it
+ * was never consumed. A no-op on a definitively empty reservation, which never acquired the
+ * lock.
+ */
+ @Override
+ public void close() {
+ if (state == null) {
+ return;
+ }
+ try {
+ if (slotClaimed && !consumed) {
+ state.sizeManager.decrement();
+ }
+ } finally {
+ state.writeLock.unlock();
+ }
+ }
+ }
+
+ /** Three-argument analogue of {@link java.util.function.BiFunction}. */
+ @Strategy
+ @FunctionalInterface
+ public interface Function3 {
+ R apply(A a, B b, C c);
+ }
+
+ /** Four-argument analogue of {@link java.util.function.BiFunction}. */
+ @Strategy
+ @FunctionalInterface
+ public interface Function4 {
+ R apply(A a, B b, C c, D d);
+ }
+
/**
* Reserves one slot in {@code state}, evicting an entry matching {@code evictable} when
* necessary. Returns {@code false} if the table is full and nothing can be evicted. This method
@@ -883,10 +1375,14 @@ public static boolean isFull(@Nonnull State> state) {
* The reservation survives drain and clear operations. Complete it with {@link
* #insertReserved}; abandoning it permanently consumes capacity.
*/
- public static Each step follows {@link Entry#next()}, so the iterator reflects entries linked at the time
+ * each step runs rather than a point-in-time snapshot -- entries inserted ahead of the iterator's
+ * current position after iteration starts may or may not be observed, and a concurrently removed
+ * entry remains reachable because {@code unlink()} deliberately retains its {@code next} link for
+ * in-flight readers.
+ */
+ @Nonnull
+ public static {@code
+ * try (Reservation
+ *
+ * {@code
+ * try (Reservation
+ *
+ * See {@link #tryGetOrInsertOrNull(Object, Object, BiFunction)} for the general contract.
+ */
+ @Nullable
+ public TEntry tryGetOrInsertOrNull(@Nonnull TEntry newEntry) {
+ return state == null ? null : finish(newEntry);
+ }
+
+ /**
+ * One key component; see {@link #tryGetOrInsertOrNull(Object, Object, BiFunction)} for the
+ * general contract.
+ */
+ @StrategyConsumer
+ @Nullable
+ public TEntry tryGetOrInsertOrNull(
+ A a, @Strategy @Nonnull Function super A, ? extends TEntry> factory) {
+ return state == null ? null : finish(factory.apply(a));
+ }
+
+ /**
+ * Two key components. Builds {@code factory.apply(...)} (skipped entirely if this reservation
+ * is definitively empty — see {@link #isReserved()}) and either links the result as a new entry
+ * or discards it in favor of an existing match found under the write lock. Returns {@code null}
+ * when there is neither a slot to claim nor a match to return -- either because this
+ * reservation is definitively empty, or (see {@link #tryReserve}) because the table was still
+ * full even under the lock and no concurrent match was found either; otherwise always returns a
+ * real entry (the newly built one, or the concurrent match).
+ *
+ *