Skip to content

Wire mergedTracerTags as a read-through parent at span build (level-split) (phase 1a)#11932

Draft
dougqh wants to merge 6 commits into
dougqh/tagmap-read-throughfrom
dougqh/tagmap-read-through-consumer
Draft

Wire mergedTracerTags as a read-through parent at span build (level-split) (phase 1a)#11932
dougqh wants to merge 6 commits into
dougqh/tagmap-read-throughfrom
dougqh/tagmap-read-through-consumer

Conversation

@dougqh

@dougqh dougqh commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

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 mergedTracerTags as 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 / CoreTracer copy-vs-share gate), and a TagMapReadThroughBenchmark.

Draft — description to be refined; posting now so review can start this week ahead of the release cut.

🤖 Generated with Claude Code

@dd-octo-sts

dd-octo-sts Bot commented Jul 13, 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 13.95 s 14.00 s [-1.2%; +0.5%] (no difference)
startup:insecure-bank:tracing:Agent 12.92 s 13.05 s [-1.6%; -0.3%] (maybe better)
startup:petclinic:appsec:Agent 16.85 s 16.77 s [-0.4%; +1.4%] (no difference)
startup:petclinic:iast:Agent 16.92 s 16.51 s [-1.9%; +6.7%] (no difference)
startup:petclinic:profiling:Agent 16.58 s 16.17 s [-2.0%; +7.0%] (no difference)
startup:petclinic:sca:Agent 16.87 s 16.77 s [-0.2%; +1.4%] (no difference)
startup:petclinic:tracing:Agent 16.20 s 16.10 s [-0.4%; +1.6%] (no difference)

