From 0ab936829d268ccb5179ce3861bd9b84a8cd6698 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 11 Sep 2026 12:32:47 -0400 Subject: [PATCH 1/6] Annotate DDCache/SimpleUtf8Cache/GenerationalUtf8Cache with @ForegroundSafe / @BackgroundOnly DDCache is cheap (small, bounded probe count) -- safe to call from application threads. SimpleUtf8Cache and GenerationalUtf8Cache trade CPU for allocation savings that only pay off when run on a background thread, so they're confined to the background serializer thread. APMLP-1544 Co-Authored-By: Claude Sonnet 5 --- .../communication/serialization/GenerationalUtf8Cache.java | 6 ++++++ .../communication/serialization/SimpleUtf8Cache.java | 6 ++++++ .../src/main/java/datadog/trace/api/cache/DDCache.java | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java b/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java index 099ff56e0e0..9dd1c3b2818 100644 --- a/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java +++ b/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java @@ -1,5 +1,6 @@ package datadog.communication.serialization; +import datadog.trace.api.function.BackgroundOnly; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.nio.charset.StandardCharsets; import javax.annotation.concurrent.ThreadSafe; @@ -15,6 +16,11 @@ * String#getBytes(java.nio.charset.Charset)}. * *

The cache is thread safe. + * + *

