Separate store into young/old content to better handle churn from sustained load

[mcculls]

What Does This Do

Previously we sampled a small set of "old" keys to approximate LRU-ish eviction. This degraded under sustained load and didn't scale well with GLOBAL_HARD_LIMIT. We're replacing it with a young/old generational design: writes land in a young concurrent map that ages into the old map at a fixed threshold.

Motivation

Works better with DataDog/dd-trace-java#10479

Contributor Checklist

Jira ticket: [PROJ-IDENT]

[datadog-prod-us1-5]

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 6c1d2ce | Docs | View more details | Give us feedback!}

[Copilot]

Pull request overview

This PR replaces the previous sampled “old key” eviction approach in the field-injection global object store with a two-generation (young/old) design intended to behave more predictably under sustained load and large capacity limits.

Changes:

• Introduces a young/old generational ConcurrentHashMap design in GlobalObjectStore, including ageing and inline/periodic old-generation trimming.
• Updates lookup/write paths to read from young first, then old; writes land in young with capacity enforcement hooks.
• Adjusts the JMH benchmark allocator to simulate non-trivial allocation cost by filling allocated buffers with random bytes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java	Replaces sampled-key eviction with a young/old generational store and new capacity enforcement logic.
field-inject/src/jmh/java/datadog/instrument/fieldinject/ObjectStoreBenchmark.java	Makes the benchmark allocator more realistic by adding random-byte initialization.

Comments suppressed due to low confidence (1)

field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java:209

• Inline stale eviction also removes stale keys only from the captured g snapshot. For the same reason as in removeStaleEntries(), if a StoreKey from a newer generation is polled, it can be dropped without removing the corresponding map entry.

 int attempts = MAX_INLINE_EVICTION_ATTEMPTS;
while (attempts-- > 0 && (staleKey = StoreKey.pollStaleKeys()) != null) {
if (removeEntry(g, staleKey) != null) {
return;
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java:201

• The new generational capacity/eviction behavior (inline stale eviction, ageing, hard-limit old eviction) is not currently covered by unit tests. Existing tests cover basic CRUD/GC cleanup via ObjectStoreTest, but none assert behavior near GLOBAL_SOFT_LIMIT/GLOBAL_HARD_LIMIT or the ageing path.

Add a focused test that drives the store above the thresholds (e.g., by inserting many distinct keys and triggering removeStaleEntries() / inline enforcement) and asserts that size is bounded and entries are evicted as expected.

 private static void enforceCapacity() {
Generations g = generations;
int youngSize = g.young.size();
int totalSize = youngSize + g.old.size();
if (totalSize < INLINE_CLEANUP_THRESHOLD) {

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java:84

• In removeStaleEntries(), the soft-limit trim decrements estimatedSize unconditionally after Iterator.remove(). Under concurrent modifications, the iterator removal may be a no-op (or race with another removal), which can cause the loop to stop early and report/leave the store above GLOBAL_SOFT_LIMIT. Prefer removing by key and only decrementing when a mapping was actually removed.

 while (estimatedSize >= GLOBAL_SOFT_LIMIT && itr.hasNext()) {
itr.next();
itr.remove();
estimatedSize--;
}

field-inject/src/test/java/datadog/instrument/fieldinject/ObjectStoreTest.java:238

• This test allocates and retains 240k distinct keys, which is a relatively expensive unit test (time + heap) for exercising the generational thresholds. The same scenario (age at least once and end above the soft limit) can be covered with fewer inserts while still validating bounded growth.

 int totalInserts = (AGEING_THRESHOLD * 4) + 40_000;

field-inject/src/test/java/datadog/instrument/fieldinject/ObjectStoreTest.java:259

• This test leaves up to ~GLOBAL_SOFT_LIMIT live entries in the static GlobalObjectStore after it completes (the @BeforeEach only drains stale entries). That can make later tests in the same JVM observe a pre-filled store and can change eviction behavior. Consider explicitly removing the entries created by this test before returning.

 int finalSize = ObjectStore.removeStaleEntries();
assertTrue(
finalSize < GLOBAL_HARD_LIMIT,
"Sustained insertion should have triggered eviction rather than unbounded growth");
assertTrue(

[mcculls]

Current state works well in DataDog/dd-trace-java#12119 but too close to release to merge (also needs a feature flag to toggle between the two approaches)

Moving back to draft for now - when I get back I'll look at optimizing the get path to improve inlinability and reduce volatile reads to a minimum.

[mcculls]

@codex review

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java:87

• removeStaleEntries() operates on whatever generation store points to at the time of the volatile read. If the store is aged concurrently while this call is running, the cleanup/soft-limit trim may apply to an older generation and miss the latest generation’s maps. Consider capturing store to a local, running cleanup, and then (optionally) retrying once if store changed during the call, so callers get a best-effort cleanup of the current generation.

 public static int removeStaleEntries() {
return store.doRemoveStaleEntries();
}

field-inject/src/main/java/datadog/instrument/fieldinject/GlobalObjectStore.java:113

• The comment says eviction is "random", but this code evicts in the iterator’s traversal order (which is not random, even if it’s weakly consistent). Either update the comment to reflect that this is arbitrary/iterator-order eviction, or implement actual randomness if that behavior is important.

 // randomly evict old content to keep us below the soft limit
Iterator<StoreKey> itr = oldMap.keySet().iterator();
while (estimatedSize >= GLOBAL_SOFT_LIMIT && itr.hasNext()) {
itr.next();
itr.remove();
estimatedSize--;
}

field-inject/src/test/java/datadog/instrument/fieldinject/ObjectStoreTest.java:244

• This test keeps a List of ~240k strongly referenced keys solely to retrieve the last key, which increases memory pressure and can slow/flakify unit test runs. You can keep only a single lastKey variable updated inside the loop (and avoid the List allocation entirely) while preserving the assertion intent.

 int totalInserts = (AGEING_THRESHOLD * 4) + 40_000;
List<Object> keys = new ArrayList<>(totalInserts);
for (int i = 0; i < totalInserts; i++) {
Object key = new Object();
keys.add(key);
capStore.put(key, i);
}

field-inject/src/test/java/datadog/instrument/fieldinject/ObjectStoreTest.java:249

• This test keeps a List of ~240k strongly referenced keys solely to retrieve the last key, which increases memory pressure and can slow/flakify unit test runs. You can keep only a single lastKey variable updated inside the loop (and avoid the List allocation entirely) while preserving the assertion intent.

 Object lastKey = keys.get(keys.size() - 1);