[dougqh]

What Does This Do

Makes "prefer a compare-and-set over a lock whenever possible" part of the perf-review skill (SKILL.md and references/checks.md):

1. New Java check J13: lock guarding publishable state → compare-and-set on an immutable snapshot. It flags:

   • a new synchronized block or ReentrantLock whose critical section only reads the current state, computes the next and writes it back (one field, or several that could form one immutable snapshot)
   • a per-instance lock object (private final Object lock = new Object()) on a per-span or per-trace type

   The suggested fix is an immutable snapshot behind one volatile field, updated by a CAS loop. J13 also tells the reviewer what to check for correctness in a lock → CAS change, and when a lock is still the right choice.

2. Replacing a lock with such a CAS no longer triggers the "visibly-contestable tradeoff" benchmark nudge. That nudge fires for changes that remove a lock. A lock → CAS swap now counts as adopting the preferred idiom, and is reviewed for correctness instead.

3. The SKILL.md summary lists J13, and notes that it's a trigger in its own right, unlike J7–J11, which only route existing findings to a fix.

Motivation

Measured while replacing PTags' per-trace sampling lock with a CAS (2026-10-22, 3 forks per arm):

• −24 B per PTags on every JDK: no lock object or lock field.
• +7–28% throughput in span-level benchmarks on JDK 17 and 21, with forks not overlapping.
• JDK 8 with biased locking off: +40–57%.
• Slower only on JDK 8/11 with default flags. There, biased locking makes an uncontended synchronized nearly free. Those JVMs are about 30% of orgs and shrinking, so the cost is accepted.

From JDK 15 a monitor enter is itself a CAS on the object header, so a lock costs the atomic plus monitor bookkeeping, often plus a lock object per instance. synchronized also pins virtual-thread carriers on JDK 21–23.

The correctness checks in J13 come from the same change. Two mutations were tried against its stress test:

• Blind writes instead of CAS (lost updates): failed the test in 2 of 2 runs.
• Clearing the decision-maker flag after its CAS instead of before: wasn't caught in 3 × 2000 trials. That's why J13 says to reason about ordering rather than rely on a stress test.

Tracked in APMLP-2100. Related: APMLP-2099 (an adaptive updater that would choose synchronized on biased-locking JVMs, coming) and APMLP-1838 (synchronized vs ReentrantLock under virtual threads).

Additional Notes

• Documentation-only change to the skill.
• Overlaps Flag code a change carries into a hotter context, and weight header paths up (AI perf review) #12902 on one line: that PR adds bullets right above the "visibly-contestable tradeoff" clause this PR amends. Whichever merges second needs a trivial rebase.
• J12 is already taken by the benchmark-validity check, so this is J13.
• The PTags lock → CAS change that motivates this will be its own PR.

🤖 Generated with Claude Code

[dd-octo-sts]

🟢 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	13.94 s	14.00 s	[-1.1%; +0.2%] (no difference)
startup:insecure-bank:tracing:Agent	12.93 s	12.99 s	[-1.1%; +0.2%] (no difference)
startup:petclinic:appsec:Agent	17.16 s	17.02 s	[+0.0%; +1.6%] (maybe worse)
startup:petclinic:iast:Agent	16.86 s	17.06 s	[-2.1%; -0.3%] (maybe better)
startup:petclinic:profiling:Agent	16.55 s	16.66 s	[-1.6%; +0.3%] (no difference)
startup:petclinic:sca:Agent	17.06 s	16.93 s	[-0.1%; +1.7%] (no difference)
startup:petclinic:tracing:Agent	16.07 s	16.18 s	[-1.5%; +0.2%] (no difference)

Commit: 2c0afa21 · 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.