Skip to content

Add TagMap read-through support (level-split mechanism) (phase 1a)#11789

Open
dougqh wants to merge 1 commit into
masterfrom
dougqh/tagmap-read-through
Open

Add TagMap read-through support (level-split mechanism) (phase 1a)#11789
dougqh wants to merge 1 commit into
masterfrom
dougqh/tagmap-read-through

Conversation

@dougqh

@dougqh dougqh commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Adds read-through support to TagMap - currently unused

A TagMap can now be constructed via createFromParent(TagMap) to resolve map queries against a frozen parent TagMap. The plan is to use this to separate trace-level tags from span-level tags without any semantic changes.

Motivation

When apply to the tracer core will enable lighter and faster span construction by removing some map-to-map copying.

Additional Notes

From Claude...

Pure mechanism — inert until a consumer attaches a parent. No map has a parent yet (parent == null for every existing map), so there is no behavior change off the read-through path. The consumer wiring lands in a stacked follow-up.

A child TagMap references a frozen parent (createFromParent) and reads through to it on a local miss, while local entries shadow the parent. This is the enabler for level-split phase 1 — a span will stop copying the shared trace-level tags (mergedTracerTags) down into every span; reads route through instead.

Design

  • Single parent by design (anti-false-generalization): the consumer needs one parent (mergedTracerTags). Written so generalizing to multiple flattened parents is additive (the bulk walk is already bucket-aligned, the degenerate single-parent case of the multi-parent merge).
  • Removal via a lazy removedFromParent side-set, not inline tombstones. Inline-in-buckets kept breaking on the bare-Entry-vs-BucketGroup duality (re-type, per-group bitfield + single-Entry gap, two bitfields — all awkward). The side-set is shape-agnostic, keeps Entry/BucketGroup completely untouched, and the lazy null field doubles as the gate. Tombstones are rare (only when a parent-exposed key is removed).
  • Bulk reads via a bucket-aligned merge: exploits universal hashing (fixed 16-bucket table, no resize) so a parent entry's shadow check is scoped to the same-index local bucket, reusing the entry's cached hash — no re-hash, no global seen-set, no Set/BloomFilter. forEach (×3) stays alloc-free; IteratorBase does a two-phase local-then-parent walk so iterator/entrySet/keySet/values/stream all emit the deduped union.

What's covered

  • Read path: getEntry fall-through (the whole get*/containsKey family inherits it). isDefinitelyEmpty() + estimateSize() added as cheap conservative/upper-bound variants (mirroring Ledger); isEmpty()/size() stay exact (Map contract) and resolve the union.
  • Removal: tombstone a parent-exposed key; re-setting clears it; remove() returns the prior visible value (Map contract holds via read-through).
  • Bulk: forEach/iterators/collection views all emit the deduped, first-occurrence-wins union, skipping shadowed/tombstoned parent entries.
  • copy() preserves read-through (shares the frozen parent + copies tombstones) — was dropping both.
  • Behavior-identical-to-flat-map tests are the safety contract for the consumer flip.

TagMapReadThroughTest covers the read path, removal/tombstoning, the deduped bulk union, and copy() preservation; the full TagMap* suite stays green (inert when parent == null).

Deferred / follow-ups

  • Inlining baseline for get/set across the parent != null branch — the perf-gate before marking this ready.
  • Consumer PR (stacked): the !needsIntercept-gated mergedTracerTags → createFromParent wiring. Includes the removeTag(VERSION) cleanup — config version moves out of the read-through bundle so the existing InternalTagsAdder conditional-add works unchanged and no per-span tombstone is minted (the apply-then-remove dance is vestigial; read-through keeps config-in-parent / manual-in-local). Plus an audit of any other per-span parent-key removals and of read-through maps as a merge/clone source (only copy() handled here).
  • Multi-parent + the cross-parent dedup clause — additive, with the dream.

tag: ai generated · pure mechanism, no behavior change until a consumer attaches a parent.

🤖 Generated with Claude Code

@dougqh dougqh added comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: feature Enhancements and improvements labels Jun 29, 2026
@dd-octo-sts

