Fold OptimizedTagMap into a final class TagMap by dougqh · Pull Request #11967 · DataDog/dd-trace-java

dougqh · 2026-07-15T17:24:51Z

What Does This Do

TagMap was an interface with a single implementation, OptimizedTagMap. The split was vestigial scaffolding from when a second (HashMap-backed) implementation existed. This folds the impl into one public final class TagMap and drops the interface.

Interface abstract declarations 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 EMPTY 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 with one class) and calls the fast path directly.

Motivation

Code simplicity — not performance. A single final class is monomorphic by construction, but CHA already devirtualized the sole impl, so no steady-state change is expected. Public API is preserved, so callers are unchanged.

Additional Notes

Testing

:internal-api:test green (TagMap suites: 181 + 112 + 4, 0 failures).
:internal-api:{spotbugsMain,spotlessJavaCheck,forbiddenApisMain} green.
Cross-module compile green (dd-trace-core, dd-trace-api, dd-trace-ot); full assemble gate before marking ready.
No mocks or subclasses of TagMap anywhere, so final is safe.

Stacked on #11963.

From Claude: folded per the long-standing "simplify TagMap" plan; verified public-API-preserving and monomorphism-neutral.

🤖 Generated with Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d735dec7e

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

datadog-datadog-prod-us1-2

More details

48 adversarial scenarios validated the four riskiest areas of this fold: EMPTY direct class-init (safe — private constructor reads no statics), putAll(TagMap) with mismatched 1-vs-16 bucket arrays (correct — loop bounds min-clamped, EMPTY always empty), compute* delegation from Map.super vs. old TagMap.super (identical behavior), and freeze/immutableCopy invariants. No regressions found.

Was this helpful? React 👍 or 👎

_{📊 Validated against 48 scenarios · Open Bits AI session}

_{🤖 Datadog Autotest · Commit 8d735de · What is Autotest? · Any feedback? Reach out in #autotest}

dd-octo-sts · 2026-07-15T18:00:55Z

🟢 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	14.02 s	13.95 s	[-0.4%; +1.4%] (no difference)
startup:insecure-bank:tracing:Agent	12.96 s	12.95 s	[-0.8%; +0.9%] (no difference)
startup:petclinic:appsec:Agent	15.79 s	16.70 s	[-11.2%; +0.2%] (unstable)
startup:petclinic:iast:Agent	16.82 s	16.84 s	[-0.8%; +0.6%] (no difference)
startup:petclinic:profiling:Agent	16.60 s	16.75 s	[-2.0%; +0.2%] (no difference)
startup:petclinic:sca:Agent	16.91 s	16.84 s	[-0.5%; +1.4%] (no difference)
startup:petclinic:tracing:Agent	16.10 s	16.14 s	[-1.3%; +0.7%] (no difference)

Commit: 0737ca6b · 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.

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>

…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

More details

76 synthetic scenarios covering EMPTY initialization, putAll, compute/computeIfAbsent/computeIfPresent, Entry.create with all types, freeze/copy, and ledger operations all pass cleanly. The one non-trivial behavioral change — Entry.create(String, Object) now returning null for empty CharSequence typed as Object — is intentional, documented, and aligns existing callers (e.g. GitInfo) whose comments already stated that empty values should be treated as absent.

Was this helpful? React 👍 or 👎

_{📊 Validated against 76 scenarios · Open Bits AI session}

_{🤖 Datadog Autotest · Commit 5f8632d · What is Autotest? · Any feedback? Reach out in #autotest}

…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>

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

@nonnull

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

dougqh added tag: ai generated Largely based on code generated by an AI or LLM tag: no release note comp: core Tracer core type: refactoring labels Jul 15, 2026

dougqh marked this pull request as ready for review July 15, 2026 17:41

dougqh requested a review from a team as a code owner July 15, 2026 17:41

dougqh requested a review from PerfectSlayer July 15, 2026 17:41

chatgpt-codex-connector Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java

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

View reviewed changes

This comment has been minimized.

Sign in to view

Base automatically changed from dougqh/tagmap-null-entry-tolerance to master July 15, 2026 20:25

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

View reviewed changes

dougqh commented Jul 16, 2026

View reviewed changes

Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java Outdated

dougqh commented Jul 16, 2026

View reviewed changes

Comment thread internal-api/src/main/java/datadog/trace/api/TagMap.java Outdated

dougqh and others added 4 commits July 16, 2026 12:53

Mark fromMap/fromMapImmutable map param @nonnull

0737ca6

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 <noreply@anthropic.com>

dougqh force-pushed the dougqh/fold-optimized-tagmap branch from 6abe29d to 0737ca6 Compare July 16, 2026 16:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fold OptimizedTagMap into a final class TagMap#11967

Fold OptimizedTagMap into a final class TagMap#11967
dougqh wants to merge 4 commits into
masterfrom
dougqh/fold-optimized-tagmap

dougqh commented Jul 15, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

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

Uh oh!

This comment has been minimized.

dd-octo-sts Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

dougqh commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Does This Do

Motivation

Additional Notes

Testing

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

dd-octo-sts Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟢 Java Benchmark SLOs — All performance SLOs passed

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dougqh commented Jul 15, 2026 •

edited

Loading

dd-octo-sts Bot commented Jul 15, 2026 •

edited

Loading