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. @@ -64,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 8eb12d48465..2398c4d4e05 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
@@ -43,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;
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 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 {@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 {@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 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.
+ *
+ * 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
+ * performance override, addendum J16) -- without needing to read this class's prose above.
+ *
+ * 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.
+ *
+ * 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:
+ * 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 {}
+ *
*/
@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 b1c0985dc3c..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
@@ -29,8 +29,21 @@
*