Skip to content

Encapsulate ConcurrentHashtable.SizeManager and relocate createBounded (perf toolbox) #12453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dougqh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Encapsulate ConcurrentHashtable.SizeManager and relocate createBounded (perf toolbox) #12453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 29 additions & 12 deletions internal-api/src/main/java/datadog/trace/util/ConcurrentHashtable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private D1(State<TEntry> state) {
@Nonnull
public static <K, TEntry extends D1.Entry<K>> D1<K, TEntry> createBounded(
@Nonnull Class<TEntry> entryClass, int maxCapacity) {
return new D1<>(State.createBounded(entryClass, maxCapacity));
return new D1<>(ConcurrentHashtable.createBounded(entryClass, maxCapacity));
}

public int size() {
Expand Down Expand Up @@ -414,7 +414,7 @@ private D2(State<TEntry> state) {
@Nonnull
public static <K1, K2, TEntry extends D2.Entry<K1, K2>> D2<K1, K2, TEntry> createBounded(
@Nonnull Class<TEntry> entryClass, int maxCapacity) {
return new D2<>(State.createBounded(entryClass, maxCapacity));
return new D2<>(ConcurrentHashtable.createBounded(entryClass, maxCapacity));
}

public int size() {
Expand Down Expand Up @@ -812,25 +812,30 @@ public <TEntry extends Entry> 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.
*
* <p>{@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<TEntry extends Entry> {
public final AtomicReferenceArray<TEntry> buckets;
public final SizeManager sizeManager;
final SizeManager sizeManager;

private State(AtomicReferenceArray<TEntry> 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 <TEntry extends Entry> State<TEntry> createBounded(
@Nonnull Class<TEntry> 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 <TEntry extends Entry> State<TEntry> createBounded(
@Nonnull Class<TEntry> entryClass, int maxCapacity) {
return new State<>(createFixedBuckets(entryClass, maxCapacity), maxCapacity);
}

/** Live entries in {@code state}; see {@link SizeManager#estimateSize()}. Lock-free. */
Expand All @@ -845,6 +850,18 @@ 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.
*
* <p>Build the entry before reserving: there is no cancellation operation, so abandoning a
* successful reservation permanently consumes capacity. Complete it with {@link #insertReserved}.
*/
public static <TEntry extends Entry> boolean tryReserve(@Nonnull State<TEntry> state) {
return state.sizeManager.tryReserve();
Comment on lines +861 to +862

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep reservation and duplicate recheck atomic

When a custom table follows the advertised lock-free find→reserve→insert flow, concurrent misses for the same logical key cannot be handled correctly by this boolean-only reservation. If one thread claims the last slot before linking its entry, another thread misses and receives false, dropping an occurrence that should have matched the in-flight entry; with multiple slots available, both threads can reserve, after which a locked duplicate recheck must either insert a duplicate or abandon one reservation and permanently consume capacity. The reservation must cover the locked duplicate recheck and insertion, or provide a way to release an unused claim.

Useful? React with 👍 / 👎.

}

/**
* 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
Expand Down
28 changes: 14 additions & 14 deletions internal-api/src/test/java/datadog/trace/util/ConcurrentHashtableSizeManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void tryReserveSucceedsUnderCapacityAndFailsWhenFull() {
@Test
void tryReserveOrEvictReservesDirectlyWhenUnderCapacity() {
ConcurrentHashtable.State<TestEntry> state =
ConcurrentHashtable.State.createBounded(TestEntry.class, 2);
ConcurrentHashtable.createBounded(TestEntry.class, 2);

boolean reserved = tryReserveOrEvict(state, e -> true);
assertTrue(reserved);
Expand All @@ -51,7 +51,7 @@ void tryReserveOrEvictReservesDirectlyWhenUnderCapacity() {
@Test
void tryReserveOrEvictEvictsWhenFullAndSomethingMatches() {
ConcurrentHashtable.State<TestEntry> 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());
Expand All @@ -65,7 +65,7 @@ void tryReserveOrEvictEvictsWhenFullAndSomethingMatches() {
@Test
void tryReserveOrEvictFailsAndLeavesTableUntouchedWhenNothingEvictable() {
ConcurrentHashtable.State<TestEntry> state =
ConcurrentHashtable.State.createBounded(TestEntry.class, 1);
ConcurrentHashtable.createBounded(TestEntry.class, 1);
TestEntry existing = insertAt(state, 0, "existing");
assertTrue(state.sizeManager.tryReserve());

Expand All @@ -78,7 +78,7 @@ void tryReserveOrEvictFailsAndLeavesTableUntouchedWhenNothingEvictable() {
@Test
void insertReservedSplicesWithoutTouchingTheCountAfterATryReserve() {
ConcurrentHashtable.State<TestEntry> state =
ConcurrentHashtable.State.createBounded(TestEntry.class, 2);
ConcurrentHashtable.createBounded(TestEntry.class, 2);
assertTrue(state.sizeManager.tryReserve());
assertEquals(1, state.sizeManager.estimateSize());

Expand All @@ -95,7 +95,7 @@ void insertReservedSplicesWithoutTouchingTheCountAfterATryReserve() {
@Test
void evictOneReturnsNullAndLeavesCountUnchangedWhenNothingMatches() {
ConcurrentHashtable.State<TestEntry> state =
ConcurrentHashtable.State.createBounded(TestEntry.class, 4);
ConcurrentHashtable.createBounded(TestEntry.class, 4);
insertAt(state, 0, "a");
state.sizeManager.increment();

Expand All @@ -107,7 +107,7 @@ void evictOneReturnsNullAndLeavesCountUnchangedWhenNothingMatches() {
@Test
void evictOneUnlinksMatchAndDecrementsCount() {
ConcurrentHashtable.State<TestEntry> 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();
Expand All @@ -130,7 +130,7 @@ void evictOneUnlinksMatchAndDecrementsCount() {
void evictOneResumesFromLastEvictedBucketAndWrapsAround() {
// Bucket-array length 4: keyHash i lands in bucket i.
ConcurrentHashtable.State<TestEntry> 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");
Expand Down Expand Up @@ -159,7 +159,7 @@ void evictOneResumesFromLastEvictedBucketAndWrapsAround() {
@Test
void evictAllRemovesEveryMatchAndReturnsCount() {
ConcurrentHashtable.State<TestEntry> 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();
Expand All @@ -179,7 +179,7 @@ void evictAllRemovesEveryMatchAndReturnsCount() {
@Test
void evictAllResetsCursorSoSubsequentEvictOneScansFromBucketZero() {
ConcurrentHashtable.State<TestEntry> 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.
Expand All @@ -203,7 +203,7 @@ void evictAllResetsCursorSoSubsequentEvictOneScansFromBucketZero() {
@Test
void releaseGivesBackRemovedSlotsAndRestartsScan() {
ConcurrentHashtable.State<TestEntry> 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
Expand All @@ -226,7 +226,7 @@ void releaseGivesBackRemovedSlotsAndRestartsScan() {
@Test
void stateCreateCappedBundlesBucketsAndSizeManager() {
ConcurrentHashtable.State<TestEntry> 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);
Expand All @@ -235,7 +235,7 @@ void stateCreateCappedBundlesBucketsAndSizeManager() {
@Test
void stateLevelTryReserveOrEvictAndEvictOneAndEvictAllDelegateToSizeManager() {
ConcurrentHashtable.State<TestEntry> 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"));
}
Expand Down Expand Up @@ -280,7 +280,7 @@ void stateLevelTryReserveOrEvictAndEvictOneAndEvictAllDelegateToSizeManager() {
@Test
void clearCannotInterleaveBetweenReservationAndInsert() throws InterruptedException {
ConcurrentHashtable.State<TestEntry> state =
ConcurrentHashtable.State.createBounded(TestEntry.class, 1);
ConcurrentHashtable.createBounded(TestEntry.class, 1);
insertAt(state, 0, "a");
state.sizeManager.increment();
assertTrue(ConcurrentHashtable.isFull(state));
Expand Down Expand Up @@ -318,7 +318,7 @@ void clearCannotInterleaveBetweenReservationAndInsert() throws InterruptedExcept
@Test
void reservationSurvivesAClearLandingBetweenReserveAndInsert() {
ConcurrentHashtable.State<TestEntry> state =
ConcurrentHashtable.State.createBounded(TestEntry.class, 2);
ConcurrentHashtable.createBounded(TestEntry.class, 2);
insertAt(state, 0, "a");
state.sizeManager.increment();

Expand Down
Loading