Add BenchmarkUtils.warmUp to restore cross-arm branch profiles in benchmarks (perf toolkit - benchmarking tools)

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

@@ -52,7 +52,8 @@ Refines the universal checks with JVM mechanics. Quarantined here, for the Java
 - **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.
 - **J11 — composite / multi-dimensional key maps on a hot path → `Hashtable` / `ConcurrentHashtable`** *(refines #1, #3)*. `Map<A, Map<B,V>>` nesting, or a `HashMap` keyed by a composite key (client-side stats, per-`(service, operation, …)` aggregation), allocates nested maps + `Entry` objects + boxes keys on the hot aggregation path. Fix: `datadog.trace.util.Hashtable` (single-threaded, composite-key D1/D2 tables — landed) or `datadog.trace.util.ConcurrentHashtable` (lock-free concurrent, **coming**) — positional composite keys, fewer allocations. flag-as-measure — SEV-2/3.
-- **Toolkit availability — cite only what exists.** Available today: `Strings`, `SubSequence`, `HashingUtils`, `Hashtable` (all `datadog.trace.util`), `RE2J` (`com.google.re2j`). Coming (name as "coming", don't imply it's present): `ConcurrentHashtable`, `StringIndex` (immutable string set/map), `UTF8BytesString.Cache` (recurring-string interner), wider `IntegerCache` (http-status/port boxing), `DDCache` inlining. **J7–J11 route an *existing* #1/#2/#3 finding to a reusable fix — they are not new flag-triggers. Don't raise a finding you wouldn't have raised anyway; the posture (precision, silent-when-unsure, findings-cap-scales-with-diff — see `SKILL.md`) is unchanged.**
+- **J12 — Benchmark validity: is the dispatch under test actually profile-dependent?** *(methodology check on a submitted benchmark, not a production hot-path finding — refines the flag-as-measure posture in J2)*. JMH forks a fresh JVM per benchmark method, so each arm is compiled in a process that has only ever executed that one arm — leaving shared `hashCode()`/`equals()`/`compareTo` call sites unrealistically clean compared with production, where erasure makes them serve every map in the application. This only affects the measurement when the receiver type is **not statically deducible at the benchmark's own call site**: if the key is a final class (`String`) or an exact type from an allocation, C2 devirtualizes on that proof and never consults the profile, so pollution is a no-op that only adds setup allocation. Note that one key class per benchmark is normal and correct — a production `Map<String, V>` call site is monotyped too; variety at the caller is not what makes the internal site megamorphic. **flag-as-measure** when a Map/Set JMH benchmark's measured operation dispatches on a non-final key type, or reaches a strategy/comparator interface with several loaded implementors, and its `@Setup` does not pollute — fix: call `BenchmarkUtils.warmUpHashDispatch(Blackhole)` from `@Setup(Level.Trial)` (trial level, not per-iteration: it runs 50,000 allocation-heavy passes). A one-off measurement made while working on #12298 (not recorded in a committed benchmark) showed both sides — `FlatHashtableIteratorBenchmark`'s four-implementor strategy dispatch fell 37.9 → 18.1 M ops/s when its profile was poisoned, while the `String`-keyed map benchmarks in the same module were unaffected, with LogCompilation confirming the dispatch resolved statically.
+- **Toolkit availability — cite only what exists.** Available today: `Strings`, `SubSequence`, `HashingUtils`, `Hashtable`, `BenchmarkUtils.warmUpHashDispatch(Blackhole)` for JMH setup (all `datadog.trace.util`), `RE2J` (`com.google.re2j`). Coming (name as "coming", don't imply it's present): `ConcurrentHashtable`, `StringIndex` (immutable string set/map), `UTF8BytesString.Cache` (recurring-string interner), wider `IntegerCache` (http-status/port boxing), `DDCache` inlining. **J7–J11 route an *existing* #1/#2/#3 finding to a reusable fix — they are not new flag-triggers. Don't raise a finding you wouldn't have raised anyway; the posture (precision, silent-when-unsure, findings-cap-scales-with-diff — see `SKILL.md`) is unchanged.**
 - **J5 — Cardinality-sensitive aggregator** *(domain-specialized #3)*. Some structures are invisible to the generic "unbounded collection" check because the risk is *cardinality*, not raw size: a config- or user-driven value (tag key, resource name, HTTP URL) feeding a **cardinality-sensitive aggregator** (e.g. the conflating metrics aggregator — each unique label combination = one aggregate; a `maxAggregates` cap bounds OOM but high-cardinality input *thrashes* it: constant eviction, garbled metrics). flag-with-confidence when config/user-driven values reach an aggregator with a per-key budget — **SEV-1** (same class as unbounded memory: correctness + heap impact). Fix: bound the source cardinality before it enters the aggregator, or use sentinel substitution for over-cap values. This surfaced on merged production code more than once in back-test calibration — the capstone pattern where the bot's value concentrates.
 
 ## Instrumentation (ByteBuddy Advice) idioms — dd-trace-java-specific fixes

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

@@ -161,6 +161,27 @@ escape analysis eliminates *local* short-lived allocations, but only when the ob
 Stored in a map, returned, captured by a lambda, or passed to a non-inlined virtual call: it
 escapes, and it's real.
 
+**Map/Set JMH benchmarks whose dispatch is actually reachable — treat as unverified.** JMH forks a
+fresh JVM per benchmark method, so each arm is compiled in a process that has only ever seen that
+one arm's key types. A collection's internal `hashCode()`/`equals()` (or `compareTo`) call site is
+shared JVM-wide in production and hit with every key type the process uses, so the benchmark's
+version of it is unrealistically clean.
+
+But this only matters when the receiver type is **not statically deducible at the benchmark's own
+call site**. If the key is a final class (`String`) or an exact type from an allocation, C2
+devirtualizes on that proof and never consults the profile — pollution is then a no-op, and
+demanding it only adds setup allocation. Having one key class per benchmark is normal and correct;
+a real `Map<String, V>` call site is monotyped too. Erasure is what makes the *internal* site
+megamorphic, not variety at the caller.
+
+A one-off measurement made while working on #12298 (not recorded in a committed benchmark) showed
+both sides: `FlatHashtableIteratorBenchmark`'s strategy dispatch fell 37.9 → 18.1
+M ops/s (2.1x) when its profile was poisoned, because the strategy is an interface with four loaded
+implementors; the `String`-keyed map benchmarks in the same module were unaffected, and
+LogCompilation confirmed the dispatch was resolved statically. Check that `@Setup` calls
+`datadog.trace.util.BenchmarkUtils.warmUpHashDispatch(Blackhole)` when the dispatch is genuinely
+profile-dependent.
+
 **EA claims for scope/wrapper objects spanning I/O — treat as unverified.** A microbenchmark
 tight-loop can show zero allocation for a scope or wrapper object because C2 inlines through
 everything and scalar-replaces it. In production, scopes almost always wrap I/O — and C2 cannot

internal-api/src/jmh/java/datadog/trace/api/TagMapAccessBenchmark.java

@@ -1,5 +1,6 @@
 package datadog.trace.api;
 
+import datadog.trace.util.BenchmarkUtils;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -56,6 +57,45 @@
  * TagMapAccessBenchmark.insert_hashMap_builderStyle  thrpt    5  28057827.189 ± 1359655.664  ops/s
  * TagMapAccessBenchmark.insert_via_ledger            thrpt    5  41169656.095 ±  773264.754  ops/s
  * </code>
+ *
+ * <p>Rerun on the same machine and JDK (Zulu 17.0.7, {@code @Fork(2)}, {@code @Threads(8)}) with a
+ * new top-level {@code @Setup(Level.Trial)} calling {@link BenchmarkUtils#warmUpHashDispatch} (this
+ * file had none before):
+ *
+ * <pre>{@code
+ * Benchmark                      Score (M ops/s)    Error
+ * getEntry                              97.00      ± 2.37
+ * getObject                             97.70      ± 0.88
+ * insert                                52.67      ± 1.28
+ * insert_hashMap                        69.57      ± 0.65
+ * insert_hashMap_builderStyle           29.51      ± 0.50
+ * insert_via_ledger                     37.15      ± 0.78
+ * }</pre>
+ *
+ * <p>Pollution left these paths materially unchanged, but not for the same reason on both sides.
+ * {@code TagMap} is typed to {@code String} keys throughout ({@link TagMap#getEntry(String)},
+ * {@link TagMap.Ledger#set(String, Object)}) and compares against a {@code String}-declared field,
+ * so it has no {@code Object}-typed dispatch site to pollute — it is insulated by construction,
+ * whatever the JIT decides. {@code HashMap} does have such sites: {@code getNode} calls {@code
+ * hashCode()}/{@code equals()} on an {@code Object}-declared key. Erasure makes that one bytecode
+ * index serve every map in the application, so in production it sees many receiver types — the
+ * condition {@code warmUpHashDispatch} approximates here by broadening that profile. Even so,
+ * {@code get}/{@code put} inline into a caller holding a statically exact {@code String}, so C2
+ * sharpens the argument and devirtualizes without consulting that profile. That is a realistic
+ * condition rather than an artifact — plenty of production call sites do hand HashMap a statically
+ * known key type, and it legitimately gets that benefit. Where the key type is not deducible, the
+ * broadened profile governs HashMap and still does not reach TagMap.
+ *
+ * <p>Consistent with that, most entries move by a couple of percent with overlapping intervals, and
+ * the larger moves go up, which pollution cannot cause.
+ *
+ * <p>The exception is {@code insert_via_ledger}, down about 10% (41.17 to 37.15) with
+ * non-overlapping intervals. That is the most allocation-heavy path measured, and {@code
+ * warmUpHashDispatch} itself allocates heavily before measurement starts, so setup churn shifting
+ * GC state for the trial is a likelier cause than dispatch. Not investigated further.
+ *
+ * <p>Both tables are the same machine and JDK, but the baseline above ran at {@code Cnt 5} against
+ * {@code Cnt 10} here, and on a different day, so small differences carry no weight.
  */
 @BenchmarkMode(Mode.Throughput)
 @OutputTimeUnit(TimeUnit.SECONDS)
@@ -95,6 +135,11 @@ public class TagMapAccessBenchmark {
     }
   }
 
+  @Setup(Level.Trial)
+  public void setUp(Blackhole bh) {
+    BenchmarkUtils.warmUpHashDispatch(bh);
+  }
+
   /**
    * Pre-populated read map, PER-THREAD ({@code Scope.Thread}): each thread owns its own map so
    * reads don't contend on shared mutable state under {@code @Threads(8)}.