Wire mergedTracerTags as a read-through parent at span build (level-split) (phase 1a) - #11932
Wire mergedTracerTags as a read-through parent at span build (level-split) (phase 1a)#11932dougqh wants to merge 3 commits into
Conversation
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
7bb20f4 to
e65e58f
Compare
3a0a318 to
e692601
Compare
e65e58f to
3b9156e
Compare
|
🎯 Code Coverage (details) 🔗 Commit SHA: 5761f89 | Docs | Datadog PR Page | Give us feedback! |
StringIndex is a compact open-addressed string→index structure (the keyOf substrate the dense tag store builds on): parallel hash/name arrays, linear probing, on par with HashSet on lookup at a smaller footprint. Includes unit tests, a footprint test (jol), and comparison benchmarks (vs HashSet/switch). No TagMap changes — standalone util. Rebased onto the level-split stack (consumer #11932) as the layer dense-store sits on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3b9156e to
d2aa48f
Compare
StringIndex is a compact open-addressed string→index structure (the keyOf substrate the dense tag store builds on): parallel hash/name arrays, linear probing, on par with HashSet on lookup at a smaller footprint. Includes unit tests, a footprint test (jol), and comparison benchmarks (vs HashSet/switch). No TagMap changes — standalone util. Rebased onto the level-split stack (consumer #11932) as the layer dense-store sits on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e692601 to
b58bcc0
Compare
d2aa48f to
1f257aa
Compare
StringIndex is a compact open-addressed string→index structure (the keyOf substrate the dense tag store builds on): parallel hash/name arrays, linear probing, on par with HashSet on lookup at a smaller footprint. Includes unit tests, a footprint test (jol), and comparison benchmarks (vs HashSet/switch). No TagMap changes — standalone util. Rebased onto the level-split stack (consumer #11932) as the layer dense-store sits on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bef20fa to
50cf53d
Compare
1f257aa to
3237b90
Compare
50cf53d to
ae64f2a
Compare
3237b90 to
2225a42
Compare
ae64f2a to
5cd8289
Compare
2225a42 to
ea094ce
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea094ce0bc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
More details
54 adversarial synthetic scenarios — simulating the full CoreTracer.buildSpan() wiring from the PR across 8 core behavioral equivalences and 5 edge cases — all passed. The read-through TagMap path is observationally identical to the old copy-down path for every tested scenario: trace-tag visibility, coreTags/contextualTags precedence override, builder-ledger tombstoning, multi-span parent isolation across config updates, and the version-exclusion change that prevents tombstone allocation on the shared frozen parent.
📊 Validated against 54 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit ea094ce · What is Autotest? · Any feedback? Reach out in #autotest
5cd8289 to
ab56044
Compare
ab56044 to
64c42e6
Compare
ea094ce to
8c7bf14
Compare
…plit phase 1) Attach the trace's merged tracer tags to each span's TagMap as a frozen read-through parent (via TagMap.createFromParent) at span construction, instead of copying them into every span. The span sees the shared tags on read and only stores its own local tags, so the common trace-level bundle is held once per trace rather than duplicated per span. - CoreTracer builds the frozen merged-tracer-tags parent once; config version is kept out of that bundle. - DDSpanContext attaches the parent at construction (fixed, no re-parenting). - Adds TagMapReadThroughBenchmark (copy-down vs read-through, -prof gc). Stacked on the read-through mechanism (#11789), which builds on the folded final-class TagMap (#11967). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8c7bf14 to
170eedb
Compare
StringIndex is a compact open-addressed string→index structure (the keyOf substrate the dense tag store builds on): parallel hash/name arrays, linear probing, on par with HashSet on lookup at a smaller footprint. Includes unit tests, a footprint test (jol), and comparison benchmarks (vs HashSet/switch). No TagMap changes — standalone util. Rebased onto the level-split stack (consumer #11932) as the layer dense-store sits on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Before / after: allocation drops ~130–200 B/op per spanRan the front-half span-creation microbenchmark across this PR's commit range ( Allocation —
|
| Arm | Before | After | Δ |
|---|---|---|---|
| bareStartSpan | 1056.0 ± 0.0 | 896.0 ± 12.5 | −160.0 (−15.2%) |
| bareBuildSpan | 1049.6 ± 11.7 | 930.7 ± 49.2 | −118.9 (−11.3%) |
| jdbcClientSpan | 1498.1 ± 42.3 | 1295.4 ± 44.8 | −202.7 (−13.5%) |
| webServerSpan | 1768.0 ± 12.5 | 1640.0 ± 0.0 | −128.0 (−7.2%) |
| webServerSpanViaBuilder | 1949.3 ± 41.7 | 1752.0 ± 0.0 | −197.3 (−10.1%) |
The saving is ~130–200 B/op roughly flat across arms, not scaling with each span's own tag count — the signature of the level-split mechanism: it avoids cloning the fixed shared trace-level bundle, so every span saves about the same regardless of how many of its own tags it sets (even the "bare" arms drop ~150 B/op, since a tagless span still merges that bundle). All deltas clear their error bars.
Throughput — ops/us (directional corroboration only)
| Arm | Before | After | Δ |
|---|---|---|---|
| bareStartSpan | 5.30 ± 0.20 | 5.74 ± 0.52 | +8.4% |
| bareBuildSpan | 5.94 ± 0.31 | 5.77 ± 0.18 | −2.8% |
| jdbcClientSpan | 5.36 ± 0.14 | 5.48 ± 0.60 | +2.1% |
| webServerSpan | 4.24 ± 0.18 | 5.17 ± 0.24 | +22.0% |
| webServerSpanViaBuilder | 4.41 ± 0.09 | 4.98 ± 0.09 | +12.9% |
Throughput agrees directionally (less allocation → less GC pressure). The trustworthy points are the two web arms — tight error bars, clearing their intervals decisively. bareBuildSpan's −2.8% sits inside the combined error, so it's flat, not a regression; the wide after-side bars on bareStartSpan/jdbcClientSpan are per-fork bimodality — don't over-read them.
Methodology
- JMH:
@Threads(8),@Fork(3),@Warmup(5),@Measurement(5),@BenchmarkMode(Throughput),-prof gc. - Front-half only: a no-op
DropWriterdrops finished traces so no serialization/agent I/O leaks into the alloc number. - Tracer config via
@ForkjvmArgs (the realConfigpath, not hand-injected tags):-Ddd.service=petclinic -Ddd.env=staging -Ddd.version=1.2.3 -Ddd.tags=team:apm,dc:us1,cluster:prod-1,owner:tracing,tier:backend,region:us-east-1. - Commit range:
64c42e6(read-through mechanism, parent of this PR's commit) →170eedb(this PR — wires the parent in). - Primary signal is
gc.alloc.rate.norm(deterministic B/op); throughput is directional-only (thermal/bimodality-fragile).
Note: with a bare (unconfigured) tracer this win is invisible — mergedTracerTags is near-empty, so there's nothing to avoid copying. Same code, the only variable is whether the tracer is configured like production.
Bits found no code fix to apply🟢 Investigated · ⚪ No code fix needed System-test jobs fail because View in Datadog | Reviewed commit 3610500 · Any feedback? Reach out in #deveng-pr-agent |
What Does This Do?
Splits trace-level tags & span-level tags into two separate TagMap-s
To maintain full semantic compatibility, this is done via the read-through parent capability introduced in #11789.
Motivation
Reduces some allocation by avoiding the cloning of the BucketGroup collision chain
Reduces span creation time by replacing O(n) map-walk with O(1) referencing of parent TagMap
Additional Notes
Stacked on #11789 (TagMap read-through mechanism) — review that first.
Level-split phase 1: the wiring. Attaches
mergedTracerTagsas a read-through parent at span build (gated on!mergedTracerTagsNeedsIntercept) instead of copying it into every span's storage. This is the change that actually activates read-through — #11789 alone is inert (parent == null).Commits: config-version handling out of the trace-level bundle, the read-through flip (
DDSpanContext.parentTags/CoreTracercopy-vs-share gate), and aTagMapReadThroughBenchmark.Draft — description to be refined; posting now so review can start this week ahead of the release cut.
🤖 Generated with Claude Code