Commit: 1f257aa1 · 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 force-pushed the dougqh/tagmap-read-through-consumer branch 2 times, most recently from 7bb20f4 to e65e58f Compare July 13, 2026 18:24
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from 3a0a318 to e692601 Compare July 15, 2026 18:20
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through-consumer branch from e65e58f to 3b9156e Compare July 15, 2026 18:24
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🎯 Code Coverage (details)
Patch Coverage: 66.67%
Overall Coverage: 57.19% (+0.01%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 1f257aa | Docs | Datadog PR Page | Give us feedback!

@dougqh

dougqh commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Directional span-creation benchmark: master vs this branch

Grafted SpanCreationBenchmark (from #11915) onto both master and this branch and ran a directional comparison with a realistic trace-level bundle. Not committed to the branch — a one-off directional check.

Config: -f 2 -wi 3 -i 3 -w 5 -r 5 -prof gc, @Threads(8), JDK 17, quiet box. Trace-level bundle via -jvmArgsAppend "-Ddd.env=bench -Ddd.version=1.0.0 -Ddd.tags=team:apm,tier:web,region:us1,shard:7,build:1234" (7 non-interceptable tags → a non-empty mergedTracerTags for read-through to fall through to). Alloc = gc.alloc.rate.norm.

method master B/op branch B/op Δ Δ%
bareStartSpan 52,511 52,457 −54 −0.10%
bareBuildSpan 52,514 52,447 −67 −0.13%
webServerSpan 62,871 62,872 +1 ~0%
webServerSpanViaBuilder 69,543 69,484 −59 −0.08%
jdbcClientSpan 64,508 64,356 −153 −0.24%

Throughput deltas were all within noise (≤1.6%, no trend) — directional only, not quoted.

Read: the deltas are small but real — a consistent ~50–150 B/op reduction (the mergedTracerTags bucket structure read-through shares instead of copying), visible even on bare spans since the bundle applies to every span. But it's <0.25% of total span-creation alloc, which is dominated by a ~52–69 KB/op baseline (span/context/PendingTrace object graph). So this benchmark confirms read-through does what it should but is the wrong lens for its alloc win — that shows in the isolated TagMapReadThroughBenchmark (~300–600 B/op, −13% to −48% by bundle size).

Consistent with the settled framing: read-through is structural groundwork (dense-store enabler), not a standalone span-creation alloc headline.

From Claude: directional check requested during the fold/read-through rebase; grafted benches, not part of the branch diff.

🤖 Generated with Claude Code

dougqh added a commit that referenced this pull request Jul 15, 2026
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 <[email protected]>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through-consumer branch from 3b9156e to d2aa48f Compare July 15, 2026 21:19
dougqh added a commit that referenced this pull request Jul 15, 2026
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 <[email protected]>
dougqh and others added 5 commits July 16, 2026 12:53
TagMap was an interface with a single implementation, OptimizedTagMap. The
split was vestigial scaffolding from when a second (HashMap-backed) impl
existed; with one impl it is false generalization. Collapse them into one
`public final class TagMap`:

- The interface's abstract method declarations are removed; OptimizedTagMap's
  bodies become TagMap's methods.
- Nested types that were implicitly `public static` in the interface
  (EntryChange, EntryRemoval, EntryReader, Entry, Ledger) are now written out
  explicitly as `public static`.
- Static factories (create/fromMap/ledger/...) and the EMPTY constant become
  explicit `public static` members; the EmptyHolder lazy-init note is updated
  now that there is no interface<->impl class-init cycle.
- putAll(TagMap) loses its `instanceof` dispatch (always true once there is one
  class) and calls the fast path directly.

No behavior change; motivation is code simplicity, not performance (a single
final class is monomorphic by construction, but CHA already devirtualized the
sole impl). Public API is preserved, so callers are unchanged; the 3 tests that
referenced OptimizedTagMap now reference TagMap.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Post-fold tidy, all TagMap-scoped:

- Remove EmptyHolder: with one class there is no interface<->impl class-init
  cycle to break, and the private constructor reads no statics, so EMPTY is a
  direct `new TagMap(new Object[1], 0)` initializer.
- Static factories (create/fromMap/ledger/...) are now `public static final`
  (not expressible on the old interface).
- assertSize/assertNotEmpty/assertEmpty/checkIntegrity test helpers dropped
  their now-always-true `instanceof TagMap` guard + redundant cast.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
The interface -> final-class fold removed the TagMap interface decls, which
carried the @Nullable/@nonnull param annotations from #11963. Re-home them
onto the now-concrete methods: @nonnull tag keys and strict-setter values,
@nullable on the set(EntryReader)/getAndSet(Entry) sinks (+ the getAndSet
contract javadoc). The null-tolerance behavior was already preserved by the
fold; this restores the self-describing contract on the write surface.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Per review: these factories NPE on a null map (map.size()/putAll), so the
input is non-null by contract.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
A fresh, mutable TagMap can read through to a frozen parent on local
misses, so a span can layer its own tags over a shared, immutable set
(e.g. merged tracer tags) without copying them.

- createFromParent(parent): the only way to attach a parent; the parent
  must be frozen and is fixed at construction (no re-parenting), so
  read-through can treat it as stable. Single-parent by design in phase 1.
- Reads resolve local-first, then the parent; a local entry shadows the
  parent's (local-wins). Removing a parent key locally records a lazy
  tombstone (removedFromParent) so it stops reading through; the tombstone
  set is null until first needed, keeping the hot paths untouched.
- size()/isEmpty() are exact (Map contract) and resolve the parent;
  isDefinitelyEmpty()/estimateSize() are the cheap conservative variants
  for the hot path. copy() preserves the parent and tombstones; forEach
  walks local then parent.

Built on the folded final-class TagMap (#11967); composes cleanly with the
null-tolerant Entry pathway (#11963).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from e692601 to b58bcc0 Compare July 20, 2026 13:59
…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 <[email protected]>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through-consumer branch from d2aa48f to 1f257aa Compare July 20, 2026 15:43
dougqh added a commit that referenced this pull request Jul 20, 2026
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 <[email protected]>
@dougqh dougqh changed the title Wire mergedTracerTags as a read-through parent at span build (level-split phase 1) Wire mergedTracerTags as a read-through parent at span build (level-split) (phase 1a) Jul 20, 2026
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch 2 times, most recently from bef20fa to 50cf53d Compare July 21, 2026 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant