Extend type-profile pollution to the remaining map/set benchmarks (perf toolbox - benchmark)

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

@@ -51,7 +51,9 @@ 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: pollute Map/Set dispatch before trusting a JMH comparison** *(methodology check on a submitted benchmark, not a production hot-path finding — refines the flag-as-measure posture in J2)*. A JMH benchmark comparing `HashMap`/`HashSet`/`ConcurrentHashMap` (or `TreeMap`/`TreeSet`/`ConcurrentSkipListMap`, or the JDK's immutable `Map.of`/`Set.of` `MapN`/`SetN`) against another collection, that only ever looks up one key class for the whole run, leaves that collection's internal `hashCode()`/`equals()` (or `compareTo`) dispatch site artificially monomorphic — a state production rarely reaches, since those call sites are shared JVM-wide across every key type the whole process uses. An unpolluted benchmark can inflate the affected structure's throughput and flip or exaggerate a comparison (this is exactly what PR #12298 found and fixed: synchronized `HashMap` collapsed ~70% once its dispatch was made megamorphic). Before trusting such a benchmark's numbers, check its `@Setup` calls `datadog.trace.util.BenchmarkUtils.polluteHashDispatch()` (covers `HashSet`/`HashMap`/`ConcurrentHashMap`/`SetN`/`MapN` `equals()` dispatch and `TreeMap`/`TreeSet`/`ConcurrentSkipListMap` `compareTo` dispatch) before the measured operations run. **flag-with-confidence** when a new or edited Map/Set JMH benchmark in this module omits it — fix: call `BenchmarkUtils.polluteHashDispatch()` in `@Setup(Level.Trial)`/`Level.Iteration`, ahead of the benchmarked calls.
+
+- **Toolkit availability — cite only what exists.** Available today: `Strings`, `SubSequence`, `HashingUtils`, `Hashtable`, `BenchmarkUtils.polluteHashDispatch()`/`polluteCompareToDispatch()` 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,15 @@ 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 that never pollute dispatch — treat as unverified.** A benchmark that
+looks up only one key class for its whole run leaves the collection's internal
+`hashCode()`/`equals()` (or `compareTo` for sorted maps/sets) call site artificially
+monomorphic — production hits that same shared call site with whatever key types the whole
+process uses, so it's realistically almost always megamorphic. An unpolluted benchmark can
+overstate a structure's throughput and flip a comparison (PR #12298: a synchronized `HashMap`
+collapsed ~70% once pollution was added). Check that the benchmark's `@Setup` calls
+`datadog.trace.util.BenchmarkUtils.polluteHashDispatch()` before trusting its numbers.
+
 **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 and is
+ * genuinely megamorphic — which is the condition {@code warmUpHashDispatch} reproduces here. 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
+ * megamorphic 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)}.