Skip to content

Commit 1d73e24

Browse files
committed
Replace weak-ref NoopContextScope cache with simple allocation
Benchmarking shows these scopes aren't alive long enough to pay for the caching
1 parent fc11418 commit 1d73e24

2 files changed

Lines changed: 11 additions & 65 deletions

File tree

Lines changed: 6 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,24 @@
11
package datadog.context;
22

3-
import java.lang.ref.WeakReference;
4-
53
/** {@link ContextScope} that has no effect on execution units. */
6-
final class NoopContextScope extends WeakReference<Context> implements ContextScope {
4+
final class NoopContextScope implements ContextScope {
75
static final ContextScope ROOT_SCOPE = new NoopContextScope(Context.root());
86

9-
private static final int CACHE_SIZE = 32; // must be power of 2
10-
private static final int SLOT_MASK = CACHE_SIZE - 1;
11-
private static final int MAX_HASH_ATTEMPTS = 3;
12-
13-
/** Bounded cache of no-op scopes to reduce (re)allocations. */
14-
private static final NoopContextScope[] cache = new NoopContextScope[CACHE_SIZE];
15-
16-
@SuppressWarnings({"resource", "StatementWithEmptyBody"})
177
static ContextScope create(Context context) {
18-
if (context == Context.root()) {
19-
return ROOT_SCOPE;
20-
}
21-
int hash = System.identityHashCode(context);
22-
int evictedSlot = -1;
23-
// search by repeated hashing; stop when we find an empty slot,
24-
// a matching slot, or we exhaust all attempts and re-use a slot
25-
for (int i = 1, h = hash; true; i++, h = rehash(h)) {
26-
int slot = SLOT_MASK & h;
27-
NoopContextScope existing = cache[slot];
28-
if (existing != null) {
29-
// slot already used
30-
Context existingContext = existing.get();
31-
if (context == existingContext) {
32-
return existing; // match found
33-
}
34-
if (i < MAX_HASH_ATTEMPTS) {
35-
// still more slots to search
36-
if (existingContext == null && evictedSlot < 0) {
37-
// record first evicted slot for re-use later
38-
evictedSlot = slot;
39-
}
40-
continue; // rehash and try again
41-
}
42-
// exhausted attempts, pick best slot to re-use
43-
if (evictedSlot >= 0) {
44-
slot = evictedSlot; // re-use first evicted slot
45-
} else if (existingContext == null) {
46-
// last hashed slot is itself evicted, re-use it
47-
} else {
48-
slot = SLOT_MASK & hash; // re-use first hashed slot
49-
}
50-
}
51-
return (cache[slot] = new NoopContextScope(context));
52-
}
8+
return context == Context.root() ? ROOT_SCOPE : new NoopContextScope(context);
539
}
5410

11+
private final Context context;
12+
5513
private NoopContextScope(Context context) {
56-
super(context);
14+
this.context = context;
5715
}
5816

5917
@Override
6018
public Context context() {
61-
Context context = get();
62-
// no-op scopes are used when the context is already attached so the reference
63-
// value should still be there; if not then we fall back to empty (root) context
64-
return context != null ? context : Context.root();
19+
return context;
6520
}
6621

6722
@Override
6823
public void close() {}
69-
70-
private static int rehash(int oldHash) {
71-
// scatter in both directions using value close to golden ratio
72-
return Integer.reverseBytes(oldHash * 0x9e3775cd) * 0x9e3775cd;
73-
}
7424
}

components/context/src/test/java/datadog/context/ContextManagerTest.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import static datadog.context.ContextTest.STRING_KEY;
66
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
77
import static org.junit.jupiter.api.Assertions.assertEquals;
8-
import static org.junit.jupiter.api.Assertions.assertSame;
98

109
import java.util.concurrent.ExecutorService;
1110
import java.util.concurrent.Executors;
@@ -62,26 +61,23 @@ void testNoopScopeContextReturnsAttachedContext() {
6261
}
6362

6463
@Test
65-
void testNoopScopeCacheHitReturnsSameScope() {
64+
void testNoopScopeReturnsCorrectContext() {
6665
Context context = root().with(STRING_KEY, "value");
6766
try (ContextScope outer = context.attach()) {
68-
// two consecutive noop scopes for the same context should be the same cached instance
6967
try (ContextScope noop1 = context.attach();
7068
ContextScope noop2 = context.attach()) {
71-
assertSame(noop1, noop2);
69+
assertEquals(context, noop1.context());
70+
assertEquals(context, noop2.context());
7271
}
7372
}
7473
}
7574

7675
@Test
77-
void testNoopScopeCacheHandlesHashCollisions() {
78-
// Cycle through enough contexts to overflow the 32-slot cache and exercise
79-
// the rehash, collision, and slot-reuse paths inside NoopContextScope.create()
76+
void testNoopScopeCorrectContextAcrossManyContexts() {
8077
for (int i = 0; i < 200; i++) {
8178
Context ctx = root().with(STRING_KEY, "ctx-" + i);
8279
try (ContextScope outer = ctx.attach()) {
83-
try (ContextScope noop =
84-
ctx.attach()) { // same-context attach exercises NoopContextScope.create()
80+
try (ContextScope noop = ctx.attach()) {
8581
assertEquals(ctx, noop.context());
8682
}
8783
}

0 commit comments

Comments
 (0)