Uh oh!
There was an error while loading. Please reload this page.
Add per-operation self-tuning dense-store sizing (phase 2) - #12056
Add per-operation self-tuning dense-store sizing (phase 2)#12056dougqh wants to merge 14 commits into
Conversation
Known tags (keyOf resolves to a stored id) are held in insertion-ordered parallel arrays (knownIds/knownValues) with NO per-tag Entry object — the allocation lever. Lazily allocated on the first known-tag write; custom tags stay in the hash buckets. Disjoint by construction (known-ness is global), so read-through shadow checks stay within-region and the bucket path is unchanged. - KnownTagCodec (id encoding + resolver) + hand-written KnownTags (keyOf substrate over StringIndex). Off-by-default: dormant until a resolver registers, so production is byte-identical. - CoreTracer flips it live behind `-Ddd.trace.dense.tags.enabled` for A/B. - Sizing is a generous fixed stopgap (KNOWN_INIT_CAP=12, the per-type max); exact per-type sizing comes with the tag registry. Reconciled onto the level-split stack (fold + read-through + StringIndex); built on the folded final-class TagMap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🎯 Code Coverage (details) 🔗 Commit SHA: b424214 | Docs | View more details | Give us feedback! |
🟢 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. |
Bits has a CI fix ready🟢 Investigated · 🟢 Fix prepared · ⚪ Validation skipped · 🟠 Ready Registered View in Datadog | Reviewed commit 956f2c9 · Any feedback? Reach out in #deveng-pr-agent |
Reframe of the dense-store presence layer. Replaces the earlier two-tier (group-decl mask + field-decl bloom) design with a single global colored slot: the tag-id middle 16 bits carry one graph-colored slot coordinate (SLOT_SHIFT=32, SLOT_MASK=0xFFFF), so the dense store tracks presence with one occupancy long instead of a group mask plus a field bloom. Adds the trace-level bit (LEVEL_TRACE) and level-bit read-through in the parent visibility check. KnownTags remains hand-maintained here (src/main); the tag-registry code generator that produces these colored ids lands in the following commit, which relocates the file to src/generated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce the tag-registry code generator (buildSrc plugin + tag-conventions YAML) and use it to assign the colored slots from the previous commit by graph-coloring the tag co-occurrence graph (each concrete span type's resolved set plus the <trace> clique), packing all stored tags into SLOT_COUNT=16 slots. Relocates the hand-maintained KnownTags from src/main to the generated src/generated source set and renames the codec accessors to their generated form (serialNum/makeTagId). Adds the generator verify task so CI fails if the checked-in KnownTags drifts from the conventions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e6800b8 to
9b7c3d1Compare956f2c9 to
8fb6bc3Comparedougqh
commented
Aug 18, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:8fb6bc38bb
ℹ️ 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".
| // resolved hint sizes the span's TagMap and self-tunes on finish; null (no/unkeyable | ||
| // operation name) falls back to the generic default capacity. | ||
| final boolean entrySpan = !(resolvedParentSpanContext instanceof DDSpanContext); | ||
| final SizingHint sizingHint = SizingHintTable.hintFor(operationName, entrySpan); |
There was a problem hiding this comment.
perf: When dd.trace.dense.tags.enabled is absent—the documented default at CoreTracer lines 662–668—every span still performs this operation-name conversion and global-table probe, and the table retains up to 1,024 names even though KnownTagCodec routes no tags into the dense store. This adds unconditional work to span creation with no possible sizing benefit, so resolve hints only when the dense-tag feature is enabled.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| this.unsafeTags = | ||
| readThroughParent != null | ||
| ? TagMap.createFromParent(readThroughParent) | ||
| : TagMap.create(capacity); | ||
| : sizingHint != null ? TagMap.create(sizingHint) : TagMap.create(capacity); |
There was a problem hiding this comment.
perf: When mergedTracerTagsNeedsIntercept is false, ConfigSnapshot supplies a non-null mergedTracerTags, so this branch always calls createFromParent and discards the sizing hint. This remains true for an empty parent because createFromParent drops it only after this choice, leaving the map at the fixed capacity of 12; consequently the normal shared-parent path does not receive the per-operation sizing introduced by this change. Add a parent-aware creation path that also applies the hint.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| private void finishAndAddToTrace(final long durationNano) { | ||
| // ensure a min duration of 1 | ||
| if (DURATION_NANO_UPDATER.compareAndSet(this, 0, Math.max(1, durationNano))) { | ||
| context.recordDenseSize(); |
There was a problem hiding this comment.
perf: The feedback call exists only in finishAndAddToTrace, but spans completed through phasedFinish() followed by publish()—including the gRPC, Netty, WebFlux, and async-servlet paths in this repository—publish directly without entering this method. Those operations therefore never tune their hints and repeatedly start from the seed capacity; invoke the feedback from a terminal path shared by both finish modes.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| private void finishAndAddToTrace(final long durationNano) { | ||
| // ensure a min duration of 1 | ||
| if (DURATION_NANO_UPDATER.compareAndSet(this, 0, Math.max(1, durationNano))) { | ||
| context.recordDenseSize(); |
There was a problem hiding this comment.
perf: Recording here occurs before serialization, while DDSpanContext.processTagsAndBaggage later runs the lazy processors that append dense known tags such as _dd.integration, _dd.svc_src, and _dd.tracer_host. The learned high-water mark therefore systematically excludes those entries, so affected spans can still grow and copy their dense arrays during serialization on every occurrence; record after the last tag-processing additions instead.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| if (!hint.capped && this.knownCount > hint.size) { | ||
| hint.size = this.knownCount; // monotonic-max; benign racy plain-int write |
There was a problem hiding this comment.
perf: When two spans of the same operation finish concurrently, both can pass this comparison using the same old value; if a span with 10 tags writes first and one with 5 tags writes afterward, the hint decreases from 10 to 5 despite the claimed monotonic-max behavior. Because size is also a plain non-volatile field, prompt visibility to span-creation threads is not guaranteed. This can make later spans repeatedly under-allocate and grow, so use an atomic max/CAS or equivalent synchronization.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| if (entrySpan) { | ||
| entrySize++; // racy approximate count | ||
| } else { | ||
| childSize++; | ||
| } |
There was a problem hiding this comment.
perf: When many first spans for the same operation race, they can all miss the initial get; after one inserts the hint, getOrCreate returns that existing hint to the remaining callers, but every caller still increments this counter. A sufficiently concurrent cold start can therefore consume the 512-entry budget while storing only one distinct operation, forcing all later operations onto the overflow hint. Have insertion report whether the caller actually claimed a new slot and increment only in that case.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
…oint, lane count, read-through sizing - Do no harm: resolve a SizingHint only when KnownTagCodec.isActive() (the dense store is live). With dd.trace.dense.tags.enabled off (the default), skip the operationName.toString() + global-table probe entirely -- a hint buys nothing when known tags don't take the dense path. - Record point: move recordDenseSize() from DDSpan.finishAndAddToTrace to DDSpanContext.processTagsAndBaggage (at serialization). That is the single terminal point that runs for BOTH finish modes -- plain finish() and phasedFinish()+publish() (gRPC/Netty/WebFlux) -- and runs AFTER the lazy tag post-processors append serialization-time tags (_dd.integration, host, ...), so the recorded high-water mark is the span's true final footprint. - Read-through sizing: add TagMap.createFromParent(parent, hint) so the LOCAL dense store of a read-through map is sized from the hint too; route it in DDSpanContext. Sizing now fires on the common child / shared-parent path (previously the hint was discarded whenever mergedTracerTags was non-null). - Lane count: FlatHashtable.getOrCreate reports genuine inserts; SizingHintTable bumps the per-lane cardinality count only on a real insert, so a cold-start burst of one operation can't spend the budget on a single name. - Docs: correct the 'monotonic-max' over-claim to best-effort racy max (a concurrent finish can transiently lower size; benign -- only mis-sizes an array). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dougqh
commented
Aug 18, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:390a3432d1
ℹ️ 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".
| jvmArgsAppend = { | ||
| "-DTEST_LOG_LEVEL=warn", |
There was a problem hiding this comment.
perf: This benchmark documents its results as “dense on” and attributes the allocation delta to SizingHint, but its fork omits -Ddd.trace.dense.tags.enabled=true; CoreTracer therefore leaves KnownTagCodec inactive and assigns sizingHint = null for every span. Unlike SpanCreationBenchmark, these runs exercise the legacy tag path, so the committed historical results cannot validate this change; add the dense-store flag to this fork.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| public void write(List<DDSpan> trace) { | ||
| blackhole.consume(trace); |
There was a problem hiding this comment.
perf: Both new span/trace benchmarks use this writer, but write only consumes the trace and never calls DDSpanContext.processTagsAndBaggage, which is the sole path invoking recordDenseSize. Consequently, even in SpanCreationBenchmark, where dense tags are enabled, every hint remains at the seed size of 1 and every heavy span repeats the grow/copy sequence; the benchmarks therefore do not measure the advertised steady-state self-tuning behavior. Use a benchmark writer that executes metadata processing, or explicitly train the hints before measurement.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| if (KnownTagCodec.isActive()) { | ||
| final boolean entrySpan = !(resolvedParentSpanContext instanceof DDSpanContext); | ||
| sizingHint = SizingHintTable.hintFor(operationName, entrySpan); |
There was a problem hiding this comment.
perf: When an integration changes a span's operation after start(), this stores the hint for the initial name and later records the final tag count back into that stale hint. This is systematic for the OTel shim: OtelTracer.spanBuilder starts every span as SPAN_KIND_INTERNAL, while OtelSpan.onSpanFinished computes the real operation name, so heterogeneous OTel spans share an internal hint per lane and lean spans eventually allocate for the largest shape while final operation names never learn. Re-resolve the hint when setOperationName changes the name, or defer selecting it until the final name is known.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| for (int i = 0; i < 4096 && firstOverflow == null; i++) { | ||
| SizingHint hint = SizingHintTable.hintFor("registry.flood." + i, true); |
There was a problem hiding this comment.
If this test runs before freshHintIsSeededAndUncapped or sizingHintFeedsAndTunesTheDenseStore—for example under randomized or parallel JUnit execution—it permanently exhausts the process-wide entry lane, so those tests receive the capped overflow hint with size 8 instead of a fresh size-1 hint and fail. Distinct operation names do not isolate tests once the global cardinality counter is exhausted; reset/use an isolated registry or explicitly guarantee that this destructive test runs last.
Useful? React with 👍 / 👎.
| final String key = | ||
| operationName.toString(); // O(1) for String / UTF8BytesString (see class doc) |
There was a problem hiding this comment.
perf: The cardinality limit bounds the number of keys, but not their byte size: hintFor converts an arbitrary public-API CharSequence to a String, and each created SizingHint keeps that string strongly for the process lifetime. Operation names are only truncated later by TraceUtils.normalizeOperationName in the DD-intake interceptor, so an application producing up to 512 entry and 512 child spans with very large dynamic names can make this supposedly bounded registry permanently retain hundreds of megabytes or more. Reject, truncate, or avoid caching oversized names before inserting them.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
Replaces the raw Boolean.getBoolean("dd.trace.dense.tags.enabled") system-property
read with a proper trace.dense.tags.enabled Config flag, captured once into the
DENSE_TAGS_ENABLED static constant so it constant-propagates on the span-creation path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>…oint, lane count, read-through sizing - Do no harm: resolve a SizingHint only when KnownTagCodec.isActive() (the dense store is live). With dd.trace.dense.tags.enabled off (the default), skip the operationName.toString() + global-table probe entirely -- a hint buys nothing when known tags don't take the dense path. - Record point: move recordDenseSize() from DDSpan.finishAndAddToTrace to DDSpanContext.processTagsAndBaggage (at serialization). That is the single terminal point that runs for BOTH finish modes -- plain finish() and phasedFinish()+publish() (gRPC/Netty/WebFlux) -- and runs AFTER the lazy tag post-processors append serialization-time tags (_dd.integration, host, ...), so the recorded high-water mark is the span's true final footprint. - Read-through sizing: add TagMap.createFromParent(parent, hint) so the LOCAL dense store of a read-through map is sized from the hint too; route it in DDSpanContext. Sizing now fires on the common child / shared-parent path (previously the hint was discarded whenever mergedTracerTags was non-null). - Lane count: FlatHashtable.getOrCreate reports genuine inserts; SizingHintTable bumps the per-lane cardinality count only on a real insert, so a cold-start burst of one operation can't spend the budget on a single name. - Docs: correct the 'monotonic-max' over-claim to best-effort racy max (a concurrent finish can transiently lower size; benign -- only mis-sizes an array). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
390a343 to
9f8ff10Comparedougqh
commented
Aug 18, 2026
@codex review |
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Expose the known-tag id on EntryReader: TagMap.Entry resolves it lazily via KnownTagCodec.keyOf (same memoized-field idiom as lazyTagHash; 0L means unknown tag / inactive codec, so the sentinel is Long.MIN_VALUE). The dense reader flyweight already has the id in hand at emit time and records it directly, skipping the keyOf resolve. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ting Address two Codex robustness findings on the generator: - generateKnownTags now clears its owned destination tree before regenerating, so a report/source file retired by a later revision cannot linger and fail verifyKnownTags with no way for the fix task to remove it. - All String.format calls in the report/emitter use Locale.ROOT, so %d output no longer localizes on machines whose default locale uses non-ASCII digits, preserving the byte-identical-output invariant verifyKnownTags relies on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address the Codex finding that the new experimental dense-tags flag was stored but never surfaced in Config.toString (repo convention requires new configs appear there for diagnostics). Surface it only when it diverges from the default, so normal config dumps stay uncluttered, and compare against a new DEFAULT_TRACE_DENSE_TAGS_ENABLED constant (also now the getBoolean default) so the condition stays correct if the default ever changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address the Codex finding that gradle/spotless.gradle excluded src/generated/**, bypassing google-java-format on committed generated code. Drop the exclude and make KnownTagsEmitter emit google-java-format-clean output (blank line before the keyOf static initializer; wrap the long id-assignment), so spotlessCheck and the byte-identical verifyKnownTags gate both pass on the emitted file. internal-api is the only module with a committed src/generated Java tree, so the exclude removal is otherwise a no-op. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a per-namespace name to the tag registry: each tag may declare an `open-telemetry-name` (replacing the vestigial `aliases` list). The generator now: - parses it off both YAMLs into the model, - validates it (an OTel name may not collide with a canonical tag name nor be claimed by two tags -> build fails loudly rather than silently picking one), - emits it into the keyOf table so keyOf(otelName) resolves to the canonical tag's id (inbound, many->one), and - emits a reverse switch so openTelemetryNameOf(tagId) recovers it (outbound). KnownTagCodec gains datadogTagOf(tagId) (== nameOf, the canonical name) and openTelemetryTagOf(tagId) (the OTel name, or null). nameOf is unchanged and still returns the Datadog name -- outbound is namespace-specific, not normalized. Serializer applicability (when a span renders under OTel names, fallback policy) and additional namespaces are a follow-on concern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reconciles #11901's SizingHint half onto the colored generator-v2 base as its own layer. A per-operation SizingHint (kept in a bounded, self-tuning SizingHintTable keyed by operation name, two lanes for entry vs child spans) sizes a span's dense TagMap at create and records the observed known-tag high-water mark back on finish, so the reused hint converges to the operation's real size. Erases the fixed KNOWN_INIT_CAP dense-array floor tax that regressed bare/small spans in the #12047-vs-1.65 A/B; the id-keyed write API stacks on top of this as a separate layer. New: SizingHint, SizingHelper, SizingHintTable, FlatHashtable (+ tests, jmh). TagMap: create(SizingHint), recordSize, denseCapHint. DDSpanContext threads a sizingHint through the primary ctor + recordDenseSize on finish; DDSpan hooks it at finishAndAddToTrace; CoreTracer resolves the per-operation hint/lane. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Near-no-op DropWriter (blackhole-consuming Writer) plus two front-half benchmarks used to measure the dense-store / per-operation-sizing (SizingHint) allocation behavior with the dense flag on: - SpanCreationBenchmark: single span create -> (set tags) -> finish, bare/web/jdbc arms; the drift-stable old-API shape for version A/Bs. - TraceCreationBenchmark: a whole trace (local root + children) so the entry-vs-child sizing win, which a single-span bench can't reach, surfaces and scales with childCount. Read gc.alloc.rate.norm (B/op, deterministic); throughput is directional. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oint, lane count, read-through sizing - Do no harm: resolve a SizingHint only when KnownTagCodec.isActive() (the dense store is live). With dd.trace.dense.tags.enabled off (the default), skip the operationName.toString() + global-table probe entirely -- a hint buys nothing when known tags don't take the dense path. - Record point: move recordDenseSize() from DDSpan.finishAndAddToTrace to DDSpanContext.processTagsAndBaggage (at serialization). That is the single terminal point that runs for BOTH finish modes -- plain finish() and phasedFinish()+publish() (gRPC/Netty/WebFlux) -- and runs AFTER the lazy tag post-processors append serialization-time tags (_dd.integration, host, ...), so the recorded high-water mark is the span's true final footprint. - Read-through sizing: add TagMap.createFromParent(parent, hint) so the LOCAL dense store of a read-through map is sized from the hint too; route it in DDSpanContext. Sizing now fires on the common child / shared-parent path (previously the hint was discarded whenever mergedTracerTags was non-null). - Lane count: FlatHashtable.getOrCreate reports genuine inserts; SizingHintTable bumps the per-lane cardinality count only on a real insert, so a cold-start burst of one operation can't spend the budget on a single name. - Docs: correct the 'monotonic-max' over-claim to best-effort racy max (a concurrent finish can transiently lower size; benign -- only mis-sizes an array). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nownTagCodec.isActive
9f8ff10 to
b424214Compare19222e1 to
33f32c4Compare
What
Sizes each span's dense
TagMapstore from a per-operation sizing hint instead of a generic default, then feeds the actual known-tag count back on span finish so the hint converges to that operation's real high-water mark. Over-provisioning shrinks toward the operation's true shape without any hand-tuned per-type tables.This is the SizingHint layer of the dense-store stack, sitting between the generator (
dougqh/generator-v2) and the id-keyedTagMap.set(long)work (dougqh/tag-id-api, #11901).How
FlatHashtable— open-addressed find-or-create over self-contained entries; static-polymorphism via a concrete-typedHelpersingleton so the JIT devirtualizes/inlineshash/matches/createper call site. Single-reference publish → torn-free under racy access.SizingHint/SizingHelper— opaque per-operation hint (label + cached hash + tuned size) and itsString-key helper.SizingHintTable— process-wide, pure-static two-lane (entry vs child span) registry keyed by operation name. Fixed-capacity and deliberately lock-free/racy: a lost update or double-mint only mis-sizes an array (over/under-provision) for a span or two — it never corrupts tag data.CoreTracer.buildSpanContextresolves the hint by operation name + entry-ness and hands it toDDSpanContext, which sizes itsTagMapfrom it and records the final size back viaDDSpan.finishAndAddToTrace.Testing
FlatHashtableTest,SizingHintTableTest(incl. the self-tuningrecordSizeloop and content-keying so aUTF8BytesStringoperation name resolves to the same hint as itsStringform).FlatHashtableBenchmark; abuildMapSizedarm inDenseStoreAllocBenchmark.spotlessJavaCheck,:internal-api:spotbugsMain,:dd-trace-core:spotbugsMainall green.Notes
DDSpanContextctors are marked@Deprecated @VisibleForTestingto steer new code to the full ctor.🤖 Generated with Claude Code