Add @NoEscape marker annotation and apply it to SubSequence and Maybe

.agents/skills/perf-review/references/checks.md

@@ -25,14 +25,15 @@ Format: **pattern** — *expensive when (the interprocedural condition to trace)
 5. **Polymorphic dispatch on a hot path** — *a hot call site becomes polymorphic enough to defeat the runtime's inlining/devirtualization (real for JIT runtimes — JVM/.NET/V8; AOT/interpreted differ)* — **flag-as-measure** ("may defeat devirtualization; verify on the target runtime") — SEV-2/3 — fix: keep hot call sites mono/bi-morphic; specialize.
 6. **FFI / native-boundary crossing on a hot path** *(central to the shared-core effort)* — *a native crossing per-span/per-item (not batched), or transporting strings/objects rather than primitives/IDs* — flag-with-confidence (boundary cost is mechanism-determined; runtime-specific pinning → addendum) — SEV-1/2 (SEV-1 if it blocks/pins under concurrency) — fix: batch (one per flush, not per item); transport interned IDs not strings; keep crossings off the hot/concurrency path.
 7. **Escape / allocation-elision defeated** *(Java/Go/.NET/V8 all have a version)* — *a refactor makes a previously-local object escape (stored, returned, captured by a closure, passed to a virtual/non-inlined call) → silent heap allocation on a hot path* — **flag-as-measure** ("may now escape and allocate; verify with an allocation profiler") — SEV-2/3 — fix: keep it local; avoid the escaping store/capture.
+8. **`@NoEscape` field-storage violation** — *a field (instance or static, directly or as a generic type argument) declared with an `@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: <reason>`-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). **No lint enforces this yet — the AI reviewer is the only check, so this stays here (not under deterministic-lint candidates below) until a checker lands and it can migrate down.**
 
 ## Deterministic-lint candidates (DON'T spend AI budget — make these real lints)
 Fixed-signature, mechanically checkable:
 - per-call cache / regex / expensive-object creation that should be static/once
 - 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)
-- *(grows as patterns prove mechanically checkable — migrate them off the AI as they stabilize)*
+- *(grows as patterns prove mechanically checkable — migrate them off the AI as they stabilize; the `@NoEscape` field-storage check above belongs here once its checker lands)*
 
 ## Java addendum (JVM-specific — mechanism authored with JIT-developer authority; **calibrate production-priority against your own escalation history**)
 Refines the universal checks with JVM mechanics. Quarantined here, for the Java audience that has the substrate.
@@ -45,7 +46,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.

internal-api/src/main/java/datadog/trace/api/function/NoEscape.java

@@ -0,0 +1,78 @@
+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 type that should never be <em>retained</em> -- 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.
+ *
+ * <p>"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.
+ *
+ * <p>Two motivating shapes, both real for existing types in this codebase:
+ *
+ * <ul>
+ *   <li><b>Shared-backing view.</b> A type that 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. Storing an instance pins the entire backing object alive for as long as
+ *       the view is retained, turning an allocation-avoidance trick into a memory leak the moment
+ *       it is kept around rather than consumed and dropped.
+ *   <li><b>Escape-analysis-dependent value.</b> A type deliberately shaped to scalar-replace under
+ *       escape analysis rather than actually allocate, on the assumption that it is constructed,
+ *       consumed, and discarded rather than stored. Assigning an instance to a field or collection
+ *       forces the JIT to materialize it as a real, permanent allocation, defeating the reason the
+ *       type exists -- even though passing the same instance through several method calls first is
+ *       fine.
+ * </ul>
+ *
+ * <p>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 a field (instance
+ * or static) declared with a {@code @NoEscape} type has a reason to be there. The discipline it
+ * names is <b>not yet enforced</b>; hold to it by hand until the checker lands.
+ *
+ * <p>Retention is {@link RetentionPolicy#CLASS}, not {@code SOURCE}: a checker that only has the
+ * compiled classfiles of a module defining a {@code @NoEscape} type (as opposed to its source)
+ * still needs to see the annotation when checking a <em>different</em>, dependent module's fields.
+ * {@code CLASS} keeps it there without exposing it to runtime reflection, which nothing needs.
+ *
+ * <p><b>On a type</b> ({@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 needs a reason is anything that keeps it alive past the operation using it.
+ *
+ * <p><b>Checker contract.</b> 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. 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.
+ *
+ * <ul>
+ *   <li><b>Trigger:</b> a field (instance or static, in any class) whose declared type is annotated
+ *       {@code @NoEscape}, either directly (e.g. {@code SubSequence field;}) or as a generic type
+ *       argument of the field's declared type (e.g. {@code List<SubSequence>}, {@code Map<K,
+ *       Maybe<V>>}), with no comment at the declaration explaining why the retention is safe.
+ *   <li><b>Not a trigger:</b> a local variable, a method parameter, or a method return type -- this
+ *       rule flags <em>storage</em> that outlives the call, not ordinary use within it. Also not a
+ *       trigger: the same field shape, annotated with a comment justifying the retention.
+ *   <li><b>Violation example:</b> {@code private final SubSequence cached;}
+ *   <li><b>Compliant example:</b> {@code private final String cached;} -- materialize the view
+ *       (e.g. call {@code toString()}) before storing it. Or, if retention is a deliberate,
+ *       reviewed exception: {@code // Retained on purpose: <reason>} above the field.
+ *   <li><b>Out of scope (v1):</b> escape through a non-generic/raw container, a capturing lambda,
+ *       or a returned value the caller goes on to store. Flag only the field-declaration shape
+ *       above; widen this contract only once a real case proves it insufficient, rather than
+ *       guessing ahead of one.
+ * </ul>
+ */
+@Documented
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.TYPE)
+public @interface NoEscape {}

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,14 @@
  * 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.
+ *
+ * <p>{@link NoEscape}: the scalar-replacement discipline above only holds while a {@code Maybe} is
+ * 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<T> {
   @Nullable private final T value;
 

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,8 +15,15 @@
  * (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 <code>String</code> only when
  * the value must be retained or handed off.
+ *
+ * <p>{@link NoEscape}: because a <code>SubSequence</code> shares its parent <code>String</code>'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 {
+  // @NoEscape exemption: backed by the interned "" literal, which is already permanently
+  // retained by the JVM -- holding this instance pins nothing beyond what's already immortal.
   public static final SubSequence EMPTY = new SubSequence("", 0, 0);
 
   /**
@@ -29,6 +37,15 @@ public static final SubSequence of(String str, int startIndex) {
   /**
    * SubSequence from <code>beginIndex</code> inclusive to <code>endIndex</code> exclusive of <code>
    * str</code> Equivalent to str.subSequence(str, startIndex, endIndex)
+   *
+   * <p>Unlike {@code String.substring}, this always allocates a view, even when <code>startIndex
+   * == endIndex</code> -- there is no free empty case here, because returning {@link #EMPTY}
+   * instead would mean this factory sometimes returns the singleton and sometimes a fresh instance,
+   * which defeats escape analysis at the call site (see {@link NoEscape}). {@code String.substring}
+   * pays no such cost for an empty range: it returns the interned {@code ""} literal
+   * unconditionally, independent of whether escape analysis succeeds. So on an empty range
+   * specifically, this is a real allocation with no offsetting copy avoided; use {@link #EMPTY}
+   * directly when the range is known to be empty.
    */
   public static final SubSequence of(String str, int startIndex, int endIndex) {
     return new SubSequence(str, startIndex, endIndex);