Skip to content

Add @ForegroundSafe/@BackgroundOnly declared-contract annotations - #12606

Closed
dougqh wants to merge 6 commits into
masterfrom
dougqh/foreground-background-only-check
Closed

dougqh wants to merge 6 commits into
masterfrom
dougqh/foreground-background-only-check

Conversation

@dougqh

@dougqh dougqh commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

  • Adds datadog.trace.api.function.ForegroundSafe/.BackgroundOnly annotations declaring a one-way foreground/background cost contract.
  • Applies @BackgroundOnly/@ForegroundSafe to DDCache, FixedSizeCache, EncodingCache, SimpleUtf8Cache, and GenerationalUtf8Cache as initial markers.
  • Documents the contract in both annotations' Javadoc and a new section in docs/instrumentation_design_guidelines.md.
  • Uses CLASS retention so a future bytecode-level checker (APMLP-1645) can consume the markers.

Motivation

Part of APMLP-1513 / APMLP-1645: giving a declared contract to check foreground/background thread-cost assumptions against, rather than relying on ad hoc judgment calls about whether code is cheap enough for a foreground thread.

Notes

Contributor Checklist

  • Format the title according to the contribution guidelines
  • Assign the type: and (comp: or inst:) labels in addition to any other useful labels
  • Avoid using close, fix, or any linking keywords when referencing an issue
  • Update the CODEOWNERS file on source file addition, migration, or deletion (n/a — no new module/ownership boundary)
  • Update public documentation with any new configuration flags or behaviors (n/a — internal-only marker annotations)
  • Once approved, use merge queue to merge the PR

Jira ticket: APMLP-1513

🤖 Generated with Claude Code

dougqh and others added 6 commits September 22, 2026 12:48
…ndSafe / @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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…16 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 <noreply@anthropic.com>
…lse-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 <noreply@anthropic.com>
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.
@dougqh dougqh added tag: no release notes Changes to exclude from release notes inst: java Core Java language instrumentation tag: ai generated Largely based on code generated by an AI or LLM labels Sep 22, 2026
@dd-octo-sts

dd-octo-sts Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.81 s 14.77 s [-0.5%; +1.0%] (no difference)
startup:insecure-bank:tracing:Agent 13.64 s 13.74 s [-1.6%; +0.2%] (no difference)
startup:petclinic:appsec:Agent 16.98 s 16.97 s [-0.9%; +1.0%] (no difference)
startup:petclinic:iast:Agent 16.83 s 16.91 s [-1.2%; +0.2%] (no difference)
startup:petclinic:profiling:Agent 16.42 s 16.64 s [-2.1%; -0.5%] (maybe better)
startup:petclinic:sca:Agent 16.97 s 16.78 s [+0.2%; +2.0%] (maybe worse)
startup:petclinic:tracing:Agent 16.00 s 16.23 s [-2.3%; -0.6%] (maybe better)

Commit: bad45b54 · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@dougqh

dougqh commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #12860, which adds the perf-review check this PR deferred and makes @ForegroundSafe / @BackgroundOnly perf contracts. Applying the markers to the caches can follow separately.

Written by Claude, reviewed by @dougqh

@dougqh dougqh closed this Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inst: java Core Java language instrumentation tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant