Uh oh!
There was an error while loading. Please reload this page.
Id-keyed TagMap.set(long) + insertion-comparison benchmark (phase 2) - #11901
Id-keyed TagMap.set(long) + insertion-comparison benchmark (phase 2)#11901dougqh wants to merge 1 commit 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. |
267ebad to
56550cbCompare🎯 Code Coverage (details) 🔗 Commit SHA: e102e67 | Docs | View more details | Give us feedback! |
ad3c1f7 to
369a591CompareBits has a CI fix ready🟢 Investigated · 🟢 Fix prepared · ⚪ Validation skipped · 🟠 Ready Added focused coverage for View in Datadog | Reviewed commit cca1b81 · Any feedback? Reach out in #deveng-pr-agent |
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>
956f2c9 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:8886a69b70
ℹ️ 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".
| if (KnownTagCodec.isIntercepted(id)) { | ||
| return setTag(KnownTagCodec.nameOf(id), value); | ||
| } | ||
| context.setTag(id, value); |
There was a problem hiding this comment.
With the normal configuration where dd.trace.dense.tags.enabled is unset, CoreTracer never calls KnownTags.init(), so KnownTagCodec.nameOf(id) returns null. Once an instrumentation uses these new overloads, intercepted IDs such as SPAN_KIND_ID therefore delegate to a null-key String setter and silently lose the tag; non-intercepted IDs are stored but later materialize with null names. Initialize the resolver independently of the experimental dense-store flag, or otherwise make the ID path handle an inactive codec.
Useful? React with 👍 / 👎.
| if (value == null) { | ||
| context.setTag(id, (Object) null); | ||
| } else { | ||
| context.setTag(id, value); |
There was a problem hiding this comment.
For a non-intercepted known ID, calling setTag(id, "") stores the empty value, whereas the existing setTag(String, String) contract removes null or empty strings and the adjacent CharSequence ID overload does the same. This makes behavior depend on whether the compile-time value type is String or CharSequence; include value.isEmpty() in this removal branch.
Useful? React with 👍 / 👎.
| if (KnownTagCodec.isIntercepted(id)) { | ||
| return setTag(KnownTagCodec.nameOf(id), value); | ||
| } | ||
| context.setTag(id, value); |
There was a problem hiding this comment.
When a caller uses the permitted stored ID KnownTags.HTTP_STATUS_CODE_ID, that ID lacks the intercepted flag, so the int overload takes this direct context path. Unlike setTag(Tags.HTTP_STATUS, int), it never updates DDSpanContext.httpStatusCode or runs interceptHttpStatusCode; consequently getHttpStatusCode(), 404 resource naming, and status serialization from the dedicated field remain incorrect. Route this ID through the existing String/status path or mark it intercepted.
Useful? React with 👍 / 👎.
| if (KnownTagCodec.isIntercepted(id)) { | ||
| return setTag(KnownTagCodec.nameOf(id), value); | ||
| } | ||
| context.setTag(id, value); |
There was a problem hiding this comment.
With dd.trace.split-by-tags configured to a known but normally non-intercepted key such as component, TagInterceptor.needsIntercept(tag) dynamically returns true and the name-keyed setter derives the service name. The ID path consults only the static intercepted bit, bypasses the interceptor, and stores the tag without applying the configured service split. Ensure the fast path also accounts for dynamically configured interceptor keys before migrating such call sites.
Useful? React with 👍 / 👎.
991fde5 to
42b7e00Comparedougqh
commented
Aug 18, 2026
@codex review |
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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". |
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>
390a343 to
9f8ff10Comparedougqh
commented
Aug 18, 2026
@codex review |
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
Threads pre-resolved KnownTags.* ids through the span API so callers can skip keyOf name resolution and, for non-intercepted tags, the tag interceptor: - TagMap.set(long, ...) family -> putKnownById -> putKnownValue. - AgentSpan gets default id-keyed setTag methods that resolve the id back to its name and delegate to the String setter (correctness-preserving fallback; only DDSpan overrides for the fast path, so the other 20 implementers are untouched). - DDSpan.setTag(long, ...) routes intercepted ids (span.kind, http.method, http.url, db.statement) back through the String path and sends the rest straight to DDSpanContext.setTag(long, ...) -> unsafeTags.set(long, ...). - TagMapSetByIdForkedTest proves the id path is observationally identical to the name path; SpanCreationByIdBenchmark measures the throughput win. 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>
9f8ff10 to
b424214Compare
What
On top of the bloom fast-path (#11900):
TagMap.set(long id, Object)— the id-keyed insertion path: given a resolvedKnownTags.*_ID, store densely and skipkeyOfname resolution. SoleTagMapimpl (OptimizedTagMap); the id must be a stored known id (custom names keep the name setters).TagMapInsertionComparisonBenchmark— HashMap / 1.0-bucket / 2.0-name / 2.0-id, alloc + throughput; the numbers behind the deck's slides 3/7/8.Results (idle box,
-prof gc -f 5 -wi 5 -i 5; persisted in the benchmark header)hashMaptagMapByIdtagMapByNametagMapCustom(1.0 bucket)id==name(the bloom/dense win is CPU, not alloc).fieldPos & 63.Scope
Minimal id API (TagMap level). Full
AgentSpan.setTag(long)+ theKnownTagscall-site migration are follow-ons.set(long)has no production caller yet — by design, it lands with its benchmark evidence ahead of the migration that consumes it (each PR stands alone in sequence).Stacking
#11814 (dense storage) → #11900 (bloom) → this (id API + comparison benchmark).
Follow-ups
TagMap.Ledger+SpanBuilderid support; per-type graph coloring; master run to pin true-1.0 (nokeyOf).🤖 Generated with Claude Code