{@link BackgroundOnly}: the eden/tenured promotion and recalibration bookkeeping costs more + * than the {@code getBytes} call it replaces, so the saving only shows up as reduced allocation/GC + * pressure on the thread that pays it -- confine it to the background serializer thread, not an + * application thread. */ /* * Cache works by using a 2-level promotion based scheme. diff --git a/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java b/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java index 8eb12d48465..4af30539053 100644 --- a/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java +++ b/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java @@ -1,5 +1,6 @@ package datadog.communication.serialization; +import datadog.trace.api.function.BackgroundOnly; import java.nio.charset.StandardCharsets; import javax.annotation.concurrent.ThreadSafe; @@ -13,6 +14,11 @@ * String#getBytes(java.nio.charset.Charset)}. * *

The cache is thread safe. + * + *

{@link BackgroundOnly}: the bookkeeping (hit counting, LFU eviction scan) costs more than the + * {@code getBytes} call it replaces, so the saving only shows up as reduced allocation/GC pressure + * on the thread that pays it -- confine it to the background serializer thread, not an application + * thread. */ /* * Thread safety is achieved through using CacheEntry objects where the key data diff --git a/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java b/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java index d1cd8024847..17888187a83 100644 --- a/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java +++ b/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java @@ -1,8 +1,15 @@ package datadog.trace.api.cache; +import datadog.trace.api.function.ForegroundSafe; import java.util.function.BiConsumer; import java.util.function.Function; +/** + * {@link ForegroundSafe}: every implementation looks up a key via a small, bounded number of probes + * (no growth, no eviction bookkeeping beyond overwriting a slot) -- cheap enough to call from an + * application thread. + */ +@ForegroundSafe public interface DDCache { /** * Look up or create and store a value in the cache. From f48481002f474d29d92e1e8d0c692bd0bffe34b8 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 11 Sep 2026 13:02:11 -0400 Subject: [PATCH 2/6] Actually apply @BackgroundOnly to the Utf8 cache classes The prior commit documented the constraint in a Javadoc @link but never applied the annotation itself, so J16's grep-based resolution had nothing to match. APMLP-1544 Co-Authored-By: Claude Sonnet 5 --- .../communication/serialization/GenerationalUtf8Cache.java | 1 + .../datadog/communication/serialization/SimpleUtf8Cache.java | 1 + 2 files changed, 2 insertions(+) diff --git a/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java b/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java index 9dd1c3b2818..d0911907116 100644 --- a/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java +++ b/communication/src/main/java/datadog/communication/serialization/GenerationalUtf8Cache.java @@ -70,6 +70,7 @@ * provide better cache utilization. */ @ThreadSafe +@BackgroundOnly @SuppressFBWarnings( value = "IS2_INCONSISTENT_SYNC", justification = diff --git a/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java b/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java index 4af30539053..2398c4d4e05 100644 --- a/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java +++ b/communication/src/main/java/datadog/communication/serialization/SimpleUtf8Cache.java @@ -49,6 +49,7 @@ * a LFU: least frequently used eviction policy is used to free up a slot. */ @ThreadSafe +@BackgroundOnly public final class SimpleUtf8Cache implements EncodingCache { static final int MAX_CAPACITY = 1024; From 04946dea3d413e010b4b4db7ab5ef35d7993014a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 11 Sep 2026 13:57:02 -0400 Subject: [PATCH 3/6] Match @NoEscape's Javadoc shape and clarify @ThreadSafe orthogonality Add the Checker contract section (Trigger/Not-a-trigger/Violation/ Compliant/Out-of-scope) that @NoEscape already uses, so the rule is machine-checkable straight from the annotation's own Javadoc without needing J16's prose. Also note that @BackgroundOnly is orthogonal to JSR-305 @ThreadSafe -- one governs concurrent-caller safety, the other which category of caller is valid at all -- since both now land on the same cache classes. APMLP-1543 Co-Authored-By: Claude Sonnet 5 --- .../trace/api/function/BackgroundOnly.java | 29 +++++++++++++++++++ .../trace/api/function/ForegroundSafe.java | 5 ++++ 2 files changed, 34 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java b/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java index 242652be53d..3c63c885672 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java +++ b/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java @@ -20,12 +20,41 @@ *

The two markers are not symmetric -- see {@link ForegroundSafe} for why it, not this * one, is the strictly stronger guarantee. * + *

Orthogonal to {@code @ThreadSafe} (JSR-305): that annotation says concurrent calls from + * multiple callers are safe; this one says which category of caller is valid at all -- background + * threads (there may be more than one), never the foreground/application thread. A class can, and + * often will, carry both. + * *

On a type ({@link ElementType#TYPE}): every method of this type is background-only * unless a method-level {@link ForegroundSafe} widens it. * *

On a method ({@link ElementType#METHOD}): this method specifically is background-only, * regardless of what the enclosing type declares -- a method-level marker always wins over the * type-level one. + * + *

Checker contract. The rule below is written to be machine-checkable -- by a future + * static checker, or in the meantime by an AI reviewer (see the {@code dd-apm-sdk-review} skill's + * performance override, addendum J16) -- without needing to read this class's prose above. + * + *

*/ @Documented @Retention(RetentionPolicy.SOURCE) diff --git a/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java b/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java index b1c0985dc3c..f25526097a8 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java +++ b/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java @@ -29,6 +29,11 @@ *

On a method ({@link ElementType#METHOD}): this method specifically is foreground-safe, * regardless of what the enclosing type declares -- a method-level marker always wins over the * type-level one. + * + *

Checker contract. This annotation is not itself a trigger -- it is what makes a call + * site not suspect. See {@link BackgroundOnly}'s checker contract for the actual rule: + * code marked {@code @ForegroundSafe} (or carrying no marker) is exactly the caller side of the + * violation that contract flags when it reaches a {@code @BackgroundOnly} symbol. */ @Documented @Retention(RetentionPolicy.SOURCE) From 900fac5d97db5849be5492dd35c93578f4693d39 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 11 Sep 2026 15:26:28 -0400 Subject: [PATCH 4/6] Document inheritance-narrowing rule, switch to CLASS retention, add J16 test coverage Adds the declaration-site inheritance-narrowing rule to both annotations' Javadoc and to the performance lens's J16 addendum (now slugged background-only-contract for stable cross-referencing), switches both annotations from SOURCE to CLASS retention so a future bytecode checker (APMLP-1645) can see them, adds llm-validation fixtures covering the interface-hop and inheritance-widening cases, and documents the foreground/background contract in instrumentation_design_guidelines.md. Co-Authored-By: Claude Sonnet 5 --- .../serialization/EncodingCache.java | 7 ++++++ docs/instrumentation_design_guidelines.md | 16 ++++++++++++++ .../java/datadog/trace/api/cache/DDCache.java | 7 ------ .../trace/api/cache/FixedSizeCache.java | 6 +++++ .../trace/api/function/BackgroundOnly.java | 22 ++++++++++++++++++- .../trace/api/function/ForegroundSafe.java | 9 +++++++- 6 files changed, 58 insertions(+), 9 deletions(-) diff --git a/communication/src/main/java/datadog/communication/serialization/EncodingCache.java b/communication/src/main/java/datadog/communication/serialization/EncodingCache.java index 4833eb8695a..d0b6bc086b3 100644 --- a/communication/src/main/java/datadog/communication/serialization/EncodingCache.java +++ b/communication/src/main/java/datadog/communication/serialization/EncodingCache.java @@ -1,6 +1,13 @@ package datadog.communication.serialization; +import datadog.trace.api.function.BackgroundOnly; + // TODO @FunctionalInterface +/** + * {@link BackgroundOnly}: every implementation ({@link SimpleUtf8Cache}, {@link + * GenerationalUtf8Cache}) is only safe to call from the writer/serializer thread that owns it. + */ +@BackgroundOnly public interface EncodingCache { byte[] encode(CharSequence s); diff --git a/docs/instrumentation_design_guidelines.md b/docs/instrumentation_design_guidelines.md index 305ff651439..6e4676e49d1 100644 --- a/docs/instrumentation_design_guidelines.md +++ b/docs/instrumentation_design_guidelines.md @@ -81,3 +81,19 @@ Context ctx = Context.current(); // GOOD - use the bytecode bridge, static-imported Context ctx = currentContext(); ``` + +### 4. Foreground/background thread cost contract + +**Why it matters:** + +- Advice bodies and span lifecycle methods run on the application (foreground) thread, where added cost is + customer-visible latency, as opposed to a background thread the tracer owns and paces itself +- Code meant to run only on a background thread must never become reachable from a foreground call site + +**What to do:** + +Mark code with `datadog.trace.api.function.ForegroundSafe` (cheap enough for either thread) or `.BackgroundOnly` +(must never be reached from the foreground) where the distinction matters, and see their Javadoc for the full +contract, including the inheritance-narrowing rule for overrides. This is a documentation-and-tooling convention +today (not yet compiler- or runtime-enforced); it is checked by hand and by the `dd-apm-sdk-review` skill's +performance lens. diff --git a/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java b/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java index 17888187a83..d1cd8024847 100644 --- a/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java +++ b/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java @@ -1,15 +1,8 @@ package datadog.trace.api.cache; -import datadog.trace.api.function.ForegroundSafe; import java.util.function.BiConsumer; import java.util.function.Function; -/** - * {@link ForegroundSafe}: every implementation looks up a key via a small, bounded number of probes - * (no growth, no eviction bookkeeping beyond overwriting a slot) -- cheap enough to call from an - * application thread. - */ -@ForegroundSafe public interface DDCache { /** * Look up or create and store a value in the cache. diff --git a/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java b/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java index a8f5fc5f3fa..7818e67ce29 100644 --- a/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java +++ b/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java @@ -1,6 +1,7 @@ package datadog.trace.api.cache; import datadog.trace.api.Pair; +import datadog.trace.api.function.ForegroundSafe; import java.util.Arrays; import java.util.function.BiConsumer; import java.util.function.Function; @@ -16,9 +17,14 @@ * computeIfAbsent is idempotent, or otherwise you might not get back the value you expect * from a cache lookup. * + *

{@link ForegroundSafe}: {@code computeIfAbsent} always does a small, bounded number of probes + * against a fixed-size array -- no growth, no eviction sweep -- cheap enough to call from an + * application thread. + * * @param key type * @param value type */ +@ForegroundSafe abstract class FixedSizeCache implements DDCache { static final int MAXIMUM_CAPACITY = 1 << 30; diff --git a/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java b/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java index 3c63c885672..c20e80df70c 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java +++ b/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java @@ -32,6 +32,17 @@ * regardless of what the enclosing type declares -- a method-level marker always wins over the * type-level one. * + *

Inheritance direction. An override may only narrow a supertype's declared cost, never + * widen it -- the same variance rule as a covariant return type, applied to a cost contract instead + * of a value type. A method overriding a {@code @BackgroundOnly} supertype/interface method may + * itself be marked {@link ForegroundSafe} if that override happens to be cheap: every caller + * holding a reference typed to the supertype already assumed the worse (background-only) case, so a + * cheaper override can't surprise them. The reverse can't be done safely: overriding a {@link + * ForegroundSafe} or unannotated supertype method and marking the override {@code @BackgroundOnly} + * breaks the promise for every existing caller holding a supertype-typed reference, without their + * code changing at all -- this is a violation at the declaration site itself, independent of + * whether any foreground call site currently exists in the diff. + * *

Checker contract. The rule below is written to be machine-checkable -- by a future * static checker, or in the meantime by an AI reviewer (see the {@code dd-apm-sdk-review} skill's * performance override, addendum J16) -- without needing to read this class's prose above. @@ -51,12 +62,21 @@ * {@code @BackgroundOnly}. *

  • Compliant example: the same call made only from the background serializer thread * that owns the cache, or from a method itself marked {@code @BackgroundOnly}. + *
  • Also a trigger, at the declaration site: a method overriding a {@link + * ForegroundSafe} or unannotated supertype/interface method that marks the override + * {@code @BackgroundOnly} -- see "Inheritance direction" above. Flag this the moment it + * appears; it does not require a foreground call site to exist yet. *
  • Out of scope (v1): resolution is grep-only today (there is no APT-generated manifest * yet -- {@code APMLP-1645}), so a callee outside the diff costs a grep per unfamiliar symbol * rather than a lookup; reflection and dynamic-proxy call sites are not resolved at all. + *
  • Propagation across an interface boundary (concrete implementation to the interface + * method it implements, and one hop further into a default method that calls it) is a bounded + * extension of the direct-call trigger above, not a separate rule -- see the {@code + * dd-apm-sdk-review} skill's performance override, addendum {@code background-only-contract}, + * for its exact two-hop scope. * */ @Documented -@Retention(RetentionPolicy.SOURCE) +@Retention(RetentionPolicy.CLASS) @Target({ElementType.TYPE, ElementType.METHOD}) public @interface BackgroundOnly {} diff --git a/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java b/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java index f25526097a8..71fe982d21c 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java +++ b/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java @@ -30,12 +30,19 @@ * regardless of what the enclosing type declares -- a method-level marker always wins over the * type-level one. * + *

    Inheritance direction: an override may narrow a {@link BackgroundOnly} supertype/ + * interface method to {@code @ForegroundSafe} (a cheaper override can't surprise a caller who + * already assumed the worse case) but may never widen a {@code @ForegroundSafe} or unannotated + * supertype method to {@link BackgroundOnly} -- see {@link BackgroundOnly}'s "Inheritance + * direction" for the full rule and why the reverse direction is a declaration-site violation on its + * own. + * *

    Checker contract. This annotation is not itself a trigger -- it is what makes a call * site not suspect. See {@link BackgroundOnly}'s checker contract for the actual rule: * code marked {@code @ForegroundSafe} (or carrying no marker) is exactly the caller side of the * violation that contract flags when it reaches a {@code @BackgroundOnly} symbol. */ @Documented -@Retention(RetentionPolicy.SOURCE) +@Retention(RetentionPolicy.CLASS) @Target({ElementType.TYPE, ElementType.METHOD}) public @interface ForegroundSafe {} From a2b2cac1058fe0997112f1dd58352c792a7fa11c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 11 Sep 2026 15:45:34 -0400 Subject: [PATCH 5/6] Address Codex findings: scope FixedSizeCache's cost claim, fix J16 false-positive FixedSizeCache's @ForegroundSafe Javadoc overclaimed: computeIfAbsent's producer and visit()'s consumer are caller-supplied and can be arbitrarily expensive, so the annotation now scopes its guarantee to the cache's own bookkeeping only. J16's interface-propagation hop-1 flagged any interface-typed call site reachable to a @BackgroundOnly implementer C, even when the diff visibly binds the receiver to a different, safe implementation -- guaranteed unreachable. Hop-1 now checks for a visible binding first and only falls back to flag-with-confidence when the receiver's concrete type isn't resolvable from the diff. Co-Authored-By: Claude Sonnet 5 --- .../java/datadog/trace/api/cache/FixedSizeCache.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java b/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java index 7818e67ce29..3f832874b5c 100644 --- a/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java +++ b/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java @@ -17,9 +17,13 @@ * computeIfAbsent is idempotent, or otherwise you might not get back the value you expect * from a cache lookup. * - *

    {@link ForegroundSafe}: {@code computeIfAbsent} always does a small, bounded number of probes - * against a fixed-size array -- no growth, no eviction sweep -- cheap enough to call from an - * application thread. + *

    {@link ForegroundSafe}: the cache's own bookkeeping -- {@code computeIfAbsent}'s probing, + * {@code clear()}, {@code visit()}'s traversal -- always does a small, bounded amount of work + * against a fixed-size array: no growth, no eviction sweep. This guarantee covers only that + * bookkeeping, not the caller-supplied {@code producer} passed to {@code computeIfAbsent} or the + * {@code consumer} passed to {@code visit()} -- their cost is the caller's responsibility, exactly + * as with any higher-order method; a slow or blocking producer/consumer is not made safe by this + * annotation. * * @param key type * @param value type From bad45b54f135cdf1895968e74a3e95d1b9631c90 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 21 Sep 2026 11:15:47 -0400 Subject: [PATCH 6/6] Move BackgroundOnly to DDCache.visit(), fix unannotated-override wording FixedSizeCache's per-method markers were unreachable by the checker at real call sites, since every caller holds a DDCache-typed reference. Move @BackgroundOnly onto DDCache.visit() itself (mirroring the EncodingCache precedent) and drop it from FixedSizeCache.clear(), which wasn't cost-justified at the small capacities real callers use. Also fixes an inverted "narrow"/"widen" phrase in both markers' javadoc (three independent reviewers flagged it) and updates the J16 example in performance.md to match. --- .../java/datadog/trace/api/cache/DDCache.java | 10 +++++- .../trace/api/cache/FixedSizeCache.java | 19 ++++++----- .../trace/api/function/BackgroundOnly.java | 34 +++++++++++-------- .../trace/api/function/ForegroundSafe.java | 13 +++---- 4 files changed, 47 insertions(+), 29 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java b/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java index d1cd8024847..6a39b5bda96 100644 --- a/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java +++ b/internal-api/src/main/java/datadog/trace/api/cache/DDCache.java @@ -1,5 +1,6 @@ package datadog.trace.api.cache; +import datadog.trace.api.function.BackgroundOnly; import java.util.function.BiConsumer; import java.util.function.Function; @@ -16,6 +17,13 @@ public interface DDCache { /** Clear the cache. */ void clear(); - /** Visits elements currently in the cache; for debugging/triage purposes. */ + /** + * Visits elements currently in the cache; for debugging/triage purposes. + * + *

    {@link BackgroundOnly}: every implementation invokes the caller-supplied {@code consumer} + * once per entry over a full scan, so the cost is unbounded regardless of the implementation's + * own scan shape. + */ + @BackgroundOnly void visit(BiConsumer consumer); } diff --git a/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java b/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java index 3f832874b5c..3baef9a2d13 100644 --- a/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java +++ b/internal-api/src/main/java/datadog/trace/api/cache/FixedSizeCache.java @@ -1,6 +1,7 @@ package datadog.trace.api.cache; import datadog.trace.api.Pair; +import datadog.trace.api.function.BackgroundOnly; import datadog.trace.api.function.ForegroundSafe; import java.util.Arrays; import java.util.function.BiConsumer; @@ -17,18 +18,17 @@ * computeIfAbsent is idempotent, or otherwise you might not get back the value you expect * from a cache lookup. * - *

    {@link ForegroundSafe}: the cache's own bookkeeping -- {@code computeIfAbsent}'s probing, - * {@code clear()}, {@code visit()}'s traversal -- always does a small, bounded amount of work - * against a fixed-size array: no growth, no eviction sweep. This guarantee covers only that - * bookkeeping, not the caller-supplied {@code producer} passed to {@code computeIfAbsent} or the - * {@code consumer} passed to {@code visit()} -- their cost is the caller's responsibility, exactly - * as with any higher-order method; a slow or blocking producer/consumer is not made safe by this - * annotation. + *

    {@link ForegroundSafe} applies to {@code computeIfAbsent}'s probing -- at most 3 slot checks + * against a fixed-size array, no growth, no eviction sweep -- and to {@code clear()}, a plain array + * fill over the same fixed size. Neither covers the caller-supplied {@code producer} passed to + * {@code computeIfAbsent}, whose cost is the caller's responsibility exactly as with any + * higher-order method. {@code visit()} is {@link BackgroundOnly}: it invokes an arbitrary + * caller-supplied {@code consumer} once per entry, so its cost is unbounded regardless of the + * cache's own (bounded) scan. * * @param key type * @param value type */ -@ForegroundSafe abstract class FixedSizeCache implements DDCache { static final int MAXIMUM_CAPACITY = 1 << 30; @@ -76,6 +76,7 @@ static int calculateSize(int capacity) { * @param producer how to create a cached value base on the key if the lookup fails * @return the cached or created and stored value */ + @ForegroundSafe @Override public final V computeIfAbsent(K key, Function producer) { if (key == null) { @@ -109,11 +110,13 @@ public final V computeIfAbsent(K key, Function producer) { return value; } + @ForegroundSafe @Override public void clear() { Arrays.fill(elements, null); } + @BackgroundOnly @Override public void visit(BiConsumer consumer) { for (Pair e : elements) { diff --git a/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java b/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java index c20e80df70c..495cceff866 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java +++ b/internal-api/src/main/java/datadog/trace/api/function/BackgroundOnly.java @@ -32,16 +32,20 @@ * regardless of what the enclosing type declares -- a method-level marker always wins over the * type-level one. * - *

    Inheritance direction. An override may only narrow a supertype's declared cost, never - * widen it -- the same variance rule as a covariant return type, applied to a cost contract instead - * of a value type. A method overriding a {@code @BackgroundOnly} supertype/interface method may - * itself be marked {@link ForegroundSafe} if that override happens to be cheap: every caller - * holding a reference typed to the supertype already assumed the worse (background-only) case, so a - * cheaper override can't surprise them. The reverse can't be done safely: overriding a {@link - * ForegroundSafe} or unannotated supertype method and marking the override {@code @BackgroundOnly} - * breaks the promise for every existing caller holding a supertype-typed reference, without their - * code changing at all -- this is a violation at the declaration site itself, independent of - * whether any foreground call site currently exists in the diff. + *

    Inheritance direction. An override may only narrow a supertype's declared + * cost, never widen it -- the same variance rule as a covariant return type, applied to a cost + * contract instead of a value type, and the same logic LSP applies to pre/postconditions: only a + * promise the supertype actually made can be broken by an override. A method overriding a + * {@code @BackgroundOnly} supertype/interface method may itself be marked {@link ForegroundSafe} if + * that override happens to be cheap: every caller holding a reference typed to the supertype + * already assumed the worse (background-only) case, so a cheaper override can't surprise them. The + * reverse can't be done safely when the supertype method is explicitly {@link ForegroundSafe}: + * overriding it and marking the override {@code @BackgroundOnly} breaks the promise for every + * existing caller holding a supertype-typed reference, without their code changing at all -- this + * is a violation at the declaration site itself, independent of whether any foreground call site + * currently exists in the diff. An unannotated supertype/interface method makes no cost + * promise at all, so marking an override {@code @BackgroundOnly} breaks nothing -- no caller + * holding the supertype-typed reference had assumed either case. * *

    Checker contract. The rule below is written to be machine-checkable -- by a future * static checker, or in the meantime by an AI reviewer (see the {@code dd-apm-sdk-review} skill's @@ -62,10 +66,12 @@ * {@code @BackgroundOnly}. *

  • Compliant example: the same call made only from the background serializer thread * that owns the cache, or from a method itself marked {@code @BackgroundOnly}. - *
  • Also a trigger, at the declaration site: a method overriding a {@link - * ForegroundSafe} or unannotated supertype/interface method that marks the override - * {@code @BackgroundOnly} -- see "Inheritance direction" above. Flag this the moment it - * appears; it does not require a foreground call site to exist yet. + *
  • Also a trigger, at the declaration site: a method overriding an explicitly {@link + * ForegroundSafe} supertype/interface method that marks the override {@code @BackgroundOnly} + * -- see "Inheritance direction" above. Flag this the moment it appears; it does not require + * a foreground call site to exist yet. Overriding an unannotated supertype/interface + * method with {@code @BackgroundOnly} is not a trigger -- the supertype made no promise to + * break. *
  • Out of scope (v1): resolution is grep-only today (there is no APT-generated manifest * yet -- {@code APMLP-1645}), so a callee outside the diff costs a grep per unfamiliar symbol * rather than a lookup; reflection and dynamic-proxy call sites are not resolved at all. diff --git a/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java b/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java index 71fe982d21c..1bb1ce5d2df 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java +++ b/internal-api/src/main/java/datadog/trace/api/function/ForegroundSafe.java @@ -30,12 +30,13 @@ * regardless of what the enclosing type declares -- a method-level marker always wins over the * type-level one. * - *

    Inheritance direction: an override may narrow a {@link BackgroundOnly} supertype/ - * interface method to {@code @ForegroundSafe} (a cheaper override can't surprise a caller who - * already assumed the worse case) but may never widen a {@code @ForegroundSafe} or unannotated - * supertype method to {@link BackgroundOnly} -- see {@link BackgroundOnly}'s "Inheritance - * direction" for the full rule and why the reverse direction is a declaration-site violation on its - * own. + *

    Inheritance direction: like a behavioral-subtyping (LSP) contract, only an actual + * promise from the supertype can be broken -- an override may narrow a {@link BackgroundOnly} + * supertype/interface method to {@code @ForegroundSafe} (a cheaper override can't surprise a caller + * who already assumed the worse case) but may never widen an explicitly {@code @ForegroundSafe} + * supertype method to {@link BackgroundOnly}. An unannotated supertype method makes no + * promise either way, so marking an override {@link BackgroundOnly} is not a violation -- see + * {@link BackgroundOnly}'s "Inheritance direction" for the full rule. * *

    Checker contract. This annotation is not itself a trigger -- it is what makes a call * site not suspect. See {@link BackgroundOnly}'s checker contract for the actual rule: