Skip to content

Fix bugs in GenerationalUtf8Cache constructor and access-time handling #11323

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 4 commits into
base: master
Choose a base branch
from
Open

Fix bugs in GenerationalUtf8Cache constructor and access-time handling #11323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd77994
Fix bugs in GenerationalUtf8Cache constructor and access-time handling
dougqh May 8, 2026
778953c
Address review feedback
dougqh May 8, 2026
adee794
Revert deprecated refreshAcessTime alias
dougqh May 8, 2026
fbea611
Merge branch 'master' into dougqh/utf8-cache-fixes
dougqh May 8, 2026
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
19 changes: 9 additions & 10 deletions communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ public final class GenerationalUtf8Cache implements EncodingCache {

static final int MAX_ENTRY_LEN = 256;

private final CacheEntry[] edenEntries;
final CacheEntry[] edenEntries;
private final int[] edenMarkers;

private final CacheEntry[] tenuredEntries;
final CacheEntry[] tenuredEntries;

private long accessTimeMs;
private double promotionThreshold = INITIAL_PROMOTION_THRESHOLD;
Expand Down Expand Up @@ -120,7 +120,7 @@ public GenerationalUtf8Cache(int capacity) {
public GenerationalUtf8Cache(int edenCapacity, int tenuredCapacity) {
this.accessTimeMs = System.currentTimeMillis();

int edenSize = Caching.cacheSizeFor(Math.min(tenuredCapacity, MAX_EDEN_CAPACITY));
int edenSize = Caching.cacheSizeFor(Math.min(edenCapacity, MAX_EDEN_CAPACITY));
this.edenEntries = new CacheEntry[edenSize];
this.edenMarkers = new int[edenSize];

Expand All @@ -143,7 +143,7 @@ public void updateAccessTime(long accessTimeMs) {
}

/** Updates access time to the @link {@link System#currentTimeMillis()} */
public void refreshAcessTime() {
public void refreshAccessTime() {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 8, 2026

Choose a reason for hiding this comment

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

P2 Badge Restore the old refreshAcessTime method

Renaming this public method removes the refreshAcessTime() symbol from the published GenerationalUtf8Cache class. Any external or out-of-tree code compiled against the previous communication artifact that calls the misspelled method will now fail with NoSuchMethodError at runtime (or fail source compilation) even though the implementation could remain as a deprecated forwarding alias to refreshAccessTime().

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@dougqh dougqh May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

My choice - as described by Claude...

Decided not to keep the deprecated alias. GenerationalUtf8Cache lives in datadog.communication.serialization and is internal to the agent — not a documented public API. grep across the repo confirms there are no callers outside this class itself, and the package isn't shipped as a stable extension point. Carrying a misspelled forwarding method just to hedge against hypothetical out-of-tree compilation isn't worth the API surface debt.

this.updateAccessTime(System.currentTimeMillis());
}

Expand Down Expand Up @@ -215,14 +215,13 @@ public final byte[] getUtf8(String value, long accessTimeMs) {
if (value.length() > MAX_ENTRY_LEN) return CacheEntry.utf8(value);

int adjHash = Caching.adjHash(value);
long lookupTimeMs = this.accessTimeMs;

CacheEntry[] tenuredEntries = this.tenuredEntries;
int matchingTenuredIndex = lookupEntryIndex(tenuredEntries, MAX_TENURED_PROBES, adjHash, value);
if (matchingTenuredIndex != -1) {
CacheEntry tenuredEntry = tenuredEntries[matchingTenuredIndex];

tenuredEntry.hit(lookupTimeMs);
tenuredEntry.hit(accessTimeMs);

this.tenuredHits += 1;
return tenuredEntry.utf8();
Expand All @@ -233,7 +232,7 @@ public final byte[] getUtf8(String value, long accessTimeMs) {
if (matchingEdenIndex != -1) {
CacheEntry edenEntry = edenEntries[matchingEdenIndex];

double hits = edenEntry.hit(lookupTimeMs);
double hits = edenEntry.hit(accessTimeMs);
if (hits > this.promotionThreshold) {
// mark promoted first - to avoid racy insertions
this.promotions += 1;
Expand All @@ -256,8 +255,8 @@ public final byte[] getUtf8(String value, long accessTimeMs) {

CacheEntry newEntry = new CacheEntry(adjHash, value);
// First request was swallowed by marking, so double hit
newEntry.hit(lookupTimeMs);
newEntry.hit(lookupTimeMs);
newEntry.hit(accessTimeMs);
newEntry.hit(accessTimeMs);

// search for empty slot or failing that the MFU entry
int edenMfuIndex = findFirstAvailableOrMfuIndex(edenEntries, MAX_EDEN_PROBES, adjHash);
Expand Down Expand Up @@ -346,7 +345,7 @@ static final boolean lfuInsert(CacheEntry[] entries, int numProbes, CacheEntry n
}
}

// If we get here, then we're evicted the LRU
// If we get here, then we're evicting the LFU
entries[lfuIndex] = newEntry;
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ static final boolean lfuInsert(CacheEntry[] entries, CacheEntry newEntry) {
}
}

// If we get here, then we're evicting the LRU
// If we get here, then we're evicting the LFU
entries[lfuIndex] = newEntry;
return true;
}
Expand Down
48 changes: 48 additions & 0 deletions ...nication/src/test/java/datadog/communication/serialization/GenerationalUtf8CacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ public void capacity() {
assertEquals(128, cache.tenuredCapacity());
}

@Test
public void capacity_twoArg() {
GenerationalUtf8Cache cache = new GenerationalUtf8Cache(64, 256);
assertEquals(64, cache.edenCapacity());
assertEquals(256, cache.tenuredCapacity());
}

@Test
public void maxCapacity() {
GenerationalUtf8Cache cache =
Expand Down Expand Up @@ -82,6 +89,29 @@ public void caching() {
assertNotEquals(0, cache.edenHits);
}

@Test
public void getUtf8_perCallAccessTime_overridesField() {
GenerationalUtf8Cache cache = create();
// The field value should not leak into the entry when an explicit time is supplied.
cache.updateAccessTime(0L);

String value = "bar";
long callTime = 12345L;

// First call only marks; the second call creates the entry.
cache.getUtf8(value, callTime);
cache.getUtf8(value, callTime);

assertEquals(callTime, lookupEdenLastUsedMs(cache, value));

// Drive enough hits to promote into tenured.
while (cache.promotions == 0) {
cache.getUtf8(value, callTime);
}

assertEquals(callTime, lookupTenuredLastUsedMs(cache, value));
}

@Test
public void promotion() {
GenerationalUtf8Cache cache = create();
Expand Down Expand Up @@ -205,6 +235,24 @@ static final String nextValue() {
return baseString + valueSuffix;
}

static long lookupEdenLastUsedMs(GenerationalUtf8Cache cache, String value) {
return lookupLastUsedMs(cache.edenEntries, "edenEntries", value);
}

static long lookupTenuredLastUsedMs(GenerationalUtf8Cache cache, String value) {
return lookupLastUsedMs(cache.tenuredEntries, "tenuredEntries", value);
}

private static long lookupLastUsedMs(
GenerationalUtf8Cache.CacheEntry[] entries, String arrayName, String value) {
for (GenerationalUtf8Cache.CacheEntry entry : entries) {
if (entry != null && value.equals(entry.value)) {
return entry.lastUsedMs();
}
}
throw new AssertionError("entry for value '" + value + "' not found in " + arrayName);
}

static final void printStats(GenerationalUtf8Cache cache) {
System.out.printf(
"eden hits: %5d\tpromotion hits: %5d\tpromotions: %5d\tearly: %5d\tlocal evictions: %5d\tglobal evictions: %5d%n",
Expand Down
Loading