From 5a78239397cb0d1bfa505bc1cb89bfcd404a556b Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 1 Sep 2026 16:11:09 -0400 Subject: [PATCH 1/6] Add @NoEscape marker annotation Documentation-and-tooling marker for transient, zero-copy view types (e.g. a substring view sharing its parent String's backing array) meant to live only within a single call frame -- holding one beyond that frame pins the shared backing object alive. No application to a real type yet and no checker -- just the annotation type, following the Strategy/StrategyConsumer marker convention (APMLP-1787). --- .../datadog/trace/api/function/NoEscape.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 internal-api/src/main/java/datadog/trace/api/function/NoEscape.java diff --git a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java new file mode 100644 index 00000000000..0e121993518 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java @@ -0,0 +1,32 @@ +package datadog.trace.api.function; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a transient, zero-copy view type meant to live only within a single call frame -- it must + * not be stored in a field, a collection, a cache, or anywhere else that outlives the call that + * produced it. + * + *

A type like this typically shares backing storage with something it was derived from (e.g. a + * substring view that shares its parent {@code String}'s backing array) to avoid a copy. That + * sharing is exactly why it must not escape: holding an instance can pin the entire backing object + * alive for as long as the view survives, turning an allocation-avoidance trick into a memory leak + * the moment it outlives the frame it was built for. + * + *

This is a documentation-and-tooling marker; it changes no behavior. It exists to telegraph the + * constraint to readers and to give a future checker (see {@code APMLP-1787}) something to verify + * -- that no field (instance or static) is declared with a {@code @NoEscape} type. The discipline + * it names is not yet enforced; hold to it by hand until the checker lands. + * + *

On a type ({@link ElementType#TYPE}): instances of this type must not escape the call + * frame that created them -- do not assign one to a field, put one in a collection, or return one + * to a caller that might hold onto it beyond the current call. + */ +@Documented +@Retention(RetentionPolicy.SOURCE) +@Target(ElementType.TYPE) +public @interface NoEscape {} From ce97c648f43956c80f00664a64d0194e92856435 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 1 Sep 2026 16:16:21 -0400 Subject: [PATCH 2/6] Generalize @NoEscape javadoc beyond shared-backing views Broadens the motivating-shapes list to cover escape-analysis-dependent value types (constructed, consumed, and discarded within one call frame by design) alongside shared-backing views, ahead of applying the annotation to Maybe in addition to SubSequence. --- .../datadog/trace/api/function/NoEscape.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java index 0e121993518..d9cda68f0f1 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java +++ b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java @@ -7,15 +7,23 @@ import java.lang.annotation.Target; /** - * Marks a transient, zero-copy view type meant to live only within a single call frame -- it must - * not be stored in a field, a collection, a cache, or anywhere else that outlives the call that - * produced it. + * Marks a transient type meant to live only within a single call frame -- it must not be stored in + * a field, a collection, a cache, or anywhere else that outlives the call that produced it. * - *

A type like this typically shares backing storage with something it was derived from (e.g. a - * substring view that shares its parent {@code String}'s backing array) to avoid a copy. That - * sharing is exactly why it must not escape: holding an instance can pin the entire backing object - * alive for as long as the view survives, turning an allocation-avoidance trick into a memory leak - * the moment it outlives the frame it was built for. + *

Two motivating shapes, both real for existing types in this codebase: + * + *

* *

This is a documentation-and-tooling marker; it changes no behavior. It exists to telegraph the * constraint to readers and to give a future checker (see {@code APMLP-1787}) something to verify From 2306667bec40935995f7b27cc38493022f590119 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 1 Sep 2026 16:16:21 -0400 Subject: [PATCH 3/6] Apply @NoEscape to SubSequence and Maybe SubSequence shares its parent String's backing array, so holding one past its call frame pins the parent alive. Maybe is designed to scalar-replace under escape analysis and becomes a real, permanent allocation if stored anywhere longer-lived than the call that produced it. Both are exactly the shape the annotation exists to flag; no checker yet (APMLP-1787). --- internal-api/src/main/java/datadog/trace/util/Maybe.java | 7 +++++++ .../src/main/java/datadog/trace/util/SubSequence.java | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/util/Maybe.java b/internal-api/src/main/java/datadog/trace/util/Maybe.java index 5c0a1fb0499..ffd06a127b7 100644 --- a/internal-api/src/main/java/datadog/trace/util/Maybe.java +++ b/internal-api/src/main/java/datadog/trace/util/Maybe.java @@ -1,5 +1,6 @@ package datadog.trace.util; +import datadog.trace.api.function.NoEscape; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Function; @@ -33,7 +34,13 @@ * phiOfTwoAllocations} arm, which uses two distinct interface implementations rather than one * concrete type and therefore fails to scalar-replace on every JDK including 25 -- a different, * stronger failure mode than the one demonstrated here. + * + *

{@link NoEscape}: the scalar-replacement discipline above only holds while a {@code Maybe} is + * constructed, consumed (typically via {@link #update}/{@link #getOrNull}), and discarded within + * one call frame. Storing an instance in a field or collection forces the JIT to materialize it as + * a real, permanent allocation, defeating the reason it exists. */ +@NoEscape public final class Maybe { @Nullable private final T value; diff --git a/internal-api/src/main/java/datadog/trace/util/SubSequence.java b/internal-api/src/main/java/datadog/trace/util/SubSequence.java index abac3ea6a7c..5cc3b257168 100644 --- a/internal-api/src/main/java/datadog/trace/util/SubSequence.java +++ b/internal-api/src/main/java/datadog/trace/util/SubSequence.java @@ -1,5 +1,6 @@ package datadog.trace.util; +import datadog.trace.api.function.NoEscape; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** @@ -14,7 +15,12 @@ * (an offset + length into the existing backing array), so the same parse allocates nothing per * slice. Use it for transient, read-only views; materialize a real String only when * the value must be retained or handed off. + * + *

{@link NoEscape}: because a SubSequence shares its parent String's + * backing array, holding one anywhere longer-lived than the call that produced it (a field, a + * cache, a collection) pins the entire parent string alive for as long as the view survives. */ +@NoEscape public final class SubSequence implements CharSequence { public static final SubSequence EMPTY = new SubSequence("", 0, 0); From e4ff2dba81d11b41ff60e3912277e29777ebb36f Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 1 Sep 2026 16:33:36 -0400 Subject: [PATCH 4/6] Reframe @NoEscape around retention, not call-frame boundaries "Live only within a single call frame" wrongly implied a NoEscape value couldn't be returned or passed to a callback -- both Maybe (returned, chained through update()/getOrNull()) and SubSequence (passed to callbacks, returned from subSequence()) do exactly that. The actual constraint is storage: never a field, collection, or cache, however far the value otherwise travels through ordinary calls. Also adds a "Checker contract" section to the annotation's javadoc (trigger / not-a-trigger / violation / compliant examples) so an AI reviewer can check compliance without reading the prose. --- .../datadog/trace/api/function/NoEscape.java | 52 ++++++++++++++----- .../main/java/datadog/trace/util/Maybe.java | 7 +-- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java index d9cda68f0f1..60eb643983a 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java +++ b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java @@ -7,32 +7,56 @@ import java.lang.annotation.Target; /** - * Marks a transient type meant to live only within a single call frame -- it must not be stored in - * a field, a collection, a cache, or anywhere else that outlives the call that produced it. + * Marks a type that must never be retained -- never assigned to a field, put into a + * collection, or cached -- though it may otherwise flow normally through ordinary code: returned + * from a method, passed to a callback, chained through further calls. The line is storage, not how + * far the value travels: a {@code @NoEscape} value can cross many methods and frames as long as + * nothing along the way parks it somewhere that outlives the operation using it. * *

Two motivating shapes, both real for existing types in this codebase: * *

* *

This is a documentation-and-tooling marker; it changes no behavior. It exists to telegraph the - * constraint to readers and to give a future checker (see {@code APMLP-1787}) something to verify - * -- that no field (instance or static) is declared with a {@code @NoEscape} type. The discipline - * it names is not yet enforced; hold to it by hand until the checker lands. + * constraint to readers and to give a future checker something to verify -- that no field (instance + * or static) is declared with a {@code @NoEscape} type. The discipline it names is not yet + * enforced; hold to it by hand until the checker lands. * - *

On a type ({@link ElementType#TYPE}): instances of this type must not escape the call - * frame that created them -- do not assign one to a field, put one in a collection, or return one - * to a caller that might hold onto it beyond the current call. + *

On a type ({@link ElementType#TYPE}): instances of this type must never be stored in a + * field or collection. Returning one, passing it to a callback, or chaining further calls on it is + * fine -- what is not fine is anything that keeps it alive past the operation using it. + * + *

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 perf-review skill's {@code + * checks.md}) -- without needing to read this class's prose above. + * + *

*/ @Documented @Retention(RetentionPolicy.SOURCE) diff --git a/internal-api/src/main/java/datadog/trace/util/Maybe.java b/internal-api/src/main/java/datadog/trace/util/Maybe.java index ffd06a127b7..4aec3fe290e 100644 --- a/internal-api/src/main/java/datadog/trace/util/Maybe.java +++ b/internal-api/src/main/java/datadog/trace/util/Maybe.java @@ -36,9 +36,10 @@ * stronger failure mode than the one demonstrated here. * *

{@link NoEscape}: the scalar-replacement discipline above only holds while a {@code Maybe} is - * constructed, consumed (typically via {@link #update}/{@link #getOrNull}), and discarded within - * one call frame. Storing an instance in a field or collection forces the JIT to materialize it as - * a real, permanent allocation, defeating the reason it exists. + * constructed, consumed (typically via {@link #update}/{@link #getOrNull}), and discarded rather + * than stored -- returning one, or passing it along to be consumed further downstream, is fine. + * Assigning an instance to a field or collection forces the JIT to materialize it as a real, + * permanent allocation, defeating the reason it exists. */ @NoEscape public final class Maybe { From ded4603e54e9203420a188ea56f4b78353b51b63 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 1 Sep 2026 16:33:37 -0400 Subject: [PATCH 5/6] Wire @NoEscape into perf-review's deterministic-lint checks Adds a deterministic-lint entry pointing at NoEscape's new self-contained checker contract, and cross-references it from J7 (SubSequence's existing retention-trap entry) so the perf-review skill can flag violations from the diff alone. --- .agents/skills/perf-review/references/checks.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.agents/skills/perf-review/references/checks.md b/.agents/skills/perf-review/references/checks.md index cdf601f4083..047be423b9f 100644 --- a/.agents/skills/perf-review/references/checks.md +++ b/.agents/skills/perf-review/references/checks.md @@ -32,6 +32,14 @@ Fixed-signature, mechanically checkable: - boxing in specific hot APIs - using a string-API where an id-API exists on a hot decorator - the existing convention rules (e.g. don't extract one-shot instrumentation methods to constants) +- a field (instance or static, directly or as a generic type argument) declared with an + `@NoEscape`-annotated type (`datadog.trace.api.function.NoEscape`) — the annotation's own + javadoc carries a self-contained "Checker contract" section (trigger / not-a-trigger / + violation example / compliant example) written so this can be checked from the diff alone, + with no other context needed. **flag-with-confidence** — SEV-2/3 (SEV-1 if the annotated type + shares backing storage with something large, per its own javadoc). Current wearers: + `SubSequence`, `Maybe` (see J7 below for `SubSequence`'s specific retention-vs-transient + discriminator). - *(grows as patterns prove mechanically checkable — migrate them off the AI as they stabilize)* ## Java addendum (JVM-specific — mechanism authored with JIT-developer authority; **calibrate production-priority against your own escalation history**) @@ -45,7 +53,7 @@ Refines the universal checks with JVM mechanics. Quarantined here, for the Java - **J4 — GC pressure → *tail* latency** *(refines #1)*. Hot-path allocation → more GC → STW pauses → app **tail** latency, not just throughput (the tracer shares the app heap). ZGC has short pauses but isn't common — assume G1/Parallel. flag-as-measure ("may raise tail latency; verify under load at a realistic heap"). - **J6 — Reference strengthening in weak-cache scans** *(refines #1, #2)*. Calling `WeakReference.get()` (or `SoftReference.get()`) inside a cache-probe loop to identify the referent **strengthens** the reference — the returned strong ref keeps the object alive until it goes out of scope, defeating the purpose of the weak reference. Pattern to flag: a loop over a weak-ref cache that calls `.get()` for identity/equality comparison on every slot probed. Fix: store a stable key (e.g. `System.identityHashCode(context)`) in the wrapper at construction time; compare the key first (plain int, no strengthening); call `.get()` only on a key match (the right moment — you're about to use the referent anyway) or to detect eviction (`get() == null`). **flag-with-confidence** when `.get()` appears inside a probe loop on a hot path — SEV-2/3. -- **J7 — `substring`/slice → `SubSequence` zero-copy view** *(refines #1, #7)*. `String.substring`/`subSequence` allocates a fresh backing array per call. On a hot parse path (headers, tags, query strings, SQL/DBM, propagation) where the slice is **transient** — compared (`equals`/`startsWith`/`contains`/`indexOf`), parsed, or appended, then discarded — a `SubSequence` (offset+length view) is zero-copy and EA-elided **iff** the consumer takes a `CharSequence`/range (else the boundary `toString()` erases the win → add the overload or skip). **flag-as-measure** for the transient case (EA-dependent). The retention trap is **flag-with-confidence**: a `SubSequence` stored in a field/tag/collection/cache pins its *entire* backing String — a small window over a large string is a net memory loss — so a retained view must be materialized or `compact()`'d. Discriminator = transient (view, measure) vs retained (must detach). +- **J7 — `substring`/slice → `SubSequence` zero-copy view** *(refines #1, #7)*. `String.substring`/`subSequence` allocates a fresh backing array per call. On a hot parse path (headers, tags, query strings, SQL/DBM, propagation) where the slice is **transient** — compared (`equals`/`startsWith`/`contains`/`indexOf`), parsed, or appended, then discarded — a `SubSequence` (offset+length view) is zero-copy and EA-elided **iff** the consumer takes a `CharSequence`/range (else the boundary `toString()` erases the win → add the overload or skip). **flag-as-measure** for the transient case (EA-dependent). The retention trap is **flag-with-confidence**: a `SubSequence` stored in a field/tag/collection/cache pins its *entire* backing String — a small window over a large string is a net memory loss — so a retained view must be materialized or `compact()`'d. Discriminator = transient (view, measure) vs retained (must detach). `SubSequence` carries `@NoEscape` — see the deterministic-lint entry above for the general field-storage check this refines. - **J8 — Backtracking regex on external input → RE2J / bounded input** *(distinct from #2 compile-per-call)*. `java.util.regex` backtracks → exponential worst-case (ReDoS) on adversarial input — a CPU / tail-latency / DoS hazard, **not** an allocation one. Flag the **conjunction**: (a) input is user/external-controllable AND (b) the pattern is backtracking-prone (nested/overlapping quantifiers, `(a+)+`, unanchored `.*` around a quantified group). **flag-with-confidence** when both hold — SEV-2 (tail latency), **SEV-1** on a per-request AppSec/security-scan path (IAST Reporter / WAF run regex on untrusted input every request). Fix: RE2J (`com.google.re2j`, guaranteed linear; no backrefs/lookaround), anchor/de-nest, or hard-cap input length. - **J9 — `Objects.hash(...)` varargs / boxing hash on a hot path → `HashingUtils`** *(refines #1)*. The allocation is specific to the **varargs/boxing forms**: `Objects.hash(a, b, …)` allocates an `Object[]` per call and **boxes every primitive** arg; same for boxing primitives into a `new Object[]{…}` (or `Arrays.hashCode` over such an array). Per-span tag/key building, or a hot value object's `hashCode()` built this way, → a guaranteed per-call allocation + boxing. Fix: `datadog.trace.util.HashingUtils` — primitive `hash(long/int/boolean/char/…)` overloads (no boxing), `hash(Object,Object)` and `hash(int,int)` combiners (no array); for >2 fields fold pairwise through `hash(int,int)` (there is no varargs form, by design). flag-with-confidence for the varargs/boxing form — SEV-2/3. **Do NOT flag allocation-free combines** — a hand-rolled `31*h + Long.hashCode(x)` / `31*h + intField`, or `Arrays.hashCode` over an *existing primitive array*, allocates nothing (`HashingUtils` is itself 31-based); flagging them would recommend replacing already-correct code. - **J10 — hot-path `String.format` / string munging → `Strings` (+ `SubSequence`)** *(refines #2)*. `String.format` parses the format string, boxes its args, and allocates on every call — never on a hot path; hand-rolled case-conversion, class/resource-name munging, blank-checks, and truncation recomputed per call qualify too. Fix: `datadog.trace.util.Strings` — allocation-aware `replace`/`truncate(CharSequence)`/`isBlank`/`getResourceName`/`getClassName`/…; for **transient substring compares** prefer a `SubSequence` view (J7); for plain assembly, direct concatenation beats `format`. flag-with-confidence for `String.format` on a hot path; flag-as-measure for borderline munging — SEV-2/3. From f20b2969db4eb9daebfadfbc2fd766ecbdc76197 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 1 Sep 2026 16:58:16 -0400 Subject: [PATCH 6/6] Soften @NoEscape from must to should (RFC-2119 sense) Retention was framed as an absolute prohibition, but a deliberate, reviewed exception (e.g. a container retaining a SubSequence as a precaution) is legitimate as long as it's called out at the retention site rather than done silently. The checker contract's trigger now requires an explanatory comment to be compliant, rather than treating every match as an automatic violation. --- .../skills/perf-review/references/checks.md | 15 ++++++---- .../datadog/trace/api/function/NoEscape.java | 29 ++++++++++++------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/.agents/skills/perf-review/references/checks.md b/.agents/skills/perf-review/references/checks.md index 047be423b9f..a92979d7724 100644 --- a/.agents/skills/perf-review/references/checks.md +++ b/.agents/skills/perf-review/references/checks.md @@ -33,12 +33,15 @@ Fixed-signature, mechanically checkable: - using a string-API where an id-API exists on a hot decorator - the existing convention rules (e.g. don't extract one-shot instrumentation methods to constants) - a field (instance or static, directly or as a generic type argument) declared with an - `@NoEscape`-annotated type (`datadog.trace.api.function.NoEscape`) — the annotation's own - javadoc carries a self-contained "Checker contract" section (trigger / not-a-trigger / - violation example / compliant example) written so this can be checked from the diff alone, - with no other context needed. **flag-with-confidence** — SEV-2/3 (SEV-1 if the annotated type - shares backing storage with something large, per its own javadoc). Current wearers: - `SubSequence`, `Maybe` (see J7 below for `SubSequence`'s specific retention-vs-transient + `@NoEscape`-annotated type (`datadog.trace.api.function.NoEscape`), with no comment at the + declaration justifying the retention — the annotation's own javadoc carries a self-contained + "Checker contract" section (trigger / not-a-trigger / violation example / compliant example) + written so this can be checked from the diff alone, with no other context needed. The + underlying rule is "should", not "must" (RFC-2119 sense): a trigger is a presumptive + finding, not an automatic failure — a field with a `// Retained on purpose: `-style + comment is compliant. **flag-with-confidence** — SEV-2/3 (SEV-1 if the annotated type shares + backing storage with something large, per its own javadoc). Current wearers: `SubSequence`, + `Maybe` (see J7 below for `SubSequence`'s specific retention-vs-transient discriminator). - *(grows as patterns prove mechanically checkable — migrate them off the AI as they stabilize)* diff --git a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java index 60eb643983a..8408709367a 100644 --- a/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java +++ b/internal-api/src/main/java/datadog/trace/api/function/NoEscape.java @@ -7,12 +7,17 @@ import java.lang.annotation.Target; /** - * Marks a type that must never be retained -- never assigned to a field, put into a + * Marks a type that should never be retained -- never assigned to a field, put into a * collection, or cached -- though it may otherwise flow normally through ordinary code: returned * from a method, passed to a callback, chained through further calls. The line is storage, not how * far the value travels: a {@code @NoEscape} value can cross many methods and frames as long as * nothing along the way parks it somewhere that outlives the operation using it. * + *

"Should", in the RFC-2119 sense: retaining an instance is presumed wrong and needs a reason, + * not an absolute prohibition. A deliberate, reviewed exception -- e.g. a container that retains a + * {@code @NoEscape} value as a precaution and has weighed the tradeoff -- is legitimate as long as + * it is called out at the retention site (e.g. a comment explaining why) rather than done silently. + * *

Two motivating shapes, both real for existing types in this codebase: * *

* *

This is a documentation-and-tooling marker; it changes no behavior. It exists to telegraph the - * constraint to readers and to give a future checker something to verify -- that no field (instance - * or static) is declared with a {@code @NoEscape} type. The discipline it names is not yet - * enforced; hold to it by hand until the checker lands. + * constraint to readers and to give a future checker something to verify -- that a field (instance + * or static) declared with a {@code @NoEscape} type has a reason to be there. The discipline it + * names is not yet enforced; hold to it by hand until the checker lands. * - *

On a type ({@link ElementType#TYPE}): instances of this type must never be stored in a + *

On a type ({@link ElementType#TYPE}): instances of this type should not be stored in a * field or collection. Returning one, passing it to a callback, or chaining further calls on it is - * fine -- what is not fine is anything that keeps it alive past the operation using it. + * fine -- what needs a reason is anything that keeps it alive past the operation using it. * *

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 perf-review skill's {@code - * checks.md}) -- without needing to read this class's prose above. + * checks.md}) -- without needing to read this class's prose above. Because the underlying rule is + * "should" rather than "must", a trigger is a presumptive finding to raise, not an automatic + * failure: a field that carries a comment explaining the deliberate exception is compliant. * *