diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapCounterBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapCounterBenchmark.java index bc8bc07b9f5..239dd3890f4 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapCounterBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapCounterBenchmark.java @@ -23,11 +23,10 @@ * Measures lookup followed by an atomic counter increment in a shared, pre-populated table. Models * per-class or per-method hit counters in the tracer. * - *

The {@link ConcurrentHashtable.D1} case embeds a {@code volatile long} in each entry. {@link - * AtomicLongFieldUpdater} updates that field atomically without allocating an {@link AtomicLong} - * per key. The map baselines store a separate {@link AtomicLong} or {@link LongAdder}; {@code - * LongAdder} spreads contention across internal cells at the cost of more memory and a more - * expensive read. + *

The {@link ConcurrentHashtable.D1} case embeds a {@code volatile long} counter directly in the + * entry and increments it via {@link AtomicLongFieldUpdater}, avoiding a second heap object. The + * map baselines store a separate {@link AtomicLong} or {@link LongAdder}; {@code LongAdder} spreads + * contention across internal cells at the cost of more memory and a more expensive read. * *

Lookups reuse the key instances installed during setup. {@code Objects.equals} therefore * returns on its identity check without dispatching to {@code equals}, so this measures the @@ -50,8 +49,8 @@ *

  • {@code LongAdder} is marginally faster (79 vs 71 ops/us) because it shards the counter * across cells to reduce CAS contention; the advantage grows with thread count. *
  • {@code ConcurrentHashtable} matches {@code AtomicLong} throughput (69 vs 71 ops/us) while - * embedding the counter directly in the entry — one object instead of two, with no throughput - * penalty. + * embedding the counter directly in the entry via {@code AtomicLongFieldUpdater} — one object + * instead of two, with no throughput penalty. * */ @Fork(2) @@ -73,8 +72,12 @@ public class ThreadSafeMapCounterBenchmark { } } + /** + * Shared state ({@link Scope#Benchmark}): one instance of each map across all threads, modelling + * a shared instrumentation counter table. + */ static final class CounterEntry extends ConcurrentHashtable.D1.Entry { - private static final AtomicLongFieldUpdater COUNT = + static final AtomicLongFieldUpdater COUNT = AtomicLongFieldUpdater.newUpdater(CounterEntry.class, "count"); volatile long count; @@ -82,16 +85,8 @@ static final class CounterEntry extends ConcurrentHashtable.D1.Entry { CounterEntry(String key) { super(key); } - - long increment() { - return COUNT.incrementAndGet(this); - } } - /** - * Shared state ({@link Scope#Benchmark}): one instance of each map across all threads, modelling - * a shared instrumentation counter table. - */ @State(Scope.Benchmark) public static class SharedState { ConcurrentHashtable.D1 table; @@ -125,7 +120,7 @@ int next() { @Benchmark public long increment_concurrentHashtable(SharedState s, ThreadState t) { - return s.table.get(KEYS[t.next()]).increment(); + return CounterEntry.COUNT.incrementAndGet(s.table.get(KEYS[t.next()])); } @Benchmark diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD1Benchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD1Benchmark.java index 8fd07544264..6f5a8dcd769 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD1Benchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD1Benchmark.java @@ -83,34 +83,35 @@ public class ThreadSafeMapD1Benchmark { } } - static final class D1Entry extends ConcurrentHashtable.D1.Entry { + /** + * Shared state ({@link Scope#Benchmark}): one instance of each map across all threads, modelling + * a shared instrumentation cache. + */ + static final class LongEntry extends ConcurrentHashtable.D1.Entry { final long value; - D1Entry(String key) { + LongEntry(String key, long value) { super(key); - this.value = 1L; + this.value = value; } } - /** - * Shared state ({@link Scope#Benchmark}): one instance of each map across all threads, modelling - * a shared instrumentation cache. - */ @State(Scope.Benchmark) public static class SharedState { - ConcurrentHashtable.D1 table; + ConcurrentHashtable.D1 table; ConcurrentHashMap concurrentHashMap; ConcurrentSkipListMap skipListMap; Map synchronizedHashMap; @Setup(Level.Iteration) public void setUp() { - table = ConcurrentHashtable.D1.createBounded(D1Entry.class, CAPACITY); + table = ConcurrentHashtable.D1.createBounded(LongEntry.class, CAPACITY); concurrentHashMap = new ConcurrentHashMap<>(CAPACITY); skipListMap = new ConcurrentSkipListMap<>(); synchronizedHashMap = Collections.synchronizedMap(new HashMap<>(CAPACITY)); for (int i = 0; i < N_KEYS; ++i) { - table.tryGetOrCreateOrNull(KEYS[i], D1Entry::new); + long value = i; + table.tryGetOrCreateOrNull(KEYS[i], k -> new LongEntry(k, value)); concurrentHashMap.put(KEYS[i], (long) i); skipListMap.put(KEYS[i], (long) i); synchronizedHashMap.put(KEYS[i], (long) i); @@ -131,7 +132,7 @@ int next() { } @Benchmark - public D1Entry get_concurrentHashtable(SharedState s, ThreadState t) { + public LongEntry get_concurrentHashtable(SharedState s, ThreadState t) { return s.table.get(KEYS[t.next()]); } @@ -151,8 +152,8 @@ public Long get_synchronizedHashMap(SharedState s, ThreadState t) { } @Benchmark - public D1Entry getOrCreate_concurrentHashtable(SharedState s, ThreadState t) { - return s.table.tryGetOrCreateOrNull(KEYS[t.next()], D1Entry::new); + public LongEntry getOrCreate_concurrentHashtable(SharedState s, ThreadState t) { + return s.table.tryGetOrCreateOrNull(KEYS[t.next()], k -> new LongEntry(k, 0L)); } /** diff --git a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD2Benchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD2Benchmark.java index aff30dd0a33..57506a12230 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD2Benchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ThreadSafeMapD2Benchmark.java @@ -108,7 +108,7 @@ static final class D2Entry extends ConcurrentHashtable.D2.Entry * Entry used with the static helpers. Its primitive second key keeps storage and lookup unboxed, * independently of {@link Integer} caching or JVM escape analysis. */ - static final class SupportEntry extends ConcurrentHashtable.Entry { + static final class SupportEntry extends ConcurrentHashtable.Entry { final String k1; final int k2; final long value; @@ -127,6 +127,11 @@ static long hash(String k1, int k2) { boolean matches(String k1, int k2) { return this.k2 == k2 && this.k1.equals(k1); } + + @Override + public boolean matches(SupportEntry other) { + return matches(other.k1, other.k2); + } } /** Composite key for map-based baselines. */ diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/LogCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/LogCollector.java index b7ad3cb0eb0..2528cebddaa 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/LogCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/LogCollector.java @@ -1,16 +1,15 @@ package datadog.trace.api.telemetry; +import datadog.trace.util.ConcurrentHashtable; import datadog.trace.util.HashingUtils; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Objects; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.slf4j.Marker; import org.slf4j.MarkerFactory; @@ -20,8 +19,7 @@ public class LogCollector { public static final Marker EXCLUDE_TELEMETRY = MarkerFactory.getMarker("EXCLUDE_TELEMETRY"); private static final int DEFAULT_MAX_CAPACITY = 10; private static final LogCollector INSTANCE = new LogCollector(); - private final Map rawLogMessages; - private final int maxCapacity; + private final ConcurrentHashtable.State rawLogMessages; public static LogCollector get() { return INSTANCE; @@ -35,8 +33,7 @@ private LogCollector() { value = "SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR", justification = "Usage in tests") LogCollector(int maxCapacity) { - this.maxCapacity = maxCapacity; - this.rawLogMessages = new ConcurrentHashMap<>(maxCapacity); + this.rawLogMessages = ConcurrentHashtable.createBounded(RawLogMessage.class, maxCapacity); } public void addLogMessage(String logLevel, String message, @Nullable Throwable throwable) { @@ -54,57 +51,70 @@ public void addLogMessage(String logLevel, String message, @Nullable Throwable t */ public void addLogMessage( String logLevel, String message, @Nullable Throwable throwable, @Nullable String tags) { - if (rawLogMessages.size() >= maxCapacity) { - // TODO: We could emit a metric for dropped logs. - return; + long keyHash = RawLogMessage.hash(logLevel, message, throwable); + + // Memoized once per call and shared across every candidate below, rather than letting + // matchesKey call throwable.getStackTrace() (a defensive-copy allocation) on each comparison. + StackTraceElement[] throwableStackTrace = null; + + // Lock-free scan first: most calls are re-observations of an already-seen message, so this + // avoids paying for a reservation and a RawLogMessage allocation on the common path. + for (RawLogMessage existing : ConcurrentHashtable.hashIterable(rawLogMessages, keyHash)) { + if (throwable != null && existing.throwable != null && existing.throwable != throwable) { + if (throwableStackTrace == null) { + throwableStackTrace = throwable.getStackTrace(); + } + } + if (existing.matchesKey(logLevel, message, throwable, throwableStackTrace)) { + existing.count.incrementAndGet(); + return; + } + } + + try (ConcurrentHashtable.Reservation reservation = + ConcurrentHashtable.tryReserve(rawLogMessages)) { + // TODO: We could emit a metric for dropped logs when the reservation is empty (table full). + RawLogMessage rawLogMessage = + reservation.tryGetOrInsertOrNull(logLevel, message, throwable, tags, RawLogMessage::new); + if (rawLogMessage != null) { + rawLogMessage.count.incrementAndGet(); + } } - RawLogMessage rawLogMessage = - new RawLogMessage(logLevel, message, throwable, tags, System.currentTimeMillis() / 1000); - AtomicInteger count = rawLogMessages.computeIfAbsent(rawLogMessage, k -> new AtomicInteger()); - count.incrementAndGet(); } public Collection drain() { - if (rawLogMessages.isEmpty()) { + int size = ConcurrentHashtable.estimateSize(rawLogMessages); + if (size == 0) { return Collections.emptyList(); } - List list = new ArrayList<>(rawLogMessages.size()); - Iterator> iterator = - rawLogMessages.entrySet().iterator(); - - while (iterator.hasNext()) { - Map.Entry entry = iterator.next(); - RawLogMessage logMessage = entry.getKey(); - // XXX: There might be lost writers to the counters under concurrency if another thread - // increments it - // while we are reading it here. At the moment, we are not overdoing this to prevent some - // counter losses. - logMessage.count = entry.getValue().get(); - iterator.remove(); - list.add(logMessage); - } - + List list = new ArrayList<>(size); + ConcurrentHashtable.drain(rawLogMessages, list::add); return list; } - public static final class RawLogMessage { + public static final class RawLogMessage extends ConcurrentHashtable.Entry { public final String message; public final String logLevel; public final Throwable throwable; public final String tags; public final long timestamp; - public int count; + public final AtomicInteger count = new AtomicInteger(); private StackTraceElement[] cachedStackTrace = null; public RawLogMessage( - String logLevel, String message, Throwable throwable, String tags, long timestamp) { + String logLevel, String message, Throwable throwable, @Nullable String tags) { + super(hash(logLevel, message, throwable)); this.logLevel = logLevel; this.message = message; this.throwable = throwable; this.tags = tags; - this.timestamp = timestamp; + this.timestamp = System.currentTimeMillis() / 1000; + } + + static long hash(String logLevel, String message, @Nullable Throwable throwable) { + return HashingUtils.hash(logLevel, message, throwable == null ? null : throwable.getClass()); } public StackTraceElement[] stackTrace() { @@ -122,25 +132,30 @@ public StackTraceElement[] stackTrace() { return stackTrace; } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RawLogMessage that = (RawLogMessage) o; - - if (!Objects.equals(logLevel, that.logLevel)) return false; - if (!Objects.equals(message, that.message)) return false; - - if (throwable == that.throwable) { + /** + * @param throwableStackTrace {@code throwable.getStackTrace()}, memoized once by the caller and + * shared across every candidate scanned for a given {@code addLogMessage} call -- avoids + * paying {@code getStackTrace()}'s defensive-copy allocation on each comparison. Only + * non-null when {@code throwable} needs a deep comparison against some candidate. + */ + private boolean matchesKey( + String logLevel, + String message, + @Nullable Throwable throwable, + @Nullable StackTraceElement[] throwableStackTrace) { + if (!Objects.equals(this.logLevel, logLevel)) return false; + if (!Objects.equals(this.message, message)) return false; + + if (this.throwable == throwable) { // DQH - While this path may seem unlikely, it does happen if the JVM fast // throws optimization kicks-in (for NPE, etc), so this case is worth optimizing. // This also covers the case where both throwables are null return true; - } else if (throwable != null && that.throwable != null) { + } else if (this.throwable != null && throwable != null) { // Both have a throwable perform a deeper comparison - return throwable.getClass().equals(that.throwable.getClass()) - && Objects.deepEquals(stackTrace(), that.stackTrace()); + return this.throwable.getClass().equals(throwable.getClass()) + && Objects.deepEquals(stackTrace(), throwableStackTrace); } else { // One has an exception & the other doesn't, not equal return false; @@ -148,8 +163,18 @@ public boolean equals(Object o) { } @Override - public int hashCode() { - return HashingUtils.hash(logLevel, message, throwable == null ? null : throwable.getClass()); + public boolean matches(@Nonnull RawLogMessage other) { + if (!Objects.equals(logLevel, other.logLevel)) return false; + if (!Objects.equals(message, other.message)) return false; + + if (throwable == other.throwable) { + return true; + } else if (throwable != null && other.throwable != null) { + return throwable.getClass().equals(other.throwable.getClass()) + && Objects.deepEquals(stackTrace(), other.stackTrace()); + } else { + return false; + } } } } diff --git a/internal-api/src/main/java/datadog/trace/util/ConcurrentHashtable.java b/internal-api/src/main/java/datadog/trace/util/ConcurrentHashtable.java index 693ca2a1b16..c205af5099f 100644 --- a/internal-api/src/main/java/datadog/trace/util/ConcurrentHashtable.java +++ b/internal-api/src/main/java/datadog/trace/util/ConcurrentHashtable.java @@ -1,6 +1,10 @@ package datadog.trace.util; +import datadog.trace.api.function.Strategy; +import datadog.trace.api.function.StrategyConsumer; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.util.Iterator; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReferenceArray; @@ -69,10 +73,16 @@ private ConcurrentHashtable() {} * arity. See {@link D1.Entry} and {@link D2.Entry}; for higher arities, or for primitive key * components, subclass this directly and drive the table with the static building blocks on * {@link ConcurrentHashtable}. + * + *

    The self-bound type parameter ({@code TEntry extends Entry}, mirroring {@code Enum>}) exists so {@link #matches(Entry)} can compare two already-built entries + * without an {@code Object}/{@code instanceof} boundary. It requires every concrete subclass to + * declare itself as its own type argument (see {@link D1.Entry}/{@link D2.Entry}); Java has no + * true {@code Self} type, so this is enforced by convention, not the compiler. */ - public abstract static class Entry { + public abstract static class Entry> { public final long keyHash; - private volatile Entry next = null; + private volatile TEntry next = null; protected Entry(long keyHash) { this.keyHash = keyHash; @@ -81,15 +91,26 @@ protected Entry(long keyHash) { // Package-private: the only writers are the static insert/remove building blocks // (insertHeadEntry, unlink) on the enclosing class, which reach it via the Entry bound. Custom // tables mutate chains through those helpers, never by touching next directly. - final void setNext(TEntry next) { + final void setNext(TEntry next) { this.next = next; } - @SuppressWarnings("unchecked") @Nullable - public final TEntry next() { - return (TEntry) this.next; + public final TEntry next() { + return this.next; } + + /** + * Returns {@code true} if {@code other} is logically the same entry as this one (same key(s)), + * used to compare two already-built entries under the write lock (e.g. in {@link + * Reservation#tryGetOrInsertOrNull}). Deliberately narrower than {@code equals}/{@code + * hashCode}: this class doesn't need reflexive/symmetric-with-null-and-unrelated-types contract + * baggage, and a bespoke method avoids entries accidentally working as {@code HashSet}/{@code + * HashMap} keys via an unrelated identity notion. {@link D1.Entry}/{@link D2.Entry} implement + * this in terms of their existing key-based {@code matches(...)}, so most callers never write + * it directly. + */ + public abstract boolean matches(@Nonnull TEntry other); } /** @@ -105,10 +126,17 @@ public static final class D1> { * Abstract base for {@link D1} entries. Subclass to add value fields you wish to mutate in * place after retrieving the entry via {@link D1#get}. * + *

    Deliberately parameterized on {@code K} alone, not self-bound on the concrete subclass: + * {@link D1} stores and links entries internally as {@code Entry} and casts back to {@code + * TEntry} at its API boundary, trading one unchecked cast (always sound -- the table only ever + * holds instances the caller's own {@code creator} produced) for a simpler subclass signature, + * e.g. {@code class MyEntry extends D1.Entry} rather than {@code D1.Entry}. + * * @param the key type */ - public abstract static class Entry extends ConcurrentHashtable.Entry { - final K key; + public abstract static class Entry extends ConcurrentHashtable.Entry> { + @Nullable final K key; protected Entry(@Nullable K key) { super(hash(key)); @@ -127,6 +155,12 @@ public boolean matches(@Nullable Object key) { return Objects.equals(key, this.key); } + /** {@link ConcurrentHashtable.Entry#matches(Entry)} in terms of the key-based overload. */ + @Override + public final boolean matches(@Nonnull Entry other) { + return matches(other.key); + } + /** * Returns the 64-bit lookup hash for {@code key}. Null keys map to {@link Long#MIN_VALUE} so * they don't collide with a real key that hashes to 0; real-key collisions in chains are @@ -137,21 +171,51 @@ public static long hash(@Nullable Object key) { } } - private final State state; + private final State> state; - private D1(State state) { + private D1(State> state) { this.state = state; } /** * Creates a fixed-size table holding at most {@code maxCapacity} entries. {@code entryClass} is - * used only to infer the concrete entry type; entries are created by the functions passed to - * the insertion methods. The table does not resize. + * used only to allocate the backing array with the right component type; entries themselves are + * created by the {@code creator}/{@code evictable} functions passed to the insertion methods. + * The table does not resize. */ @Nonnull + @SuppressWarnings("unchecked") public static > D1 createBounded( @Nonnull Class entryClass, int maxCapacity) { - return new D1<>(State.createBounded(entryClass, maxCapacity)); + // entryClass is erased away (see ConcurrentHashtable#createFixedBuckets), so treating it as + // Class> instead of the caller's concrete Class is safe. + Class> baseEntryClass = (Class>) (Class) entryClass; + return new D1<>(ConcurrentHashtable.createBounded(baseEntryClass, maxCapacity)); + } + + /** + * Sound because every entry ever inserted into {@link #state} was produced as a {@code TEntry}. + */ + @SuppressWarnings("unchecked") + private TEntry cast(@Nullable Entry entry) { + return (TEntry) entry; + } + + /** See {@link #cast(Entry)}; casts the functional-interface reference, not each element. */ + @SuppressWarnings("unchecked") + private Predicate> castPredicate(Predicate predicate) { + return (Predicate>) predicate; + } + + @SuppressWarnings("unchecked") + private Consumer> castConsumer(Consumer consumer) { + return (Consumer>) consumer; + } + + @SuppressWarnings("unchecked") + private BiConsumer> castConsumer( + BiConsumer consumer) { + return (BiConsumer>) consumer; } public int size() { @@ -165,11 +229,11 @@ public boolean isFull() { @Nullable public TEntry get(@Nullable K key) { long keyHash = D1.Entry.hash(key); - for (TEntry curEntry = bucketFor(state, keyHash); + for (Entry curEntry = bucketFor(state, keyHash); curEntry != null; curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key)) { - return curEntry; + return cast(curEntry); } } return null; @@ -182,7 +246,7 @@ public TEntry get(@Nullable K key) { */ @Nonnull public Maybe tryGetOrCreate( - @Nullable K key, @Nonnull Function creator) { + @Nullable K key, @Strategy @Nonnull Function creator) { return Maybe.of(tryGetOrCreateOrNull(key, creator)); } @@ -192,22 +256,25 @@ public Maybe tryGetOrCreate( * {@code key} was not already present. Re-checks under the lock to avoid duplicate entries * under concurrent misses. */ + @StrategyConsumer @Nullable public TEntry tryGetOrCreateOrNull( - @Nullable K key, @Nonnull Function creator) { + @Nullable K key, @Strategy @Nonnull Function creator) { long keyHash = D1.Entry.hash(key); int index = bucketIndex(state.buckets, keyHash); - for (TEntry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { + for (Entry curEntry = bucketAt(state, index); + curEntry != null; + curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key)) { - return curEntry; + return cast(curEntry); } } synchronized (getTableWriteLock(state)) { - for (TEntry curEntry = bucketAt(state, index); + for (Entry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key)) { - return curEntry; + return cast(curEntry); } } // isFull() is checked before creating the entry, not before reserving a slot for it: @@ -233,8 +300,8 @@ public TEntry tryGetOrCreateOrNull( @Nonnull public Maybe tryGetOrCreateOrEvict( @Nullable K key, - @Nonnull Function creator, - @Nonnull Predicate evictable) { + @Strategy @Nonnull Function creator, + @Strategy @Nonnull Predicate evictable) { return Maybe.of(tryGetOrCreateOrEvictOrNull(key, creator, evictable)); } @@ -245,28 +312,31 @@ public Maybe tryGetOrCreateOrEvict( * ever leaving a slot double-booked. A creator that throws after a successful eviction simply * leaves the table one entry smaller — no corruption, just a wasted eviction. */ + @StrategyConsumer @Nullable public TEntry tryGetOrCreateOrEvictOrNull( @Nullable K key, - @Nonnull Function creator, - @Nonnull Predicate evictable) { + @Strategy @Nonnull Function creator, + @Strategy @Nonnull Predicate evictable) { long keyHash = D1.Entry.hash(key); int index = bucketIndex(state.buckets, keyHash); - for (TEntry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { + for (Entry curEntry = bucketAt(state, index); + curEntry != null; + curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key)) { - return curEntry; + return cast(curEntry); } } synchronized (getTableWriteLock(state)) { - for (TEntry curEntry = bucketAt(state, index); + for (Entry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key)) { - return curEntry; + return cast(curEntry); } } if (state.sizeManager.isFull() - && state.sizeManager.evictOne(state.buckets, evictable) == null) { + && state.sizeManager.evictOne(state.buckets, castPredicate(evictable)) == null) { return null; } TEntry newEntry = creator.apply(key); @@ -286,14 +356,14 @@ public TEntry remove(@Nullable K key) { long keyHash = D1.Entry.hash(key); int index = bucketIndex(state.buckets, keyHash); synchronized (getTableWriteLock(state)) { - TEntry prev = null; - for (TEntry curEntry = bucketAt(state, index); + Entry prev = null; + for (Entry curEntry = bucketAt(state, index); curEntry != null; prev = curEntry, curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key)) { unlink(state, index, prev, curEntry); state.sizeManager.decrement(); - return curEntry; + return cast(curEntry); } } return null; @@ -305,8 +375,8 @@ public TEntry remove(@Nullable K key) { * Holds the table-level lock for the whole sweep, so the predicate sees a stable table and * concurrent writers are excluded; lock-free readers continue throughout. */ - public boolean removeIf(@Nonnull Predicate predicate) { - return ConcurrentHashtable.removeIf(state, predicate); + public boolean removeIf(@Strategy @Nonnull Predicate predicate) { + return ConcurrentHashtable.removeIf(state, castPredicate(predicate)); } /** @@ -316,8 +386,8 @@ public boolean removeIf(@Nonnull Predicate predicate) { * *

    Use {@link #drain(Object, BiConsumer)} to avoid a capturing lambda. */ - public void drain(@Nonnull Consumer sink) { - ConcurrentHashtable.drain(state, sink); + public void drain(@Strategy @Nonnull Consumer sink) { + ConcurrentHashtable.drain(state, castConsumer(sink)); } /** @@ -325,8 +395,9 @@ public void drain(@Nonnull Consumer sink) { * a {@code static final}) plus the accumulator as {@code context} (e.g. the target list or * event builder) to avoid a capturing-lambda allocation. */ - public void drain(C context, @Nonnull BiConsumer sink) { - ConcurrentHashtable.drain(state, context, sink); + public void drain( + C context, @Strategy @Nonnull BiConsumer sink) { + ConcurrentHashtable.drain(state, context, castConsumer(sink)); } /** Removes all entries. Lock-free readers mid-walk complete against the entries they hold. */ @@ -334,16 +405,17 @@ public void clear() { ConcurrentHashtable.clear(state); } - public void forEach(@Nonnull Consumer consumer) { - ConcurrentHashtable.forEach(state, consumer); + public void forEach(@Strategy @Nonnull Consumer consumer) { + ConcurrentHashtable.forEach(state, castConsumer(consumer)); } /** * Context-passing forEach. Avoids a capturing-lambda allocation — pass a non-capturing {@link * BiConsumer} (typically a {@code static final}) plus whatever side-band state it needs. */ - public void forEach(C context, @Nonnull BiConsumer consumer) { - ConcurrentHashtable.forEach(state, context, consumer); + public void forEach( + C context, @Strategy @Nonnull BiConsumer consumer) { + ConcurrentHashtable.forEach(state, context, castConsumer(consumer)); } } @@ -361,14 +433,17 @@ public static final class D2> { /** * Abstract base for {@link D2} entries. Subclass to add value fields you wish to mutate in - * place. + * place after retrieving the entry via {@link D2#get}. + * + *

    Deliberately parameterized on {@code K1}/{@code K2} alone, not self-bound on the concrete + * subclass -- see {@link D1.Entry} for why. * * @param first key type * @param second key type */ - public abstract static class Entry extends ConcurrentHashtable.Entry { - final K1 key1; - final K2 key2; + public abstract static class Entry extends ConcurrentHashtable.Entry> { + @Nullable final K1 key1; + @Nullable final K2 key2; protected Entry(@Nullable K1 key1, @Nullable K2 key2) { super(hash(key1, key2)); @@ -394,27 +469,63 @@ public boolean matches(@Nullable K1 key1, @Nullable K2 key2) { return Objects.equals(key1, this.key1) && Objects.equals(key2, this.key2); } + /** {@link ConcurrentHashtable.Entry#matches(Entry)} in terms of the key-based overload. */ + @Override + public final boolean matches(@Nonnull Entry other) { + return matches(other.key1, other.key2); + } + /** Returns the 64-bit lookup hash combining both key parts via {@link LongHashingUtils}. */ public static long hash(@Nullable Object key1, @Nullable Object key2) { return LongHashingUtils.hash(key1, key2); } } - private final State state; + private final State> state; - private D2(State state) { + private D2(State> state) { this.state = state; } /** * Creates a fixed-size table holding at most {@code maxCapacity} entries. {@code entryClass} is - * used only to infer the concrete entry type; entries are created by the functions passed to - * the insertion methods. The table does not resize. + * used only to allocate the backing array with the right component type; entries themselves are + * created by the {@code creator}/{@code evictable} functions passed to the insertion methods. + * The table does not resize. */ @Nonnull + @SuppressWarnings("unchecked") public static > D2 createBounded( @Nonnull Class entryClass, int maxCapacity) { - return new D2<>(State.createBounded(entryClass, maxCapacity)); + // entryClass is erased away (see ConcurrentHashtable#createFixedBuckets), so treating it as + // Class> instead of the caller's concrete Class is safe. + Class> baseEntryClass = (Class>) (Class) entryClass; + return new D2<>(ConcurrentHashtable.createBounded(baseEntryClass, maxCapacity)); + } + + /** + * Sound because every entry ever inserted into {@link #state} was produced as a {@code TEntry}. + */ + @SuppressWarnings("unchecked") + private TEntry cast(@Nullable Entry entry) { + return (TEntry) entry; + } + + /** See {@link #cast(Entry)}; casts the functional-interface reference, not each element. */ + @SuppressWarnings("unchecked") + private Predicate> castPredicate(Predicate predicate) { + return (Predicate>) predicate; + } + + @SuppressWarnings("unchecked") + private Consumer> castConsumer(Consumer consumer) { + return (Consumer>) consumer; + } + + @SuppressWarnings("unchecked") + private BiConsumer> castConsumer( + BiConsumer consumer) { + return (BiConsumer>) consumer; } public int size() { @@ -428,11 +539,11 @@ public boolean isFull() { @Nullable public TEntry get(@Nullable K1 key1, @Nullable K2 key2) { long keyHash = D2.Entry.hash(key1, key2); - for (TEntry curEntry = bucketFor(state, keyHash); + for (Entry curEntry = bucketFor(state, keyHash); curEntry != null; curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { - return curEntry; + return cast(curEntry); } } return null; @@ -442,15 +553,12 @@ public TEntry get(@Nullable K1 key1, @Nullable K2 key2) { * Returns the entry for {@code (key1, key2)}, creating one via {@code creator} if absent and * the table is under capacity. Lock-free on hit; acquires a table-level lock on miss. Wraps * {@link #tryGetOrCreateOrNull} — see that method for the refusal and ordering details. - * - *

    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 tryGetOrCreate( @Nullable K1 key1, @Nullable K2 key2, - @Nonnull BiFunction creator) { + @Strategy @Nonnull BiFunction creator) { return Maybe.of(tryGetOrCreateOrNull(key1, key2, creator)); } @@ -460,24 +568,27 @@ public Maybe tryGetOrCreate( * {@code (key1, key2)} was not already present. Re-checks under the lock to avoid duplicate * entries under concurrent misses. */ + @StrategyConsumer @Nullable public TEntry tryGetOrCreateOrNull( @Nullable K1 key1, @Nullable K2 key2, - @Nonnull BiFunction creator) { + @Strategy @Nonnull BiFunction creator) { long keyHash = D2.Entry.hash(key1, key2); int index = bucketIndex(state.buckets, keyHash); - for (TEntry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { + for (Entry curEntry = bucketAt(state, index); + curEntry != null; + curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { - return curEntry; + return cast(curEntry); } } synchronized (getTableWriteLock(state)) { - for (TEntry curEntry = bucketAt(state, index); + for (Entry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { - return curEntry; + return cast(curEntry); } } // isFull() is checked before creating the entry, not before reserving a slot for it: @@ -504,8 +615,8 @@ public TEntry tryGetOrCreateOrNull( public Maybe tryGetOrCreateOrEvict( @Nullable K1 key1, @Nullable K2 key2, - @Nonnull BiFunction creator, - @Nonnull Predicate evictable) { + @Strategy @Nonnull BiFunction creator, + @Strategy @Nonnull Predicate evictable) { return Maybe.of(tryGetOrCreateOrEvictOrNull(key1, key2, creator, evictable)); } @@ -516,29 +627,32 @@ public Maybe tryGetOrCreateOrEvict( * ever leaving a slot double-booked. A creator that throws after a successful eviction simply * leaves the table one entry smaller — no corruption, just a wasted eviction. */ + @StrategyConsumer @Nullable public TEntry tryGetOrCreateOrEvictOrNull( @Nullable K1 key1, @Nullable K2 key2, - @Nonnull BiFunction creator, - @Nonnull Predicate evictable) { + @Strategy @Nonnull BiFunction creator, + @Strategy @Nonnull Predicate evictable) { long keyHash = D2.Entry.hash(key1, key2); int index = bucketIndex(state.buckets, keyHash); - for (TEntry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { + for (Entry curEntry = bucketAt(state, index); + curEntry != null; + curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { - return curEntry; + return cast(curEntry); } } synchronized (getTableWriteLock(state)) { - for (TEntry curEntry = bucketAt(state, index); + for (Entry curEntry = bucketAt(state, index); curEntry != null; curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { - return curEntry; + return cast(curEntry); } } if (state.sizeManager.isFull() - && state.sizeManager.evictOne(state.buckets, evictable) == null) { + && state.sizeManager.evictOne(state.buckets, castPredicate(evictable)) == null) { return null; } TEntry newEntry = creator.apply(key1, key2); @@ -558,14 +672,14 @@ public TEntry remove(@Nullable K1 key1, @Nullable K2 key2) { long keyHash = D2.Entry.hash(key1, key2); int index = bucketIndex(state.buckets, keyHash); synchronized (getTableWriteLock(state)) { - TEntry prev = null; - for (TEntry curEntry = bucketAt(state, index); + Entry prev = null; + for (Entry curEntry = bucketAt(state, index); curEntry != null; prev = curEntry, curEntry = curEntry.next()) { if (curEntry.keyHash == keyHash && curEntry.matches(key1, key2)) { unlink(state, index, prev, curEntry); state.sizeManager.decrement(); - return curEntry; + return cast(curEntry); } } return null; @@ -577,8 +691,8 @@ public TEntry remove(@Nullable K1 key1, @Nullable K2 key2) { * Holds the table-level lock for the whole sweep, so the predicate sees a stable table and * concurrent writers are excluded; lock-free readers continue throughout. */ - public boolean removeIf(@Nonnull Predicate predicate) { - return ConcurrentHashtable.removeIf(state, predicate); + public boolean removeIf(@Strategy @Nonnull Predicate predicate) { + return ConcurrentHashtable.removeIf(state, castPredicate(predicate)); } /** @@ -588,8 +702,8 @@ public boolean removeIf(@Nonnull Predicate predicate) { * *

    Use {@link #drain(Object, BiConsumer)} to avoid a capturing lambda. */ - public void drain(@Nonnull Consumer sink) { - ConcurrentHashtable.drain(state, sink); + public void drain(@Strategy @Nonnull Consumer sink) { + ConcurrentHashtable.drain(state, castConsumer(sink)); } /** @@ -597,8 +711,9 @@ public void drain(@Nonnull Consumer sink) { * a {@code static final}) plus the accumulator as {@code context} (e.g. the target list or * event builder) to avoid a capturing-lambda allocation. */ - public void drain(C context, @Nonnull BiConsumer sink) { - ConcurrentHashtable.drain(state, context, sink); + public void drain( + C context, @Strategy @Nonnull BiConsumer sink) { + ConcurrentHashtable.drain(state, context, castConsumer(sink)); } /** Removes all entries. Lock-free readers mid-walk complete against the entries they hold. */ @@ -606,16 +721,17 @@ public void clear() { ConcurrentHashtable.clear(state); } - public void forEach(@Nonnull Consumer consumer) { - ConcurrentHashtable.forEach(state, consumer); + public void forEach(@Strategy @Nonnull Consumer consumer) { + ConcurrentHashtable.forEach(state, castConsumer(consumer)); } /** * Context-passing forEach. Avoids a capturing-lambda allocation — pass a non-capturing {@link * BiConsumer} (typically a {@code static final}) plus whatever side-band state it needs. */ - public void forEach(C context, @Nonnull BiConsumer consumer) { - ConcurrentHashtable.forEach(state, context, consumer); + public void forEach( + C context, @Strategy @Nonnull BiConsumer consumer) { + ConcurrentHashtable.forEach(state, context, castConsumer(consumer)); } } @@ -663,8 +779,9 @@ public boolean isFull() { * cannot both acquire the last slot. Returns {@code false} with the count unchanged when the * table is full. * - *

    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) { @@ -674,6 +791,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. @@ -682,9 +820,9 @@ public boolean tryReserve() { * an abandoned reservation permanently consumes capacity. */ @GuardedBy("getTableWriteLock(buckets)") - public boolean tryReserveOrEvict( + public > boolean tryReserveOrEvict( @Nonnull AtomicReferenceArray buckets, - @Nonnull Predicate evictable) { + @Strategy @Nonnull Predicate evictable) { if (tryReserve()) { return true; } @@ -733,9 +871,9 @@ public void release(int removed) { */ @GuardedBy("getTableWriteLock(buckets)") @Nullable - public TEntry evictOne( + public > TEntry evictOne( @Nonnull AtomicReferenceArray buckets, - @Nonnull Predicate evictable) { + @Strategy @Nonnull Predicate evictable) { TEntry evicted = evictOneInRange(buckets, evictable, evictionCursor, buckets.length()); if (evicted == null && evictionCursor != 0) { evicted = evictOneInRange(buckets, evictable, 0, evictionCursor); @@ -757,10 +895,11 @@ public TEntry evictOne( justification = "evictionCursor is read and written only under synchronized (getTableWriteLock(buckets)); SpotBugs" + " cannot model that dynamic guard") + @StrategyConsumer @Nullable - private TEntry evictOneInRange( + private > TEntry evictOneInRange( @Nonnull AtomicReferenceArray buckets, - @Nonnull Predicate evictable, + @Strategy @Nonnull Predicate evictable, int startBucket, int endBucket) { for (int i = startBucket; i < endBucket; i++) { @@ -788,9 +927,10 @@ private TEntry evictOneInRange( justification = "evictionCursor is read and written only under synchronized (getTableWriteLock(buckets)); SpotBugs" + " cannot model that dynamic guard") - public int evictAll( + @StrategyConsumer + public > int evictAll( @Nonnull AtomicReferenceArray buckets, - @Nonnull Predicate evictable) { + @Strategy @Nonnull Predicate evictable) { int count = 0; for (int i = 0; i < buckets.length(); i++) { TEntry prev = null; @@ -812,25 +952,30 @@ public int evictAll( /** * Bucket array and occupancy manager for a caller-defined capped table. Keep them paired and * prefer the {@code State}-accepting helpers so structural changes update the count consistently. + * + *

    {@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 { + public static final class State> { public final AtomicReferenceArray buckets; - public final SizeManager sizeManager; + final SizeManager sizeManager; private State(AtomicReferenceArray buckets, int maxCapacity) { this.buckets = buckets; this.sizeManager = new SizeManager(maxCapacity); } + } - /** - * Creates a bucket array for {@code maxCapacity} entries and pairs it with a manager enforcing - * that cap. {@code entryClass} is used only to infer {@code TEntry}. - */ - @Nonnull - public static State createBounded( - @Nonnull Class entryClass, int maxCapacity) { - return new State<>(createFixedBuckets(entryClass, maxCapacity), maxCapacity); - } + /** + * Creates a bucket array for {@code maxCapacity} entries and pairs it with a manager enforcing + * that cap. {@code entryClass} is used only to infer {@code TEntry}. + */ + @Nonnull + public static > State createBounded( + @Nonnull Class entryClass, int maxCapacity) { + return new State<>(createFixedBuckets(entryClass, maxCapacity), maxCapacity); } /** Live entries in {@code state}; see {@link SizeManager#estimateSize()}. Lock-free. */ @@ -845,6 +990,237 @@ public static boolean isFull(@Nonnull State state) { return state.sizeManager.isFull(); } + /** + * Reserves one slot in {@code state} without evicting; see {@link SizeManager#tryReserve()}. + * Lock-free — does not acquire the table write lock. Returns {@code false} with the table + * unchanged when it is full. + * + *

    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 > boolean tryReserveSlot( + @Nonnull State state) { + return state.sizeManager.tryReserve(); + } + + /** + * Claims one slot in {@code state} and returns a handle for completing the find-or-insert + * protocol, or an empty handle if the table is full. Lock-free — does not acquire the table write + * lock. Never returns {@code null}, so this always composes with try-with-resources: + * + *

    {@code
    +   * try (Reservation r = ConcurrentHashtable.tryReserve(state)) {
    +   *   return r.tryGetOrInsertOrNull(component1, component2, component3, TEntry::new);
    +   * }
    +   * }
    + * + *

    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. + * + *

    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. + */ + @Nonnull + public static > Reservation tryReserve( + @Nonnull State state) { + return new Reservation<>(state.sizeManager.tryReserve() ? state : null); + } + + /** + * Handle returned by {@link #tryReserve}, gating {@link #tryGetOrInsertOrNull} behind a claimed + * slot and auto-cancelling it on {@link #close} if it's never consumed. A single {@code + * Reservation} must be used for at most one {@code tryGetOrInsertOrNull} call. + * + *

    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. + * + * @param the table's entry type, itself self-bound (see {@link + * ConcurrentHashtable.Entry}) + */ + public static final class Reservation> implements AutoCloseable { + @Nullable private final State state; + private boolean consumed; + + private Reservation(@Nullable State state) { + this.state = state; + } + + /** {@code true} if this is a real, claimed reservation rather than an empty one. */ + public boolean isReserved() { + return state != null; + } + + /** + * Escape hatch for a caller that already built {@code newEntry} itself -- e.g. more than 4 key + * components, or components the caller wants to keep as primitives rather than boxing them into + * a {@code Function}'s type argument: + * + *

    {@code
    +     * try (Reservation r = ConcurrentHashtable.tryReserve(state)) {
    +     *   if (!r.isReserved()) {
    +     *     return null;
    +     *   }
    +     *   return r.tryGetOrInsertOrNull(new TEntry(longComponent1, longComponent2));
    +     * }
    +     * }
    + * + * 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 factory) { + return state == null ? null : finish(factory.apply(a)); + } + + /** + * Two key components. Builds {@code factory.apply(...)} (skipped entirely if this reservation + * is empty — the table was full) 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} only when this + * reservation is empty; otherwise always returns a real entry (the newly built one, or the + * concurrent match). + * + *

    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 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 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 factory) { + return state == null ? null : finish(factory.apply(a, b, c, d)); + } + + /** {@link Maybe}-wrapping counterpart of {@link #tryGetOrInsertOrNull(Entry)}. */ + @Nonnull + public Maybe tryGetOrInsert(@Nonnull TEntry newEntry) { + return Maybe.of(tryGetOrInsertOrNull(newEntry)); + } + + /** + * {@link Maybe}-wrapping counterpart of {@link #tryGetOrInsertOrNull(Object, Function)}, for + * callers who'd rather make the "this can fail unlike unbounded collections" outcome visible in + * the return type than rely on a {@code null} check — mirrors {@link D1#tryGetOrCreate} / + * {@link D2#tryGetOrCreate} wrapping their own {@code ...OrNull} methods. + */ + @Nonnull + public Maybe tryGetOrInsert( + A a, @Strategy @Nonnull Function factory) { + return Maybe.of(tryGetOrInsertOrNull(a, factory)); + } + + /** Two key components; see {@link #tryGetOrInsert(Object, Function)}. */ + @Nonnull + public Maybe tryGetOrInsert( + A a, B b, @Strategy @Nonnull BiFunction factory) { + return Maybe.of(tryGetOrInsertOrNull(a, b, factory)); + } + + /** Three key components; see {@link #tryGetOrInsert(Object, Function)}. */ + @Nonnull + public Maybe tryGetOrInsert( + A a, + B b, + C c, + @Strategy @Nonnull Function3 factory) { + return Maybe.of(tryGetOrInsertOrNull(a, b, c, factory)); + } + + /** Four key components; see {@link #tryGetOrInsert(Object, Function)}. */ + @Nonnull + public Maybe tryGetOrInsert( + A a, + B b, + C c, + D d, + @Strategy @Nonnull + Function4 factory) { + return Maybe.of(tryGetOrInsertOrNull(a, b, c, d, factory)); + } + + private TEntry finish(@Nonnull TEntry newEntry) { + synchronized (getTableWriteLock(state)) { + 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; + } + } + insertHeadEntryAt(state, index, newEntry); + consumed = true; + return newEntry; + } + } + + /** Gives back an unconsumed reservation's slot; a no-op on an empty reservation. */ + @Override + public void close() { + if (state != null && !consumed) { + state.sizeManager.cancelReservation(); + } + } + } + + /** 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 @@ -853,8 +1229,8 @@ 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 boolean tryReserveOrEvict( - @Nonnull State state, @Nonnull Predicate evictable) { + public static > boolean tryReserveOrEvict( + @Nonnull State state, @Strategy @Nonnull Predicate evictable) { synchronized (getTableWriteLock(state)) { return state.sizeManager.tryReserveOrEvict(state.buckets, evictable); } @@ -866,8 +1242,8 @@ public static boolean tryReserveOrEvict( * Self-locking. */ @Nullable - public static TEntry evictOne( - @Nonnull State state, @Nonnull Predicate evictable) { + public static > TEntry evictOne( + @Nonnull State state, @Strategy @Nonnull Predicate evictable) { synchronized (getTableWriteLock(state)) { return state.sizeManager.evictOne(state.buckets, evictable); } @@ -877,8 +1253,8 @@ public static TEntry evictOne( * Unlinks every entry in {@code state} matching {@code evictable}, decrementing per removal, and * returns how many went. Self-locking. */ - public static int evictAll( - @Nonnull State state, @Nonnull Predicate evictable) { + public static > int evictAll( + @Nonnull State state, @Strategy @Nonnull Predicate evictable) { synchronized (getTableWriteLock(state)) { return state.sizeManager.evictAll(state.buckets, evictable); } @@ -901,7 +1277,7 @@ public static int evictAll( * reflective allocation or runtime type checks; it only lets the compiler infer {@code TEntry}. */ @Nonnull - public static AtomicReferenceArray createFixedBuckets( + public static > AtomicReferenceArray createFixedBuckets( @Nonnull Class entryClass, int capacity) { return new AtomicReferenceArray<>(sizeFor(capacity)); } @@ -996,14 +1372,14 @@ public static int bucketIndex(@Nonnull AtomicReferenceArray buckets, long key * bucket. */ @Nullable - public static TEntry bucketFor( + public static > TEntry bucketFor( @Nonnull AtomicReferenceArray buckets, long keyHash) { return buckets.get(bucketIndex(buckets, keyHash)); } /** {@link #bucketFor(AtomicReferenceArray, long)} over a {@link State}. */ @Nullable - public static TEntry bucketFor( + public static > TEntry bucketFor( @Nonnull State state, long keyHash) { return bucketFor(state.buckets, keyHash); } @@ -1015,17 +1391,86 @@ public static TEntry bucketFor( * overload of it. */ @Nullable - public static TEntry bucketAt( + public static > TEntry bucketAt( @Nonnull AtomicReferenceArray buckets, int index) { return buckets.get(index); } /** {@link #bucketAt(AtomicReferenceArray, int)} over a {@link State}. */ @Nullable - public static TEntry bucketAt(@Nonnull State state, int index) { + public static > TEntry bucketAt( + @Nonnull State state, int index) { return bucketAt(state.buckets, index); } + /** + * Returns a lock-free iterator over the candidates for {@code keyHash}: entries in the bucket + * chain that {@code keyHash} maps to (starting from {@link #bucketFor(AtomicReferenceArray, + * long)}) whose own {@link Entry#keyHash} equals it, skipping any other entry sharing the same + * bucket via hash collision on {@link #bucketIndex}. Callers only need a {@code matches} check + * against the entries this yields, not a {@code keyHash} check of their own. + * + *

    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 > Iterator hashIterator( + @Nonnull AtomicReferenceArray buckets, long keyHash) { + return new Iterator() { + private TEntry next = advance(bucketFor(buckets, keyHash)); + + private TEntry advance(TEntry candidate) { + while (candidate != null && candidate.keyHash != keyHash) { + candidate = candidate.next(); + } + return candidate; + } + + @Override + public boolean hasNext() { + return next != null; + } + + @Override + public TEntry next() { + TEntry current = next; + if (current == null) { + throw new NoSuchElementException(); + } + next = advance(current.next()); + return current; + } + }; + } + + /** {@link #hashIterator(AtomicReferenceArray, long)} over a {@link State}. */ + @Nonnull + public static > Iterator hashIterator( + @Nonnull State state, long keyHash) { + return hashIterator(state.buckets, keyHash); + } + + /** + * {@link Iterable} wrapper around {@link #hashIterator(AtomicReferenceArray, long)}, for callers + * that want a plain for-each loop over the candidates for {@code keyHash} rather than driving the + * {@link Iterator} by hand. + */ + @Nonnull + public static > Iterable hashIterable( + @Nonnull AtomicReferenceArray buckets, long keyHash) { + return () -> hashIterator(buckets, keyHash); + } + + /** {@link #hashIterable(AtomicReferenceArray, long)} over a {@link State}. */ + @Nonnull + public static > Iterable hashIterable( + @Nonnull State state, long keyHash) { + return hashIterable(state.buckets, keyHash); + } + /** * Publishes {@code entry} as the head of bucket {@code index}. The helper writes the entry's * {@code next} link before the volatile {@link AtomicReferenceArray#set}; a volatile bucket read @@ -1036,7 +1481,7 @@ public static TEntry bucketAt(@Nonnull State stat * retains its {@code next} link for readers already traversing that chain. */ @GuardedBy("getWriteLockAt(buckets, index)") - public static void insertHeadEntryAt( + public static > void insertHeadEntryAt( @Nonnull AtomicReferenceArray buckets, int index, @Nonnull TEntry entry) { assert Thread.holdsLock(getWriteLockAt(buckets, index)) : "insertHeadEntryAt called without holding getWriteLockAt(buckets, index)"; @@ -1050,7 +1495,7 @@ public static void insertHeadEntryAt( /** {@link #insertHeadEntryAt(AtomicReferenceArray, int, Entry)} over a {@link State}. */ @GuardedBy("getWriteLockAt(state, index)") - public static void insertHeadEntryAt( + public static > void insertHeadEntryAt( @Nonnull State state, int index, @Nonnull TEntry entry) { insertHeadEntryAt(state.buckets, index, entry); } @@ -1061,7 +1506,7 @@ public static void insertHeadEntryAt( * getOrCreate} that reuses it across the lock-free pre-check). */ @GuardedBy("getWriteLock(buckets, keyHash)") - public static void insertHeadEntryFor( + public static > void insertHeadEntryFor( @Nonnull AtomicReferenceArray buckets, long keyHash, @Nonnull TEntry entry) { insertHeadEntryAt(buckets, bucketIndex(buckets, keyHash), entry); } @@ -1075,7 +1520,7 @@ public static void insertHeadEntryFor( * reservations. */ @GuardedBy("getTableWriteLock(state)") - public static void insertReserved( + public static > void insertReserved( @Nonnull State state, long keyHash, @Nonnull TEntry entry) { insertHeadEntryFor(state.buckets, keyHash, entry); } @@ -1090,7 +1535,7 @@ public static void insertReserved( * Does not touch size accounting. */ @GuardedBy("getWriteLockAt(buckets, index)") - public static void unlink( + public static > void unlink( @Nonnull AtomicReferenceArray buckets, int index, @Nullable TEntry prev, @@ -1107,7 +1552,7 @@ public static void unlink( /** {@link #unlink(AtomicReferenceArray, int, Entry, Entry)} over a {@link State}. */ @GuardedBy("getWriteLockAt(state, index)") - public static void unlink( + public static > void unlink( @Nonnull State state, int index, @Nullable TEntry prev, @Nonnull TEntry entry) { unlink(state.buckets, index, prev, entry); } @@ -1118,10 +1563,11 @@ public static void unlink( * predicate sees a stable table and concurrent writers are excluded; lock-free readers continue * throughout. */ - public static boolean removeIf( + @StrategyConsumer + public static > boolean removeIf( @Nonnull AtomicReferenceArray buckets, @Nonnull AtomicInteger size, - @Nonnull Predicate predicate) { + @Strategy @Nonnull Predicate predicate) { synchronized (getTableWriteLock(buckets)) { boolean removed = false; for (int i = 0; i < buckets.length(); i++) { @@ -1146,8 +1592,9 @@ public static boolean removeIf( * occupancy with a {@link State} instead of a bare counter — used by {@link D1#removeIf} and * {@link D2#removeIf}. */ - public static boolean removeIf( - @Nonnull State state, @Nonnull Predicate predicate) { + @StrategyConsumer + public static > boolean removeIf( + @Nonnull State state, @Strategy @Nonnull Predicate predicate) { AtomicReferenceArray buckets = state.buckets; synchronized (getTableWriteLock(state)) { boolean removed = false; @@ -1175,8 +1622,9 @@ public static boolean removeIf( * *

    The sink must not throw. If it does, the partial drain is not rolled back. */ - public static void drain( - @Nonnull AtomicReferenceArray buckets, @Nonnull Consumer sink) { + public static > void drain( + @Nonnull AtomicReferenceArray buckets, + @Strategy @Nonnull Consumer sink) { drainCounting(buckets, sink); } @@ -1185,8 +1633,10 @@ public static void drain( * sink}, so a {@link State} form can subtract exactly that from its {@link SizeManager} instead * of zeroing. The count is free here: the sweep already visits every entry. */ - private static int drainCounting( - @Nonnull AtomicReferenceArray buckets, @Nonnull Consumer sink) { + @StrategyConsumer + private static > int drainCounting( + @Nonnull AtomicReferenceArray buckets, + @Strategy @Nonnull Consumer sink) { int removed = 0; synchronized (getTableWriteLock(buckets)) { for (int i = 0; i < buckets.length(); i++) { @@ -1205,18 +1655,19 @@ private static int drainCounting( } /** Context-passing variant of {@link #drain(AtomicReferenceArray, Consumer)}. Self-locking. */ - public static void drain( + public static > void drain( @Nonnull AtomicReferenceArray buckets, C context, - @Nonnull BiConsumer sink) { + @Strategy @Nonnull BiConsumer sink) { drainCounting(buckets, context, sink); } /** {@link #drainCounting(AtomicReferenceArray, Consumer)}, context-passing form. */ - private static int drainCounting( + @StrategyConsumer + private static > int drainCounting( @Nonnull AtomicReferenceArray buckets, C context, - @Nonnull BiConsumer sink) { + @Strategy @Nonnull BiConsumer sink) { int removed = 0; synchronized (getTableWriteLock(buckets)) { for (int i = 0; i < buckets.length(); i++) { @@ -1240,18 +1691,18 @@ private static int drainCounting( * freed. Draining without that leaves the cap permanently consumed, so the two belong in one call * rather than as a pair the caller has to remember. */ - public static void drain( - @Nonnull State state, @Nonnull Consumer sink) { + public static > void drain( + @Nonnull State state, @Strategy @Nonnull Consumer sink) { synchronized (getTableWriteLock(state)) { state.sizeManager.release(drainCounting(state.buckets, sink)); } } /** Context-passing form of {@link #drain(State, Consumer)}. */ - public static void drain( + public static > void drain( @Nonnull State state, C context, - @Nonnull BiConsumer sink) { + @Strategy @Nonnull BiConsumer sink) { synchronized (getTableWriteLock(state)) { state.sizeManager.release(drainCounting(state.buckets, context, sink)); } @@ -1273,16 +1724,16 @@ public static void clear(@Nonnull AtomicReferenceArray buckets) { * rather than O(buckets); clear is a rare, whole-table operation, so the walk is affordable and * keeping the count honest is worth more than the constant. */ - private static int clearCounting(@Nonnull AtomicReferenceArray buckets) { + private static int clearCounting(@Nonnull AtomicReferenceArray> buckets) { int removed = 0; synchronized (getTableWriteLock(buckets)) { for (int i = 0; i < buckets.length(); i++) { - Entry head = buckets.get(i); + Entry head = buckets.get(i); if (head == null) { continue; } buckets.set(i, null); - for (Entry e = head; e != null; e = e.next()) { + for (Entry e = head; e != null; e = e.next()) { removed++; } } @@ -1299,8 +1750,10 @@ public static void clear(@Nonnull State state) { } } - public static void forEach( - @Nonnull AtomicReferenceArray buckets, @Nonnull Consumer consumer) { + @StrategyConsumer + public static > void forEach( + @Nonnull AtomicReferenceArray buckets, + @Strategy @Nonnull Consumer consumer) { for (int i = 0; i < buckets.length(); i++) { for (TEntry curEntry = buckets.get(i); curEntry != null; curEntry = curEntry.next()) { consumer.accept(curEntry); @@ -1308,10 +1761,11 @@ public static void forEach( } } - public static void forEach( + @StrategyConsumer + public static > void forEach( @Nonnull AtomicReferenceArray buckets, C context, - @Nonnull BiConsumer consumer) { + @Strategy @Nonnull BiConsumer consumer) { for (int i = 0; i < buckets.length(); i++) { for (TEntry curEntry = buckets.get(i); curEntry != null; curEntry = curEntry.next()) { consumer.accept(context, curEntry); @@ -1320,16 +1774,16 @@ public static void forEach( } /** {@link #forEach(AtomicReferenceArray, Consumer)} over a {@link State}. */ - public static void forEach( - @Nonnull State state, @Nonnull Consumer consumer) { + public static > void forEach( + @Nonnull State state, @Strategy @Nonnull Consumer consumer) { forEach(state.buckets, consumer); } /** {@link #forEach(AtomicReferenceArray, Object, BiConsumer)} over a {@link State}. */ - public static void forEach( + public static > void forEach( @Nonnull State state, C context, - @Nonnull BiConsumer consumer) { + @Strategy @Nonnull BiConsumer consumer) { forEach(state.buckets, context, consumer); } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/LogCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/LogCollectorTest.groovy index 4f798f5bf9f..e82ad0dbad6 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/telemetry/LogCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/LogCollectorTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.api.telemetry import datadog.trace.test.util.DDSpecification +import datadog.trace.util.ConcurrentHashtable class LogCollectorTest extends DDSpecification { @@ -30,7 +31,7 @@ class LogCollectorTest extends DDSpecification { logCollector.addLogMessage("ERROR", "Message 4", null) then: - logCollector.rawLogMessages.size() == 3 + ConcurrentHashtable.estimateSize(logCollector.rawLogMessages) == 3 } void "grouping messages in LogCollector"() { @@ -57,7 +58,7 @@ class LogCollectorTest extends DDSpecification { boolean listContains(Collection list, String logLevel, String message, Throwable t, int count) { for (final def logMsg in list) { - if (logMsg.logLevel == logLevel && logMsg.message == message && logMsg.throwable == t && logMsg.count == count) { + if (logMsg.logLevel == logLevel && logMsg.message == message && logMsg.throwable == t && logMsg.count.get() == count) { return true } } diff --git a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD1Test.java b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD1Test.java index 540eb036c2f..46963236366 100644 --- a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD1Test.java +++ b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD1Test.java @@ -80,7 +80,7 @@ void forEachVisitsAllEntries() { table.tryGetOrCreateOrNull("b", k -> new StringEntry(k, 2)); table.tryGetOrCreateOrNull("c", k -> new StringEntry(k, 3)); Set seen = new HashSet<>(); - table.forEach(e -> seen.add(e.key)); + table.forEach(e -> seen.add(e.key())); assertEquals(3, seen.size()); assertTrue(seen.contains("a")); assertTrue(seen.contains("b")); @@ -94,7 +94,7 @@ void forEachWithContextPassesContext() { table.tryGetOrCreateOrNull("x", k -> new StringEntry(k, 10)); table.tryGetOrCreateOrNull("y", k -> new StringEntry(k, 20)); Set seen = new HashSet<>(); - table.forEach(seen, (ctx, e) -> ctx.add(e.key)); + table.forEach(seen, (ctx, e) -> ctx.add(e.key())); assertEquals(2, seen.size()); assertTrue(seen.contains("x")); assertTrue(seen.contains("y")); @@ -258,7 +258,7 @@ void removeIfRemovesMatchingEntries() { assertTrue(removed); assertEquals(5, table.size()); Set seen = new HashSet<>(); - table.forEach(e -> seen.add(e.key)); + table.forEach(e -> seen.add(e.key())); assertEquals(5, seen.size()); for (String key : seen) { assertNotNull(table.get(key)); @@ -301,7 +301,7 @@ void drainRemovesEveryEntryAndFeedsSink() { int[] sum = {0}; table.drain( e -> { - drained.add(e.key); + drained.add(e.key()); sum[0] += e.value; }); @@ -323,7 +323,7 @@ void drainWithContextFeedsSink() { table.tryGetOrCreateOrNull("b", k -> new StringEntry(k, 2)); Set drained = new HashSet<>(); - table.drain(drained, (ctx, e) -> ctx.add(e.key)); + table.drain(drained, (ctx, e) -> ctx.add(e.key())); assertEquals(new HashSet<>(Arrays.asList("a", "b")), drained); assertEquals(0, table.size()); @@ -421,7 +421,7 @@ void tryGetOrCreateOrEvictEvictsWhenFullAndInsertsNewEntry() { Maybe created = table.tryGetOrCreateOrEvict("new", k -> new StringEntry(k, 2), e -> true); assertTrue(created.isPresent()); - assertEquals("new", created.getOrNull().key); + assertEquals("new", created.getOrNull().key()); assertEquals(1, table.size()); assertNull(table.get("old")); assertSame(created.getOrNull(), table.get("new")); @@ -464,8 +464,9 @@ void tryGetOrCreateOrEvictOrNullEvictionRunsBeforeThrowingCreator() { assertNull(table.get("new")); } + /** Entry holding a key plus one mutable {@code int} payload. */ private static final class StringEntry extends ConcurrentHashtable.D1.Entry { - final int value; + volatile int value; StringEntry(String key, int value) { super(key); @@ -473,6 +474,13 @@ private static final class StringEntry extends ConcurrentHashtable.D1.Entry { + CollidingEntry(CollidingKey key) { + super(key); + } + } + /** Key with a fixed hashCode to force deterministic bucket placement. */ private static final class CollidingKey { final String label; @@ -497,10 +505,4 @@ public boolean equals(Object o) { return fixedHash == that.fixedHash && label.equals(that.label); } } - - private static final class CollidingEntry extends ConcurrentHashtable.D1.Entry { - CollidingEntry(CollidingKey key) { - super(key); - } - } } diff --git a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD2Test.java b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD2Test.java index 182cb4c4f25..e2b3f8dd353 100644 --- a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD2Test.java +++ b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableD2Test.java @@ -45,8 +45,8 @@ void getOrCreateOnMissBuildsEntryViaCreator() { return new PairEntry(k1, k2); }); assertNotNull(created); - assertEquals("a", created.key1); - assertEquals(Integer.valueOf(1), created.key2); + assertEquals("a", created.key1()); + assertEquals(Integer.valueOf(1), created.key2()); assertEquals(1, table.size()); assertEquals(1, createCount[0]); assertSame(created, table.get("a", 1)); @@ -78,7 +78,7 @@ void forEachVisitsBothPairs() { table.tryGetOrCreateOrNull("a", 1, PairEntry::new); table.tryGetOrCreateOrNull("b", 2, PairEntry::new); Set seen = new HashSet<>(); - table.forEach(e -> seen.add(e.key1 + ":" + e.key2)); + table.forEach(e -> seen.add(e.key1() + ":" + e.key2())); assertEquals(2, seen.size()); assertTrue(seen.contains("a:1")); assertTrue(seen.contains("b:2")); @@ -91,7 +91,7 @@ void forEachWithContextPassesContextToConsumer() { table.tryGetOrCreateOrNull("a", 1, PairEntry::new); table.tryGetOrCreateOrNull("b", 2, PairEntry::new); Set seen = new HashSet<>(); - table.forEach(seen, (ctx, e) -> ctx.add(e.key1 + ":" + e.key2)); + table.forEach(seen, (ctx, e) -> ctx.add(e.key1() + ":" + e.key2())); assertEquals(2, seen.size()); assertTrue(seen.contains("a:1")); assertTrue(seen.contains("b:2")); @@ -246,11 +246,11 @@ void removeIfRemovesMatchingEntries() { for (int i = 0; i < 10; i++) { table.tryGetOrCreateOrNull("k", i, PairEntry::new); } - boolean removed = table.removeIf(e -> e.key2 % 2 == 0); // removes key2 0,2,4,6,8 + boolean removed = table.removeIf(e -> e.key2() % 2 == 0); // removes key2 0,2,4,6,8 assertTrue(removed); assertEquals(5, table.size()); Set seen = new HashSet<>(); - table.forEach(e -> seen.add(e.key1 + ":" + e.key2)); + table.forEach(e -> seen.add(e.key1() + ":" + e.key2())); assertEquals(5, seen.size()); } @@ -286,7 +286,7 @@ void drainRemovesEveryEntryAndFeedsSink() { table.tryGetOrCreateOrNull("b", 1, PairEntry::new); Set drained = new HashSet<>(); - table.drain(e -> drained.add(e.key1 + ":" + e.key2)); + table.drain(e -> drained.add(e.key1() + ":" + e.key2())); assertEquals(new HashSet<>(Arrays.asList("a:1", "a:2", "b:1")), drained); assertEquals(0, table.size()); @@ -304,7 +304,7 @@ void drainWithContextFeedsSink() { table.tryGetOrCreateOrNull("b", 2, PairEntry::new); Set drained = new HashSet<>(); - table.drain(drained, (ctx, e) -> ctx.add(e.key1 + ":" + e.key2)); + table.drain(drained, (ctx, e) -> ctx.add(e.key1() + ":" + e.key2())); assertEquals(new HashSet<>(Arrays.asList("a:1", "b:2")), drained); assertEquals(0, table.size()); @@ -348,7 +348,7 @@ void tryGetOrCreateOrEvictEvictsWhenFullAndInsertsNewEntry() { Maybe created = table.tryGetOrCreateOrEvict("new", 2, PairEntry::new, e -> true); assertTrue(created.isPresent()); - assertEquals("new", created.getOrNull().key1); + assertEquals("new", created.getOrNull().key1()); assertEquals(1, table.size()); assertNull(table.get("old", 1)); assertSame(created.getOrNull(), table.get("new", 2)); @@ -391,6 +391,7 @@ void tryGetOrCreateOrEvictOrNullEvictionRunsBeforeThrowingCreator() { assertNull(table.get("new", 2)); } + /** Entry with no payload beyond its two key parts, used to exercise the D2 identity/API. */ private static final class PairEntry extends ConcurrentHashtable.D2.Entry { PairEntry(String key1, Integer key2) { super(key1, key2); diff --git a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableReservationTest.java b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableReservationTest.java new file mode 100644 index 00000000000..f61876b2e66 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableReservationTest.java @@ -0,0 +1,184 @@ +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.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nonnull; +import org.junit.jupiter.api.Test; + +/** Exercises {@link ConcurrentHashtable#tryReserve} and {@link ConcurrentHashtable.Reservation}. */ +class ConcurrentHashtableReservationTest { + + private static final class TestEntry extends ConcurrentHashtable.Entry { + final int value; + + TestEntry(int value) { + super(value); + this.value = value; + } + + @Override + public boolean matches(@Nonnull TestEntry other) { + return value == other.value; + } + } + + @Test + void tryGetOrInsertOrNullInsertsOnMissAndFindsOnHit() { + ConcurrentHashtable.State state = + ConcurrentHashtable.createBounded(TestEntry.class, 4); + + TestEntry first; + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + assertTrue(r.isReserved()); + first = r.tryGetOrInsertOrNull(1, TestEntry::new); + } + assertEquals(1, first.value); + assertEquals(1, ConcurrentHashtable.estimateSize(state)); + + // Reserving again for a key that already exists should discard the reservation and return the + // existing entry, not double-insert or leak the claimed slot. + TestEntry second; + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + second = r.tryGetOrInsertOrNull(1, TestEntry::new); + } + assertSame(first, second); + assertEquals(1, ConcurrentHashtable.estimateSize(state)); + } + + @Test + void reserveOnFullTableIsAbsentAndSkipsTheFactory() { + ConcurrentHashtable.State state = + ConcurrentHashtable.createBounded(TestEntry.class, 1); + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + r.tryGetOrInsertOrNull(1, TestEntry::new); + } + assertTrue(ConcurrentHashtable.isFull(state)); + + AtomicInteger factoryCalls = new AtomicInteger(); + TestEntry result; + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + assertFalse(r.isReserved()); + result = + r.tryGetOrInsertOrNull( + 2, + v -> { + factoryCalls.incrementAndGet(); + return new TestEntry(v); + }); + } + assertNull(result); + assertEquals(0, factoryCalls.get()); + assertEquals(1, ConcurrentHashtable.estimateSize(state)); + } + + @Test + void closeCancelsAnUnconsumedReservation() { + ConcurrentHashtable.State state = + ConcurrentHashtable.createBounded(TestEntry.class, 1); + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + assertTrue(r.isReserved()); + // Deliberately not consuming the reservation. + } + assertEquals(0, ConcurrentHashtable.estimateSize(state)); + assertFalse(ConcurrentHashtable.isFull(state)); + } + + @Test + void tryGetOrInsertWrapsResultInMaybe() { + ConcurrentHashtable.State state = + ConcurrentHashtable.createBounded(TestEntry.class, 1); + + Maybe present; + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + present = r.tryGetOrInsert(1, TestEntry::new); + } + assertTrue(present.isPresent()); + assertEquals(1, present.getOrNull().value); + + Maybe absent; + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + absent = r.tryGetOrInsert(2, TestEntry::new); + } + assertFalse(absent.isPresent()); + assertNull(absent.getOrNull()); + } + + @Test + void closeOnAnAbsentReservationIsANoOp() { + ConcurrentHashtable.State state = + ConcurrentHashtable.createBounded(TestEntry.class, 0); + try (ConcurrentHashtable.Reservation r = ConcurrentHashtable.tryReserve(state)) { + assertFalse(r.isReserved()); + } + assertEquals(0, ConcurrentHashtable.estimateSize(state)); + } + + private static final class ThreePartEntry extends ConcurrentHashtable.Entry { + final String a; + final String b; + final String c; + + ThreePartEntry(String a, String b, String c) { + super(HashingUtils.hash(a, b, c)); + this.a = a; + this.b = b; + this.c = c; + } + + @Override + public boolean matches(@Nonnull ThreePartEntry other) { + return a.equals(other.a) && b.equals(other.b) && c.equals(other.c); + } + } + + private static final class FourPartEntry extends ConcurrentHashtable.Entry { + final String a; + final String b; + final String c; + final String d; + + FourPartEntry(String a, String b, String c, String d) { + super(HashingUtils.hash(a, b, c, d)); + this.a = a; + this.b = b; + this.c = c; + this.d = d; + } + + @Override + public boolean matches(@Nonnull FourPartEntry other) { + return a.equals(other.a) && b.equals(other.b) && c.equals(other.c) && d.equals(other.d); + } + } + + @Test + void tryGetOrInsertOrNullSupportsUpToFourComponents() { + ConcurrentHashtable.State state3 = + ConcurrentHashtable.createBounded(ThreePartEntry.class, 2); + ThreePartEntry three; + try (ConcurrentHashtable.Reservation r = + ConcurrentHashtable.tryReserve(state3)) { + three = r.tryGetOrInsertOrNull("x", "y", "z", ThreePartEntry::new); + } + assertEquals("x", three.a); + assertEquals("y", three.b); + assertEquals("z", three.c); + + ConcurrentHashtable.State state4 = + ConcurrentHashtable.createBounded(FourPartEntry.class, 2); + FourPartEntry four; + try (ConcurrentHashtable.Reservation r = + ConcurrentHashtable.tryReserve(state4)) { + four = r.tryGetOrInsertOrNull("w", "x", "y", "z", FourPartEntry::new); + } + assertEquals("w", four.a); + assertEquals("x", four.b); + assertEquals("y", four.c); + assertEquals("z", four.d); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableSizeManagerTest.java b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableSizeManagerTest.java index 24c581b2380..2fceb4ce3ea 100644 --- a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableSizeManagerTest.java +++ b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableSizeManagerTest.java @@ -40,7 +40,7 @@ void tryReserveSucceedsUnderCapacityAndFailsWhenFull() { @Test void tryReserveOrEvictReservesDirectlyWhenUnderCapacity() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 2); + ConcurrentHashtable.createBounded(TestEntry.class, 2); boolean reserved = tryReserveOrEvict(state, e -> true); assertTrue(reserved); @@ -51,7 +51,7 @@ void tryReserveOrEvictReservesDirectlyWhenUnderCapacity() { @Test void tryReserveOrEvictEvictsWhenFullAndSomethingMatches() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 1); + ConcurrentHashtable.createBounded(TestEntry.class, 1); TestEntry existing = insertAt(state, 0, "existing"); assertTrue(state.sizeManager.tryReserve()); assertTrue(state.sizeManager.isFull()); @@ -65,7 +65,7 @@ void tryReserveOrEvictEvictsWhenFullAndSomethingMatches() { @Test void tryReserveOrEvictFailsAndLeavesTableUntouchedWhenNothingEvictable() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 1); + ConcurrentHashtable.createBounded(TestEntry.class, 1); TestEntry existing = insertAt(state, 0, "existing"); assertTrue(state.sizeManager.tryReserve()); @@ -78,7 +78,7 @@ void tryReserveOrEvictFailsAndLeavesTableUntouchedWhenNothingEvictable() { @Test void insertReservedSplicesWithoutTouchingTheCountAfterATryReserve() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 2); + ConcurrentHashtable.createBounded(TestEntry.class, 2); assertTrue(state.sizeManager.tryReserve()); assertEquals(1, state.sizeManager.estimateSize()); @@ -95,7 +95,7 @@ void insertReservedSplicesWithoutTouchingTheCountAfterATryReserve() { @Test void evictOneReturnsNullAndLeavesCountUnchangedWhenNothingMatches() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 4); + ConcurrentHashtable.createBounded(TestEntry.class, 4); insertAt(state, 0, "a"); state.sizeManager.increment(); @@ -107,7 +107,7 @@ void evictOneReturnsNullAndLeavesCountUnchangedWhenNothingMatches() { @Test void evictOneUnlinksMatchAndDecrementsCount() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 4); + ConcurrentHashtable.createBounded(TestEntry.class, 4); TestEntry a = insertAt(state, 0, "a"); TestEntry b = insertAt(state, 1, "b"); state.sizeManager.increment(); @@ -130,7 +130,7 @@ void evictOneUnlinksMatchAndDecrementsCount() { void evictOneResumesFromLastEvictedBucketAndWrapsAround() { // Bucket-array length 4: keyHash i lands in bucket i. ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 4); + ConcurrentHashtable.createBounded(TestEntry.class, 4); TestEntry e0 = insertAt(state, 0, "e0"); insertAt(state, 2, "e2"); TestEntry e3 = insertAt(state, 3, "e3"); @@ -159,7 +159,7 @@ void evictOneResumesFromLastEvictedBucketAndWrapsAround() { @Test void evictAllRemovesEveryMatchAndReturnsCount() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 8); + ConcurrentHashtable.createBounded(TestEntry.class, 8); for (int i = 0; i < 6; i++) { insertAt(state, i, "e" + i); state.sizeManager.increment(); @@ -179,7 +179,7 @@ void evictAllRemovesEveryMatchAndReturnsCount() { @Test void evictAllResetsCursorSoSubsequentEvictOneScansFromBucketZero() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 4); + ConcurrentHashtable.createBounded(TestEntry.class, 4); insertAt(state, 2, "a"); state.sizeManager.increment(); // Advance the cursor away from 0 via a successful eviction at bucket 2. @@ -203,7 +203,7 @@ void evictAllResetsCursorSoSubsequentEvictOneScansFromBucketZero() { @Test void releaseGivesBackRemovedSlotsAndRestartsScan() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 4); + ConcurrentHashtable.createBounded(TestEntry.class, 4); insertAt(state, 2, "a"); state.sizeManager.increment(); evictOne(state, e -> true); // advances the cursor to 2, count back to 0 @@ -226,7 +226,7 @@ void releaseGivesBackRemovedSlotsAndRestartsScan() { @Test void stateCreateCappedBundlesBucketsAndSizeManager() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 3); + ConcurrentHashtable.createBounded(TestEntry.class, 3); assertEquals(0, state.sizeManager.estimateSize()); assertEquals(3, state.sizeManager.capacity()); assertTrue(state.buckets.length() >= 3); @@ -235,7 +235,7 @@ void stateCreateCappedBundlesBucketsAndSizeManager() { @Test void stateLevelTryReserveOrEvictAndEvictOneAndEvictAllDelegateToSizeManager() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 1); + ConcurrentHashtable.createBounded(TestEntry.class, 1); synchronized (ConcurrentHashtable.getWriteLockAt(state, 0)) { ConcurrentHashtable.insertHeadEntryAt(state, 0, new TestEntry(0, "a")); } @@ -280,7 +280,7 @@ void stateLevelTryReserveOrEvictAndEvictOneAndEvictAllDelegateToSizeManager() { @Test void clearCannotInterleaveBetweenReservationAndInsert() throws InterruptedException { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 1); + ConcurrentHashtable.createBounded(TestEntry.class, 1); insertAt(state, 0, "a"); state.sizeManager.increment(); assertTrue(ConcurrentHashtable.isFull(state)); @@ -318,7 +318,7 @@ void clearCannotInterleaveBetweenReservationAndInsert() throws InterruptedExcept @Test void reservationSurvivesAClearLandingBetweenReserveAndInsert() { ConcurrentHashtable.State state = - ConcurrentHashtable.State.createBounded(TestEntry.class, 2); + ConcurrentHashtable.createBounded(TestEntry.class, 2); insertAt(state, 0, "a"); state.sizeManager.increment(); @@ -420,12 +420,17 @@ private static int evictAll( } /** Entry with a caller-controlled {@code keyHash} so tests can place it in an exact bucket. */ - private static final class TestEntry extends ConcurrentHashtable.Entry { + private static final class TestEntry extends ConcurrentHashtable.Entry { final String label; TestEntry(long keyHash, String label) { super(keyHash); this.label = label; } + + @Override + public boolean matches(TestEntry other) { + return label.equals(other.label); + } } } diff --git a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableStaticsTest.java b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableStaticsTest.java index 8191e396e14..8be6e791b9e 100644 --- a/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableStaticsTest.java +++ b/internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableStaticsTest.java @@ -329,7 +329,7 @@ private static boolean assertionsEnabled() { } /** Primitive-{@code int}-key entry: no boxing, keyHash is the key itself. */ - private static final class IntEntry extends ConcurrentHashtable.Entry { + private static final class IntEntry extends ConcurrentHashtable.Entry { final int key; final int value; @@ -342,6 +342,11 @@ private static final class IntEntry extends ConcurrentHashtable.Entry { boolean matches(int key) { return this.key == key; } + + @Override + public boolean matches(IntEntry other) { + return matches(other.key); + } } /**