dd-octo-sts Bot commented Jun 29, 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.96 s 13.98 s [-0.8%; +0.5%] (no difference)
startup:insecure-bank:tracing:Agent 12.87 s 12.94 s [-1.3%; +0.1%] (no difference)
startup:petclinic:appsec:Agent 16.94 s 16.79 s [+0.1%; +1.7%] (maybe worse)
startup:petclinic:iast:Agent 16.92 s 16.83 s [-0.2%; +1.3%] (no difference)
startup:petclinic:profiling:Agent 16.56 s 16.40 s [-3.5%; +5.5%] (no difference)
startup:petclinic:sca:Agent 16.96 s 16.78 s [+0.2%; +1.9%] (maybe worse)
startup:petclinic:tracing:Agent 15.62 s 16.07 s [-7.1%; +1.5%] (no difference)

Commit: 64c42e60 · 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 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Inlining gate — off-path (parent == null) reads: parity confirmed

Ran -XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining (no-fork, -f0) on UnsynchronizedMapBenchmark.get_tagMap / iterate_tagMap_forEach, master vs this branch, to verify the getEntry refactor + forEach parent loop don't regress hot-read inlining for the common parent == null case (every existing map).

Reads — parity, both fully inline hot:

getEntry decision
master monolith, 90 B inline (hot)
branch getEntry 51 B → getLocalEntry 24 B → findInBucket 45 B all inline (hot)

Same fully-inlined read — the branch inlines a 120 B chain of tiny methods where master inlines a 90 B monolith (+30 B for the parent != null check + framing, all far under FreqInlineSize 325). Cold-site "callee is too large" is identical on both (> the 35 B cold cutoff). So the split is inlining-neutral.

forEach: the read-through parent loop was extracted to per-variant forEachParent(...) (called only when parent != null), so off-path forEach is back to ~90 B (it had grown to 242 B inline) and compiles as its own loop unit exactly as before; the parent-loop code is out of line and dead when parent == null.

Writes: descoped — setTag → TagInterceptor → getAndSet is already a non-inlinable big method due to the interceptor cascade, so a write-side inlining measurement is confounded. The one gated removedFromParent != null field-check is noise against that; meaningful write-path inlining waits on interceptor retirement.

Not yet measured: the with-parent hot path (read-through active) — there's no benchmark with a parent attached yet. That belongs on the consumer PR's span-level -prof gc benchmark, which also demonstrates the per-span allocation win.

@dougqh

dougqh commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Measured read-through win (quiet box, firmed)

TagMapReadThroughBenchmark (on the stacked consumer branch, where the wiring lands) models span-build tag assembly — copyDown (today: putAll the frozen trace-level bundle + span tags) vs readThrough (attach the bundle as a read-through parent, span tags local) — swept by trace-bundle size. 5 forks, @Threads(8), -prof gc, 25 measurements/point.

traceTagCount copyDown alloc readThrough alloc saved copyDown thrpt readThrough thrpt speedup
3 360 B/op 312 B/op −13% 170.8M ops/s 213.7M 1.25×
7 (realistic) 456 B/op 312 B/op −32% 133.0M ops/s 215.0M 1.62×
15 600 B/op 312 B/op −48% 101.1M ops/s 207.3M 2.05×

The headline is the shape, not a single number:

  • read-through is invariant to bundle size — 312 B/op and ~210M ops/s flat across 3/7/15 trace tags. A span no longer pays anything for the trace-level bundle; its cost is O(its own tags).
  • copyDown grows with the bundle (more BucketGroup clones + collisions to copy), so the win scales with mergedTracerTags size: −13%/1.25× at 3 tags → −48%/2.05× at 15.

Reliability: alloc deterministic (±0.001 over 25 samples); throughput tight (±2–5M).

Honest attribution / scope: the alloc delta is bucket structure (BucketGroup clones + fewer local collisions), not Entry objects — putAll-into-empty already shares the frozen entries. Bucket structures are ~3% of tracer alloc, so absolute macro impact at roomy heap is modest; the win matters most under GC pressure and on the app/request thread (the throughput column). The structural point is the invariance — read-through decouples per-span cost from trace-bundle size, which is the level-split thesis (trace tags paid once, not per span).

Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java Outdated
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from 3a0a318 to e692601 Compare July 15, 2026 18:20
@dougqh
dougqh changed the base branch from master to dougqh/fold-optimized-tagmap July 15, 2026 18:20
dougqh added a commit that referenced this pull request Jul 15, 2026
…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>
@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: 73.80%
Overall Coverage: 57.31% (+0.02%)

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

