Repository navigation
Conversation
TagMap now holds its tags in two internal stores: an IdStore for known tags by id and a name store for custom tags by name. In hash mode, the only mode so far, one HashStore serves as both, so the split adds no object per tag. The bucket structure, read-through to frozen parents and removal tombstones move into HashStore unchanged; TagMap delegates to it. The public API and behavior are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TagRouting resolves a tag name once, by KnownTagCodec.keyOrAliasOf, and routes the operation to the store that holds it: a known tag to the id store by its id, anything else to the name store by its name. TagMap's name-keyed getters, setters, removals and entry factories, and its resolved variants, all go through it, so name-to-id resolution lives in one place. A direction-scoped alias is still stored as a custom tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DDSpanContext now owns a HashStore (and, outside hash mode, an IdStore) instead of a TagMap, created when the span is constructed and read through to the frozen tracer-level tags. Every tag write and read goes through TagRouting under one lock, the HashStore itself. Direction, alias, interception and pending-alias handling stay in DDSpanContext and DirectionalTags, now applied to the stores. HashStore is no longer an IdStore: it is the name-keyed hash, which also holds known tags by id, and exposes an IdStore adapter over them. The static HASH_MODE flag lets hash mode call the hash directly, so a span needs no adapter object. Internal readers (tag post-processors, the serializer) get a short-lived TagMap view over the span's stores whose name-keyed reads honor the span's direction; getTags() is still a snapshot copy. TagMap.Internals is gone: the store API is the direct path. Its tests move to TagStoresTest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TagRouting.putAll built a TagMap view to reuse TagMap.putAll, but putAll is too big to inline, so the view escaped and cost an allocation on the span's tag-seeding path. In hash mode it now copies into the HashStore directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The view handed to the post-processor chain escapes, so building one per use cost an allocation at trace write and another at serialization. The span now creates one lazily, under its tag lock, and reuses it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A direction-scoped alias written before the span has a kind is no longer stored in the span's HashStore. It goes into a small holder that DirectionalTags owns, which stays null until the first such write, so the HashStore holds only custom tags. Kind-set resolution walks only the holder, and once every alias has resolved the holder is dropped, so the span returns to the fast path rather than staying flagged for life. The id setters' routing gate now tests the holder for null instead of reading a sticky flag. Reads of a tag with an alias probe the holder only when there is one. Last-write-wins and the removal rules are unchanged. Emitting the span's tags (trace-write and serialization processing, and the getTags snapshot) copies the holder's entries in under their literal names, so a span that never learns its kind still emits them. 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. |
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
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
Splits tag storage into strict id and name stores, and has the span own them directly.
TagMapbecomes a thin view for everyone else. No behavior change.The idea: classify a tag name once, at the boundary (known tag → id, custom tag → name, direction-scoped alias → the span's call), then keep the two kinds apart inside.
Layers
KnownTagCodecHashStoreIdStoreHashIdStore) now; a dense store slots in hereTagRoutingDDSpanContextHashStore(also its one lock) and, outside hash mode, anIdStore. All setting logic lives here, withDirectionalTagsandTagInterceptorTagMapNotable changes
TagMap.Internalsis gone. The strict store API is the direct path, so core no longer needs a trusted side door aroundTagMap. Its tests move toTagStoresTest.HASH_MODEis a compile-time constant (currentlytrue). Each id operation isHASH_MODE ? hash.x(id) : ids.x(id), so hash mode has no adapter object and no extra load.TagMapview over the span's stores.getTags()is still a snapshot copy.Entryobjects and clone onlyBucketGroupchains, as before.Motivation
TagMaphad accreted name canonicalization, known-versus-custom classification and, with #12731, direction awareness. That forced core to reach around it throughInternals, and every write route repeated the known-versus-custom and direction logic. Most of #12731's review findings were one write route missing a rule another route had.This puts policy back in the span layer, keeps storage simple and fast, and gives a dense store a clean place to replace the known-tag half. That matters for the biggest remaining allocation sites: point reads building an
Entryper read, primitive metrics stored as boxedEntrys, and the dense store's id arrays.Additional Notes
Performance (JDK 17,
-f 2,-prof gc; base is #12731's tip6aa6d4f1e2):SetTag.knownTag_byIdSetTag.knownTag_byNameSetTag.customTag_byNameGetTag.knownTag_unsafeGetTagGetTag.knownTag_getTagSpanCreation.webServerSpanSpanCreation.bareStartSpan(4 forks)Throughput is within noise. With the adapter instead of the folded
HASH_MODEpath,unsafeGetTagwas +6% and bare id writes +4%.Known cost: +40 B per span. The span's view, handed to the tag post-processor chain, escapes there and isn't scalar-replaced, so the view is cached per span (32 B plus a field). This is a stopgap: the post-processors are due for rework, and once they read the span's stores directly, the view and the field go away. Without the cache it was +32 B on the app thread and another +32 B at serialization.
Semantic note: name-keyed reads through a span's view resolve aliases by the span's direction, where today's raw
unsafeTagsreads don't. No current post-processor reads an alias name.Draft: stacked on #12731's branch, so this diff shows only the split. A follow-up commit moves pending aliases into a small lazily created holder: the custom store then holds only custom tags, and the pending-alias check becomes a null check that resets after resolution.
🤖 Generated with Claude Code