Repository navigation
Conversation
TagMap bucketed a known tag by folding its serial, so which tags shared one of the 16 buckets was luck, and each shared bucket costs a BucketGroup (48 B). Slot coloring already gives tags that appear together on a span type distinct slots, so bucketing a known tag by its slot keeps them in distinct buckets more often. Trace-level tags reuse span-level slot numbers, so the level bit moves them to the other half; known tags without a slot and custom tags keep their hashes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dougqh
added
comp: core
SpanScenarioBenchmark builds spans and traces the way today's decorators do (prototype in afterStart, then request or connection tags by name): bare spans, a server span, the server span via the builder, a jdbc span, and a server trace with 1, 5 or 20 jdbc children. TraceShapeBenchmark builds one web-request-shaped trace three ways: every tag by name, the decorators' calls, and the newest span APIs. Both finish what they start, so allocation per op covers whole spans and traces. Single-span results can hide per-child costs that only the trace scenarios show. TraceShapeBenchmark is the same file #12918 adds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Contributor
🟢 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. |
TraceShapeBenchmark takes the number of jdbc children (1, 5, 20), so each arm shows how a trace's cost grows with its span count: single-span and small-trace results can hide costs that land on every child span. It covers what SpanScenarioBenchmark's trace scenario added, and the rest of that benchmark overlapped SpanCreationBenchmark (same scenario names, different semantics), so SpanScenarioBenchmark goes. TagMapBucketHashTest pins down the bucket hash: known tags get distinct, nonzero hashes, a slotted tag lands in its slot's bucket (trace-level in the other half), and tags without a slot and custom tags keep their hashes. The inline comment now says the level bit flips bit 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dougqh
added a commit
that referenced
this pull request
Oct 22, 2026
Same change as #12992, which carries the identical file: the benchmark takes the number of jdbc children (1, 5, 20), so each arm shows how a trace's cost grows with its span count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dougqh
marked this pull request as ready for review
October 22, 2026 21:06
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
More details
The slot-based hash remains consistent across lookup and mutation, and collision handling retains tag-name checks. The completed static review found no reportable defect.
🤖 Bits Code Review · Commit 6e65a31
HeaderPathBenchmark runs one request per op through the propagation paths: extract the inbound headers with the default propagator, start the server span from what was extracted, run a few children, inject into an outbound call, finish everything. Inbound headers vary: none, Datadog, W3C (with dd, OpenTelemetry and third-party tracestate members), both, and both plus baggage. The existing propagation benchmarks each measure one step, so they can't see costs the extracted state causes later on every span; this one can. A setup check fails the run if nothing is extracted or injected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kets Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
HeaderPathBenchmark creates separate root traces for its JDBC and HTTP-client spans instead of children of the extracted server span, so its allocation measurements do not represent the intended request.
🤖 Bits Code Review · Commit 7894411
Comment on lines
+197
to
+219
| try (ContextScope serverScope = parent.with(server).attach()) { | ||
| for (int i = 0; i < JDBC_SPANS; ++i) { | ||
| AgentSpan jdbc = tracer.startSpan(JDBC_INSTRUMENTATION, JDBC_OPERATION); | ||
| jdbc.applyOverwriting(jdbcProto); | ||
| jdbc.setTag(Tags.DB_TYPE, "postgresql"); | ||
| jdbc.setTag(Tags.DB_INSTANCE, "petclinic"); | ||
| jdbc.setTag(Tags.PEER_HOSTNAME, "db.internal"); | ||
| jdbc.setResourceName("SELECT * FROM owners WHERE id = ?"); | ||
| try (ContextScope s = tracer.activateSpan(jdbc)) { | ||
| // the statement executes | ||
| } | ||
| jdbc.finish(); | ||
| } | ||
|
|
||
| // the outbound call: start the client span, inject its headers, finish | ||
| AgentSpan client = tracer.startSpan(CLIENT_INSTRUMENTATION, CLIENT_OPERATION); | ||
| client.applyOverwriting(clientProto); | ||
| client.setTag(Tags.HTTP_METHOD, "GET"); | ||
| client.setTag(Tags.HTTP_URL, "http://inventory.internal/items/7"); | ||
| client.setTag(Tags.PEER_HOSTNAME, "inventory.internal"); | ||
| client.setTag(Tags.PEER_PORT, 8080); | ||
| Propagators.defaultPropagator() | ||
| .inject(Context.current().with(client), outbound.headers, SETTER); |
Contributor
There was a problem hiding this comment.
In standalone JMH runs, setup creates a local CoreTracer without connecting it to the global Context manager. Attaching the server through parent.with(server).attach() therefore leaves the tracer's scope manager empty. The three JDBC spans and HTTP client become separate root traces instead of inheriting the server's trace and sampling state, invalidating the intended whole-request measurements. Use the local tracer for both attachment and injection context; strengthen the smoke check to verify child parentage.
Suggested change
| try (ContextScope serverScope = parent.with(server).attach()) { | |
| for (int i = 0; i < JDBC_SPANS; ++i) { | |
| AgentSpan jdbc = tracer.startSpan(JDBC_INSTRUMENTATION, JDBC_OPERATION); | |
| jdbc.applyOverwriting(jdbcProto); | |
| jdbc.setTag(Tags.DB_TYPE, "postgresql"); | |
| jdbc.setTag(Tags.DB_INSTANCE, "petclinic"); | |
| jdbc.setTag(Tags.PEER_HOSTNAME, "db.internal"); | |
| jdbc.setResourceName("SELECT * FROM owners WHERE id = ?"); | |
| try (ContextScope s = tracer.activateSpan(jdbc)) { | |
| // the statement executes | |
| } | |
| jdbc.finish(); | |
| } | |
| // the outbound call: start the client span, inject its headers, finish | |
| AgentSpan client = tracer.startSpan(CLIENT_INSTRUMENTATION, CLIENT_OPERATION); | |
| client.applyOverwriting(clientProto); | |
| client.setTag(Tags.HTTP_METHOD, "GET"); | |
| client.setTag(Tags.HTTP_URL, "http://inventory.internal/items/7"); | |
| client.setTag(Tags.PEER_HOSTNAME, "inventory.internal"); | |
| client.setTag(Tags.PEER_PORT, 8080); | |
| Propagators.defaultPropagator() | |
| .inject(Context.current().with(client), outbound.headers, SETTER); | |
| try (ContextScope serverScope = tracer.attach(parent.with(server))) { | |
| for (int i = 0; i < JDBC_SPANS; ++i) { | |
| AgentSpan jdbc = tracer.startSpan(JDBC_INSTRUMENTATION, JDBC_OPERATION); | |
| jdbc.applyOverwriting(jdbcProto); | |
| jdbc.setTag(Tags.DB_TYPE, "postgresql"); | |
| jdbc.setTag(Tags.DB_INSTANCE, "petclinic"); | |
| jdbc.setTag(Tags.PEER_HOSTNAME, "db.internal"); | |
| jdbc.setResourceName("SELECT * FROM owners WHERE id = ?"); | |
| try (ContextScope s = tracer.activateSpan(jdbc)) { | |
| // the statement executes | |
| } | |
| jdbc.finish(); | |
| } | |
| // the outbound call: start the client span, inject its headers, finish | |
| AgentSpan client = tracer.startSpan(CLIENT_INSTRUMENTATION, CLIENT_OPERATION); | |
| client.applyOverwriting(clientProto); | |
| client.setTag(Tags.HTTP_METHOD, "GET"); | |
| client.setTag(Tags.HTTP_URL, "http://inventory.internal/items/7"); | |
| client.setTag(Tags.PEER_HOSTNAME, "inventory.internal"); | |
| client.setTag(Tags.PEER_PORT, 8080); | |
| Propagators.defaultPropagator() | |
| .inject(tracer.currentContext().with(client), outbound.headers, SETTER); |
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
This was referenced Oct 23, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Buckets known tags in
TagMapby their slot instead of by a fold of their serial.TagMaphas 16 buckets. When two tags land in the same one, it allocates aBucketGroup(48 B). With the serial fold (since #12715), which tags share a bucket was luck. Slot coloring (#12882) already gives tags that appear together on a span type distinct slots, so bucketing by slot keeps a span's tags in distinct buckets far more often.slot & 15. Trace-level tags reuse the span-level slot numbers, so their level bit moves them into the other half of the buckets. The serial sits above the slot in the 32-bit bucket hash, so two known tags never share a hash.NO_SLOT) and custom tags: unchanged.The change is one method,
TagMap.Entry.bucketHash. Correctness doesn't depend on it: a collision still goes into aBucketGroupas before.This PR also adds
TraceShapeBenchmark(the same file #12918 adds) andTagMapBucketHashTest. The benchmark builds one web-request-shaped trace per op (a server root,childrenjdbc children, an internal span, all finished) three ways: every tag by name (legacy), the calls today's decorators make (normal), and the newest span APIs (newApi).children= 1, 5 or 20 shows how the cost scales with span count, since single-span results can hide costs that land on every child. It also addsHeaderPathBenchmark, one whole request through the header code paths (extract, start from the extracted context, children, inject, finish) for several inbound header styles, since the existing propagation benchmarks measure one step each and can't see costs the extracted state causes on every span. The test pins down the bucket hash: distinct and nonzero for every known tag, slotted tags in their slot's bucket, and the fallbacks unchanged.Motivation
Part of master's unreleased allocation regression since 1.67.0: the id-based bucket hash introduced new collisions (for example
http.useragentandpeer.ipv4against tags every root span carries), each costing aBucketGroup.Measurements
TraceShapeBenchmark, bytes per op,-prof gc, JDK 17, single-threaded, one fork; master6c87b14b0ebefore and after:legacylegacylegacynormalnormalnormalnewApinewApiThe saving is about 96 B per trace at every size: the root's tags stop colliding, while child spans mostly weren't colliding before. (
newApiwith 1 child showed −280 B in this single-fork run, most likely an escape-analysis difference between runs, so it's left out.) On the split tag stores (#12918, dense store off), where each span's known-tag store collided more, the same change saves ~1 KB on a 21-span trace.Additional Notes
component(slot 2) anddb.user(slot 18) share bucket 2. A follow-up could make slot coloring prefer, among legal slots, one that doesn't repeat a common neighbor'sslot & 15, as long as it doesn't raise the slot count.TagMapmay differ, which nothing should depend on.Where this is going
This PR is one step in replacing the span's single
TagMapwith storage that knows each tag's kind. The goal is to cut what every span allocates on the application thread, without changing behavior.The end state
TagMapbecomes a thin view for code outside the span.Entryobject per tag, and each span's store is presized from a per-operation hint. With the flag on, a web-request-shaped trace allocates about 30% less on the application thread than on master.SpanContents, not aTagMap. That covers post-processors now and serializers later, so noTagMapview gets built for them.SpanContentsstarts with four methods and grows a name, id and primitive API as more readers move to it.Later, not in this stack: cheaper read-through to parent spans, trace-level tags in a per-trace holder (
TraceSegment), and primitive tag values without boxing.The stack. This replaces #12918, split into pieces that can each be reviewed, and measured, on their own.
TagMap. This is the stack's only behavior change, kept separate on purpose.TagMap, and addTraceShapeBenchmarkSpanContentsfor the eager post-processorsTagMap's bucket mechanics intoHashSupport(a pure move)TagMapa facade over the two storestrace.experimental.dense.tags.enabled(off by default)Every PR after #12999 is behavior-preserving.
The pieces' allocation figures come from a deterministic per-thread probe (
ThreadMXBeanallocated bytes) that builds the same 7-span trace asTraceShapeBenchmark. The #12992 and #12926 figures come fromTraceShapeBenchmarkitself.🤖 Generated with Claude Code