dougqh added a commit that referenced this pull request Jul 15, 2026
…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>
@dougqh
dougqh force-pushed the dougqh/fold-optimized-tagmap branch from 6abe29d to 0737ca6 Compare July 16, 2026 16:56
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from e692601 to b58bcc0 Compare July 20, 2026 13:59
dougqh added a commit that referenced this pull request Jul 20, 2026
…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>
Base automatically changed from dougqh/fold-optimized-tagmap to master July 20, 2026 17:41
@dougqh dougqh changed the title Add TagMap read-through support (level-split phase 1 mechanism) Add TagMap read-through support (level-split mechanism) (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
@dougqh
dougqh marked this pull request as ready for review July 21, 2026 02:48
@dougqh
dougqh requested a review from a team as a code owner July 21, 2026 02:48
@dougqh
dougqh requested a review from mcculls July 21, 2026 02:48
@dd-octo-sts

dd-octo-sts Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

  • Remove the issue linking keyword

If you need help, please check our contributing guidelines.

@datadog-datadog-prod-us1-2 datadog-datadog-prod-us1-2 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: PASS

More details

This PR adds read-through support to TagMap, enabling span-level maps to read through a frozen parent (trace-level) map on local misses. The mechanism is inert when no parent is attached (parent == null), ensuring no behavioral change for existing code. Comprehensive testing verifies read-through correctness, tombstone handling, iterator deduplication, and equivalence to flat-map semantics. No production bugs detected.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Datadog Autotest · Commit 50cf53d · What is Autotest? · Any feedback? Reach out in #autotest

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Arrays.fill(this.buckets, null);
this.size = 0;

P2 Badge Tombstone parent entries during clear

When clear() is called on a TagMap created with a non-empty parent, these lines only wipe the local buckets and leave the read-through parent visible. In that scenario map.clear(); map.isEmpty() is still false and get() continues returning parent tags, which violates the Map.clear() contract and can resurrect inherited tags after callers believe they removed every mapping.


if (map instanceof TagMap) {
this.putAllOptimizedMap((TagMap) map);

P2 Badge Copy visible parent tags in TagMap putAll

When the source map is a read-through TagMap, this optimized path copies only that.buckets/that.size, so entries that are visible solely through that.parent are silently skipped; for example TagMap.create().putAll(childFromParent) loses all inherited tags even though Map.putAll should add every visible mapping from the source. The optimized path needs to iterate the read-through union or fall back to the normal entrySet() path when that has a parent/tombstones.

ℹ️ 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".

Comment on lines +86 to +89
if (parent != null && !parent.frozen) {
throw new IllegalStateException("read-through parent must be frozen");
}
return new TagMap(parent);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject or flatten read-through parents

createFromParent accepts any frozen TagMap, including one produced by freezing or immutableCopy() on a read-through child, but the new bulk paths only scan this.parent.buckets while getEntry() recurses through ancestors. With a grandparent tag, grandChild.get("a") succeeds while size(), forEach(), keySet(), and iterators omit it, so either nested parents need to be rejected here or the union walkers must traverse the full parent chain.

Useful? React with 👍 / 👎.

Comment on lines +1337 to +1338
if (this.removedFromParent != null) {
this.removedFromParent.remove(newEntry.tag);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Return the inherited value when overwriting parent tags

With a read-through child, overwriting a key that exists only in the parent reaches this path, inserts a new local entry, and returns null because no local entry was replaced. That makes put("a", ...), getAndSet("a", ...), and default replace calls report that no prior value existed even though get("a") returned the parent value immediately before, violating Map.put semantics for inherited tags.

Useful? React with 👍 / 👎.

dougqh added a commit that referenced this pull request Jul 21, 2026
…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>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from 50cf53d to ae64f2a Compare July 21, 2026 14:50
dougqh added a commit that referenced this pull request Jul 21, 2026
…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>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from ae64f2a to 5cd8289 Compare July 21, 2026 15:19
dougqh added a commit that referenced this pull request Jul 21, 2026
…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>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from 5cd8289 to ab56044 Compare July 21, 2026 17:04
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 <noreply@anthropic.com>
@dougqh
dougqh force-pushed the dougqh/tagmap-read-through branch from ab56044 to 64c42e6 Compare July 21, 2026 17:06
dougqh added a commit that referenced this pull request Jul 21, 2026
…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>
dougqh added a commit that referenced this pull request Jul 21, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant