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 6 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: 1f257aa | Docs | Datadog PR Page | Give us feedback! |
Directional span-creation benchmark: master vs this branchGrafted Config:
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 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 |
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]>
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 <[email protected]>
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]>
e692601 to
b58bcc0
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 <[email protected]>
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 <[email protected]>
bef20fa to
50cf53d
Compare
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