From 4873780f9a11604b96627082822f538f3149be04 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 13:24:06 -0400 Subject: [PATCH 01/10] Fold OptimizedTagMap into a final class TagMap (drop the interface) 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 --- .../main/java/datadog/trace/api/TagMap.java | 365 ++++-------------- .../trace/api/TagMapBucketGroupTest.java | 114 +++--- .../datadog/trace/api/TagMapFuzzTest.java | 22 +- .../java/datadog/trace/api/TagMapTest.java | 18 +- 4 files changed, 141 insertions(+), 378 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 1610cb3a5f2..e8a5136aa89 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -44,19 +44,19 @@ *
  • adaptive collision * */ -public interface TagMap extends Map, Iterable { +public final class TagMap implements Map, Iterable { /** Immutable empty TagMap - similar to {@link Collections#emptyMap()} */ - TagMap EMPTY = OptimizedTagMap.EmptyHolder.EMPTY; + public static final TagMap EMPTY = EmptyHolder.EMPTY; /** Creates a new mutable TagMap that contains the contents of map */ - static TagMap fromMap(Map map) { + public static TagMap fromMap(Map map) { TagMap tagMap = TagMap.create(map.size()); tagMap.putAll(map); return tagMap; } /** Creates a new immutable TagMap that contains the contents of map */ - static TagMap fromMapImmutable(Map map) { + public static TagMap fromMapImmutable(Map map) { if (map.isEmpty()) { return TagMap.EMPTY; } else { @@ -64,226 +64,25 @@ static TagMap fromMapImmutable(Map map) { } } - static TagMap create() { - return new OptimizedTagMap(); + public static TagMap create() { + return new TagMap(); } - static TagMap create(int size) { - return new OptimizedTagMap(); + public static TagMap create(int size) { + return new TagMap(); } /** Creates a new TagMap.Ledger */ - static Ledger ledger() { + public static Ledger ledger() { return new Ledger(); } /** Creates a new TagMap.Ledger which handles size modifications before expansion */ - static Ledger ledger(int size) { + public static Ledger ledger(int size) { return new Ledger(size); } - boolean isOptimized(); - - /** Inefficiently implemented for optimized TagMap */ - @Deprecated - Set keySet(); - - Iterator tagIterator(); - - /** Inefficiently implemented for optimized TagMap - requires boxing primitives */ - @Deprecated - Collection values(); - - Iterator valueIterator(); - - // @Deprecated -- not deprecated until OptimizedTagMap becomes the default - Set> entrySet(); - - /** - * Deprecated in favor of typed getters like... - * - *
      - *
    • {@link TagMap#getObject(String)} - *
    • {@link TagMap#getString(String)} - *
    • {@link TagMap#getBoolean(String)} - *
    • ... - *
    - */ - @Deprecated - Object get(Object tag); - - /** Provides the corresponding entry value as an Object - boxing if necessary */ - Object getObject(String tag); - - /** Provides the corresponding entry value as a String - calling toString if necessary */ - String getString(String tag); - - boolean getBoolean(String tag); - - boolean getBooleanOrDefault(String tag, boolean defaultValue); - - int getInt(String tag); - - int getIntOrDefault(String tag, int defaultValue); - - long getLong(String tag); - - long getLongOrDefault(String tag, long defaultValue); - - float getFloat(String tag); - - float getFloatOrDefault(String tag, float defaultValue); - - double getDouble(String tag); - - double getDoubleOrDefault(String tag, double defaultValue); - - /** - * Provides the corresponding Entry object - preferable w/ optimized TagMap if the Entry needs to - * have its type checked - */ - Entry getEntry(String tag); - - /** - * Deprecated in favor of {@link TagMap#set} methods. set methods don't return the prior value and - * are implemented more efficiently. - */ - @Deprecated - Object put(@Nonnull String tag, Object value); - - /** Sets value without returning prior value - more efficient than {@link Map#put} */ - void set(@Nonnull String tag, @Nonnull Object value); - - /** - * Similar to {@link TagMap#set(String, Object)} but more efficient when working with - * CharSequences and Strings. Depending on this situation, this methods avoids having to do type - * resolution later on - */ - void set(@Nonnull String tag, @Nonnull CharSequence value); - - void set(@Nonnull String tag, boolean value); - - void set(@Nonnull String tag, int value); - - void set(@Nonnull String tag, long value); - - void set(@Nonnull String tag, float value); - - void set(@Nonnull String tag, double value); - - /** A null reader is a no-op (see the null-tolerance contract on {@link #getAndSet(Entry)}). */ - void set(@Nullable EntryReader newEntry); - - /** sets the value while returning the prior Entry */ - Entry getAndSet(@Nonnull String tag, Object value); - - Entry getAndSet(@Nonnull String tag, CharSequence value); - - Entry getAndSet(@Nonnull String tag, boolean value); - - Entry getAndSet(@Nonnull String tag, int value); - - Entry getAndSet(@Nonnull String tag, long value); - - Entry getAndSet(@Nonnull String tag, float value); - - Entry getAndSet(@Nonnull String tag, double value); - - /** - * Places an Entry directly into the map, avoiding a new Entry allocation. Null-tolerant: a null - * {@code newEntry} is a no-op returning null, so an Entry producer (e.g. {@link - * Entry#create(String, Object)} for a null/empty value) can emit "no tag" without the caller - * filtering. Contrast the strict {@link Nonnull} {@code set(String, value)} setters. - */ - Entry getAndSet(@Nullable Entry newEntry); - - void putAll(Map map); - - /** - * Similar to {@link Map#putAll(Map)} but optimized to quickly copy from one TagMap to another - * - *

    For optimized TagMaps, this method takes advantage of the consistent TagMap layout to - * quickly handle each bucket. And similar to {@link TagMap#getAndSet(Entry)} this method shares - * Entry objects from the source TagMap - */ - void putAll(TagMap that); - - void fillMap(Map map); - - void fillStringMap(Map stringMap); - - /** - * Deprecated in favor of {@link TagMap#remove(String)} which returns a boolean and is more - * efficiently implemented - */ - @Deprecated - Object remove(Object tag); - - /** - * Similar to {@link Map#remove(Object)} but doesn't return the prior value (orEntry). Preferred - * when the prior value isn't needed - */ - boolean remove(String tag); - - /** - * Similar to {@link Map#remove(Object)} but returns the prior Entry object rather than the prior - * value. For optimized TagMap-s, this method is preferred because it avoids additional boxing. - */ - Entry getAndRemove(String tag); - - /** Returns a mutable copy of this TagMap */ - TagMap copy(); - - /** - * Returns an immutable copy of this TagMap This method is more efficient than - * map.copy().freeze() when called on an immutable TagMap - */ - TagMap immutableCopy(); - - /** - * Provides an Iterator over the Entry-s of the TagMap Equivalent to entrySet().iterator() - * , but with less allocation - */ - @Override - Iterator iterator(); - - Stream stream(); - - /** - * Visits each Entry in this TagMap This method is more efficient than {@link TagMap#iterator()} - */ - void forEach(Consumer consumer); - - /** - * Version of forEach that takes an extra context object that is passed as the first argument to - * the consumer - * - *

    The intention is to use this method to avoid using a capturing lambda - */ - void forEach(T thisObj, BiConsumer consumer); - - /** - * Version of forEach that takes two extra context objects that are passed as the first two - * argument to the consumer - * - *

    The intention is to use this method to avoid using a capturing lambda - */ - void forEach( - T thisObj, U otherObj, TriConsumer consumer); - - /** Clears the TagMap */ - void clear(); - - /** Freeze the TagMap preventing further modification - returns this TagMap */ - TagMap freeze(); - - /** Indicates if this map is frozen */ - boolean isFrozen(); - - /** Checks if the TagMap is writable - if not throws {@link IllegalStateException} */ - void checkWriteAccess(); - - abstract class EntryChange { + public abstract static class EntryChange { public static final EntryRemoval newRemoval(String tag) { return new EntryRemoval(tag); } @@ -305,7 +104,7 @@ public final boolean matches(String tag) { public abstract boolean isRemoval(); } - final class EntryRemoval extends EntryChange { + public static final class EntryRemoval extends EntryChange { EntryRemoval(String tag) { super(tag); } @@ -316,7 +115,7 @@ public boolean isRemoval() { } } - interface EntryReader { + public interface EntryReader { public static final byte OBJECT = 1; /* @@ -366,7 +165,8 @@ interface EntryReader { Entry entry(); } - final class Entry extends EntryChange implements Map.Entry, EntryReader { + public static final class Entry extends EntryChange + implements Map.Entry, EntryReader { /* * Special value used for Objects that haven't been type checked yet. * These objects might be primitive box objects. @@ -982,7 +782,7 @@ static int _hash(String tag) { * An in-order ledger of changes to be made to a TagMap. * Ledger can also serves as a builder for TagMap-s via build & buildImmutable. */ - final class Ledger implements Iterable { + public static final class Ledger implements Iterable { EntryChange[] entryChanges; int nextPos = 0; boolean containsRemovals = false; @@ -1167,45 +967,42 @@ public EntryChange next() { } } } -} -/* - * For memory efficiency, OptimizedTagMap uses a rather complicated bucket system. - *

    - * When there is only a single Entry in a particular bucket, the Entry is stored into the bucket directly. - *

    - * Because the Entry objects can be shared between multiple TagMaps, the Entry objects cannot - * directly form a linked list to handle collisions. - *

    - * Instead when multiple entries collide in the same bucket, a BucketGroup is formed to hold multiple entries. - * But a BucketGroup is only formed when a collision occurs to keep allocation low in the common case of no collisions. - *

    - * For efficiency, BucketGroups are a fixed size, so when a BucketGroup fills up another BucketGroup is formed - * to hold the additional Entry-s. And the BucketGroup-s are connected via a linked list instead of the Entry-s. - *

    - * This does introduce some inefficiencies when Entry-s are removed. - * The assumption is that removals are rare, so BucketGroups are never consolidated. - * However as a precaution if a BucketGroup becomes completely empty, then that BucketGroup will be - * removed from the collision chain. - */ -final class OptimizedTagMap implements TagMap { - // Lazy holder so the shared empty map is built on first access via the constructor — - // never read as a still-null static during the TagMap <-> OptimizedTagMap class-init cycle. - // TagMap declares default methods, so initializing OptimizedTagMap forces TagMap init first, - // and TagMap's EMPTY constant reads back through the factory into here; deferring the build - // to a separate holder keeps that read from observing a half-initialized static. + /* + * For memory efficiency, TagMap uses a rather complicated bucket system. + *

    + * When there is only a single Entry in a particular bucket, the Entry is stored into the bucket directly. + *

    + * Because the Entry objects can be shared between multiple TagMaps, the Entry objects cannot + * directly form a linked list to handle collisions. + *

    + * Instead when multiple entries collide in the same bucket, a BucketGroup is formed to hold multiple entries. + * But a BucketGroup is only formed when a collision occurs to keep allocation low in the common case of no collisions. + *

    + * For efficiency, BucketGroups are a fixed size, so when a BucketGroup fills up another BucketGroup is formed + * to hold the additional Entry-s. And the BucketGroup-s are connected via a linked list instead of the Entry-s. + *

    + * This does introduce some inefficiencies when Entry-s are removed. + * The assumption is that removals are rare, so BucketGroups are never consolidated. + * However as a precaution if a BucketGroup becomes completely empty, then that BucketGroup will be + * removed from the collision chain. + */ + // Lazy holder for the shared empty map. EMPTY is initialized from EmptyHolder.EMPTY during + // TagMap's ; isolating the instance in a holder means its construction is deferred to + // first access and never observes a half-initialized TagMap static (the private constructor + // reads no statics, so the empty view is safe to build during class init). static final class EmptyHolder { // Using special constructor that creates a frozen view of an existing array. // Bucket calculation requires that array length is a power of 2; size 0 fails with // ArrayIndexOutOfBoundsException, but size 1 works. - static final OptimizedTagMap EMPTY = new OptimizedTagMap(new Object[1], 0); + static final TagMap EMPTY = new TagMap(new Object[1], 0); } private final Object[] buckets; private int size; private boolean frozen; - public OptimizedTagMap() { + public TagMap() { // needs to be a power of 2 for bucket masking calculation to work as intended this.buckets = new Object[1 << 4]; this.size = 0; @@ -1213,13 +1010,12 @@ public OptimizedTagMap() { } /** Used for inexpensive immutable */ - private OptimizedTagMap(Object[] buckets, int size) { + private TagMap(Object[] buckets, int size) { this.buckets = buckets; this.size = size; this.frozen = true; } - @Override public boolean isOptimized() { return true; } @@ -1320,7 +1116,6 @@ public Set keySet() { return new Keys(this); } - @Override public Iterator tagIterator() { return new KeysIterator(this); } @@ -1330,7 +1125,6 @@ public Collection values() { return new Values(this); } - @Override public Iterator valueIterator() { return new ValuesIterator(this); } @@ -1340,7 +1134,6 @@ public Set> entrySet() { return new Entries(this); } - @Override public Entry getEntry(String tag) { Object[] thisBuckets = this.buckets; @@ -1369,7 +1162,6 @@ public Object put(String tag, Object value) { return entry == null ? null : entry.objectValue(); } - @Override public void set(TagMap.EntryReader newEntryReader) { if (newEntryReader == null) { return; @@ -1377,42 +1169,34 @@ public void set(TagMap.EntryReader newEntryReader) { this.getAndSet(newEntryReader.entry()); } - @Override public void set(String tag, Object value) { this.getAndSet(Entry.newAnyEntry(tag, value)); } - @Override public void set(String tag, CharSequence value) { this.getAndSet(Entry.newObjectEntry(tag, value)); } - @Override public void set(String tag, boolean value) { this.getAndSet(Entry.newBooleanEntry(tag, value)); } - @Override public void set(String tag, int value) { this.getAndSet(Entry.newIntEntry(tag, value)); } - @Override public void set(String tag, long value) { this.getAndSet(Entry.newLongEntry(tag, value)); } - @Override public void set(String tag, float value) { this.getAndSet(Entry.newFloatEntry(tag, value)); } - @Override public void set(String tag, double value) { this.getAndSet(Entry.newDoubleEntry(tag, value)); } - @Override public Entry getAndSet(Entry newEntry) { if (newEntry == null) { return null; @@ -1465,37 +1249,30 @@ public Entry getAndSet(Entry newEntry) { return null; } - @Override public Entry getAndSet(String tag, Object value) { return this.getAndSet(Entry.newAnyEntry(tag, value)); } - @Override public Entry getAndSet(String tag, CharSequence value) { return this.getAndSet(Entry.newObjectEntry(tag, value)); } - @Override public TagMap.Entry getAndSet(String tag, boolean value) { return this.getAndSet(Entry.newBooleanEntry(tag, value)); } - @Override public TagMap.Entry getAndSet(String tag, int value) { return this.getAndSet(Entry.newIntEntry(tag, value)); } - @Override public TagMap.Entry getAndSet(String tag, long value) { return this.getAndSet(Entry.newLongEntry(tag, value)); } - @Override public TagMap.Entry getAndSet(String tag, float value) { return this.getAndSet(Entry.newFloatEntry(tag, value)); } - @Override public TagMap.Entry getAndSet(String tag, double value) { return this.getAndSet(Entry.newDoubleEntry(tag, value)); } @@ -1503,8 +1280,8 @@ public TagMap.Entry getAndSet(String tag, double value) { public void putAll(Map map) { this.checkWriteAccess(); - if (map instanceof OptimizedTagMap) { - this.putAllOptimizedMap((OptimizedTagMap) map); + if (map instanceof TagMap) { + this.putAllOptimizedMap((TagMap) map); } else { this.putAllUnoptimizedMap(map); } @@ -1518,23 +1295,18 @@ private void putAllUnoptimizedMap(Map that) } /** - * Similar to {@link Map#putAll(Map)} but optimized to quickly copy from one TagMap to another + * Similar to {@link Map#putAll(Map)} but optimized to quickly copy from one TagMap to another. * - *

    For optimized TagMaps, this method takes advantage of the consistent TagMap layout to - * quickly handle each bucket. And similar to {@link TagMap#getAndSet(Entry)} this method shares - * Entry objects from the source TagMap + *

    Takes advantage of the consistent TagMap layout to quickly handle each bucket. And similar + * to {@link TagMap#getAndSet(Entry)} this method shares Entry objects from the source TagMap. */ public void putAll(TagMap that) { this.checkWriteAccess(); - if (that instanceof OptimizedTagMap) { - this.putAllOptimizedMap((OptimizedTagMap) that); - } else { - this.putAllUnoptimizedMap(that); - } + this.putAllOptimizedMap(that); } - private void putAllOptimizedMap(OptimizedTagMap that) { + private void putAllOptimizedMap(TagMap that) { if (this.size == 0) { this.putAllIntoEmptyMap(that); } else { @@ -1542,7 +1314,7 @@ private void putAllOptimizedMap(OptimizedTagMap that) { } } - private void putAllMerge(OptimizedTagMap that) { + private void putAllMerge(TagMap that) { Object[] thisBuckets = this.buckets; Object[] thatBuckets = that.buckets; @@ -1659,7 +1431,7 @@ private void putAllMerge(OptimizedTagMap that) { /* * Specially optimized version of putAll for the common case of destination map being empty */ - private void putAllIntoEmptyMap(OptimizedTagMap that) { + private void putAllIntoEmptyMap(TagMap that) { Object[] thisBuckets = this.buckets; Object[] thatBuckets = that.buckets; @@ -1731,7 +1503,6 @@ public boolean remove(String tag) { return (this.getAndRemove(tag) != null); } - @Override public Entry getAndRemove(String tag) { this.checkWriteAccess(); @@ -1771,9 +1542,8 @@ public Entry getAndRemove(String tag) { return null; } - @Override public TagMap copy() { - OptimizedTagMap copy = new OptimizedTagMap(); + TagMap copy = new TagMap(); copy.putAllIntoEmptyMap(this); return copy; } @@ -1791,7 +1561,6 @@ public Iterator iterator() { return new EntryReaderIterator(this); } - @Override public Stream stream() { return StreamSupport.stream(spliterator(), false); } @@ -1815,7 +1584,6 @@ public void forEach(Consumer consumer) { } } - @Override public void forEach(T thisObj, BiConsumer consumer) { Object[] thisBuckets = this.buckets; @@ -1834,7 +1602,6 @@ public void forEach(T thisObj, BiConsumer con } } - @Override public void forEach( T thisObj, U otherObj, TriConsumer consumer) { Object[] thisBuckets = this.buckets; @@ -1861,7 +1628,7 @@ public void clear() { this.size = 0; } - public OptimizedTagMap freeze() { + public TagMap freeze() { this.frozen = true; return this; @@ -1960,7 +1727,7 @@ public Object compute( String key, BiFunction remappingFunction) { this.checkWriteAccess(); - return TagMap.super.compute(key, remappingFunction); + return Map.super.compute(key, remappingFunction); } @Override @@ -1968,7 +1735,7 @@ public Object computeIfAbsent( String key, Function mappingFunction) { this.checkWriteAccess(); - return TagMap.super.computeIfAbsent(key, mappingFunction); + return Map.super.computeIfAbsent(key, mappingFunction); } @Override @@ -1976,7 +1743,7 @@ public Object computeIfPresent( String key, BiFunction remappingFunction) { this.checkWriteAccess(); - return TagMap.super.computeIfPresent(key, remappingFunction); + return Map.super.computeIfPresent(key, remappingFunction); } @Override @@ -2043,7 +1810,7 @@ abstract static class IteratorBase { private BucketGroup group = null; private int groupIndex = 0; - IteratorBase(OptimizedTagMap map) { + IteratorBase(TagMap map) { this.buckets = map.buckets; } @@ -2117,7 +1884,7 @@ private final Entry advance() { } static final class EntryReaderIterator extends IteratorBase implements Iterator { - EntryReaderIterator(OptimizedTagMap map) { + EntryReaderIterator(TagMap map) { super(map); } @@ -2592,9 +2359,9 @@ public String toString() { } static final class Entries extends AbstractSet> { - private final OptimizedTagMap map; + private final TagMap map; - Entries(OptimizedTagMap map) { + Entries(TagMap map) { this.map = map; } @@ -2617,9 +2384,9 @@ public Iterator> iterator() { } static final class Keys extends AbstractSet { - final OptimizedTagMap map; + final TagMap map; - Keys(OptimizedTagMap map) { + Keys(TagMap map) { this.map = map; } @@ -2645,7 +2412,7 @@ public Iterator iterator() { } static final class KeysIterator extends IteratorBase implements Iterator { - KeysIterator(OptimizedTagMap map) { + KeysIterator(TagMap map) { super(map); } @@ -2656,9 +2423,9 @@ public String next() { } static final class Values extends AbstractCollection { - final OptimizedTagMap map; + final TagMap map; - Values(OptimizedTagMap map) { + Values(TagMap map) { this.map = map; } @@ -2684,7 +2451,7 @@ public Iterator iterator() { } static final class ValuesIterator extends IteratorBase implements Iterator { - ValuesIterator(OptimizedTagMap map) { + ValuesIterator(TagMap map) { super(map); } diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapBucketGroupTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapBucketGroupTest.java index cecadd446b1..477a5ae58d8 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapBucketGroupTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapBucketGroupTest.java @@ -19,8 +19,8 @@ public void newGroup() { int firstHash = firstEntry.hash(); int secondHash = secondEntry.hash(); - OptimizedTagMap.BucketGroup group = - new OptimizedTagMap.BucketGroup( + TagMap.BucketGroup group = + new TagMap.BucketGroup( firstHash, firstEntry, secondHash, secondEntry); @@ -45,8 +45,8 @@ public void _insert() { int firstHash = firstEntry.hash(); int secondHash = secondEntry.hash(); - OptimizedTagMap.BucketGroup group = - new OptimizedTagMap.BucketGroup(firstHash, firstEntry, secondHash, secondEntry); + TagMap.BucketGroup group = + new TagMap.BucketGroup(firstHash, firstEntry, secondHash, secondEntry); TagMap.Entry newEntry = TagMap.Entry.newAnyEntry("baz", "lorem ipsum"); int newHash = newEntry.hash(); @@ -82,8 +82,7 @@ public void _replace() { int origHash = origEntry.hash(); int otherHash = otherEntry.hash(); - OptimizedTagMap.BucketGroup group = - new OptimizedTagMap.BucketGroup(origHash, origEntry, otherHash, otherEntry); + TagMap.BucketGroup group = new TagMap.BucketGroup(origHash, origEntry, otherHash, otherEntry); assertContainsDirectly(origEntry, group); assertContainsDirectly(otherEntry, group); @@ -112,8 +111,8 @@ public void _remove() { int firstHash = firstEntry.hash(); int secondHash = secondEntry.hash(); - OptimizedTagMap.BucketGroup group = - new OptimizedTagMap.BucketGroup( + TagMap.BucketGroup group = + new TagMap.BucketGroup( firstHash, firstEntry, secondHash, secondEntry); @@ -137,9 +136,9 @@ public void _remove() { @Test public void groupChaining() { int startingIndex = 10; - OptimizedTagMap.BucketGroup firstGroup = fullGroup(startingIndex); + TagMap.BucketGroup firstGroup = fullGroup(startingIndex); - for (int offset = 0; offset < OptimizedTagMap.BucketGroup.LEN; ++offset) { + for (int offset = 0; offset < TagMap.BucketGroup.LEN; ++offset) { assertChainContainsTag(tag(startingIndex + offset), firstGroup); } @@ -151,79 +150,78 @@ public void groupChaining() { assertFalse(firstGroup._insert(newHash, newEntry)); assertDoesntContainDirectly(newEntry, firstGroup); - OptimizedTagMap.BucketGroup newHeadGroup = - new OptimizedTagMap.BucketGroup(newHash, newEntry, firstGroup); + TagMap.BucketGroup newHeadGroup = new TagMap.BucketGroup(newHash, newEntry, firstGroup); assertContainsDirectly(newEntry, newHeadGroup); assertSame(firstGroup, newHeadGroup.prev); assertChainContainsTag("new", newHeadGroup); - for (int offset = 0; offset < OptimizedTagMap.BucketGroup.LEN; ++offset) { + for (int offset = 0; offset < TagMap.BucketGroup.LEN; ++offset) { assertChainContainsTag(tag(startingIndex + offset), newHeadGroup); } } @Test public void removeInChain() { - OptimizedTagMap.BucketGroup firstGroup = fullGroup(10); - OptimizedTagMap.BucketGroup headGroup = fullGroup(20, firstGroup); + TagMap.BucketGroup firstGroup = fullGroup(10); + TagMap.BucketGroup headGroup = fullGroup(20, firstGroup); - for (int offset = 0; offset < OptimizedTagMap.BucketGroup.LEN; ++offset) { + for (int offset = 0; offset < TagMap.BucketGroup.LEN; ++offset) { assertChainContainsTag(tag(10, offset), headGroup); assertChainContainsTag(tag(20, offset), headGroup); } - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); String firstRemovedTag = tag(10, 1); int firstRemovedHash = TagMap.Entry._hash(firstRemovedTag); - OptimizedTagMap.BucketGroup firstContainingGroup = + TagMap.BucketGroup firstContainingGroup = headGroup.findContainingGroupInChain(firstRemovedHash, firstRemovedTag); assertSame(firstContainingGroup, firstGroup); assertNotNull(firstContainingGroup._remove(firstRemovedHash, firstRemovedTag)); assertChainDoesntContainTag(firstRemovedTag, headGroup); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2 - 1, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2 - 1, headGroup.sizeInChain()); String secondRemovedTag = tag(20, 2); int secondRemovedHash = TagMap.Entry._hash(secondRemovedTag); - OptimizedTagMap.BucketGroup secondContainingGroup = + TagMap.BucketGroup secondContainingGroup = headGroup.findContainingGroupInChain(secondRemovedHash, secondRemovedTag); assertSame(secondContainingGroup, headGroup); assertNotNull(secondContainingGroup._remove(secondRemovedHash, secondRemovedTag)); assertChainDoesntContainTag(secondRemovedTag, headGroup); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2 - 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2 - 2, headGroup.sizeInChain()); } @Test public void replaceInChain() { - OptimizedTagMap.BucketGroup firstGroup = fullGroup(10); - OptimizedTagMap.BucketGroup headGroup = fullGroup(20, firstGroup); + TagMap.BucketGroup firstGroup = fullGroup(10); + TagMap.BucketGroup headGroup = fullGroup(20, firstGroup); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); TagMap.Entry firstReplacementEntry = TagMap.Entry.newObjectEntry(tag(10, 1), "replaced"); assertNotNull(headGroup.replaceInChain(firstReplacementEntry.hash(), firstReplacementEntry)); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); TagMap.Entry secondReplacementEntry = TagMap.Entry.newObjectEntry(tag(20, 2), "replaced"); assertNotNull(headGroup.replaceInChain(secondReplacementEntry.hash(), secondReplacementEntry)); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); } @Test public void insertInChain() { // set-up a chain with some gaps in it - OptimizedTagMap.BucketGroup firstGroup = fullGroup(10); - OptimizedTagMap.BucketGroup headGroup = fullGroup(20, firstGroup); + TagMap.BucketGroup firstGroup = fullGroup(10); + TagMap.BucketGroup headGroup = fullGroup(20, firstGroup); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); String firstHoleTag = tag(10, 1); int firstHoleHash = TagMap.Entry._hash(firstHoleTag); @@ -233,7 +231,7 @@ public void insertInChain() { int secondHoleHash = TagMap.Entry._hash(secondHoleTag); headGroup._remove(secondHoleHash, secondHoleTag); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2 - 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2 - 2, headGroup.sizeInChain()); String firstNewTag = "new-tag-0"; TagMap.Entry firstNewEntry = TagMap.Entry.newObjectEntry(firstNewTag, "new"); @@ -256,18 +254,18 @@ public void insertInChain() { assertFalse(headGroup.insertInChain(thirdNewHash, thirdNewEntry)); assertChainDoesntContainTag(thirdNewTag, headGroup); - assertEquals(OptimizedTagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); + assertEquals(TagMap.BucketGroup.LEN * 2, headGroup.sizeInChain()); } @Test public void cloneChain() { - OptimizedTagMap.BucketGroup firstGroup = fullGroup(10); - OptimizedTagMap.BucketGroup secondGroup = fullGroup(20, firstGroup); - OptimizedTagMap.BucketGroup headGroup = fullGroup(30, secondGroup); + TagMap.BucketGroup firstGroup = fullGroup(10); + TagMap.BucketGroup secondGroup = fullGroup(20, firstGroup); + TagMap.BucketGroup headGroup = fullGroup(30, secondGroup); - OptimizedTagMap.BucketGroup clonedHeadGroup = headGroup.cloneChain(); - OptimizedTagMap.BucketGroup clonedSecondGroup = clonedHeadGroup.prev; - OptimizedTagMap.BucketGroup clonedFirstGroup = clonedSecondGroup.prev; + TagMap.BucketGroup clonedHeadGroup = headGroup.cloneChain(); + TagMap.BucketGroup clonedSecondGroup = clonedHeadGroup.prev; + TagMap.BucketGroup clonedFirstGroup = clonedSecondGroup.prev; assertGroupContentsStrictEquals(headGroup, clonedHeadGroup); assertGroupContentsStrictEquals(secondGroup, clonedSecondGroup); @@ -276,11 +274,11 @@ public void cloneChain() { @Test public void removeGroupInChain() { - OptimizedTagMap.BucketGroup tailGroup = fullGroup(10); - OptimizedTagMap.BucketGroup secondGroup = fullGroup(20, tailGroup); - OptimizedTagMap.BucketGroup thirdGroup = fullGroup(30, secondGroup); - OptimizedTagMap.BucketGroup fourthGroup = fullGroup(40, thirdGroup); - OptimizedTagMap.BucketGroup headGroup = fullGroup(50, fourthGroup); + TagMap.BucketGroup tailGroup = fullGroup(10); + TagMap.BucketGroup secondGroup = fullGroup(20, tailGroup); + TagMap.BucketGroup thirdGroup = fullGroup(30, secondGroup); + TagMap.BucketGroup fourthGroup = fullGroup(40, thirdGroup); + TagMap.BucketGroup headGroup = fullGroup(50, fourthGroup); assertChain(headGroup, fourthGroup, thirdGroup, secondGroup, tailGroup); // need to test group removal - at head, middle, and tail of the chain @@ -298,15 +296,14 @@ public void removeGroupInChain() { assertChain(fourthGroup, secondGroup); } - static final OptimizedTagMap.BucketGroup fullGroup(int startingIndex) { + static final TagMap.BucketGroup fullGroup(int startingIndex) { TagMap.Entry firstEntry = TagMap.Entry.newObjectEntry(tag(startingIndex), value(startingIndex)); TagMap.Entry secondEntry = TagMap.Entry.newObjectEntry(tag(startingIndex + 1), value(startingIndex + 1)); - OptimizedTagMap.BucketGroup group = - new OptimizedTagMap.BucketGroup( - firstEntry.hash(), firstEntry, secondEntry.hash(), secondEntry); - for (int offset = 2; offset < OptimizedTagMap.BucketGroup.LEN; ++offset) { + TagMap.BucketGroup group = + new TagMap.BucketGroup(firstEntry.hash(), firstEntry, secondEntry.hash(), secondEntry); + for (int offset = 2; offset < TagMap.BucketGroup.LEN; ++offset) { TagMap.Entry anotherEntry = TagMap.Entry.newObjectEntry(tag(startingIndex + offset), value(startingIndex + offset)); group._insert(anotherEntry.hash(), anotherEntry); @@ -314,9 +311,8 @@ static final OptimizedTagMap.BucketGroup fullGroup(int startingIndex) { return group; } - static final OptimizedTagMap.BucketGroup fullGroup( - int startingIndex, OptimizedTagMap.BucketGroup prev) { - OptimizedTagMap.BucketGroup group = fullGroup(startingIndex); + static final TagMap.BucketGroup fullGroup(int startingIndex, TagMap.BucketGroup prev) { + TagMap.BucketGroup group = fullGroup(startingIndex); group.prev = prev; return group; } @@ -333,7 +329,7 @@ static final String value(int i) { return "value-" + i; } - static void assertContainsDirectly(TagMap.Entry entry, OptimizedTagMap.BucketGroup group) { + static void assertContainsDirectly(TagMap.Entry entry, TagMap.BucketGroup group) { int hash = entry.hash(); String tag = entry.tag(); @@ -343,32 +339,32 @@ static void assertContainsDirectly(TagMap.Entry entry, OptimizedTagMap.BucketGro assertSame(group, group.findContainingGroupInChain(hash, tag)); } - static void assertDoesntContainDirectly(TagMap.Entry entry, OptimizedTagMap.BucketGroup group) { - for (int i = 0; i < OptimizedTagMap.BucketGroup.LEN; ++i) { + static void assertDoesntContainDirectly(TagMap.Entry entry, TagMap.BucketGroup group) { + for (int i = 0; i < TagMap.BucketGroup.LEN; ++i) { assertNotSame(entry, group._entryAt(i)); } } - static void assertChainContainsTag(String tag, OptimizedTagMap.BucketGroup group) { + static void assertChainContainsTag(String tag, TagMap.BucketGroup group) { int hash = TagMap.Entry._hash(tag); assertNotNull(group.findInChain(hash, tag)); } - static void assertChainDoesntContainTag(String tag, OptimizedTagMap.BucketGroup group) { + static void assertChainDoesntContainTag(String tag, TagMap.BucketGroup group) { int hash = TagMap.Entry._hash(tag); assertNull(group.findInChain(hash, tag)); } static void assertGroupContentsStrictEquals( - OptimizedTagMap.BucketGroup expected, OptimizedTagMap.BucketGroup actual) { - for (int i = 0; i < OptimizedTagMap.BucketGroup.LEN; ++i) { + TagMap.BucketGroup expected, TagMap.BucketGroup actual) { + for (int i = 0; i < TagMap.BucketGroup.LEN; ++i) { assertEquals(expected._hashAt(i), actual._hashAt(i)); assertSame(expected._entryAt(i), actual._entryAt(i)); } } - static void assertChain(OptimizedTagMap.BucketGroup... chain) { - OptimizedTagMap.BucketGroup cur; + static void assertChain(TagMap.BucketGroup... chain) { + TagMap.BucketGroup cur; int index; for (cur = chain[0], index = 0; cur != null; cur = cur.prev, ++index) { assertSame(chain[index], cur); diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java index 48254ae9bd1..685dc1d9cdf 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java @@ -30,8 +30,8 @@ void testMerge() { TestCase mapACase = generateTest(); TestCase mapBCase = generateTest(); - OptimizedTagMap tagMapA = test(mapACase); - OptimizedTagMap tagMapB = test(mapBCase); + TagMap tagMapA = test(mapACase); + TagMap tagMapB = test(mapBCase); HashMap hashMapA = new HashMap<>(tagMapA); HashMap hashMapB = new HashMap<>(tagMapB); @@ -858,7 +858,7 @@ void priorFailingCase2() { put("key-41", "values--904162962")); Map expected = makeMap(testCase); - OptimizedTagMap actual = makeTagMap(testCase); + TagMap actual = makeTagMap(testCase); MapAction failingAction = remove("key-127"); failingAction.applyToExpectedMap(expected); @@ -889,27 +889,27 @@ public static final Map makeMap(List actions) { return map; } - public static final OptimizedTagMap makeTagMap(TestCase testCase) { + public static final TagMap makeTagMap(TestCase testCase) { return makeTagMap(testCase.actions); } - public static final OptimizedTagMap makeTagMap(MapAction... actions) { + public static final TagMap makeTagMap(MapAction... actions) { return makeTagMap(Arrays.asList(actions)); } - public static final OptimizedTagMap makeTagMap(List actions) { - OptimizedTagMap map = new OptimizedTagMap(); + public static final TagMap makeTagMap(List actions) { + TagMap map = new TagMap(); for (MapAction action : actions) { action.applyToTestMap(map); } return map; } - public static final OptimizedTagMap test(TestCase test) { + public static final TagMap test(TestCase test) { List actions = test.actions(); Map hashMap = new HashMap<>(); - OptimizedTagMap tagMap = new OptimizedTagMap(); + TagMap tagMap = new TagMap(); int actionIndex = 0; try { @@ -1014,7 +1014,7 @@ public static final MapAction getAndRemove(String key) { return new GetAndRemove(key); } - static final void assertMapEquals(Map expected, OptimizedTagMap actual) { + static final void assertMapEquals(Map expected, TagMap actual) { // checks entries in both directions to make sure there's full intersection for (Map.Entry expectedEntry : expected.entrySet()) { @@ -1100,7 +1100,7 @@ static final Map mapOf(String... keysAndValues) { } static final TagMap tagMapOf(String... keysAndValues) { - OptimizedTagMap map = new OptimizedTagMap(); + TagMap map = new TagMap(); for (int i = 0; i < keysAndValues.length; i += 2) { String key = keysAndValues[i]; String value = keysAndValues[i + 1]; diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapTest.java index 0227a5b3886..a91cb4999ed 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapTest.java @@ -944,7 +944,7 @@ public void computeIfPresent() { @ParameterizedTest @ValueSource(ints = {0, 5, 25, 125}) public void _toInternalString(int size) { - OptimizedTagMap tagMap = new OptimizedTagMap(); + TagMap tagMap = new TagMap(); fillMap(tagMap, size); String str = tagMap.toInternalString(); @@ -1064,8 +1064,8 @@ static final void assertEntry(String key, String value, TagMap map) { } static final void assertSize(int size, TagMap map) { - if (map instanceof OptimizedTagMap) { - assertEquals(size, ((OptimizedTagMap) map).computeSize()); + if (map instanceof TagMap) { + assertEquals(size, ((TagMap) map).computeSize()); } assertEquals(size, map.size()); @@ -1094,15 +1094,15 @@ static void assertEmptiness(boolean expectEmpty, TagMap map) { } static void assertNotEmpty(TagMap map) { - if (map instanceof OptimizedTagMap) { - assertFalse(((OptimizedTagMap) map).checkIfEmpty()); + if (map instanceof TagMap) { + assertFalse(((TagMap) map).checkIfEmpty()); } assertFalse(map.isEmpty()); } static void assertEmpty(TagMap map) { - if (map instanceof OptimizedTagMap) { - assertTrue(((OptimizedTagMap) map).checkIfEmpty()); + if (map instanceof TagMap) { + assertTrue(((TagMap) map).checkIfEmpty()); } assertTrue(map.isEmpty()); } @@ -1128,8 +1128,8 @@ static void assertFrozen(Runnable runnable) { } static void checkIntegrity(TagMap map) { - if (map instanceof OptimizedTagMap) { - OptimizedTagMap optMap = (OptimizedTagMap) map; + if (map instanceof TagMap) { + TagMap optMap = (TagMap) map; optMap.checkIntegrity(); } } From 5bb0fee016924cdab57bc9ff1a3b2c2d559958bb Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 13:39:11 -0400 Subject: [PATCH 02/10] Drop EmptyHolder, make factories final, simplify test-helper instanceof 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 --- .../main/java/datadog/trace/api/TagMap.java | 29 +++++++------------ .../java/datadog/trace/api/TagMapTest.java | 17 +++-------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index e8a5136aa89..8c95a34aa39 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -46,17 +46,21 @@ */ public final class TagMap implements Map, Iterable { /** Immutable empty TagMap - similar to {@link Collections#emptyMap()} */ - public static final TagMap EMPTY = EmptyHolder.EMPTY; + // Frozen view over a length-1 array: bucket masking needs a power-of-two array length (size 0 + // would fail with ArrayIndexOutOfBoundsException, size 1 works), and the private constructor + // reads + // no statics, so this is safe to build directly during TagMap's . + public static final TagMap EMPTY = new TagMap(new Object[1], 0); /** Creates a new mutable TagMap that contains the contents of map */ - public static TagMap fromMap(Map map) { + public static final TagMap fromMap(Map map) { TagMap tagMap = TagMap.create(map.size()); tagMap.putAll(map); return tagMap; } /** Creates a new immutable TagMap that contains the contents of map */ - public static TagMap fromMapImmutable(Map map) { + public static final TagMap fromMapImmutable(Map map) { if (map.isEmpty()) { return TagMap.EMPTY; } else { @@ -64,21 +68,21 @@ public static TagMap fromMapImmutable(Map map) { } } - public static TagMap create() { + public static final TagMap create() { return new TagMap(); } - public static TagMap create(int size) { + public static final TagMap create(int size) { return new TagMap(); } /** Creates a new TagMap.Ledger */ - public static Ledger ledger() { + public static final Ledger ledger() { return new Ledger(); } /** Creates a new TagMap.Ledger which handles size modifications before expansion */ - public static Ledger ledger(int size) { + public static final Ledger ledger(int size) { return new Ledger(size); } @@ -987,17 +991,6 @@ public EntryChange next() { * However as a precaution if a BucketGroup becomes completely empty, then that BucketGroup will be * removed from the collision chain. */ - // Lazy holder for the shared empty map. EMPTY is initialized from EmptyHolder.EMPTY during - // TagMap's ; isolating the instance in a holder means its construction is deferred to - // first access and never observes a half-initialized TagMap static (the private constructor - // reads no statics, so the empty view is safe to build during class init). - static final class EmptyHolder { - // Using special constructor that creates a frozen view of an existing array. - // Bucket calculation requires that array length is a power of 2; size 0 fails with - // ArrayIndexOutOfBoundsException, but size 1 works. - static final TagMap EMPTY = new TagMap(new Object[1], 0); - } - private final Object[] buckets; private int size; private boolean frozen; diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapTest.java index a91cb4999ed..da179553f56 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapTest.java @@ -1064,9 +1064,7 @@ static final void assertEntry(String key, String value, TagMap map) { } static final void assertSize(int size, TagMap map) { - if (map instanceof TagMap) { - assertEquals(size, ((TagMap) map).computeSize()); - } + assertEquals(size, map.computeSize()); assertEquals(size, map.size()); assertEquals(size, count(map)); @@ -1094,16 +1092,12 @@ static void assertEmptiness(boolean expectEmpty, TagMap map) { } static void assertNotEmpty(TagMap map) { - if (map instanceof TagMap) { - assertFalse(((TagMap) map).checkIfEmpty()); - } + assertFalse(map.checkIfEmpty()); assertFalse(map.isEmpty()); } static void assertEmpty(TagMap map) { - if (map instanceof TagMap) { - assertTrue(((TagMap) map).checkIfEmpty()); - } + assertTrue(map.checkIfEmpty()); assertTrue(map.isEmpty()); } @@ -1128,9 +1122,6 @@ static void assertFrozen(Runnable runnable) { } static void checkIntegrity(TagMap map) { - if (map instanceof TagMap) { - TagMap optMap = (TagMap) map; - optMap.checkIntegrity(); - } + map.checkIntegrity(); } } From 8ee8c2892c3f112677c73f9cc88ea6c57439a6f3 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 14:02:31 -0400 Subject: [PATCH 03/10] Restore null-contract annotations lost when folding away the interface 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 --- .../main/java/datadog/trace/api/TagMap.java | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 8c95a34aa39..ed81edac466 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -1150,47 +1150,54 @@ public Entry getEntry(String tag) { @Deprecated @Override - public Object put(String tag, Object value) { + public Object put(@Nonnull String tag, Object value) { TagMap.Entry entry = this.getAndSet(Entry.newAnyEntry(tag, value)); return entry == null ? null : entry.objectValue(); } - public void set(TagMap.EntryReader newEntryReader) { + /** A null reader is a no-op (see the null-tolerance contract on {@link #getAndSet(Entry)}). */ + public void set(@Nullable TagMap.EntryReader newEntryReader) { if (newEntryReader == null) { return; } this.getAndSet(newEntryReader.entry()); } - public void set(String tag, Object value) { + public void set(@Nonnull String tag, @Nonnull Object value) { this.getAndSet(Entry.newAnyEntry(tag, value)); } - public void set(String tag, CharSequence value) { + public void set(@Nonnull String tag, @Nonnull CharSequence value) { this.getAndSet(Entry.newObjectEntry(tag, value)); } - public void set(String tag, boolean value) { + public void set(@Nonnull String tag, boolean value) { this.getAndSet(Entry.newBooleanEntry(tag, value)); } - public void set(String tag, int value) { + public void set(@Nonnull String tag, int value) { this.getAndSet(Entry.newIntEntry(tag, value)); } - public void set(String tag, long value) { + public void set(@Nonnull String tag, long value) { this.getAndSet(Entry.newLongEntry(tag, value)); } - public void set(String tag, float value) { + public void set(@Nonnull String tag, float value) { this.getAndSet(Entry.newFloatEntry(tag, value)); } - public void set(String tag, double value) { + public void set(@Nonnull String tag, double value) { this.getAndSet(Entry.newDoubleEntry(tag, value)); } - public Entry getAndSet(Entry newEntry) { + /** + * Places an Entry directly into the map, avoiding a new Entry allocation. Null-tolerant: a null + * {@code newEntry} is a no-op returning null, so an Entry producer (e.g. {@link + * Entry#create(String, Object)} for a null/empty value) can emit "no tag" without the caller + * filtering. Contrast the strict {@link Nonnull} {@code set(String, value)} setters. + */ + public Entry getAndSet(@Nullable Entry newEntry) { if (newEntry == null) { return null; } @@ -1242,31 +1249,31 @@ public Entry getAndSet(Entry newEntry) { return null; } - public Entry getAndSet(String tag, Object value) { + public Entry getAndSet(@Nonnull String tag, Object value) { return this.getAndSet(Entry.newAnyEntry(tag, value)); } - public Entry getAndSet(String tag, CharSequence value) { + public Entry getAndSet(@Nonnull String tag, CharSequence value) { return this.getAndSet(Entry.newObjectEntry(tag, value)); } - public TagMap.Entry getAndSet(String tag, boolean value) { + public TagMap.Entry getAndSet(@Nonnull String tag, boolean value) { return this.getAndSet(Entry.newBooleanEntry(tag, value)); } - public TagMap.Entry getAndSet(String tag, int value) { + public TagMap.Entry getAndSet(@Nonnull String tag, int value) { return this.getAndSet(Entry.newIntEntry(tag, value)); } - public TagMap.Entry getAndSet(String tag, long value) { + public TagMap.Entry getAndSet(@Nonnull String tag, long value) { return this.getAndSet(Entry.newLongEntry(tag, value)); } - public TagMap.Entry getAndSet(String tag, float value) { + public TagMap.Entry getAndSet(@Nonnull String tag, float value) { return this.getAndSet(Entry.newFloatEntry(tag, value)); } - public TagMap.Entry getAndSet(String tag, double value) { + public TagMap.Entry getAndSet(@Nonnull String tag, double value) { return this.getAndSet(Entry.newDoubleEntry(tag, value)); } From 0737ca6bad331c4ed8ce3dd727492e1b2f1dd9b1 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Jul 2026 07:01:57 -0400 Subject: [PATCH 04/10] Mark fromMap/fromMapImmutable map param @Nonnull 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 --- internal-api/src/main/java/datadog/trace/api/TagMap.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index ed81edac466..761320a2205 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -53,14 +53,14 @@ public final class TagMap implements Map, Iterablemap */ - public static final TagMap fromMap(Map map) { + public static final TagMap fromMap(@Nonnull Map map) { TagMap tagMap = TagMap.create(map.size()); tagMap.putAll(map); return tagMap; } /** Creates a new immutable TagMap that contains the contents of map */ - public static final TagMap fromMapImmutable(Map map) { + public static final TagMap fromMapImmutable(@Nonnull Map map) { if (map.isEmpty()) { return TagMap.EMPTY; } else { From b58bcc00330ce2d4cf4bf62b4889371798a63b00 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 14:14:27 -0400 Subject: [PATCH 05/10] Add TagMap read-through support (level-split phase 1 mechanism) 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 --- .../main/java/datadog/trace/api/TagMap.java | 358 ++++++++++++++++-- .../trace/api/TagMapReadThroughTest.java | 316 ++++++++++++++++ 2 files changed, 641 insertions(+), 33 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 761320a2205..2f519b75311 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -6,6 +6,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; @@ -48,8 +49,7 @@ public final class TagMap implements Map, Iterable. + // reads no statics, so this is safe to build directly during TagMap's . public static final TagMap EMPTY = new TagMap(new Object[1], 0); /** Creates a new mutable TagMap that contains the contents of map */ @@ -76,6 +76,19 @@ public static final TagMap create(int size) { return new TagMap(); } + /** + * Creates a fresh, mutable TagMap that reads through to {@code parent} on local misses. The + * parent must be frozen and is fixed for the life of the returned map (no re-parenting), so + * read-through relies on a stable parent rather than an unenforced convention. Level-split phase + * 1. + */ + public static final TagMap createFromParent(TagMap parent) { + if (parent != null && !parent.frozen) { + throw new IllegalStateException("read-through parent must be frozen"); + } + return new TagMap(parent); + } + /** Creates a new TagMap.Ledger */ public static final Ledger ledger() { return new Ledger(); @@ -991,15 +1004,45 @@ public EntryChange next() { * However as a precaution if a BucketGroup becomes completely empty, then that BucketGroup will be * removed from the collision chain. */ + private final Object[] buckets; private int size; private boolean frozen; + /** + * Optional frozen parent for read-through (level-split phase 1). When non-null, reads that miss + * the local buckets fall through to the parent; a local entry shadows the parent's (local-wins). + * Phase 1 is single-parent by design (anti-false-generalization); generalizing to multiple + * flattened parents is additive. Must be frozen when attached, so it is safely shareable. + */ + private final TagMap parent; + + /** + * Parent keys removed locally (read-through tombstones). Lazily allocated on the first such + * removal; {@code null} both means "no tombstones" and serves as the gate that keeps the hot + * paths untouched. Only meaningful when {@link #parent} != null. A tombstone stops read-through + * fall-through for its key, so a key removed from a child no longer reads through to the parent. + * Kept off the bucket structure deliberately — it is shape-agnostic (bare-Entry vs BucketGroup) + * and rare, so it costs a lazy allocation on removal rather than complicating the hot bucket + * code. + */ + private Set removedFromParent; + public TagMap() { + this((TagMap) null); + } + + /** + * Fresh mutable map that reads through to {@code parent} (may be null). The parent is set once, + * here at construction, and is final — there is no re-parenting, so read-through optimizations + * can treat it as fixed. + */ + private TagMap(TagMap parent) { // needs to be a power of 2 for bucket masking calculation to work as intended this.buckets = new Object[1 << 4]; this.size = 0; this.frozen = false; + this.parent = parent; } /** Used for inexpensive immutable */ @@ -1007,6 +1050,7 @@ private TagMap(Object[] buckets, int size) { this.buckets = buckets; this.size = size; this.frozen = true; + this.parent = null; } public boolean isOptimized() { @@ -1015,12 +1059,63 @@ public boolean isOptimized() { @Override public int size() { - return this.size; + // Exact (Map contract). Under read-through resolves the union; prefer estimateSize() for hints. + TagMap parent = this.parent; + return parent == null ? this.size : this.size + this.visibleParentCount(); + } + + /** + * Exact count of parent entries not shadowed locally or tombstoned (the read-through addition). + */ + private int visibleParentCount() { + Object[] parentBuckets = this.parent.buckets; + Object[] thisBuckets = this.buckets; + int count = 0; + for (int i = 0; i < parentBuckets.length; ++i) { + Object parentBucket = parentBuckets[i]; + Object localBucket = thisBuckets[i]; + if (parentBucket instanceof Entry) { + if (parentEntryVisibleInBucket(localBucket, (Entry) parentBucket)) count++; + } else if (parentBucket instanceof BucketGroup) { + for (BucketGroup cuGroup = (BucketGroup) parentBucket; + cuGroup != null; + cuGroup = cuGroup.prev) { + for (int j = 0; j < BucketGroup.LEN; ++j) { + Entry parentEntry = cuGroup._entryAt(j); + if (parentEntry != null && parentEntryVisibleInBucket(localBucket, parentEntry)) + count++; + } + } + } + } + return count; } @Override public boolean isEmpty() { - return (this.size == 0); + // Exact (Map contract). Under read-through resolves the parent; prefer isDefinitelyEmpty(). + if (this.size != 0) { + return false; + } + TagMap parent = this.parent; + if (parent == null) { + return true; + } + if (this.removedFromParent == null) { + // no local entries and no tombstones -> empty iff the parent is empty (nothing shadows it) + return parent.isEmpty(); + } + // size == 0 with tombstones (rare): empty iff every parent entry is tombstoned + return this.visibleParentCount() == 0; + } + + public boolean isDefinitelyEmpty() { + return this.size == 0 && (this.parent == null || this.parent.isDefinitelyEmpty()); + } + + public int estimateSize() { + // Upper bound: local + parent, ignoring read-through shadowing/removals (over-counts). + return this.parent == null ? this.size : this.size + this.parent.estimateSize(); } @Deprecated @@ -1128,26 +1223,59 @@ public Set> entrySet() { } public Entry getEntry(String tag) { - Object[] thisBuckets = this.buckets; + Entry local = this.getLocalEntry(tag); + if (local != null) { + // Local entry shadows the parent (local-wins) — unchanged hot path. + return local; + } + // Read-through: miss locally, defer to the frozen parent. Single-parent in phase 1. + // The tombstone check lives only here, on the cold miss+parent path — the hot local hit above + // never touches it. + TagMap parent = this.parent; + if (parent == null) { + return null; + } + if (this.removedFromParent != null && this.removedFromParent.contains(tag)) { + return null; // tombstoned: removed locally, do not read through + } + return parent.getEntry(tag); + } + /** Looks up an entry in this map's own buckets only — no read-through to the parent. */ + private Entry getLocalEntry(String tag) { + Object[] thisBuckets = this.buckets; int hash = TagMap.Entry._hash(tag); - int bucketIndex = hash & (thisBuckets.length - 1); + return findInBucket(thisBuckets[hash & (thisBuckets.length - 1)], hash, tag); + } - Object bucket = thisBuckets[bucketIndex]; - if (bucket == null) { - return null; - } else if (bucket instanceof Entry) { + /** + * Finds an entry by hash/tag within a single bucket object (Entry | BucketGroup chain | null). + */ + private static Entry findInBucket(Object bucket, int hash, String tag) { + if (bucket instanceof Entry) { Entry tagEntry = (Entry) bucket; - if (tagEntry.matches(tag)) return tagEntry; + return tagEntry.matches(tag) ? tagEntry : null; } else if (bucket instanceof BucketGroup) { - BucketGroup lastGroup = (BucketGroup) bucket; - - Entry tagEntry = lastGroup.findInChain(hash, tag); - return tagEntry; + return ((BucketGroup) bucket).findInChain(hash, tag); } return null; } + /** + * Whether a parent entry is visible through this child at its (shared) bucket: not tombstoned and + * not shadowed by a local entry. Exploits universal hashing — by {@code _hash}, the only local + * entry that could shadow {@code parentEntry} lives in this map's same-index bucket, so we + * compare against {@code localBucket} alone, reusing {@code parentEntry}'s cached hash (no + * re-hash, no full-map probe). + */ + private boolean parentEntryVisibleInBucket(Object localBucket, Entry parentEntry) { + if (this.removedFromParent != null && this.removedFromParent.contains(parentEntry.tag)) { + return false; // tombstoned: removed locally + } + return findInBucket(localBucket, parentEntry.hash(), parentEntry.tag) + == null; // not shadowed by a local entry + } + @Deprecated @Override public Object put(@Nonnull String tag, Object value) { @@ -1204,6 +1332,12 @@ public Entry getAndSet(@Nullable Entry newEntry) { this.checkWriteAccess(); + // Re-setting a key clears any read-through tombstone for it (the new value overrides the + // removal). Gated on the lazy field, so this is a no-op for the common no-tombstone case. + if (this.removedFromParent != null) { + this.removedFromParent.remove(newEntry.tag); + } + Object[] thisBuckets = this.buckets; int newHash = newEntry.hash(); @@ -1295,10 +1429,11 @@ private void putAllUnoptimizedMap(Map that) } /** - * Similar to {@link Map#putAll(Map)} but optimized to quickly copy from one TagMap to another. + * Similar to {@link Map#putAll(Map)} but optimized to quickly copy from one TagMap to another * - *

    Takes advantage of the consistent TagMap layout to quickly handle each bucket. And similar - * to {@link TagMap#getAndSet(Entry)} this method shares Entry objects from the source TagMap. + *

    For optimized TagMaps, this method takes advantage of the consistent TagMap layout to + * quickly handle each bucket. And similar to {@link TagMap#getAndSet(Entry)} this method shares + * Entry objects from the source TagMap */ public void putAll(TagMap that) { this.checkWriteAccess(); @@ -1506,6 +1641,32 @@ public boolean remove(String tag) { public Entry getAndRemove(String tag) { this.checkWriteAccess(); + Entry localRemoved = this.removeLocal(tag); + + TagMap parent = this.parent; + if (parent != null) { + // Read-through: if the parent still exposes this key, removing it must also hide it from + // fall-through — install a tombstone. The prior *visible* value (Map.remove contract) is the + // local entry if there was one, otherwise the parent's (which we now hide). Single-parent in + // phase 1; rare path (only when removing a parent-exposed key). + boolean alreadyTombstoned = + this.removedFromParent != null && this.removedFromParent.contains(tag); + if (!alreadyTombstoned) { + Entry parentEntry = parent.getEntry(tag); + if (parentEntry != null) { + if (this.removedFromParent == null) { + this.removedFromParent = new HashSet<>(); + } + this.removedFromParent.add(tag); + return localRemoved != null ? localRemoved : parentEntry; + } + } + } + return localRemoved; + } + + /** Removes an entry from this map's own buckets only — no parent/tombstone handling. */ + private Entry removeLocal(String tag) { Object[] thisBuckets = this.buckets; int hash = TagMap.Entry._hash(tag); @@ -1543,8 +1704,14 @@ public Entry getAndRemove(String tag) { } public TagMap copy() { - TagMap copy = new TagMap(); + // Construct with the same (frozen, shared) parent up front — parent is final. putAll then + // clones this map's own (local) buckets + size. The copy stays independently mutable (writes + // land on its local buckets, never the shared parent). + TagMap copy = new TagMap(this.parent); copy.putAllIntoEmptyMap(this); + if (this.removedFromParent != null) { + copy.removedFromParent = new HashSet<>(this.removedFromParent); + } return copy; } @@ -1582,6 +1749,35 @@ public void forEach(Consumer consumer) { thisGroup.forEachInChain(consumer); } } + + // read-through: parent entries not shadowed locally or tombstoned. Kept out of line so the + // common parent == null path stays byte-identical to before (small / inlinable). + if (this.parent != null) { + this.forEachParent(consumer); + } + } + + private void forEachParent(Consumer consumer) { + Object[] localBuckets = this.buckets; + Object[] parentBuckets = this.parent.buckets; // leaf parent: same length, same bucket per key + for (int i = 0; i < parentBuckets.length; ++i) { + Object parentBucket = parentBuckets[i]; + Object localBucket = localBuckets[i]; + if (parentBucket instanceof Entry) { + Entry parentEntry = (Entry) parentBucket; + if (parentEntryVisibleInBucket(localBucket, parentEntry)) consumer.accept(parentEntry); + } else if (parentBucket instanceof BucketGroup) { + for (BucketGroup cuGroup = (BucketGroup) parentBucket; + cuGroup != null; + cuGroup = cuGroup.prev) { + for (int j = 0; j < BucketGroup.LEN; ++j) { + Entry parentEntry = cuGroup._entryAt(j); + if (parentEntry != null && parentEntryVisibleInBucket(localBucket, parentEntry)) + consumer.accept(parentEntry); + } + } + } + } } public void forEach(T thisObj, BiConsumer consumer) { @@ -1600,6 +1796,36 @@ public void forEach(T thisObj, BiConsumer con thisGroup.forEachInChain(thisObj, consumer); } } + + // read-through: parent entries not shadowed locally or tombstoned (kept out of line). + if (this.parent != null) { + this.forEachParent(thisObj, consumer); + } + } + + private void forEachParent(T thisObj, BiConsumer consumer) { + Object[] localBuckets = this.buckets; + Object[] parentBuckets = this.parent.buckets; // leaf parent: same length, same bucket per key + for (int i = 0; i < parentBuckets.length; ++i) { + Object parentBucket = parentBuckets[i]; + Object localBucket = localBuckets[i]; + if (parentBucket instanceof Entry) { + Entry parentEntry = (Entry) parentBucket; + if (parentEntryVisibleInBucket(localBucket, parentEntry)) + consumer.accept(thisObj, parentEntry); + } else if (parentBucket instanceof BucketGroup) { + for (BucketGroup cuGroup = (BucketGroup) parentBucket; + cuGroup != null; + cuGroup = cuGroup.prev) { + for (int j = 0; j < BucketGroup.LEN; ++j) { + Entry parentEntry = cuGroup._entryAt(j); + if (parentEntry != null && parentEntryVisibleInBucket(localBucket, parentEntry)) { + consumer.accept(thisObj, parentEntry); + } + } + } + } + } } public void forEach( @@ -1619,6 +1845,37 @@ public void forEach( thisGroup.forEachInChain(thisObj, otherObj, consumer); } } + + // read-through: parent entries not shadowed locally or tombstoned (kept out of line). + if (this.parent != null) { + this.forEachParent(thisObj, otherObj, consumer); + } + } + + private void forEachParent( + T thisObj, U otherObj, TriConsumer consumer) { + Object[] localBuckets = this.buckets; + Object[] parentBuckets = this.parent.buckets; // leaf parent: same length, same bucket per key + for (int i = 0; i < parentBuckets.length; ++i) { + Object parentBucket = parentBuckets[i]; + Object localBucket = localBuckets[i]; + if (parentBucket instanceof Entry) { + Entry parentEntry = (Entry) parentBucket; + if (parentEntryVisibleInBucket(localBucket, parentEntry)) + consumer.accept(thisObj, otherObj, parentEntry); + } else if (parentBucket instanceof BucketGroup) { + for (BucketGroup cuGroup = (BucketGroup) parentBucket; + cuGroup != null; + cuGroup = cuGroup.prev) { + for (int j = 0; j < BucketGroup.LEN; ++j) { + Entry parentEntry = cuGroup._entryAt(j); + if (parentEntry != null && parentEntryVisibleInBucket(localBucket, parentEntry)) { + consumer.accept(thisObj, otherObj, parentEntry); + } + } + } + } + } } public void clear() { @@ -1683,7 +1940,10 @@ void checkIntegrity() { if (this.size != this.computeSize()) { throw new IllegalStateException("incorrect size"); } - if (this.isEmpty() != this.checkIfEmpty()) { + // Local-structure invariant: the size counter's emptiness must match the local buckets. Uses + // the + // local (this.size == 0), NOT isEmpty(), which under read-through resolves the parent too. + if ((this.size == 0) != this.checkIfEmpty()) { throw new IllegalStateException("incorrect empty status"); } } @@ -1801,7 +2061,12 @@ String toInternalString() { } abstract static class IteratorBase { - private final Object[] buckets; + private final TagMap map; + private final Object[] localBuckets; + + // current array being walked: local buckets first, then the parent's (read-through union) + private Object[] buckets; + private boolean inParent = false; private Entry nextEntry; @@ -1811,18 +2076,16 @@ abstract static class IteratorBase { private int groupIndex = 0; IteratorBase(TagMap map) { + this.map = map; + this.localBuckets = map.buckets; this.buckets = map.buckets; } public final boolean hasNext() { if (this.nextEntry != null) return true; - while (this.bucketIndex < this.buckets.length) { - this.nextEntry = this.advance(); - if (this.nextEntry != null) return true; - } - - return false; + this.nextEntry = this.advance(); + return this.nextEntry != null; } final Entry nextEntryOrThrowNoSuchElement() { @@ -1850,6 +2113,35 @@ final Entry nextEntryOrNull() { } private final Entry advance() { + while (true) { + Entry tagEntry = this.rawAdvance(); + if (tagEntry != null) { + // local entries emit as-is; parent entries only if not shadowed locally or tombstoned. + // bucketIndex indexes the parent buckets here, which (universal hashing) line up with the + // same-index local bucket — so localBuckets[bucketIndex] is the bucket that could shadow. + if (!this.inParent + || this.map.parentEntryVisibleInBucket( + this.localBuckets[this.bucketIndex], tagEntry)) { + return tagEntry; + } + continue; // parent entry shadowed/tombstoned -> skip + } + + // current array exhausted; switch to the parent's buckets once (read-through union) + if (!this.inParent && this.map.parent != null) { + this.inParent = true; + this.buckets = this.map.parent.buckets; + this.bucketIndex = -1; + this.group = null; + this.groupIndex = 0; + continue; + } + return null; + } + } + + /** Next raw entry in the current bucket array, ignoring shadowing/tombstones. */ + private final Entry rawAdvance() { while (this.bucketIndex < this.buckets.length) { if (this.group != null) { for (++this.groupIndex; this.groupIndex < BucketGroup.LEN; ++this.groupIndex) { @@ -2367,12 +2659,12 @@ static final class Entries extends AbstractSet> { @Override public int size() { - return this.map.computeSize(); + return this.map.size(); } @Override public boolean isEmpty() { - return this.map.checkIfEmpty(); + return this.map.isEmpty(); } @Override @@ -2392,12 +2684,12 @@ static final class Keys extends AbstractSet { @Override public int size() { - return this.map.computeSize(); + return this.map.size(); } @Override public boolean isEmpty() { - return this.map.checkIfEmpty(); + return this.map.isEmpty(); } @Override @@ -2431,12 +2723,12 @@ static final class Values extends AbstractCollection { @Override public int size() { - return this.map.computeSize(); + return this.map.size(); } @Override public boolean isEmpty() { - return this.map.checkIfEmpty(); + return this.map.isEmpty(); } @Override diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java new file mode 100644 index 00000000000..aecf7fbce5b --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java @@ -0,0 +1,316 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; + +/** + * Read-through support, slice 1 (read path): a child {@link TagMap} with a frozen parent reads + * through to the parent on a local miss, while local entries shadow the parent (local-wins). + * Removal/tombstones and bulk (iteration/serialize) union come in later slices. + */ +class TagMapReadThroughTest { + + private static TagMap frozenParent() { + TagMap parent = (TagMap) TagMap.create(); + parent.set("a", "parent-a"); + parent.set("b", "parent-b"); + parent.freeze(); + return parent; + } + + @Test + void readsThroughToParentOnMiss() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("c", "child-c"); + + assertEquals("parent-a", child.getString("a")); // miss locally -> read through + assertEquals("parent-b", child.getString("b")); + assertEquals("child-c", child.getString("c")); // local + assertNull(child.getString("missing")); + assertTrue(child.containsKey("a")); + assertFalse(child.containsKey("missing")); + } + + @Test + void localEntryShadowsParent() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); // same key as parent + + assertEquals("child-b", child.getString("b")); // local wins + assertEquals("parent-a", child.getString("a")); // parent still visible + } + + @Test + void estimateSizeIsUpperBound() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); // shadows parent "b" + child.set("c", "child-c"); + + // true union = {a, b, c} = 3; estimate over-counts the shadowed "b": local 2 + parent 2 = 4 + assertEquals(4, child.estimateSize()); + assertTrue(child.estimateSize() >= 3, "estimateSize must be an upper bound on the true size"); + } + + @Test + void emptinessSemantics() { + TagMap emptyOverEmpty = (TagMap) TagMap.createFromParent((TagMap) TagMap.create().freeze()); + assertTrue(emptyOverEmpty.isEmpty()); + assertTrue(emptyOverEmpty.isDefinitelyEmpty()); + + TagMap emptyOverNonEmpty = (TagMap) TagMap.createFromParent(frozenParent()); + assertFalse(emptyOverNonEmpty.isEmpty(), "a non-empty parent makes the map non-empty"); + assertFalse(emptyOverNonEmpty.isDefinitelyEmpty()); + + assertTrue(((TagMap) TagMap.create()).isDefinitelyEmpty()); + } + + @Test + void parentMustBeFrozen() { + TagMap mutableParent = (TagMap) TagMap.create(); + assertThrows(IllegalStateException.class, () -> TagMap.createFromParent(mutableParent)); + } + + // --- slice 2: removal / tombstones --- + + @Test + void removingParentKeyHidesItFromChildButNotFromParent() { + TagMap parent = frozenParent(); + TagMap child = (TagMap) TagMap.createFromParent(parent); + + assertEquals("parent-a", child.getString("a")); // visible before removal + child.remove("a"); + + assertNull(child.getString("a")); // tombstoned: no longer reads through + assertFalse(child.containsKey("a")); + assertEquals("parent-b", child.getString("b")); // other parent keys unaffected + assertEquals("parent-a", parent.getString("a")); // frozen parent untouched + } + + @Test + void removeReturnsPriorVisibleValueViaParent() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + + // Map.remove contract: the key was present (via read-through), so removal reports it. + assertTrue(child.remove("a"), "removing a parent-exposed key should report it was present"); + assertNull(child.getString("a")); + } + + @Test + void reSettingARemovedKeyRestoresVisibility() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + + child.remove("a"); + assertNull(child.getString("a")); + + child.set("a", "child-a"); // re-set clears the tombstone + assertEquals("child-a", child.getString("a")); + } + + @Test + void removingAKeyThatIsBothLocalAndParentHidesBoth() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); // shadows parent "b" + + assertEquals("child-b", child.getString("b")); + child.remove("b"); + + assertNull(child.getString("b"), "removal must hide both the local entry and the parent's"); + assertEquals("parent-b", frozenParent().getString("b")); // parent still has it + } + + // --- slice 3a: bulk forEach union + exact size/isEmpty --- + + private static Map collect(TagMap map) { + Map out = new HashMap<>(); + map.forEach(e -> out.put(e.tag(), e.objectValue())); + return out; + } + + @Test + void forEachEmitsDedupedUnionLocalWins() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); // parent {a, b} + child.set("b", "child-b"); // shadows parent "b" + child.set("c", "child-c"); + + Map u = collect(child); + assertEquals(3, u.size(), "union {a, b, c} with b deduped"); + assertEquals("parent-a", u.get("a")); // read-through + assertEquals("child-b", u.get("b")); // local wins (no duplicate emit) + assertEquals("child-c", u.get("c")); + } + + @Test + void forEachSkipsTombstonedParentKeys() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("c", "child-c"); + child.remove("a"); // tombstone parent's "a" + + Map u = collect(child); + assertEquals(2, u.size()); + assertFalse(u.containsKey("a")); + assertEquals("parent-b", u.get("b")); + assertEquals("child-c", u.get("c")); + } + + @Test + void biConsumerForEachAlsoEmitsUnion() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("c", "child-c"); + + Map out = new HashMap<>(); + child.forEach(out, (m, e) -> m.put(e.tag(), e.objectValue())); // non-capturing: alloc-free path + assertEquals(3, out.size()); + assertEquals("parent-a", out.get("a")); + assertEquals("child-c", out.get("c")); + } + + @Test + void sizeIsExactUnion() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); // shadows + child.set("c", "child-c"); + assertEquals(3, child.size()); // {a, b, c} — b deduped, not 4 + + child.remove("a"); + assertEquals(2, child.size()); // {b, c} + } + + @Test + void isEmptyExactWhenAllParentKeysTombstonedAndNoLocal() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); // parent {a, b} + assertFalse(child.isEmpty()); + + child.remove("a"); + child.remove("b"); + assertTrue(child.isEmpty(), "all parent keys tombstoned and no local entries -> empty"); + assertEquals(0, child.size()); + } + + // --- slice 3b: pull-based iterators / collection views --- + + @Test + void iteratorEmitsDedupedUnion() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); // shadows parent "b" + child.set("c", "child-c"); + + Map u = new HashMap<>(); + Iterator it = child.iterator(); + while (it.hasNext()) { + TagMap.EntryReader e = it.next(); + u.put(e.tag(), e.objectValue()); + } + assertEquals(3, u.size()); + assertEquals("parent-a", u.get("a")); + assertEquals("child-b", u.get("b")); // local wins, emitted once + assertEquals("child-c", u.get("c")); + } + + @Test + void keySetReflectsUnionAndTombstones() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("c", "child-c"); + + Set keys = child.keySet(); + assertEquals(3, keys.size()); // a, b, c + assertTrue(keys.contains("a")); + assertTrue(keys.contains("c")); + + child.remove("a"); + assertEquals(2, child.keySet().size()); + assertFalse(child.keySet().contains("a")); + } + + @Test + void valuesAndEntrySetReflectUnion() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); // shadows parent "b" + + assertEquals(2, child.entrySet().size()); // {a, b} — b deduped + assertTrue(child.values().contains("child-b")); // local-won value + assertTrue(child.values().contains("parent-a")); + assertFalse(child.values().contains("parent-b"), "shadowed parent value must not appear"); + } + + // --- slice 4: behavior-identical to a copy-down / flat map --- + + @Test + void copyIsObservationallyIdentical() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); // {a, b} + child.set("b", "child-b"); // shadows parent "b" + child.set("c", "child-c"); + + TagMap copy = (TagMap) child.copy(); + assertEquals(child.size(), copy.size()); + assertEquals("parent-a", copy.getString("a")); // copy still reads through + assertEquals("child-b", copy.getString("b")); + assertEquals("child-c", copy.getString("c")); + assertEquals(collect(child), collect(copy)); // same union + } + + @Test + void copyIsIndependentlyMutable() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("c", "child-c"); + + TagMap copy = (TagMap) child.copy(); + copy.set("c", "copy-c"); // mutate copy's local + copy.remove("a"); // tombstone on copy only + + assertEquals("child-c", child.getString("c"), "original unaffected by copy mutation"); + assertEquals("parent-a", child.getString("a"), "original still reads through a"); + assertEquals("copy-c", copy.getString("c")); + assertNull(copy.getString("a")); + } + + @Test + void copyPreservesTombstones() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.remove("a"); // tombstone "a" + + TagMap copy = (TagMap) child.copy(); + assertNull(copy.getString("a"), "tombstone must carry into the copy"); + assertEquals("parent-b", copy.getString("b")); + } + + /** The contract that lets the consumer flip mergedTracerTags to a parent. */ + @Test + void readThroughMatchesAnEquivalentFlatMap() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("b", "child-b"); + child.set("c", "child-c"); + + TagMap flat = (TagMap) TagMap.create(); + flat.set("a", "parent-a"); + flat.set("b", "child-b"); + flat.set("c", "child-c"); + + assertEquals(flat.size(), child.size()); + assertEquals(collect(flat), collect(child)); + assertEquals(flat.keySet(), child.keySet()); + for (String k : new String[] {"a", "b", "c", "missing"}) { + assertEquals(flat.getString(k), child.getString(k), "mismatch for key " + k); + } + } + + @Test + void immutableCopyOfReadThroughIsFrozenAndStillReadsThrough() { + TagMap child = (TagMap) TagMap.createFromParent(frozenParent()); + child.set("c", "child-c"); + + TagMap frozen = (TagMap) child.immutableCopy(); + assertTrue(frozen.isFrozen()); + assertEquals("parent-a", frozen.getString("a")); // union preserved + assertEquals("child-c", frozen.getString("c")); + assertThrows(IllegalStateException.class, () -> frozen.set("x", "y")); // frozen blocks writes + } +} From 1f257aa1386a3bea5a6700834051d55658bcdfcd Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 14:22:44 -0400 Subject: [PATCH 06/10] Wire mergedTracerTags as a read-through parent at span build (level-split 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 --- .../java/datadog/trace/core/CoreTracer.java | 28 ++++++- .../datadog/trace/core/DDSpanContext.java | 68 ++++++++++++++- .../util/TagMapReadThroughBenchmark.java | 84 +++++++++++++++++++ 3 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 internal-api/src/jmh/java/datadog/trace/util/TagMapReadThroughBenchmark.java diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index d8b3fef89ce..0e005e10820 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -2206,18 +2206,31 @@ protected static final DDSpanContext buildSpanContext( propagationTags, tracer.profilingContextIntegration, tracer.injectBaggageAsTags, - tracer.injectLinksAsTags); + tracer.injectLinksAsTags, + mergedTracerTagsNeedsIntercept ? null : mergedTracerTags); // By setting the tags on the context we apply decorators to any tags that have been set via // the builder. This is the order that the tags were added previously, but maybe the `tags` // set in the builder should come last, so that they override other tags. - context.setAllTags(mergedTracerTags, mergedTracerTagsNeedsIntercept); + // + // mergedTracerTags is trace-level shared state and the precedence floor (everything below + // overrides it). When it carries no interceptable tags it is attached as a read-through + // PARENT at construction (shared by reference, no per-span copy). When it does need + // interception, copy its entries in (the interceptor's per-span side-effects can't be + // shared by reference). + if (mergedTracerTagsNeedsIntercept) { + context.setAllTags(mergedTracerTags, true); + } context.setAllTags(tagLedger); context.setAllTags(coreTags, coreTagsNeedsIntercept); context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); context.setAllTags(contextualTags); - // remove version here since will be done later on the postProcessor. - // it will allow knowing if it will be set manually or not + // Version is added later by the postProcessor (InternalTagsAdder), only if not already set + // during the request. Config version is kept out of the trace-level bundle (see + // withTracerTags), so this removal now only wipes a version set via the span builder — + // keeping + // the existing semantics where a builder-set version is replaced by the config version. Under + // read-through this is a cheap local removal (version isn't in the parent, so no tombstone). context.removeTag(Tags.VERSION); return context; } @@ -2448,6 +2461,13 @@ static TagMap withTracerTags( Map userSpanTags, Config config, TraceConfig traceConfig) { final TagMap result = TagMap.create(userSpanTags.size() + 5); result.putAll(userSpanTags); + // Version is conditionally managed by InternalTagsAdder (added only when service == DD_SERVICE + // and not set during the request), so keep it OUT of the trace-level bundle. This matters under + // read-through: the bundle becomes a shared parent, and a per-span removeTag(VERSION) on a key + // that lived in the parent would mint a per-span tombstone. With version excluded here, the + // per-span removeTag (retained, to wipe a builder-set version) is a cheap local op, never a + // tombstone. Behavior is unchanged: version was applied-then-removed at build today. + result.remove(Tags.VERSION); if (null != config) { // static if (!config.getEnv().isEmpty()) { result.set("env", config.getEnv()); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 6120502ec09..a2d87e2c18b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -243,7 +243,8 @@ public DDSpanContext( propagationTags, ProfilingContextIntegration.NoOp.INSTANCE, true, - true); + true, + null); } public DDSpanContext( @@ -293,9 +294,11 @@ public DDSpanContext( propagationTags, ProfilingContextIntegration.NoOp.INSTANCE, injectBaggageAsTags, - injectLinksAsTags); + injectLinksAsTags, + null); } + /** Back-compat ctor (no read-through parent); delegates with a null parent. */ public DDSpanContext( final DDTraceId traceId, final long spanId, @@ -322,6 +325,62 @@ public DDSpanContext( final ProfilingContextIntegration profilingContextIntegration, final boolean injectBaggageAsTags, final boolean injectLinksAsTags) { + this( + traceId, + spanId, + parentId, + parentServiceName, + serviceNameSource, + serviceName, + operationName, + resourceName, + samplingPriority, + origin, + baggageItems, + w3cBaggage, + errorFlag, + spanType, + tagsSize, + traceCollector, + requestContextDataAppSec, + requestContextDataIast, + CiVisibilityContextData, + pathwayContext, + disableSamplingMechanismValidation, + propagationTags, + profilingContextIntegration, + injectBaggageAsTags, + injectLinksAsTags, + null); + } + + public DDSpanContext( + final DDTraceId traceId, + final long spanId, + final long parentId, + final CharSequence parentServiceName, + final CharSequence serviceNameSource, + final String serviceName, + final CharSequence operationName, + final CharSequence resourceName, + final int samplingPriority, + final CharSequence origin, + final Map baggageItems, + final Baggage w3cBaggage, + final boolean errorFlag, + final CharSequence spanType, + final int tagsSize, + final TraceCollector traceCollector, + final Object requestContextDataAppSec, + final Object requestContextDataIast, + final Object CiVisibilityContextData, + final PathwayContext pathwayContext, + final boolean disableSamplingMechanismValidation, + final PropagationTags propagationTags, + final ProfilingContextIntegration profilingContextIntegration, + final boolean injectBaggageAsTags, + final boolean injectLinksAsTags, + final TagMap readThroughParent) { assert traceCollector != null; this.traceCollector = traceCollector; @@ -350,7 +409,10 @@ public DDSpanContext( // The +1 is the magic number from the tags below that we set at the end, // and "* 4 / 3" is to make sure that we don't resize immediately final int capacity = Math.max((tagsSize <= 0 ? 3 : (tagsSize + 1)) * 4 / 3, 8); - this.unsafeTags = TagMap.create(capacity); + this.unsafeTags = + readThroughParent != null + ? TagMap.createFromParent(readThroughParent) + : TagMap.create(capacity); // must set this before setting the service and resource names below this.profilingContextIntegration = profilingContextIntegration; diff --git a/internal-api/src/jmh/java/datadog/trace/util/TagMapReadThroughBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/TagMapReadThroughBenchmark.java new file mode 100644 index 00000000000..ed8768ef1cf --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/TagMapReadThroughBenchmark.java @@ -0,0 +1,84 @@ +package datadog.trace.util; + +import datadog.trace.api.TagMap; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Models span-build tag assembly with vs without read-through of the shared trace-level bundle. + * + *
      + *
    • copyDown — today's path: {@code putAll} the (frozen) trace-level bundle into the + * fresh span map, then set the span-specific tags. {@code putAll}-into-empty shares the + * frozen entry references (bucket-clone), so this does NOT allocate new Entry objects for the + * trace tags — its cost is cloned {@code BucketGroup}s plus the collisions caused by the + * trace tags sharing the local buckets with the span tags. + *
    • readThrough — attach the frozen bundle as a read-through parent; only the + * span-specific tags are stored locally. + *
    + * + *

    Run with {@code -prof gc}; the B/op delta is the per-span allocation read-through saves. Both + * arms set the same span tags, so the delta isolates the trace-bundle handling. {@code + * traceTagCount} sweeps the bundle size — the win scales with it (more trace tags → more cloned + * BucketGroups and local collisions avoided). {@code traceTagCount = 7} ≈ a realistic + * mergedTracerTags (env, version, language, runtime-id, a propagation tag, a couple global tags). + */ +@State(Scope.Benchmark) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 5, time = 2) +@Fork(3) +@Threads(8) +public class TagMapReadThroughBenchmark { + + @Param({"3", "7", "15"}) + int traceTagCount; + + private TagMap traceTags; + + @Setup(Level.Trial) + public void setup() { + TagMap m = TagMap.create(Math.max(16, traceTagCount * 2)); + for (int i = 0; i < traceTagCount; i++) { + m.set("_dd.trace.tag." + i, "trace-value-" + i); + } + this.traceTags = m.freeze(); + } + + @Benchmark + public TagMap copyDown() { + TagMap m = TagMap.create(16); + m.putAll(traceTags); // putAll-into-empty: shares frozen entries, clones BucketGroups + setSpanTags(m); + return m; + } + + @Benchmark + public TagMap readThrough() { + // no copy; trace tags read through the shared frozen parent (fixed at construction) + TagMap m = TagMap.createFromParent(traceTags); + setSpanTags(m); + return m; + } + + private static void setSpanTags(TagMap m) { + m.set("http.method", "GET"); + m.set("http.url", "/api/checkout/cart"); + m.set("component", "spring-web-controller"); + m.set("span.kind", "server"); + m.set("http.status_code", 200); + } +} From dcbc5ff9932429606d19ff7bf4692c29b06e40bd Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 15:41:32 -0400 Subject: [PATCH 07/10] Add StringIndex: a generic open-addressed string set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StringIndex is a compact open-addressed string→index structure (the keyOf substrate the dense tag store builds on): parallel hash/name arrays, linear probing, on par with HashSet on lookup at a smaller footprint. Includes unit tests, a footprint test (jol), and comparison benchmarks (vs HashSet/switch). No TagMap changes — standalone util. Rebased onto the level-split stack (consumer #11932) as the layer dense-store sits on. Co-Authored-By: Claude Opus 4.8 --- gradle/libs.versions.toml | 2 + internal-api/build.gradle.kts | 1 + .../trace/util/ImmutableMapBenchmark.java | 89 +++++- .../trace/util/ImmutableSetBenchmark.java | 84 ++++- .../util/StringIndexSwitchBenchmark.java | 299 ++++++++++++++++++ .../java/datadog/trace/util/StringIndex.java | 293 +++++++++++++++++ .../trace/util/StringIndexFootprintTest.java | 88 ++++++ .../datadog/trace/util/StringIndexTest.java | 171 ++++++++++ 8 files changed, 1012 insertions(+), 15 deletions(-) create mode 100644 internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java create mode 100644 internal-api/src/main/java/datadog/trace/util/StringIndex.java create mode 100644 internal-api/src/test/java/datadog/trace/util/StringIndexFootprintTest.java create mode 100644 internal-api/src/test/java/datadog/trace/util/StringIndexTest.java diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 3f9412b2042..fa09df00f5f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -42,6 +42,7 @@ jmh = "1.37" # Profiling jmc = "8.1.0" jafar = "0.16.0" +jol = "0.17" # Web & Network jnr-unixsocket = "0.38.25" @@ -125,6 +126,7 @@ instrument-java = { module = "com.datadoghq:dd-instrument-java", version.ref = " jmc-common = { module = "org.openjdk.jmc:common", version.ref = "jmc" } jmc-flightrecorder = { module = "org.openjdk.jmc:flightrecorder", version.ref = "jmc" } jafar-tools = { module = "io.btrace:jafar-tools", version.ref = "jafar" } +jol-core = { module = "org.openjdk.jol:jol-core", version.ref = "jol" } # Web & Network okio = { module = "com.datadoghq.okio:okio", version.ref = "okio" } diff --git a/internal-api/build.gradle.kts b/internal-api/build.gradle.kts index 65be1563eda..4d48a434c19 100644 --- a/internal-api/build.gradle.kts +++ b/internal-api/build.gradle.kts @@ -282,6 +282,7 @@ dependencies { testImplementation("org.junit.vintage:junit-vintage-engine:${libs.versions.junit5.get()}") testImplementation(libs.commons.math) testImplementation(libs.bundles.mockito) + testImplementation(libs.jol.core) } jmh { diff --git a/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java index e42a67ec9ea..46fabf7a312 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java @@ -33,11 +33,55 @@ * 10+, falls back to the input map pre-10). {@code Map.copyOf}/{@code MapN} is the honest * immutable-map baseline, not {@code HashMap}. * + *

    Also compared: {@link StringIndex} used as a string->int map — an open-addressed index plus + * a slot-aligned {@code int[]} of values ({@code SI_VALUES[indexOf(key)]}). {@code + * stringIndex_get*} goes through the instance wrapper; {@code support_get*} reads via {@code static + * final} arrays (the JIT folds the refs). No {@code iterate} arm — StringIndex is a lookup index, + * not an iteration structure; its map use case is the {@code indexOf}->parallel-array read. + * *

    Lookups use {@code EQUAL_KEYS} (distinct String instances) to exercise {@code equals()}; * {@code *_sameKey} variants reuse the original interned key instances to show the identity fast * path — which is the common tracer case, since map keys are typically interned tag-name constants. - * (Results pending a fresh multi-JVM run — {@code Map.copyOf} only materializes the compact form on - * Java 10+.) + * + *

    JDK 17 results (Apple M1, quiet machine, {@code @Fork(5)}, {@code @Threads(8)}; M ops/s). + * {@code get} uses distinct keys (exercises {@code equals()}); {@code sameKey} reuses the interned + * key (the {@code ==} fast path — the common tracer case): + * + * 

    {@code
+ * Structure              get    sameKey
+ * support (static)      1498     2081    (fastest)
+ * stringIndex (inst)    1363     1900
+ * hashMap               1216     1850
+ * linkedHashMap         1214       -
+ * tagMap                1167     1386
+ * tracerImmutableMap    1049     1364    (MapN)
+ * treeMap                656       -
+ * }
    + * + *

    {@code iterate} (full traversal): + * + * 

    {@code
+ * tagMap.forEach        148    (fastest)
+ * linkedHashMap         136
+ * tracerImmutableMap    135    (MapN)
+ * treeMap               134
+ * hashMap               104
+ * tagMap (iterator)      96
+ * }
    + * + *

    Key findings: + * + *

      + *
    • StringIndex-as-map ({@code Support}) is the fastest {@code get} — beating {@code HashMap} + * and {@code Map.copyOf}/{@code MapN}, most on the interned path; the instance wrapper trails + * it by ~10%. (vs {@code MapN} the edge is speed + the slot/parallel-array capability, not + * footprint — see {@link ImmutableSetBenchmark}.) + *
    • {@code TagMap.forEach} (148) beats its own {@code iterator} (96) by ~1.5x: TagMap's + * structure makes a faithful external {@code Iterator} expensive (externalized cursor + + * skip-empty + per-call re-entry + the iterator allocation) — all of which internal {@code + * forEach} avoids. Traverse TagMap via {@code forEach}, never its iterator; that gap only + * widens as TagMap's entry model grows. + *
    */ // @Fork(5): get_tracerImmutableMap* (MapN reached via interface dispatch) is JIT-bimodal at fewer // forks — 5 @@ -71,12 +115,31 @@ static void fill(Map map) { } } + // StringIndex as a string->int map: an open-addressed index plus a slot-aligned int[] of values + // (VALUES[indexOf(key)]). support_* reads via static final arrays (JIT folds the refs to + // constants); stringIndex_* goes through the instance wrapper. Both share one placement -- + // StringIndex.of and Support.create place identically -- so SI_VALUES aligns with either. + static final int[] SI_HASHES; + static final String[] SI_NAMES; + static final int[] SI_VALUES; + + static { + StringIndex.Data data = StringIndex.Support.create(INSERTION_KEYS); + SI_HASHES = data.hashes; + SI_NAMES = data.names; + SI_VALUES = new int[SI_HASHES.length]; + for (int i = 0; i < INSERTION_KEYS.length; ++i) { + SI_VALUES[StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, INSERTION_KEYS[i])] = i; + } + } + // Built once, never mutated -- safe to share across the reader threads. HashMap hashMap; LinkedHashMap linkedHashMap; TreeMap treeMap; TagMap tagMap; Map tracerImmutableMap; + StringIndex stringIndex; @Setup(Level.Trial) public void setUp() { @@ -92,6 +155,7 @@ public void setUp() { } // JDK compact immutable map (MapN on Java 10+); the agent's actual fixed-map representation. tracerImmutableMap = CollectionUtils.tryMakeImmutableMap(hashMap); + stringIndex = StringIndex.of(INSERTION_KEYS); } /** Per-thread lookup cursor so each reader thread cycles keys independently. */ @@ -199,4 +263,25 @@ public void iterate_tracerImmutableMap(Blackhole blackhole) { blackhole.consume(entry.getValue()); } } + + @Benchmark + public int stringIndex_get(Cursor cursor) { + return SI_VALUES[stringIndex.indexOf(cursor.nextKey())]; + } + + @Benchmark + public int stringIndex_get_sameKey(Cursor cursor) { + return SI_VALUES[stringIndex.indexOf(cursor.nextKey(INSERTION_KEYS))]; + } + + @Benchmark + public int support_get(Cursor cursor) { + return SI_VALUES[StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextKey())]; + } + + @Benchmark + public int support_get_sameKey(Cursor cursor) { + return SI_VALUES[ + StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextKey(INSERTION_KEYS))]; + } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java index 54b27604f3d..823fd9f883c 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java @@ -35,34 +35,57 @@ * ({@code ImmutableCollections.SetN}), which is what the agent actually uses for fixed config * sets. Java 10+; falls back to {@code HashSet} pre-10. The realistic baseline for any * flat/immutable set comparison. + *
  • {@code stringIndex} — {@link StringIndex#contains} on the instance wrapper (one field load + * to reach the placed arrays, then an open-addressed probe). + *
  • {@code support} — the same probe via {@link StringIndex.Support#indexOf} over {@code static + * final} arrays, so the JIT folds the refs to constants and there is nothing to dereference + * (the hot path StringIndex recommends). The {@code stringIndex}/{@code support} pair shows + * the indirection cost of the wrapper. * * *

    Lookups are interned (the {@code ==} fast path where a structure has one); misses are short * and never present. * - *

    Java 17 results (Apple M1, {@code @Fork(2)}, {@code @Threads(8)}; M ops/s = millions): + *

    JDK 17 results (Apple M1, quiet machine, {@code @Fork(5)}, {@code @Threads(8)}; M ops/s = + * millions): * * 

    {@code
  * Structure              hit     miss
- * hashSet               2159     1751    (fastest)
- * tracerImmutableSet    1946     1633    (Set.copyOf / SetN)
- * array                  926      584
- * sortedArray            664      588
- * treeSet                642      593
+ * support (static)      2320     2159    (fastest)
+ * hashSet               2198     2134
+ * stringIndex (inst)    2098     1548 *  (* miss bimodal -- see caveat)
+ * tracerImmutableSet    1914     1663    (Set.copyOf / SetN)
+ * array                  941      589
+ * sortedArray            685      610
+ * treeSet                657      610
  * }
    * *

    Key findings: * *

      - *
    • {@code HashSet} is fastest; {@link java.util.Set#copyOf} ({@code SetN}) trails by only ~10% - * on hit and ~7% on miss — and it's the compact, array-backed form the agent already uses for - * fixed config sets, so it's a strong default when the set is immutable. - *
    • {@code array} / {@code sortedArray} / {@code treeSet} cluster at ~0.6–0.9B — they scan, - * binary-search, or tree-walk per lookup, so they trail the hashed structures, most visibly - * on the miss path. + *
    • The static {@code Support} path is the fastest — it beats {@code HashSet} on hit and miss + * and crushes the scan/search/tree forms. + *
    • {@code stringIndex} (the instance wrapper) trails {@code Support} by the field-load + * indirection (~10% on hit), landing near {@code HashSet} — fine off the hot path, prefer + * {@code Support} on it. + *
    • {@link java.util.Set#copyOf} ({@code SetN}, the agent's compact fixed-set form) is ~1.2x + * behind {@code Support} on hit but the most compact (~27% smaller — no cached hashes, + * no 2x table). So StringIndex's edge over {@code SetN} is speed + the {@code + * indexOf}->parallel-array capability, not footprint; over {@code HashSet} it wins both. + *
    • {@code array} / {@code sortedArray} / {@code treeSet} trail the hashed structures, most on + * miss. *
    + * + *

    Caveat — the instance {@code stringIndex} miss is bimodal across forks (confirmed at + * {@code @Fork(10)}: 6 forks fast, 4 slow, nothing between). ~60% of forks compile to a fast mode + * (~2000, ≈ {@code support_miss} — the wrapper indirection is then free) and ~40% to a slow mode + * (~1070, ~half); each fork locks one at warmup. So the {@code 1548 ±27%} above is a mode-mix, not + * noise. Cause: C2 hoists the instance field-loads ({@code this.hashes}/{@code names}) out of the + * miss-path probe loop only in the fast mode; the static {@code Support} path const-folds those + * refs and is never bimodal ({@code support_miss} ±0.3%). Prefer {@code Support} where miss latency + * matters. */ -@Fork(2) +@Fork(5) // 5 forks settle the bimodal stringIndex_miss / interface-dispatch arms (see header) @Warmup(iterations = 2) @Measurement(iterations = 3) @Threads(8) @@ -84,12 +107,26 @@ static String[] newMisses() { return misses; } + // StringIndex static-Support mode: the placed arrays pulled into static final fields, so the JIT + // folds the refs to constants and Support.indexOf has nothing to dereference (the hot path the + // StringIndex class Javadoc recommends). Contrast support_* (these) with stringIndex_* (the + // instance wrapper, one field load) to see the indirection cost. + static final int[] SI_HASHES; + static final String[] SI_NAMES; + + static { + StringIndex.Data data = StringIndex.Support.create(STRINGS); + SI_HASHES = data.hashes; + SI_NAMES = data.names; + } + // Built once, never mutated -- safe to share across the reader threads. String[] array; String[] sortedArray; HashSet hashSet; TreeSet treeSet; Set tracerImmutableSet; + StringIndex stringIndex; @Setup(Level.Trial) public void setUp() { @@ -99,6 +136,7 @@ public void setUp() { hashSet = new HashSet<>(Arrays.asList(STRINGS)); treeSet = new TreeSet<>(Arrays.asList(STRINGS)); tracerImmutableSet = CollectionUtils.tryMakeImmutableSet(Arrays.asList(STRINGS)); + stringIndex = StringIndex.of(STRINGS); } /** Per-thread lookup cursor so each reader thread cycles keys independently. */ @@ -184,4 +222,24 @@ public boolean tracerImmutableSet_hit(Cursor cursor) { public boolean tracerImmutableSet_miss(Cursor cursor) { return tracerImmutableSet.contains(cursor.nextMiss()); } + + @Benchmark + public boolean stringIndex_hit(Cursor cursor) { + return stringIndex.contains(cursor.nextHit()); + } + + @Benchmark + public boolean stringIndex_miss(Cursor cursor) { + return stringIndex.contains(cursor.nextMiss()); + } + + @Benchmark + public boolean support_hit(Cursor cursor) { + return StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextHit()) >= 0; + } + + @Benchmark + public boolean support_miss(Cursor cursor) { + return StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextMiss()) >= 0; + } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java new file mode 100644 index 00000000000..916cfe70059 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java @@ -0,0 +1,299 @@ +package datadog.trace.util; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.CompilerControl; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * The third {@link StringIndex} use case: replacing a {@code switch} over interned {@code String} + * literals that maps a key to a small {@code int} id (exactly what {@code TagInterceptor} does to + * decide whether/how to intercept a tag). Both forms resolve a key to an id, 0 == "not found". + * + *

    Compared: + * + *

      + *
    • {@code switch} — a hand-written {@code switch(key)} over the literals ({@code hashCode} + * switch + {@code equals}), {@code default} returns 0. + *
    • {@code stringIndex} — {@code IDS[Support.indexOf(HASHES, NAMES, key)]} over {@code static + * final} arrays (a miss returns 0), the folded-constant hot path. + *
    + * + *

    What this measures: two axes. A prior investigation found the {@code TagInterceptor} + * switch wasn't being inlined / specialized into its hot caller. So each form is measured across + * (a) inlining — {@code _inlined} vs {@code _noinline} (a real call, {@code TagInterceptor}'s + * actual regime) via {@link CompilerControl} — and (b) key shape — a constant key vs a runtime, + * varied key. The results (below) land the teaching point: the dominant axis is + * key-constancy, not inlining. At steady state the inline-vs-not gap is small for both + * forms; what sinks the switch is a runtime, varied key (it can't specialize), while the + * StringIndex {@code Support} path stays flat across both axes — so the win is largest exactly + * where {@code TagInterceptor} lives. + * + *

    The {@code _inlined} and {@code _noinline} helpers carry duplicate bodies on purpose: that's + * the only way to pin each form's inlining decision independently. + * + *

    {@code @Threads(8)}; read-only, so no store dilutes the signal. Hit keys are the interned + * literals (the {@code ==} fast path StringIndex and the switch both get); misses are distinct and + * never present. Run via {@code -Pjmh.includes=StringIndexSwitchBenchmark} (add {@code -prof gc} — + * should be ~0 B/op both ways; this proves throughput, not allocation). + * + *

    JDK 17 results (Apple M1, quiet machine, {@code @Fork(5)}, {@code @Threads(8)}; M ops/s, + * ±1–5%): + * + * 

    {@code
+ * key             switch (inl / noinl)   stringIndex (inl / noinl)
+ * const            2778 / 2769            2047 / 2035
+ * hit  (runtime)   1161 / 1166            2147 / 2152
+ * miss             2083 / 2050            2546 / 2539
+ * }
    + * + *

    Two takeaways: + * + *

      + *
    • The string switch only matches StringIndex in the constant-key corner + * (~2.7B): there the JIT specializes the switch to the single known key — and the {@code + * const} arms show it does so even across a {@code DONT_INLINE} boundary (profile-driven, not + * const-prop-through-inline). Production tags are runtime-varied, so that corner never + * occurs. + *
    • In the realistic regime — a runtime, varied hit key, exactly {@code TagInterceptor} + * — the switch falls to ~1.16B while StringIndex holds ~2.15B (~1.85x). StringIndex is + * flat (~2.0–2.5B) across inline/not-inline and key shape: its throughput doesn't + * depend on the JIT's inlining decisions, which is the whole point. (Misses short-circuit for + * both; StringIndex still ~1.2x.) + *
    + * + *

    So the {@code const} arm is the control: it exposes the switch's "fast" as a single-key + * specialization artifact — drop the constant and the switch is ~half StringIndex's throughput. + */ +@Fork(5) // matches the documented @Fork(5) numbers; the switch's const-key arm is profile-bimodal +@Warmup(iterations = 2) +@Measurement(iterations = 3) +@Threads(8) +@State(Scope.Benchmark) +public class StringIndexSwitchBenchmark { + static final String[] KEYS = { + "alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel", + "india", "juliet", "kilo", "lima", "mike", "november", "oscar", "papa" + }; + + // A compile-time-constant hit key. javac inlines it, so the JIT can constant-propagate it into an + // inlined switch and fold the whole switch away -- the switch's theoretical ceiling. The const_* + // arms pair this with INLINE vs DONT_INLINE to show that ceiling only materializes when the call + // ALSO inlines: across a DONT_INLINE boundary the constant can't propagate in, so the switch runs + // in full. TagInterceptor's real regime is a runtime tag through a non-inlined call -- neither + // holds -- which is why StringIndex wins where it counts. + static final String CONST_KEY = "mike"; + + /** Distinct String instances that are never present, for the miss path. */ + static final String[] MISSES = newMisses(); + + static String[] newMisses() { + String[] misses = new String[KEYS.length * 2]; + for (int i = 0; i < misses.length; ++i) { + misses[i] = "dne-" + i; + } + return misses; + } + + // StringIndex placed arrays + slot-aligned ids, pulled into static final fields so the JIT folds + // the refs to constants (the hot path StringIndex recommends). IDS[slot] is the 1-based id; + // empty slots stay 0, which doubles as the "not found" sentinel. + static final int[] HASHES; + static final String[] NAMES; + static final int[] IDS; + + static { + StringIndex.Data data = StringIndex.Support.create(KEYS); + HASHES = data.hashes; + NAMES = data.names; + IDS = new int[HASHES.length]; + for (int i = 0; i < KEYS.length; ++i) { + IDS[StringIndex.Support.indexOf(HASHES, NAMES, KEYS[i])] = i + 1; // 1-based; 0 = not found + } + } + + /** Per-thread cursors so threads don't contend on a shared index under {@code @Threads(8)}. */ + @State(Scope.Thread) + public static class Cursor { + int hit = 0; + int miss = 0; + + String nextHit() { + int i = hit + 1; + if (i >= KEYS.length) { + i = 0; + } + hit = i; + return KEYS[i]; + } + + String nextMiss() { + int i = miss + 1; + if (i >= MISSES.length) { + i = 0; + } + miss = i; + return MISSES[i]; + } + } + + @CompilerControl(CompilerControl.Mode.INLINE) + static int switchInline(String key) { + switch (key) { + case "alpha": + return 1; + case "bravo": + return 2; + case "charlie": + return 3; + case "delta": + return 4; + case "echo": + return 5; + case "foxtrot": + return 6; + case "golf": + return 7; + case "hotel": + return 8; + case "india": + return 9; + case "juliet": + return 10; + case "kilo": + return 11; + case "lima": + return 12; + case "mike": + return 13; + case "november": + return 14; + case "oscar": + return 15; + case "papa": + return 16; + default: + return 0; + } + } + + // Duplicate body, pinned non-inlinable -- TagInterceptor's actual call regime. + @CompilerControl(CompilerControl.Mode.DONT_INLINE) + static int switchNoInline(String key) { + switch (key) { + case "alpha": + return 1; + case "bravo": + return 2; + case "charlie": + return 3; + case "delta": + return 4; + case "echo": + return 5; + case "foxtrot": + return 6; + case "golf": + return 7; + case "hotel": + return 8; + case "india": + return 9; + case "juliet": + return 10; + case "kilo": + return 11; + case "lima": + return 12; + case "mike": + return 13; + case "november": + return 14; + case "oscar": + return 15; + case "papa": + return 16; + default: + return 0; + } + } + + @CompilerControl(CompilerControl.Mode.INLINE) + static int indexInline(String key) { + int slot = StringIndex.Support.indexOf(HASHES, NAMES, key); + return slot >= 0 ? IDS[slot] : 0; + } + + @CompilerControl(CompilerControl.Mode.DONT_INLINE) + static int indexNoInline(String key) { + int slot = StringIndex.Support.indexOf(HASHES, NAMES, key); + return slot >= 0 ? IDS[slot] : 0; + } + + @Benchmark + public int switch_hit_inlined(Cursor cursor) { + return switchInline(cursor.nextHit()); + } + + @Benchmark + public int switch_miss_inlined(Cursor cursor) { + return switchInline(cursor.nextMiss()); + } + + @Benchmark + public int switch_hit_noinline(Cursor cursor) { + return switchNoInline(cursor.nextHit()); + } + + @Benchmark + public int switch_miss_noinline(Cursor cursor) { + return switchNoInline(cursor.nextMiss()); + } + + @Benchmark + public int stringIndex_hit_inlined(Cursor cursor) { + return indexInline(cursor.nextHit()); + } + + @Benchmark + public int stringIndex_miss_inlined(Cursor cursor) { + return indexInline(cursor.nextMiss()); + } + + @Benchmark + public int stringIndex_hit_noinline(Cursor cursor) { + return indexNoInline(cursor.nextHit()); + } + + @Benchmark + public int stringIndex_miss_noinline(Cursor cursor) { + return indexNoInline(cursor.nextMiss()); + } + + // --- constant key: the switch's best case (const-propagated). Inlined -> folds away; not-inlined + // -> the constant can't cross the boundary, so the switch runs in full. --- + + @Benchmark + public int switch_const_inlined() { + return switchInline(CONST_KEY); + } + + @Benchmark + public int switch_const_noinline() { + return switchNoInline(CONST_KEY); + } + + @Benchmark + public int stringIndex_const_inlined() { + return indexInline(CONST_KEY); + } + + @Benchmark + public int stringIndex_const_noinline() { + return indexNoInline(CONST_KEY); + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/StringIndex.java b/internal-api/src/main/java/datadog/trace/util/StringIndex.java new file mode 100644 index 00000000000..c5da0379cce --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/StringIndex.java @@ -0,0 +1,293 @@ +package datadog.trace.util; + +import java.lang.reflect.Array; +import java.util.function.Function; +import java.util.function.ToIntFunction; +import java.util.function.ToLongFunction; + +/** + * Flat open-addressed name set. Generic — it knows only names. + * + *

    Two ways to use it, trading convenience for indirection: + * + *

      + *
    • {@link Support} — static algorithm over raw arrays. Keep the arrays in your own + * (ideally {@code static final}) fields and the JIT folds the refs to constants. The fastest + * path; nothing to dereference. + *
    • The {@code StringIndex} instance ({@link #of}) — a convenience wrapper holding the + * arrays; {@link #indexOf}/{@link #contains} delegate to {@link Support}. Costs an + * instance-field load per call (the indirection the static path removes) — fine off the hot + * path. + *
    + * + *

    Consumers attach their own parallel payload arrays (ids, values, ...) sized to {@link + * #numSlots()} and indexed by the slot {@code indexOf} returns. {@code mapValues}/{@code + * mapIntValues}/{@code mapLongValues} build such an array at construction; {@code lookup}/{@code + * lookupOrDefault} read one back in a single call (slot resolve + array read). + * + *

    Slot 0-value is the empty sentinel: {@link Support#hash} never returns 0, so {@code hashes[i] + * == 0} unambiguously means an empty slot. + * + *

    Trades memory for simplicity (and, incidentally, speed). The table is 2x-oversized (load + * factor ≤ 0.5) so build-time placement always finds a free slot and never has to rehash or + * resize — short probe chains are a welcome side effect, not the design goal. The cached {@code + * int[]} hashes gate {@code equals()}. Both cost memory, so a tightly-packed set is more compact: + * prefer {@link java.util.Set#copyOf} (the JDK's {@code SetN}) when you only need membership, and + * reach for {@code StringIndex} for the {@code indexOf}->parallel-array (name→id) + * capability or the hot, allocation-free static {@link Support} path. (If footprint ever matters + * more than build simplicity, a higher load factor with construction-time rehashing would close the + * gap.) + */ +public final class StringIndex { + private final int[] hashes; + private final String[] names; + + private StringIndex(int[] hashes, String[] names) { + this.hashes = hashes; + this.names = names; + } + + /** + * Convenience instance — wraps the placed arrays. For the hot path prefer raw {@link Support}. + */ + public static StringIndex of(String... names) { + Data data = Support.create(names); + return new StringIndex(data.hashes, data.names); + } + + /** Slot of {@code name}, or -1. Delegates to {@link Support} on the instance's arrays. */ + public int indexOf(String name) { + return Support.indexOf(this.hashes, this.names, name); + } + + public boolean contains(String name) { + return indexOf(name) >= 0; + } + + /** Table size — allocate parallel payload arrays of this length. */ + public int numSlots() { + return hashes.length; + } + + // --- value mapping: build a slot-aligned parallel array (off the hot path) --- + + /** + * Builds a slot-aligned {@code T[]} of values: {@code out[indexOf(name)] == fn.apply(name)} for + * every indexed name; other slots stay {@code null}. {@code type} is the array element type (Java + * can't allocate a generic array without it). Pair with {@link #lookup(Object[], String)}. + */ + public T[] mapValues(Class type, Function fn) { + return Support.mapValues(this.names, type, fn); + } + + /** Slot-aligned {@code int[]} of values; absent slots stay 0. See {@link #mapValues}. */ + public int[] mapIntValues(ToIntFunction fn) { + return Support.mapIntValues(this.names, fn); + } + + /** Slot-aligned {@code long[]} of values; absent slots stay 0. See {@link #mapValues}. */ + public long[] mapLongValues(ToLongFunction fn) { + return Support.mapLongValues(this.names, fn); + } + + // --- lookup: resolve a key and read its parallel value in one call --- + + /** {@code data[indexOf(key)]}, or {@code null} when {@code key} is absent. */ + public T lookup(T[] data, String key) { + return Support.lookup(this.hashes, this.names, data, key); + } + + /** {@code data[indexOf(key)]}, or {@code defaultValue} when {@code key} is absent. */ + public T lookupOrDefault(T[] data, String key, T defaultValue) { + return Support.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); + } + + /** {@code data[indexOf(key)]}, or 0 when {@code key} is absent. */ + public int lookup(int[] data, String key) { + return Support.lookup(this.hashes, this.names, data, key); + } + + /** {@code data[indexOf(key)]}, or {@code defaultValue} when {@code key} is absent. */ + public int lookupOrDefault(int[] data, String key, int defaultValue) { + return Support.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); + } + + /** {@code data[indexOf(key)]}, or 0 when {@code key} is absent. */ + public long lookup(long[] data, String key) { + return Support.lookup(this.hashes, this.names, data, key); + } + + /** {@code data[indexOf(key)]}, or {@code defaultValue} when {@code key} is absent. */ + public long lookupOrDefault(long[] data, String key, long defaultValue) { + return Support.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); + } + + /** Build-time carrier. Pull the fields into your own (static final) fields; don't keep this. */ + public static final class Data { + public final int[] hashes; + public final String[] names; + + Data(int[] hashes, String[] names) { + this.hashes = hashes; + this.names = names; + } + } + + /** + * Static algorithm over raw arrays. Query helpers take raw arrays, never a Data or a StringIndex. + */ + public static final class Support { + private Support() {} + + /** Spread of String.hashCode; 0 reserved as the empty sentinel. */ + public static int hash(String name) { + int h = name.hashCode(); // cached on String -> field load + return h == 0 ? 0xDD06 : h ^ (h >>> 16); + } + + /** Power-of-two size, 2x-oversized so load factor stays <= 0.5. */ + public static int tableSizeFor(int n) { + int size = 1; + while (size <= n) { + size <<= 1; + } + return size << 1; + } + + /** Build the placed table. Returns a Data carrier; pull its arrays into your own fields. */ + public static Data create(String... names) { + int size = tableSizeFor(names.length); + int[] hashes = new int[size]; + String[] placed = new String[size]; + for (String name : names) { + put(hashes, placed, name, hash(name)); + } + return new Data(hashes, placed); + } + + /** + * Slot-aligned {@code T[]} over placed {@code names}: {@code out[slot] = fn(name)} per name, + * {@code null} elsewhere. {@code type} is the array element type (generic-array allocation). + */ + @SuppressWarnings("unchecked") + public static T[] mapValues(String[] names, Class type, Function fn) { + T[] out = (T[]) Array.newInstance(type, names.length); + for (int slot = 0; slot < names.length; slot++) { + String name = names[slot]; + if (name != null) { + out[slot] = fn.apply(name); + } + } + return out; + } + + /** + * Slot-aligned {@code int[]} over placed {@code names}; {@code out[slot] = fn(name)}, 0 else. + */ + public static int[] mapIntValues(String[] names, ToIntFunction fn) { + int[] out = new int[names.length]; + for (int slot = 0; slot < names.length; slot++) { + String name = names[slot]; + if (name != null) { + out[slot] = fn.applyAsInt(name); + } + } + return out; + } + + /** + * Slot-aligned {@code long[]} over placed {@code names}; {@code out[slot] = fn(name)}, 0 else. + */ + public static long[] mapLongValues(String[] names, ToLongFunction fn) { + long[] out = new long[names.length]; + for (int slot = 0; slot < names.length; slot++) { + String name = names[slot]; + if (name != null) { + out[slot] = fn.applyAsLong(name); + } + } + return out; + } + + /** Build-time placement. Returns the slot. */ + public static int put(int[] hashes, String[] names, String name, int h) { + final int mask = hashes.length - 1; + int i = h & mask; + for (int probes = 0; probes <= mask; probes++, i = (i + 1) & mask) { + if (hashes[i] == 0) { + hashes[i] = h; + names[i] = name; + return i; + } + if (hashes[i] == h && names[i].equals(name)) { + return i; // already present + } + } + throw new IllegalStateException("table full"); // impossible at LF <= 0.5 + } + + /** Probe; returns the slot or -1. Raw arrays — no Data, no instance. */ + public static int indexOf(int[] hashes, String[] names, String name, int h) { + final int mask = hashes.length - 1; + int i = h & mask; + for (int probes = 0; probes <= mask; probes++, i = (i + 1) & mask) { + int sh = hashes[i]; + if (sh == 0) { + return -1; + } + if (sh == h && names[i].equals(name)) { + return i; + } + } + return -1; + } + + public static int indexOf(int[] hashes, String[] names, String name) { + return indexOf(hashes, names, name, hash(name)); + } + + /** Number of slots — the length to size parallel payload arrays to. */ + public static int numSlots(int[] hashes) { + return hashes.length; + } + + /** {@code data[indexOf(...)]}, or {@code null} when {@code key} is absent. */ + public static T lookup(int[] hashes, String[] names, T[] data, String key) { + int slot = indexOf(hashes, names, key); + return slot >= 0 ? data[slot] : null; + } + + /** {@code data[indexOf(...)]}, or {@code defaultValue} when {@code key} is absent. */ + public static T lookupOrDefault( + int[] hashes, String[] names, T[] data, String key, T defaultValue) { + int slot = indexOf(hashes, names, key); + return slot >= 0 ? data[slot] : defaultValue; + } + + /** {@code data[indexOf(...)]}, or 0 when {@code key} is absent. */ + public static int lookup(int[] hashes, String[] names, int[] data, String key) { + int slot = indexOf(hashes, names, key); + return slot >= 0 ? data[slot] : 0; + } + + /** {@code data[indexOf(...)]}, or {@code defaultValue} when {@code key} is absent. */ + public static int lookupOrDefault( + int[] hashes, String[] names, int[] data, String key, int defaultValue) { + int slot = indexOf(hashes, names, key); + return slot >= 0 ? data[slot] : defaultValue; + } + + /** {@code data[indexOf(...)]}, or 0 when {@code key} is absent. */ + public static long lookup(int[] hashes, String[] names, long[] data, String key) { + int slot = indexOf(hashes, names, key); + return slot >= 0 ? data[slot] : 0L; + } + + /** {@code data[indexOf(...)]}, or {@code defaultValue} when {@code key} is absent. */ + public static long lookupOrDefault( + int[] hashes, String[] names, long[] data, String key, long defaultValue) { + int slot = indexOf(hashes, names, key); + return slot >= 0 ? data[slot] : defaultValue; + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/StringIndexFootprintTest.java b/internal-api/src/test/java/datadog/trace/util/StringIndexFootprintTest.java new file mode 100644 index 00000000000..9a3b1db2571 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/StringIndexFootprintTest.java @@ -0,0 +1,88 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.TreeSet; +import org.junit.jupiter.api.Test; +import org.openjdk.jol.info.GraphLayout; + +/** + * Retained-footprint comparison (JOL) for {@link StringIndex} vs the JDK set representations, over + * a fixed read-only string set. Footprint is deterministic, so this is safe to run under load + * (unlike the throughput benchmarks). + * + *

    All structures hold the same String instances, so the shared strings cancel out and the + * differences reflect structural overhead. We report total retained bytes and the overhead above a + * plain {@code String[]} (which is just the strings + a reference array). {@code Set.copyOf} yields + * the JDK's compact {@code SetN} only on Java 10+ (it falls back to {@code HashSet} pre-10), so the + * copyOf row is only meaningful on a 10+ test JVM. + * + *

    The one robust cross-JVM invariant we assert is that {@code StringIndex} is lighter than + * {@code HashSet} (no per-element {@code Node} objects). The {@code StringIndex} vs {@code SetN} + * comparison is left as reported data rather than an assertion: {@code StringIndex} caches an + * {@code int[]} of hashes that {@code SetN} does not, so which one wins on bytes is genuinely worth + * measuring. + * + *

    Measured retained bytes (Java 17, JOL estimate mode — relative ordering reliable, exact bytes + * approximate): + * + * 

    {@code
+ * n      array   hashSet  treeSet   copyOf  stringIndex
+ * 8        496      864      848      552      760
+ * 32      1936     3168     3152     2088     2872
+ * 128     7696    12384    12368     8232    11320
+ * }
    + * + * Finding: {@code StringIndex} is ~9% lighter than {@code HashSet}/{@code TreeSet} (no per-element + * {@code Node} objects), but {@code Set.copyOf} ({@code SetN}) is the most compact by a wide margin + * (~27% under {@code StringIndex} at n=128) — {@code StringIndex} pays for its cached {@code int[]} + * hashes and 2x-oversized {@code String[]}. So {@code StringIndex}'s edge over {@code SetN} is + * speed and the {@code indexOf}->parallel-array capability, not footprint. + */ +class StringIndexFootprintTest { + + static String[] elements(int n) { + String[] a = new String[n]; + for (int i = 0; i < n; ++i) { + a[i] = "element-key-" + i; + } + return a; + } + + static long bytes(Object root) { + return GraphLayout.parseInstance(root).totalSize(); + } + + @Test + void footprintComparison() { + System.out.printf( + "%-6s %12s %12s %12s %12s %12s%n", + "n", "array", "hashSet", "treeSet", "copyOf", "stringIndex"); + System.out.printf( + "%-6s %12s %12s %12s %12s %12s (overhead above array)%n", "", "", "", "", "", ""); + + for (int n : new int[] {8, 32, 128}) { + String[] el = elements(n); + + long array = bytes((Object) el); // baseline: strings + reference array + long hashSet = bytes(new HashSet<>(Arrays.asList(el))); + long treeSet = bytes(new TreeSet<>(Arrays.asList(el))); + Set copy = CollectionUtils.tryMakeImmutableSet(Arrays.asList(el)); + long copyOf = bytes(copy); + long stringIndex = bytes(StringIndex.of(el)); + + System.out.printf( + "%-6d %12d %12d %12d %12d %12d%n", n, array, hashSet, treeSet, copyOf, stringIndex); + System.out.printf( + "%-6s %12s %12d %12d %12d %12d%n", + "", "", hashSet - array, treeSet - array, copyOf - array, stringIndex - array); + + // Robust cross-JVM invariant: no per-element Node objects -> lighter than HashSet. + assertTrue( + stringIndex < hashSet, "StringIndex should retain fewer bytes than HashSet at n=" + n); + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java new file mode 100644 index 00000000000..5c8e32bfe2c --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java @@ -0,0 +1,171 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.util.StringIndex.Data; +import datadog.trace.util.StringIndex.Support; +import org.junit.jupiter.api.Test; + +class StringIndexTest { + + @Test + void hash_spread_and_zeroSentinel() { + // "".hashCode() == 0 -> remapped to the non-zero sentinel so 0 can mean "empty slot" + assertEquals(0xDD06, Support.hash("")); + + int raw = "foo".hashCode(); + assertEquals(raw ^ (raw >>> 16), Support.hash("foo")); + assertNotEquals(0, Support.hash("foo")); + } + + @Test + void tableSizeFor_isPow2_andOversized() { + assertEquals(2, Support.tableSizeFor(0)); + assertEquals(4, Support.tableSizeFor(1)); + assertEquals(8, Support.tableSizeFor(3)); + assertEquals(16, Support.tableSizeFor(4)); + } + + @Test + void instance_contains_internedAndCopy_andMiss() { + StringIndex set = StringIndex.of("foo", "bar", "baz"); + + assertEquals(8, set.numSlots()); // 3 names -> tableSizeFor(3) == 8 + + assertTrue(set.contains("foo")); // interned literal -> == fast path in eq + assertTrue(set.contains(new String("bar"))); // non-interned -> .equals path + assertFalse(set.contains("nope")); + + assertTrue(set.indexOf("baz") >= 0); + assertEquals(-1, set.indexOf("nope")); + } + + @Test + void support_create_then_indexOf() { + Data d = Support.create("x", "y"); + + int slot = Support.indexOf(d.hashes, d.names, "x"); // 3-arg overload computes the hash + assertTrue(slot >= 0); + assertEquals("x", d.names[slot]); + + assertEquals(-1, Support.indexOf(d.hashes, d.names, "q")); + } + + /** Controlled hashes force collision, linear-probe wraparound, and the already-present path. */ + @Test + void put_and_indexOf_collisionAndWraparound() { + int[] hashes = new int[4]; // mask = 3 + String[] names = new String[4]; + + assertEquals(3, Support.put(hashes, names, "a", 7)); // 7 & 3 == 3 + assertEquals(0, Support.put(hashes, names, "b", 7)); // collides at 3, probes (3+1)&3 == 0 + assertEquals(3, Support.put(hashes, names, "a", 7)); // already present -> existing slot + + assertEquals(3, Support.indexOf(hashes, names, "a", 7)); // direct hit + assertEquals(0, Support.indexOf(hashes, names, "b", 7)); // hit after collision + wraparound + assertEquals( + -1, Support.indexOf(hashes, names, "c", 7)); // miss after probing 3 -> 0 -> 1(empty) + assertEquals(-1, Support.indexOf(hashes, names, "z", 6)); // 6 & 3 == 2, empty -> immediate miss + } + + @Test + void put_throwsWhenFull() { + int[] hashes = new int[2]; // mask = 1 + String[] names = new String[2]; + + Support.put(hashes, names, "a", 4); // 4 & 1 == 0 + Support.put(hashes, names, "b", 5); // 5 & 1 == 1 + + // both slots occupied, no match -> probe exhausts -> throw + assertThrows(IllegalStateException.class, () -> Support.put(hashes, names, "c", 6)); + } + + /** The documented usage: build a StringIndex, attach a parallel payload indexed by slot. */ + @Test + void parallelPayloadBySlot() { + String[] names = {"a", "b", "c"}; + Data d = Support.create(names); + + long[] ids = new long[d.names.length]; + for (int j = 0; j < names.length; j++) { + ids[Support.indexOf(d.hashes, d.names, names[j])] = j + 1L; + } + + assertEquals(1L, ids[Support.indexOf(d.hashes, d.names, "a")]); + assertEquals(2L, ids[Support.indexOf(d.hashes, d.names, "b")]); + assertEquals(3L, ids[Support.indexOf(d.hashes, d.names, "c")]); + } + + @Test + void mapIntValues_slotAligned_andLookup() { + StringIndex idx = StringIndex.of("a", "b", "c"); + // 1-based ids; 0 stays the empty-slot / not-found sentinel. + int[] ids = idx.mapIntValues(s -> s.charAt(0) - 'a' + 1); + assertEquals(idx.numSlots(), ids.length); // sized to the table, not the name count + + assertEquals(1, idx.lookup(ids, "a")); + assertEquals(2, idx.lookup(ids, "b")); + assertEquals(3, idx.lookup(ids, "c")); + assertEquals(0, idx.lookup(ids, "z")); // miss -> 0 + assertEquals(-1, idx.lookupOrDefault(ids, "z", -1)); // miss -> supplied default + } + + @Test + void mapLongValues_slotAligned_andLookup() { + Data d = Support.create("a", "b", "c"); + long[] vals = Support.mapLongValues(d.names, s -> s.charAt(0) - 'a' + 1L); + + assertEquals(1L, Support.lookup(d.hashes, d.names, vals, "a")); + assertEquals(3L, Support.lookup(d.hashes, d.names, vals, "c")); + assertEquals(0L, Support.lookup(d.hashes, d.names, vals, "z")); // miss -> 0 + assertEquals(-1L, Support.lookupOrDefault(d.hashes, d.names, vals, "z", -1L)); + } + + @Test + void mapValues_objects_typedArray_andLookup() { + StringIndex idx = StringIndex.of("a", "bb", "ccc"); + Integer[] lengths = idx.mapValues(Integer.class, String::length); + + // Class drives a real Integer[], not an Object[]. + assertEquals(Integer[].class, lengths.getClass()); + + assertEquals(Integer.valueOf(1), idx.lookup(lengths, "a")); + assertEquals(Integer.valueOf(3), idx.lookup(lengths, "ccc")); + assertNull(idx.lookup(lengths, "z")); // miss -> null + assertEquals(Integer.valueOf(-1), idx.lookupOrDefault(lengths, "z", -1)); + } + + @Test + void support_mapValues_objects_sizedToSlots_emptyStayNull() { + Data d = Support.create("a", "b", "c"); + String[] tagged = Support.mapValues(d.names, String.class, s -> s + "!"); + + assertEquals(d.names.length, tagged.length); // sized to the table + int nonNull = 0; + for (String s : tagged) { + if (s != null) { + nonNull++; + } + } + assertEquals(3, nonNull); // only the placed names map; unfilled slots stay null + + assertEquals("a!", Support.lookup(d.hashes, d.names, tagged, "a")); + assertEquals("dflt", Support.lookupOrDefault(d.hashes, d.names, tagged, "z", "dflt")); + } + + @Test + void instance_lookup_delegatesToSupportArrays() { + StringIndex idx = StringIndex.of("x", "y"); + int[] ids = idx.mapIntValues(s -> "x".equals(s) ? 7 : 9); + + assertEquals(7, idx.lookup(ids, "x")); + assertEquals(9, idx.lookup(ids, "y")); + assertEquals(0, idx.lookup(ids, "missing")); + assertEquals(42, idx.lookupOrDefault(ids, "missing", 42)); + } +} From fc8942382133105ac24f7ad59ed599709074fd80 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 16 Jul 2026 10:30:29 -0400 Subject: [PATCH 08/10] Cover StringIndex instance long[] API + Support.numSlots for coverage gate Re-applies the coverage fix dropped by the branch rebase/restack. jacocoTestCoverageVerification flags StringIndex at 0.7 instruction coverage (min 0.8): the instance long[] API (mapLongValues / lookup / lookupOrDefault) and the Support.numSlots(int[]) static were never exercised. Add two tests covering them. Co-Authored-By: Claude Opus 4.8 --- .../datadog/trace/util/StringIndexTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java index 5c8e32bfe2c..e4763c2bef5 100644 --- a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java +++ b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java @@ -168,4 +168,23 @@ void instance_lookup_delegatesToSupportArrays() { assertEquals(0, idx.lookup(ids, "missing")); assertEquals(42, idx.lookupOrDefault(ids, "missing", 42)); } + + @Test + void instance_longValues_mapAndLookup() { + StringIndex idx = StringIndex.of("a", "b", "c"); + long[] vals = idx.mapLongValues(s -> s.charAt(0) - 'a' + 1L); + assertEquals(idx.numSlots(), vals.length); // sized to the table, not the name count + + assertEquals(1L, idx.lookup(vals, "a")); + assertEquals(3L, idx.lookup(vals, "c")); + assertEquals(0L, idx.lookup(vals, "z")); // miss -> 0 + assertEquals(2L, idx.lookupOrDefault(vals, "b", -1L)); // hit + assertEquals(-1L, idx.lookupOrDefault(vals, "z", -1L)); // miss -> supplied default + } + + @Test + void support_numSlots_matchesTableSize() { + Data d = Support.create("a", "b", "c"); + assertEquals(d.hashes.length, Support.numSlots(d.hashes)); + } } From f3c4c21eb4570803f80514a861ffa230ebe17f85 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Mon, 20 Jul 2026 09:55:55 -0400 Subject: [PATCH 09/10] Align StringIndex with the flat-collection family: EmbeddingSupport + capacityFor Two consistency changes in light of the FlatHashtable strategy family: - Rename Support -> EmbeddingSupport. StringIndex is the LightMap/object-side member (a real object with static factories), and its static-over-raw-arrays tier is exactly the "embed the backing arrays in your own fields" role that LightMap.EmbeddingSupport names. - Replace tableSizeFor with capacityFor(n[, loadFactor]) + DEFAULT_LOAD_FACTOR / LOW_LOAD_FACTOR, mirroring FlatHashtable's sizing (duplicated for now; the two branches are independent, to be unified when the family converges). This also tightens the sizing: the old `while (size <= n)` over-allocated 2x at power-of- two counts (capacityFor(16) is now 32, was 64) while still targeting load factor <= 0.5. capacityFor(0) stays valid (StringIndex allows the empty set). Updates StringIndexTest (sizing expectations + a rejects test) and the three benchmarks referencing the tier. Behavior unchanged except the tighter default table size. Co-Authored-By: Claude Opus 4.8 --- .../trace/util/ImmutableMapBenchmark.java | 8 +- .../trace/util/ImmutableSetBenchmark.java | 14 +-- .../util/StringIndexSwitchBenchmark.java | 17 +-- .../java/datadog/trace/util/StringIndex.java | 105 ++++++++++++------ .../datadog/trace/util/StringIndexTest.java | 93 +++++++++------- 5 files changed, 141 insertions(+), 96 deletions(-) diff --git a/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java index 46fabf7a312..c2e77b89451 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ImmutableMapBenchmark.java @@ -124,12 +124,12 @@ static void fill(Map map) { static final int[] SI_VALUES; static { - StringIndex.Data data = StringIndex.Support.create(INSERTION_KEYS); + StringIndex.Data data = StringIndex.EmbeddingSupport.create(INSERTION_KEYS); SI_HASHES = data.hashes; SI_NAMES = data.names; SI_VALUES = new int[SI_HASHES.length]; for (int i = 0; i < INSERTION_KEYS.length; ++i) { - SI_VALUES[StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, INSERTION_KEYS[i])] = i; + SI_VALUES[StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, INSERTION_KEYS[i])] = i; } } @@ -276,12 +276,12 @@ public int stringIndex_get_sameKey(Cursor cursor) { @Benchmark public int support_get(Cursor cursor) { - return SI_VALUES[StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextKey())]; + return SI_VALUES[StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, cursor.nextKey())]; } @Benchmark public int support_get_sameKey(Cursor cursor) { return SI_VALUES[ - StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextKey(INSERTION_KEYS))]; + StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, cursor.nextKey(INSERTION_KEYS))]; } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java index 823fd9f883c..2cc60ab7bf3 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ImmutableSetBenchmark.java @@ -37,10 +37,10 @@ * flat/immutable set comparison. *
  • {@code stringIndex} — {@link StringIndex#contains} on the instance wrapper (one field load * to reach the placed arrays, then an open-addressed probe). - *
  • {@code support} — the same probe via {@link StringIndex.Support#indexOf} over {@code static - * final} arrays, so the JIT folds the refs to constants and there is nothing to dereference - * (the hot path StringIndex recommends). The {@code stringIndex}/{@code support} pair shows - * the indirection cost of the wrapper. + *
  • {@code support} — the same probe via {@link StringIndex.EmbeddingSupport#indexOf} over + * {@code static final} arrays, so the JIT folds the refs to constants and there is nothing to + * dereference (the hot path StringIndex recommends). The {@code stringIndex}/{@code support} + * pair shows the indirection cost of the wrapper. * * *

    Lookups are interned (the {@code ==} fast path where a structure has one); misses are short @@ -115,7 +115,7 @@ static String[] newMisses() { static final String[] SI_NAMES; static { - StringIndex.Data data = StringIndex.Support.create(STRINGS); + StringIndex.Data data = StringIndex.EmbeddingSupport.create(STRINGS); SI_HASHES = data.hashes; SI_NAMES = data.names; } @@ -235,11 +235,11 @@ public boolean stringIndex_miss(Cursor cursor) { @Benchmark public boolean support_hit(Cursor cursor) { - return StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextHit()) >= 0; + return StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, cursor.nextHit()) >= 0; } @Benchmark public boolean support_miss(Cursor cursor) { - return StringIndex.Support.indexOf(SI_HASHES, SI_NAMES, cursor.nextMiss()) >= 0; + return StringIndex.EmbeddingSupport.indexOf(SI_HASHES, SI_NAMES, cursor.nextMiss()) >= 0; } } diff --git a/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java index 916cfe70059..c527cb21a54 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/StringIndexSwitchBenchmark.java @@ -19,8 +19,8 @@ *

      *
    • {@code switch} — a hand-written {@code switch(key)} over the literals ({@code hashCode} * switch + {@code equals}), {@code default} returns 0. - *
    • {@code stringIndex} — {@code IDS[Support.indexOf(HASHES, NAMES, key)]} over {@code static - * final} arrays (a miss returns 0), the folded-constant hot path. + *
    • {@code stringIndex} — {@code IDS[EmbeddingSupport.indexOf(HASHES, NAMES, key)]} over {@code + * static final} arrays (a miss returns 0), the folded-constant hot path. *
    * *

    What this measures: two axes. A prior investigation found the {@code TagInterceptor} @@ -30,8 +30,8 @@ * varied key. The results (below) land the teaching point: the dominant axis is * key-constancy, not inlining. At steady state the inline-vs-not gap is small for both * forms; what sinks the switch is a runtime, varied key (it can't specialize), while the - * StringIndex {@code Support} path stays flat across both axes — so the win is largest exactly - * where {@code TagInterceptor} lives. + * StringIndex {@code EmbeddingSupport} path stays flat across both axes — so the win is largest + * exactly where {@code TagInterceptor} lives. * *

    The {@code _inlined} and {@code _noinline} helpers carry duplicate bodies on purpose: that's * the only way to pin each form's inlining decision independently. @@ -107,12 +107,13 @@ static String[] newMisses() { static final int[] IDS; static { - StringIndex.Data data = StringIndex.Support.create(KEYS); + StringIndex.Data data = StringIndex.EmbeddingSupport.create(KEYS); HASHES = data.hashes; NAMES = data.names; IDS = new int[HASHES.length]; for (int i = 0; i < KEYS.length; ++i) { - IDS[StringIndex.Support.indexOf(HASHES, NAMES, KEYS[i])] = i + 1; // 1-based; 0 = not found + IDS[StringIndex.EmbeddingSupport.indexOf(HASHES, NAMES, KEYS[i])] = + i + 1; // 1-based; 0 = not found } } @@ -224,13 +225,13 @@ static int switchNoInline(String key) { @CompilerControl(CompilerControl.Mode.INLINE) static int indexInline(String key) { - int slot = StringIndex.Support.indexOf(HASHES, NAMES, key); + int slot = StringIndex.EmbeddingSupport.indexOf(HASHES, NAMES, key); return slot >= 0 ? IDS[slot] : 0; } @CompilerControl(CompilerControl.Mode.DONT_INLINE) static int indexNoInline(String key) { - int slot = StringIndex.Support.indexOf(HASHES, NAMES, key); + int slot = StringIndex.EmbeddingSupport.indexOf(HASHES, NAMES, key); return slot >= 0 ? IDS[slot] : 0; } diff --git a/internal-api/src/main/java/datadog/trace/util/StringIndex.java b/internal-api/src/main/java/datadog/trace/util/StringIndex.java index c5da0379cce..d7709225b0e 100644 --- a/internal-api/src/main/java/datadog/trace/util/StringIndex.java +++ b/internal-api/src/main/java/datadog/trace/util/StringIndex.java @@ -11,11 +11,13 @@ *

    Two ways to use it, trading convenience for indirection: * *

      - *
    • {@link Support} — static algorithm over raw arrays. Keep the arrays in your own - * (ideally {@code static final}) fields and the JIT folds the refs to constants. The fastest - * path; nothing to dereference. + *
    • {@link EmbeddingSupport} — static algorithm over raw arrays you embed in your + * own (ideally {@code static final}) fields; the JIT folds the refs to constants. The fastest + * path, nothing to dereference. (Named for the same role as {@code + * LightMap.EmbeddingSupport}: the static ops you reach for when you own the backing arrays + * directly.) *
    • The {@code StringIndex} instance ({@link #of}) — a convenience wrapper holding the - * arrays; {@link #indexOf}/{@link #contains} delegate to {@link Support}. Costs an + * arrays; {@link #indexOf}/{@link #contains} delegate to {@link EmbeddingSupport}. Costs an * instance-field load per call (the indirection the static path removes) — fine off the hot * path. *
    @@ -25,18 +27,19 @@ * mapIntValues}/{@code mapLongValues} build such an array at construction; {@code lookup}/{@code * lookupOrDefault} read one back in a single call (slot resolve + array read). * - *

    Slot 0-value is the empty sentinel: {@link Support#hash} never returns 0, so {@code hashes[i] - * == 0} unambiguously means an empty slot. + *

    Slot 0-value is the empty sentinel: {@link EmbeddingSupport#hash} never returns 0, so {@code + * hashes[i] == 0} unambiguously means an empty slot. * - *

    Trades memory for simplicity (and, incidentally, speed). The table is 2x-oversized (load - * factor ≤ 0.5) so build-time placement always finds a free slot and never has to rehash or - * resize — short probe chains are a welcome side effect, not the design goal. The cached {@code - * int[]} hashes gate {@code equals()}. Both cost memory, so a tightly-packed set is more compact: - * prefer {@link java.util.Set#copyOf} (the JDK's {@code SetN}) when you only need membership, and - * reach for {@code StringIndex} for the {@code indexOf}->parallel-array (name→id) - * capability or the hot, allocation-free static {@link Support} path. (If footprint ever matters - * more than build simplicity, a higher load factor with construction-time rehashing would close the - * gap.) + *

    Trades memory for simplicity (and, incidentally, speed). The table is 2x-oversized ({@link + * EmbeddingSupport#DEFAULT_LOAD_FACTOR} ≤ 0.5) so build-time placement always finds a free slot + * and never has to rehash or resize — short probe chains are a welcome side effect, not the design + * goal. The cached {@code int[]} hashes gate {@code equals()}. Both cost memory, so a + * tightly-packed set is more compact: prefer {@link java.util.Set#copyOf} (the JDK's {@code SetN}) + * when you only need membership, and reach for {@code StringIndex} for the {@code + * indexOf}->parallel-array (name→id) capability or the hot, allocation-free static {@link + * EmbeddingSupport} path. (If footprint matters more than build simplicity, build via {@link + * EmbeddingSupport#capacityFor(int, float)} at a higher load factor — placement still finds a slot + * at any factor < 1, so no rehash is needed.) */ public final class StringIndex { private final int[] hashes; @@ -48,16 +51,19 @@ private StringIndex(int[] hashes, String[] names) { } /** - * Convenience instance — wraps the placed arrays. For the hot path prefer raw {@link Support}. + * Convenience instance — wraps the placed arrays. For the hot path prefer raw {@link + * EmbeddingSupport}. */ public static StringIndex of(String... names) { - Data data = Support.create(names); + Data data = EmbeddingSupport.create(names); return new StringIndex(data.hashes, data.names); } - /** Slot of {@code name}, or -1. Delegates to {@link Support} on the instance's arrays. */ + /** + * Slot of {@code name}, or -1. Delegates to {@link EmbeddingSupport} on the instance's arrays. + */ public int indexOf(String name) { - return Support.indexOf(this.hashes, this.names, name); + return EmbeddingSupport.indexOf(this.hashes, this.names, name); } public boolean contains(String name) { @@ -77,49 +83,49 @@ public int numSlots() { * can't allocate a generic array without it). Pair with {@link #lookup(Object[], String)}. */ public T[] mapValues(Class type, Function fn) { - return Support.mapValues(this.names, type, fn); + return EmbeddingSupport.mapValues(this.names, type, fn); } /** Slot-aligned {@code int[]} of values; absent slots stay 0. See {@link #mapValues}. */ public int[] mapIntValues(ToIntFunction fn) { - return Support.mapIntValues(this.names, fn); + return EmbeddingSupport.mapIntValues(this.names, fn); } /** Slot-aligned {@code long[]} of values; absent slots stay 0. See {@link #mapValues}. */ public long[] mapLongValues(ToLongFunction fn) { - return Support.mapLongValues(this.names, fn); + return EmbeddingSupport.mapLongValues(this.names, fn); } // --- lookup: resolve a key and read its parallel value in one call --- /** {@code data[indexOf(key)]}, or {@code null} when {@code key} is absent. */ public T lookup(T[] data, String key) { - return Support.lookup(this.hashes, this.names, data, key); + return EmbeddingSupport.lookup(this.hashes, this.names, data, key); } /** {@code data[indexOf(key)]}, or {@code defaultValue} when {@code key} is absent. */ public T lookupOrDefault(T[] data, String key, T defaultValue) { - return Support.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); + return EmbeddingSupport.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); } /** {@code data[indexOf(key)]}, or 0 when {@code key} is absent. */ public int lookup(int[] data, String key) { - return Support.lookup(this.hashes, this.names, data, key); + return EmbeddingSupport.lookup(this.hashes, this.names, data, key); } /** {@code data[indexOf(key)]}, or {@code defaultValue} when {@code key} is absent. */ public int lookupOrDefault(int[] data, String key, int defaultValue) { - return Support.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); + return EmbeddingSupport.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); } /** {@code data[indexOf(key)]}, or 0 when {@code key} is absent. */ public long lookup(long[] data, String key) { - return Support.lookup(this.hashes, this.names, data, key); + return EmbeddingSupport.lookup(this.hashes, this.names, data, key); } /** {@code data[indexOf(key)]}, or {@code defaultValue} when {@code key} is absent. */ public long lookupOrDefault(long[] data, String key, long defaultValue) { - return Support.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); + return EmbeddingSupport.lookupOrDefault(this.hashes, this.names, data, key, defaultValue); } /** Build-time carrier. Pull the fields into your own (static final) fields; don't keep this. */ @@ -136,8 +142,8 @@ public static final class Data { /** * Static algorithm over raw arrays. Query helpers take raw arrays, never a Data or a StringIndex. */ - public static final class Support { - private Support() {} + public static final class EmbeddingSupport { + private EmbeddingSupport() {} /** Spread of String.hashCode; 0 reserved as the empty sentinel. */ public static int hash(String name) { @@ -145,18 +151,43 @@ public static int hash(String name) { return h == 0 ? 0xDD06 : h ^ (h >>> 16); } - /** Power-of-two size, 2x-oversized so load factor stays <= 0.5. */ - public static int tableSizeFor(int n) { - int size = 1; - while (size <= n) { - size <<= 1; + /** + * Balanced default load factor — target fill {@code <= 0.5} ({@code >= 2x} capacity). (Mirrors + * {@code FlatHashtable.DEFAULT_LOAD_FACTOR}; duplicated while the two are separate PRs, to be + * unified when the flat-collection family converges.) + */ + public static final float DEFAULT_LOAD_FACTOR = 0.5f; + + /** Sparse load factor — target fill {@code <= 0.25} ({@code >= 4x} capacity). */ + public static final float LOW_LOAD_FACTOR = 0.25f; + + /** Power-of-two capacity for {@code n} names at the {@link #DEFAULT_LOAD_FACTOR}. */ + public static int capacityFor(int n) { + return capacityFor(n, DEFAULT_LOAD_FACTOR); + } + + /** + * Power-of-two capacity for {@code n} names at {@code loadFactor}: the smallest power of two + * {@code >= ceil(n / loadFactor)} (so the achieved fill is {@code <= loadFactor}). {@code n == + * 0} yields a minimal 2-slot table (StringIndex allows the empty set, unlike FlatHashtable). + */ + public static int capacityFor(int n, float loadFactor) { + if (n < 0) { + throw new IllegalArgumentException("n must be non-negative: " + n); + } + if (!(loadFactor > 0f && loadFactor < 1f)) { + throw new IllegalArgumentException("loadFactor must be in (0, 1): " + loadFactor); + } + if (n == 0) { + return 2; // empty set -> minimal table (one always-empty slot suffices, 2 keeps it pow2) } - return size << 1; + int min = (int) Math.ceil(n / (double) loadFactor); + return Integer.highestOneBit(min - 1) << 1; } /** Build the placed table. Returns a Data carrier; pull its arrays into your own fields. */ public static Data create(String... names) { - int size = tableSizeFor(names.length); + int size = capacityFor(names.length); int[] hashes = new int[size]; String[] placed = new String[size]; for (String name : names) { diff --git a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java index e4763c2bef5..987ccc8b3ba 100644 --- a/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java +++ b/internal-api/src/test/java/datadog/trace/util/StringIndexTest.java @@ -8,7 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.trace.util.StringIndex.Data; -import datadog.trace.util.StringIndex.Support; +import datadog.trace.util.StringIndex.EmbeddingSupport; import org.junit.jupiter.api.Test; class StringIndexTest { @@ -16,26 +16,34 @@ class StringIndexTest { @Test void hash_spread_and_zeroSentinel() { // "".hashCode() == 0 -> remapped to the non-zero sentinel so 0 can mean "empty slot" - assertEquals(0xDD06, Support.hash("")); + assertEquals(0xDD06, EmbeddingSupport.hash("")); int raw = "foo".hashCode(); - assertEquals(raw ^ (raw >>> 16), Support.hash("foo")); - assertNotEquals(0, Support.hash("foo")); + assertEquals(raw ^ (raw >>> 16), EmbeddingSupport.hash("foo")); + assertNotEquals(0, EmbeddingSupport.hash("foo")); } @Test - void tableSizeFor_isPow2_andOversized() { - assertEquals(2, Support.tableSizeFor(0)); - assertEquals(4, Support.tableSizeFor(1)); - assertEquals(8, Support.tableSizeFor(3)); - assertEquals(16, Support.tableSizeFor(4)); + void capacityFor_isPow2_andAtLeastDoubled() { + assertEquals(2, EmbeddingSupport.capacityFor(0)); // empty set -> minimal table + assertEquals(2, EmbeddingSupport.capacityFor(1)); // >= 2x, smallest power of two + assertEquals(8, EmbeddingSupport.capacityFor(3)); // ceil(3/0.5)=6 -> 8 + assertEquals(8, EmbeddingSupport.capacityFor(4)); // ceil(4/0.5)=8 -> 8 (was 16: tightened) + assertEquals(64, EmbeddingSupport.capacityFor(16, EmbeddingSupport.LOW_LOAD_FACTOR)); // 4x + } + + @Test + void capacityFor_rejectsBadArgs() { + assertThrows(IllegalArgumentException.class, () -> EmbeddingSupport.capacityFor(-1)); + assertThrows(IllegalArgumentException.class, () -> EmbeddingSupport.capacityFor(4, 0f)); + assertThrows(IllegalArgumentException.class, () -> EmbeddingSupport.capacityFor(4, 1f)); } @Test void instance_contains_internedAndCopy_andMiss() { StringIndex set = StringIndex.of("foo", "bar", "baz"); - assertEquals(8, set.numSlots()); // 3 names -> tableSizeFor(3) == 8 + assertEquals(8, set.numSlots()); // 3 names -> capacityFor(3) == 8 assertTrue(set.contains("foo")); // interned literal -> == fast path in eq assertTrue(set.contains(new String("bar"))); // non-interned -> .equals path @@ -47,13 +55,13 @@ void instance_contains_internedAndCopy_andMiss() { @Test void support_create_then_indexOf() { - Data d = Support.create("x", "y"); + Data d = EmbeddingSupport.create("x", "y"); - int slot = Support.indexOf(d.hashes, d.names, "x"); // 3-arg overload computes the hash + int slot = EmbeddingSupport.indexOf(d.hashes, d.names, "x"); // 3-arg overload computes the hash assertTrue(slot >= 0); assertEquals("x", d.names[slot]); - assertEquals(-1, Support.indexOf(d.hashes, d.names, "q")); + assertEquals(-1, EmbeddingSupport.indexOf(d.hashes, d.names, "q")); } /** Controlled hashes force collision, linear-probe wraparound, and the already-present path. */ @@ -62,15 +70,20 @@ void put_and_indexOf_collisionAndWraparound() { int[] hashes = new int[4]; // mask = 3 String[] names = new String[4]; - assertEquals(3, Support.put(hashes, names, "a", 7)); // 7 & 3 == 3 - assertEquals(0, Support.put(hashes, names, "b", 7)); // collides at 3, probes (3+1)&3 == 0 - assertEquals(3, Support.put(hashes, names, "a", 7)); // already present -> existing slot + assertEquals(3, EmbeddingSupport.put(hashes, names, "a", 7)); // 7 & 3 == 3 + assertEquals( + 0, EmbeddingSupport.put(hashes, names, "b", 7)); // collides at 3, probes (3+1)&3 == 0 + assertEquals( + 3, EmbeddingSupport.put(hashes, names, "a", 7)); // already present -> existing slot - assertEquals(3, Support.indexOf(hashes, names, "a", 7)); // direct hit - assertEquals(0, Support.indexOf(hashes, names, "b", 7)); // hit after collision + wraparound + assertEquals(3, EmbeddingSupport.indexOf(hashes, names, "a", 7)); // direct hit + assertEquals( + 0, EmbeddingSupport.indexOf(hashes, names, "b", 7)); // hit after collision + wraparound + assertEquals( + -1, + EmbeddingSupport.indexOf(hashes, names, "c", 7)); // miss after probing 3 -> 0 -> 1(empty) assertEquals( - -1, Support.indexOf(hashes, names, "c", 7)); // miss after probing 3 -> 0 -> 1(empty) - assertEquals(-1, Support.indexOf(hashes, names, "z", 6)); // 6 & 3 == 2, empty -> immediate miss + -1, EmbeddingSupport.indexOf(hashes, names, "z", 6)); // 6 & 3 == 2, empty -> immediate miss } @Test @@ -78,27 +91,27 @@ void put_throwsWhenFull() { int[] hashes = new int[2]; // mask = 1 String[] names = new String[2]; - Support.put(hashes, names, "a", 4); // 4 & 1 == 0 - Support.put(hashes, names, "b", 5); // 5 & 1 == 1 + EmbeddingSupport.put(hashes, names, "a", 4); // 4 & 1 == 0 + EmbeddingSupport.put(hashes, names, "b", 5); // 5 & 1 == 1 // both slots occupied, no match -> probe exhausts -> throw - assertThrows(IllegalStateException.class, () -> Support.put(hashes, names, "c", 6)); + assertThrows(IllegalStateException.class, () -> EmbeddingSupport.put(hashes, names, "c", 6)); } /** The documented usage: build a StringIndex, attach a parallel payload indexed by slot. */ @Test void parallelPayloadBySlot() { String[] names = {"a", "b", "c"}; - Data d = Support.create(names); + Data d = EmbeddingSupport.create(names); long[] ids = new long[d.names.length]; for (int j = 0; j < names.length; j++) { - ids[Support.indexOf(d.hashes, d.names, names[j])] = j + 1L; + ids[EmbeddingSupport.indexOf(d.hashes, d.names, names[j])] = j + 1L; } - assertEquals(1L, ids[Support.indexOf(d.hashes, d.names, "a")]); - assertEquals(2L, ids[Support.indexOf(d.hashes, d.names, "b")]); - assertEquals(3L, ids[Support.indexOf(d.hashes, d.names, "c")]); + assertEquals(1L, ids[EmbeddingSupport.indexOf(d.hashes, d.names, "a")]); + assertEquals(2L, ids[EmbeddingSupport.indexOf(d.hashes, d.names, "b")]); + assertEquals(3L, ids[EmbeddingSupport.indexOf(d.hashes, d.names, "c")]); } @Test @@ -117,13 +130,13 @@ void mapIntValues_slotAligned_andLookup() { @Test void mapLongValues_slotAligned_andLookup() { - Data d = Support.create("a", "b", "c"); - long[] vals = Support.mapLongValues(d.names, s -> s.charAt(0) - 'a' + 1L); + Data d = EmbeddingSupport.create("a", "b", "c"); + long[] vals = EmbeddingSupport.mapLongValues(d.names, s -> s.charAt(0) - 'a' + 1L); - assertEquals(1L, Support.lookup(d.hashes, d.names, vals, "a")); - assertEquals(3L, Support.lookup(d.hashes, d.names, vals, "c")); - assertEquals(0L, Support.lookup(d.hashes, d.names, vals, "z")); // miss -> 0 - assertEquals(-1L, Support.lookupOrDefault(d.hashes, d.names, vals, "z", -1L)); + assertEquals(1L, EmbeddingSupport.lookup(d.hashes, d.names, vals, "a")); + assertEquals(3L, EmbeddingSupport.lookup(d.hashes, d.names, vals, "c")); + assertEquals(0L, EmbeddingSupport.lookup(d.hashes, d.names, vals, "z")); // miss -> 0 + assertEquals(-1L, EmbeddingSupport.lookupOrDefault(d.hashes, d.names, vals, "z", -1L)); } @Test @@ -142,8 +155,8 @@ void mapValues_objects_typedArray_andLookup() { @Test void support_mapValues_objects_sizedToSlots_emptyStayNull() { - Data d = Support.create("a", "b", "c"); - String[] tagged = Support.mapValues(d.names, String.class, s -> s + "!"); + Data d = EmbeddingSupport.create("a", "b", "c"); + String[] tagged = EmbeddingSupport.mapValues(d.names, String.class, s -> s + "!"); assertEquals(d.names.length, tagged.length); // sized to the table int nonNull = 0; @@ -154,8 +167,8 @@ void support_mapValues_objects_sizedToSlots_emptyStayNull() { } assertEquals(3, nonNull); // only the placed names map; unfilled slots stay null - assertEquals("a!", Support.lookup(d.hashes, d.names, tagged, "a")); - assertEquals("dflt", Support.lookupOrDefault(d.hashes, d.names, tagged, "z", "dflt")); + assertEquals("a!", EmbeddingSupport.lookup(d.hashes, d.names, tagged, "a")); + assertEquals("dflt", EmbeddingSupport.lookupOrDefault(d.hashes, d.names, tagged, "z", "dflt")); } @Test @@ -184,7 +197,7 @@ void instance_longValues_mapAndLookup() { @Test void support_numSlots_matchesTableSize() { - Data d = Support.create("a", "b", "c"); - assertEquals(d.hashes.length, Support.numSlots(d.hashes)); + Data d = EmbeddingSupport.create("a", "b", "c"); + assertEquals(d.hashes.length, EmbeddingSupport.numSlots(d.hashes)); } } From 0c0d740c38e846f0c8060d6dde5afa1766962005 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 16:30:58 -0400 Subject: [PATCH 10/10] Store known span tags densely in TagMap by tag-id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../java/datadog/trace/core/CoreTracer.java | 9 + .../trace/api/DenseStoreAllocBenchmark.java | 148 +++++ .../java/datadog/trace/api/KnownTagCodec.java | 177 ++++++ .../java/datadog/trace/api/KnownTags.java | 328 ++++++++++++ .../main/java/datadog/trace/api/TagMap.java | 504 +++++++++++++++--- .../java/datadog/trace/api/KnownTagsTest.java | 152 ++++++ .../trace/api/TagMapDenseForkedTest.java | 273 ++++++++++ .../trace/api/TagMapDenseFuzzForkedTest.java | 202 +++++++ 8 files changed, 1731 insertions(+), 62 deletions(-) create mode 100644 internal-api/src/jmh/java/datadog/trace/api/DenseStoreAllocBenchmark.java create mode 100644 internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java create mode 100644 internal-api/src/main/java/datadog/trace/api/KnownTags.java create mode 100644 internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java create mode 100644 internal-api/src/test/java/datadog/trace/api/TagMapDenseForkedTest.java create mode 100644 internal-api/src/test/java/datadog/trace/api/TagMapDenseFuzzForkedTest.java diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 0e005e10820..c335555a4bc 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -40,6 +40,7 @@ import datadog.trace.api.EndpointTracker; import datadog.trace.api.IdGenerationStrategy; import datadog.trace.api.InstrumenterConfig; +import datadog.trace.api.KnownTags; import datadog.trace.api.Pair; import datadog.trace.api.TagMap; import datadog.trace.api.TraceConfig; @@ -656,6 +657,14 @@ private CoreTracer( // preload this enum to avoid triggering classloading on the hot path TraceCollector.PublishState.values(); + // Dense known-tag store (experimental, OFF by default): registering the KnownTagCodec resolver + // flips the dense store live so known tags store without a per-tag Entry. Gated by a system + // property for A/B benchmarking; when off, keyOf stays a no-op and tag storage is byte-identical + // to today. Promote to a Config flag if this becomes a permanent rollout. + if (Boolean.getBoolean("dd.trace.dense.tags.enabled")) { + KnownTags.init(); + } + if (reportInTracerFlare) { TracerFlare.addReporter(this); } diff --git a/internal-api/src/jmh/java/datadog/trace/api/DenseStoreAllocBenchmark.java b/internal-api/src/jmh/java/datadog/trace/api/DenseStoreAllocBenchmark.java new file mode 100644 index 00000000000..6c11f8bc212 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/api/DenseStoreAllocBenchmark.java @@ -0,0 +1,148 @@ +package datadog.trace.api; + +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Deterministic allocation A/B for the dense known-tag store, using the REAL {@link KnownTags} + * resolver (a {@code StringIndex} probe + a constant-returning {@code switch} — allocation-free, + * exactly like production). An earlier synthetic prefix resolver allocated in {@code keyOf} + * (substring) and {@code nameOf} (concat), contaminating the dense arm; this measures the store, + * not the resolver. + * + *

    Models how a real span's tags route: {@code today} = all custom (what ships now — every tag + * buckets, since nothing is registered as known), {@code dense} = the same tag count with a + * realistic fraction routed to the dense store (real known tag names) and the rest custom. Run with + * {@code -prof gc}; the {@code gc.alloc.rate.norm} (B/op) delta at the same {@code tagCount} is + * what enabling the dense store does to a real span's per-build allocation. + * + *

    Results — buildMap, JDK 17 (Zulu 17.0.7, Apple Silicon), {@code -prof gc -f 1 -wi 2 -i 3}, + * 2026-07-08. Allocation is deterministic (±0.001 B/op); throughput on this run is NOT + * trustworthy (single fork, short) — read B/op only. + * + * 

    {@code
+ * scenario    tagCount=7   tagCount=12
+ * today          408 B/op     704 B/op
+ * dense          376 B/op     416 B/op
+ * allKnown       176 B/op     400 B/op
+ * }
    + * + *

    Gate met: {@code dense < today} at both counts (the over-provision artifact is gone). The + * Entry-less win scales with the known-tag fraction — ~8% at 7 tags (~70% known), ~41% at 12; + * {@code allKnown} (the codegen endgame / read-through parent shape) reaches ~57% at 7. + * + *

    Serialize paths (same run, B/op). {@code buildAndSerialize} (alloc-free {@code forEach} + * flyweight) adds a flat +16 B/op over {@code buildMap} in every scenario (7: 392, 12: 432 dense). + * {@code buildAndSerializeViaIterator} — the {@code EntryReader} enhanced-for modeling the count + * pre-pass at {@code TraceMapperV0_4:95} — adds a CONSTANT per-call cost (+56 custom / +80 dense, + * identical at 7 and 12 tags): that flat-vs-tagCount signature is the {@code EntryReaderIterator} + * OBJECT, NOT per-tag Entry — the iterator reuses a dense flyweight (TagMap:2182/2652). So the + * dense win SURVIVES serialization; the only nit is {@code iterator()} allocating one Iterator per + * call, which {@code forEach} avoids and which can be recycled away. + */ +@State(Scope.Benchmark) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 2, time = 2) +@Measurement(iterations = 3, time = 2) +@Fork(1) +@Threads(1) +public class DenseStoreAllocBenchmark { + + // Real stored (dense-routed) tag names — a realistic web/db span's known set. + static final String[] KNOWN = + new String[] { + DDTags.BASE_SERVICE, + Tags.VERSION, + Tags.COMPONENT, + Tags.SPAN_KIND, + Tags.HTTP_METHOD, + Tags.HTTP_ROUTE, + Tags.DB_TYPE, + Tags.DB_INSTANCE, + Tags.PEER_HOSTNAME, + Tags.DB_USER, + DDTags.LANGUAGE_TAG_KEY, + Tags.PEER_PORT, + }; + + // today = all custom (all bucket, what ships now); dense = ~70% known + custom (a real span); + // allKnown = 100% known (the trace-tier read-through parent's shape — exercises lazy buckets). + @Param({"today", "dense", "allKnown"}) + String scenario; + + @Param({"7", "12"}) + int tagCount; + + private String[] keys; + private String[] values; + + @Setup(Level.Trial) + public void setup() { + KnownTags.init(); // registers the real (allocation-free) resolver + int knownCount; + if ("allKnown".equals(scenario)) { + knownCount = tagCount; // 100% known (<= KNOWN.length) + } else if ("dense".equals(scenario)) { + knownCount = (tagCount * 7) / 10; // ~70% known + custom + } else { + knownCount = 0; // today: all custom (all bucket) + } + this.keys = new String[tagCount]; + this.values = new String[tagCount]; + for (int i = 0; i < tagCount; i++) { + this.keys[i] = i < knownCount ? KNOWN[i] : "custom.tag." + i; + this.values[i] = "value-" + i; + } + } + + @Benchmark + public TagMap buildMap() { + TagMap m = TagMap.create(16); + for (int i = 0; i < tagCount; i++) { + m.set(keys[i], values[i]); + } + return m; + } + + @Benchmark + public void buildAndSerialize(Blackhole bh) { + TagMap m = TagMap.create(16); + for (int i = 0; i < tagCount; i++) { + m.set(keys[i], values[i]); + } + // forEach: the alloc-free flyweight emit for dense + m.forEach(reader -> bh.consume(reader.objectValue())); + bh.consume(m); + } + + @Benchmark + public void buildAndSerializeViaIterator(Blackhole bh) { + TagMap m = TagMap.create(16); + for (int i = 0; i < tagCount; i++) { + m.set(keys[i], values[i]); + } + // models the REAL serializer's count pre-pass (TraceMapperV0_4:95). The EntryReader iterator + // uses a reused dense flyweight (NO per-tag Entry alloc — TagMap:2182/2652), so the dense win + // SURVIVES; the only extra cost vs forEach is the EntryReaderIterator object itself (a fixed + // per-call cost, constant across tagCount — not per-tag). forEach avoids even that. + for (TagMap.EntryReader reader : m) { + bh.consume(reader.objectValue()); + } + bh.consume(m); + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java b/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java new file mode 100644 index 00000000000..7ed9dff633a --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/KnownTagCodec.java @@ -0,0 +1,177 @@ +package datadog.trace.api; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Registry for generated tag ID ↔ name resolution. The code generator populates this at tracer init + * via {@link #register(Resolver)}. Once registered, HotSpot CHA devirtualizes and inlines the + * resolver's switch, making {@link #nameOf}/{@link #keyOf} effectively zero-overhead. + */ +public final class KnownTagCodec { + // Plain (non-volatile) fast-path flag: false until a resolver is ever registered. A plain read is + // free and hoistable, unlike a volatile read of `resolver` (costly on weak memory models such as + // ARM). A stale `false` is benign — callers treat the tag as unknown and use the hash buckets, + // which is correct, just unoptimized; the next read after publication takes the slot path. + private static boolean active; + + private static volatile Resolver resolver; + + /** Fast-path gate: true once a resolver has been registered. */ + public static boolean isActive() { + return active; + } + + /* + * tagId bit layout: [63 intercepted] [62-48 globalSerial (15 bits)] [47-32 fieldPos] + * [31-0 nameHash]. Bit 63 (the sign bit) marks a tag the tag interceptor must see, so the check + * is a single {@code tagId < 0}. globalSerial is globally unique per known tag; fieldPos is its + * slot in the global positional layout (TagMap.knownEntries index); nameHash is + * TagMap.Entry#_hash(name) and is layout-independent. Unknown (string-only) tags have the upper + * 32 bits zero. NOTE: TagMap.Entry decodes nameHash inline as (int) tagId on its hot path, so the + * low-32 encoding here must stay in sync with that. + */ + public static int globalSerial(long tagId) { + return (int) ((tagId >>> 48) & 0x7FFF); + } + + /** + * Flag bit (the sign bit) marking a tag the tag interceptor must process — reserved/"virtual" + * tags AND intercepted-but-stored tags (e.g. http.method, which the interceptor side-effects and + * also stores). Encoded in the id so {@code DDSpanContext.setTag(long)} can route with a single + * sign test ({@link #isIntercepted}) instead of resolving the name. Non-intercepted tags (peer.*, + * base.service, …) leave it clear and take the fast store path. Must agree with the interceptor's + * name-based {@code needsIntercept} for every assigned id. + */ + public static final long INTERCEPTED = Long.MIN_VALUE; // 1L << 63 + + /** True if the tagId is flagged for tag-interceptor processing. */ + public static boolean isIntercepted(long tagId) { + return tagId < 0L; + } + + /** Returns the tagId with the {@link #INTERCEPTED} flag set. */ + public static long intercepted(long tagId) { + return tagId | INTERCEPTED; + } + + public static int fieldPos(long tagId) { + return (int) ((tagId >>> 32) & 0xFFFF); + } + + public static int nameHash(long tagId) { + return (int) tagId; + } + + /** + * globalSerial partition. {@code [1, FIRST_STORED_SERIAL)} is reserved for "virtual" tags that + * are specially handled (redirected to span fields or processed by the tag interceptor) and are + * NOT stored in the TagMap — these are hand-assigned in tracer core. {@code [FIRST_STORED_SERIAL, + * ..]} is for generated convention tags that ARE stored (slotted/bucketed). {@code globalSerial + * == 0} means unknown / string-only. Both core and the code generator must agree on this + * boundary. + */ + public static final int FIRST_STORED_SERIAL = 256; + + /** True if the tagId names a reserved "virtual"/specially-handled tag (not stored in the map). */ + public static boolean isReserved(long tagId) { + int globalSerial = globalSerial(tagId); + return globalSerial > 0 && globalSerial < FIRST_STORED_SERIAL; + } + + /** True if the tagId names a generated, map-stored (slotted/bucketed) tag. */ + public static boolean isStored(long tagId) { + return globalSerial(tagId) >= FIRST_STORED_SERIAL; + } + + /** + * Sentinel {@code fieldPos} meaning "no positional slot". It is the maximum value the 16-bit + * fieldPos field can hold, so it always compares {@code >= slotCount()} and routes to the hash + * buckets rather than the fast positional array. Two kinds of tagId use it: + * + *

      + *
    • Reserved/virtual tags ({@code globalSerial < FIRST_STORED_SERIAL}) — not stored at all; + * the sentinel just guarantees an incidental store never lands in a slot. + *
    • Unslotted stored tags ({@code globalSerial >= FIRST_STORED_SERIAL}) — "low-priority" tags + * that get a stable id (and so {@code keyOf}/{@code nameOf} unification with their string + * form) but are deliberately not given a slot, so they live in the buckets and don't widen + * {@code knownEntries[]} for every span. {@code getEntry(long)} for these resolves the name + * and rehashes — the cost of not owning a slot. + *
    + */ + public static final int NO_SLOT = 0xFFFF; + + /** + * True if the tagId names a stored tag that deliberately has no positional slot (bucket-only). + */ + public static boolean isUnslotted(long tagId) { + return isStored(tagId) && fieldPos(tagId) == NO_SLOT; + } + + /** + * Builds a tagId from its parts: {@code globalSerial} (globally unique per known tag), {@code + * fieldPos} (the tag's slot within its span type's positional table), and the tag {@code name} + * (whose hash is computed via the same function the runtime uses, so the low 32 bits match {@link + * TagMap.Entry#hash()}). Inverse of {@link #globalSerial}/{@link #fieldPos}/{@link #nameHash}. + * Intended for the code generator and tests. + */ + public static long tagId(int globalSerial, int fieldPos, String name) { + long nameHash = TagMap.Entry._hash(name) & 0xFFFFFFFFL; + return ((long) globalSerial << 48) | ((long) (fieldPos & 0xFFFF) << 32) | nameHash; + } + + /** + * Builds a tagId with no positional slot ({@code fieldPos == }{@link #NO_SLOT}). Use for reserved + * "virtual" tags and for "low-priority" stored tags that get a stable id but are intentionally + * kept out of the fast slot array (they route to the hash buckets). See {@link #NO_SLOT}. + */ + public static long tagId(int globalSerial, String name) { + return tagId(globalSerial, NO_SLOT, name); + } + + // Number of positional slots in the global layout = (max stored fieldPos) + 1, declared by the + // registered provider. Captured once at registration and read as a dynamic constant; TagMap sizes + // its knownEntries array to exactly this rather than a hardcoded max. 0 when no resolver. + private static int slotCount; + + /** Slot count of the registered provider (max stored fieldPos + 1); 0 if none. */ + public static int slotCount() { + return slotCount; + } + + public interface Resolver { + String nameOf(long tagId); + + long keyOf(String name); + + /** Number of positional slots this provider uses: (max stored fieldPos) + 1. */ + int slotCount(); + } + + @SuppressFBWarnings( + value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", + justification = + "active/slotCount are plain by design: written once at tracer-init registration (before" + + " any span processing) and read plain on the hot path. A stale read is benign — the" + + " tag is treated as unknown and takes the hash-bucket path — so plain reads are" + + " deliberately preferred over a costly volatile read on weak memory models.") + public static void register(Resolver resolver) { + KnownTagCodec.resolver = resolver; // volatile write publishes the resolver + KnownTagCodec.slotCount = (resolver != null) ? resolver.slotCount() : 0; + KnownTagCodec.active = + (resolver != null); // plain write; readers re-read resolver volatile anyway + } + + public static String nameOf(long tagId) { + if (!active) return null; + Resolver r = resolver; + return r != null ? r.nameOf(tagId) : null; + } + + public static long keyOf(String name) { + if (!active) return 0L; + Resolver r = resolver; + return r != null ? r.keyOf(name) : 0L; + } + + private KnownTagCodec() {} +} diff --git a/internal-api/src/main/java/datadog/trace/api/KnownTags.java b/internal-api/src/main/java/datadog/trace/api/KnownTags.java new file mode 100644 index 00000000000..391b000b844 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/KnownTags.java @@ -0,0 +1,328 @@ +package datadog.trace.api; + +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.util.StringIndex; + +/** + * Hand-assigned tag-id constants for well-known tags, plus the {@link KnownTagCodec.Resolver} that + * resolves them. This is the single registry shared by the tracer core and by instrumentation + * (decorators) — it lives in {@code internal-api} so both layers can reference the ids; the + * eventual code generator will replace the hand assignment here. + * + *

    Reserved serials {@code [1, KnownTagCodec.FIRST_STORED_SERIAL)} name "virtual" tags handled by + * the tag interceptor / span fields and are NOT stored in the {@code TagMap}; their {@code + * fieldPos} is the {@link KnownTagCodec#NO_SLOT} sentinel that is out of slot range, so any + * incidental store routes to the hash buckets rather than a positional slot. Serials {@code >= + * FIRST_STORED_SERIAL} name stored tags that slot/bucket normally (or, with {@code NO_SLOT}, are + * stored bucket-only). + * + *

    The resolver registers on class initialization, so simply referencing any constant here makes + * tag-id resolution live before the first span is built. + * + *

    Slice-1 note (keyOf substrate): the {@code fieldPos} assignments below (and {@link + * #SLOT_COUNT}) describe a single universal positional layout (slots 0..25). That layout is + * currently dormant — no dense store consumes {@code fieldPos} yet — and is provisional: the + * dense-store slice replaces the universal layout with per-role / per-type sizing (see the + * over-provision finding in {@code dense-tagmap-design.md}). {@code keyOf}/{@code nameOf} depend + * only on {@code globalSerial} + name, not {@code fieldPos}, so the ids themselves are stable + * across any layout scheme. + */ +public final class KnownTags { + // slot count = (max stored fieldPos) + 1. Stored tags use fieldPos 0..25. PROVISIONAL universal + // layout — see the slice-1 note above; the dense-store slice supersedes this with role/type + // sizing. + static final int SLOT_COUNT = 26; + + // ---- reserved / virtual (tag-interceptor handled, not stored) ---- + // Reserved tags are always intercepted -> set the INTERCEPTED flag. + public static final int ERROR_SERIAL = 1; + public static final long ERROR_ID = + KnownTagCodec.intercepted(KnownTagCodec.tagId(ERROR_SERIAL, Tags.ERROR)); + + // ---- stored (slotted / bucketed) ---- + public static final int PARENT_ID_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL; + public static final long PARENT_ID = KnownTagCodec.tagId(PARENT_ID_SERIAL, 0, DDTags.PARENT_ID); + + // common (process-constant) tags added by InternalTagsAdder to ~every span + public static final int BASE_SERVICE_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 1; + public static final long BASE_SERVICE_ID = + KnownTagCodec.tagId(BASE_SERVICE_SERIAL, 1, DDTags.BASE_SERVICE); + + public static final int VERSION_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 2; + public static final long VERSION_ID = KnownTagCodec.tagId(VERSION_SERIAL, 2, Tags.VERSION); + + // build-time-known constant tags merged into defaultSpanTags (see CoreTracer.withTracerTags). + // "env" is a base-mixin tag; the *_ENABLED flags are product-mixin tags. Hand-assigned for now. + public static final String ENV = "env"; + public static final int ENV_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 3; + public static final long ENV_ID = KnownTagCodec.tagId(ENV_SERIAL, 3, ENV); + + public static final int DJM_ENABLED_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 4; + public static final long DJM_ENABLED_ID = + KnownTagCodec.tagId(DJM_ENABLED_SERIAL, 4, DDTags.DJM_ENABLED); + + public static final int DSM_ENABLED_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 5; + public static final long DSM_ENABLED_ID = + KnownTagCodec.tagId(DSM_ENABLED_SERIAL, 5, DDTags.DSM_ENABLED); + + // common tags added by the tag post-processors (RemoteHostnameAdder / IntegrationAdder / + // ServiceNameSourceAdder). Not intercepted; stored. + public static final int TRACER_HOST_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 6; + public static final long TRACER_HOST_ID = + KnownTagCodec.tagId(TRACER_HOST_SERIAL, 6, DDTags.TRACER_HOST); + + public static final int INTEGRATION_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 7; + public static final long INTEGRATION_ID = + KnownTagCodec.tagId(INTEGRATION_SERIAL, 7, DDTags.DD_INTEGRATION); + + public static final int SVC_SRC_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 8; + public static final long SVC_SRC_ID = KnownTagCodec.tagId(SVC_SRC_SERIAL, 8, DDTags.DD_SVC_SRC); + + // peer.service tags, read/written by PeerServiceCalculator (post-processor; uses Map put/get that + // bypass the interceptor). peer.service is intercepted on the set-path but STORED, so it slots. + public static final int PEER_SERVICE_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 9; + public static final long PEER_SERVICE_ID = + KnownTagCodec.intercepted(KnownTagCodec.tagId(PEER_SERVICE_SERIAL, 9, Tags.PEER_SERVICE)); + + public static final int PEER_SERVICE_REMAPPED_FROM_SERIAL = + KnownTagCodec.FIRST_STORED_SERIAL + 10; + public static final long PEER_SERVICE_REMAPPED_FROM_ID = + KnownTagCodec.tagId(PEER_SERVICE_REMAPPED_FROM_SERIAL, 10, DDTags.PEER_SERVICE_REMAPPED_FROM); + + // HTTP tags read by HttpEndpointPostProcessor. http.method/http.url are intercepted-but-stored + // (interceptTag side-effects then returns false → stored); http.route is not intercepted. All + // stored, so the string set-path slots them via keyOf and the id reads here find them. + public static final int HTTP_METHOD_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 11; + public static final long HTTP_METHOD_ID = + KnownTagCodec.intercepted(KnownTagCodec.tagId(HTTP_METHOD_SERIAL, 11, Tags.HTTP_METHOD)); + + public static final int HTTP_ROUTE_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 12; + public static final long HTTP_ROUTE_ID = + KnownTagCodec.tagId(HTTP_ROUTE_SERIAL, 12, Tags.HTTP_ROUTE); + + public static final int HTTP_URL_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 13; + public static final long HTTP_URL_ID = + KnownTagCodec.intercepted(KnownTagCodec.tagId(HTTP_URL_SERIAL, 13, Tags.HTTP_URL)); + + // peer connection tags set by BaseDecorator.onPeerConnection on ~every client/producer span. + // Not intercepted; stored. Slotted (common across client instrumentations). + public static final int PEER_HOSTNAME_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 14; + public static final long PEER_HOSTNAME_ID = + KnownTagCodec.tagId(PEER_HOSTNAME_SERIAL, 14, Tags.PEER_HOSTNAME); + + public static final int PEER_HOST_IPV4_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 15; + public static final long PEER_HOST_IPV4_ID = + KnownTagCodec.tagId(PEER_HOST_IPV4_SERIAL, 15, Tags.PEER_HOST_IPV4); + + public static final int PEER_HOST_IPV6_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 16; + public static final long PEER_HOST_IPV6_ID = + KnownTagCodec.tagId(PEER_HOST_IPV6_SERIAL, 16, Tags.PEER_HOST_IPV6); + + public static final int PEER_PORT_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 17; + public static final long PEER_PORT_ID = KnownTagCodec.tagId(PEER_PORT_SERIAL, 17, Tags.PEER_PORT); + + // Universal decorator tags — set on ~every span (component/span.kind via Base/Server/Client + // decorators, language via ServerDecorator). span.kind is intercepted (setSpanKindOrdinal). + public static final int COMPONENT_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 18; + public static final long COMPONENT_ID = KnownTagCodec.tagId(COMPONENT_SERIAL, 18, Tags.COMPONENT); + + public static final int SPAN_KIND_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 19; + public static final long SPAN_KIND_ID = + KnownTagCodec.intercepted(KnownTagCodec.tagId(SPAN_KIND_SERIAL, 19, Tags.SPAN_KIND)); + + public static final int LANGUAGE_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 20; + public static final long LANGUAGE_ID = + KnownTagCodec.tagId(LANGUAGE_SERIAL, 20, DDTags.LANGUAGE_TAG_KEY); + + // JDBC / database-client tags — set on every db span (58% of petclinic spans). Not intercepted + // (only db.statement is, and that's handled separately). + public static final int DB_TYPE_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 21; + public static final long DB_TYPE_ID = KnownTagCodec.tagId(DB_TYPE_SERIAL, 21, Tags.DB_TYPE); + + public static final int DB_INSTANCE_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 22; + public static final long DB_INSTANCE_ID = + KnownTagCodec.tagId(DB_INSTANCE_SERIAL, 22, Tags.DB_INSTANCE); + + public static final int DB_USER_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 23; + public static final long DB_USER_ID = KnownTagCodec.tagId(DB_USER_SERIAL, 23, Tags.DB_USER); + + public static final int DB_OPERATION_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 24; + public static final long DB_OPERATION_ID = + KnownTagCodec.tagId(DB_OPERATION_SERIAL, 24, Tags.DB_OPERATION); + + public static final int DB_POOL_NAME_SERIAL = KnownTagCodec.FIRST_STORED_SERIAL + 25; + public static final long DB_POOL_NAME_ID = + KnownTagCodec.tagId(DB_POOL_NAME_SERIAL, 25, Tags.DB_POOL_NAME); + + // Open-addressed name -> id table backing keyOf (data, not a switch): scales flat as the known + // set grows, where a generated switch eventually falls off the inline threshold. KEYOF_NAMES and + // KEYOF_VALUES are parallel; the table places names by hash and a parallel ids[] by slot. + private static final String[] KEYOF_NAMES = { + Tags.ERROR, + DDTags.PARENT_ID, + DDTags.BASE_SERVICE, + Tags.VERSION, + ENV, + DDTags.DJM_ENABLED, + DDTags.DSM_ENABLED, + DDTags.TRACER_HOST, + DDTags.DD_INTEGRATION, + DDTags.DD_SVC_SRC, + Tags.PEER_SERVICE, + DDTags.PEER_SERVICE_REMAPPED_FROM, + Tags.HTTP_METHOD, + Tags.HTTP_ROUTE, + Tags.HTTP_URL, + Tags.PEER_HOSTNAME, + Tags.PEER_HOST_IPV4, + Tags.PEER_HOST_IPV6, + Tags.PEER_PORT, + Tags.COMPONENT, + Tags.SPAN_KIND, + DDTags.LANGUAGE_TAG_KEY, + Tags.DB_TYPE, + Tags.DB_INSTANCE, + Tags.DB_USER, + Tags.DB_OPERATION, + Tags.DB_POOL_NAME, + }; + + private static final long[] KEYOF_VALUES = { + ERROR_ID, + PARENT_ID, + BASE_SERVICE_ID, + VERSION_ID, + ENV_ID, + DJM_ENABLED_ID, + DSM_ENABLED_ID, + TRACER_HOST_ID, + INTEGRATION_ID, + SVC_SRC_ID, + PEER_SERVICE_ID, + PEER_SERVICE_REMAPPED_FROM_ID, + HTTP_METHOD_ID, + HTTP_ROUTE_ID, + HTTP_URL_ID, + PEER_HOSTNAME_ID, + PEER_HOST_IPV4_ID, + PEER_HOST_IPV6_ID, + PEER_PORT_ID, + COMPONENT_ID, + SPAN_KIND_ID, + LANGUAGE_ID, + DB_TYPE_ID, + DB_INSTANCE_ID, + DB_USER_ID, + DB_OPERATION_ID, + DB_POOL_NAME_ID, + }; + + // Static-final raw arrays placed by StringIndex.EmbeddingSupport: the JIT folds these refs to + // constants on + // the keyOf hot path (the fastest of StringIndex's three usage modes — no instance dereference). + private static final int[] KEYOF_HASHES; + private static final String[] KEYOF_KEYS; + private static final long[] KEYOF_IDS; + + static { + StringIndex.Data data = StringIndex.EmbeddingSupport.create(KEYOF_NAMES); + long[] ids = new long[data.names.length]; + for (int j = 0; j < KEYOF_NAMES.length; j++) { + ids[StringIndex.EmbeddingSupport.indexOf(data.hashes, data.names, KEYOF_NAMES[j])] = + KEYOF_VALUES[j]; + } + KEYOF_HASHES = data.hashes; + KEYOF_KEYS = data.names; + KEYOF_IDS = ids; + } + + static final KnownTagCodec.Resolver RESOLVER = + new KnownTagCodec.Resolver() { + @Override + public String nameOf(long tagId) { + switch (KnownTagCodec.globalSerial(tagId)) { + case ERROR_SERIAL: + return Tags.ERROR; + case PARENT_ID_SERIAL: + return DDTags.PARENT_ID; + case BASE_SERVICE_SERIAL: + return DDTags.BASE_SERVICE; + case VERSION_SERIAL: + return Tags.VERSION; + case ENV_SERIAL: + return ENV; + case DJM_ENABLED_SERIAL: + return DDTags.DJM_ENABLED; + case DSM_ENABLED_SERIAL: + return DDTags.DSM_ENABLED; + case TRACER_HOST_SERIAL: + return DDTags.TRACER_HOST; + case INTEGRATION_SERIAL: + return DDTags.DD_INTEGRATION; + case SVC_SRC_SERIAL: + return DDTags.DD_SVC_SRC; + case PEER_SERVICE_SERIAL: + return Tags.PEER_SERVICE; + case PEER_SERVICE_REMAPPED_FROM_SERIAL: + return DDTags.PEER_SERVICE_REMAPPED_FROM; + case HTTP_METHOD_SERIAL: + return Tags.HTTP_METHOD; + case HTTP_ROUTE_SERIAL: + return Tags.HTTP_ROUTE; + case HTTP_URL_SERIAL: + return Tags.HTTP_URL; + case PEER_HOSTNAME_SERIAL: + return Tags.PEER_HOSTNAME; + case PEER_HOST_IPV4_SERIAL: + return Tags.PEER_HOST_IPV4; + case PEER_HOST_IPV6_SERIAL: + return Tags.PEER_HOST_IPV6; + case PEER_PORT_SERIAL: + return Tags.PEER_PORT; + case COMPONENT_SERIAL: + return Tags.COMPONENT; + case SPAN_KIND_SERIAL: + return Tags.SPAN_KIND; + case LANGUAGE_SERIAL: + return DDTags.LANGUAGE_TAG_KEY; + case DB_TYPE_SERIAL: + return Tags.DB_TYPE; + case DB_INSTANCE_SERIAL: + return Tags.DB_INSTANCE; + case DB_USER_SERIAL: + return Tags.DB_USER; + case DB_OPERATION_SERIAL: + return Tags.DB_OPERATION; + case DB_POOL_NAME_SERIAL: + return Tags.DB_POOL_NAME; + default: + return null; + } + } + + @Override + public int slotCount() { + return SLOT_COUNT; + } + + @Override + public long keyOf(String name) { + int slot = StringIndex.EmbeddingSupport.indexOf(KEYOF_HASHES, KEYOF_KEYS, name); + return slot < 0 ? 0L : KEYOF_IDS[slot]; + } + }; + + static { + KnownTagCodec.register(RESOLVER); + } + + /** + * Forces resolver registration. Merely invoking this static method runs {@code } (which + * registers {@link #RESOLVER}), so calling it once at tracer init flips the dense store live; + * idempotent. Until something references this class the registry stays dormant and {@code keyOf} + * returns 0, so tag storage is byte-identical to the bucket-only behavior. + */ + public static void init() {} + + private KnownTags() {} +} diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 2f519b75311..e4c00630cec 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -47,10 +47,9 @@ */ public final class TagMap implements Map, Iterable { /** Immutable empty TagMap - similar to {@link Collections#emptyMap()} */ - // Frozen view over a length-1 array: bucket masking needs a power-of-two array length (size 0 - // would fail with ArrayIndexOutOfBoundsException, size 1 works), and the private constructor - // reads no statics, so this is safe to build directly during TagMap's . - public static final TagMap EMPTY = new TagMap(new Object[1], 0); + // Frozen view over a power-of-two array; the private constructor reads no statics, so this is + // safe to build directly during TagMap's . + public static final TagMap EMPTY = new TagMap(new Object[1 << 4], 0); /** Creates a new mutable TagMap that contains the contents of map */ public static final TagMap fromMap(@Nonnull Map map) { @@ -1005,10 +1004,43 @@ public EntryChange next() { * removed from the collision chain. */ - private final Object[] buckets; + // Shared immutable empty buckets (all null, length 16). Every map points here until its first + // custom-tag write copies-on-write to a private array (materializeBuckets), so an all-known / + // known-heavy map (e.g. the trace-tier read-through parent) allocates ZERO buckets. Length is + // always 16, so reads need no null guard and read-through bucket alignment (hash & 15) holds. + private static final Object[] EMPTY_BUCKETS = new Object[1 << 4]; + + private Object[] buckets; private int size; private boolean frozen; + /** + * Dense known-tag store (dense-tagmap-design §5). Values for KNOWN tags (those {@link + * KnownTagCodec#keyOf} resolves to a stored id) live in these INSERTION-ORDERED parallel arrays + * with NO per-tag {@link Entry} object — the allocation win. Lazily allocated on the first + * known-tag write ({@code null} until then, so all-unknown maps pay nothing) and grown x2 from + * {@link #KNOWN_INIT_CAP}. Matched by globalSerial via a linear scan ({@link #knownIndexOf}); + * reads aren't hot, so O(knownCount) is fine and positional indexing is deferred. Dormant until a + * resolver is registered: {@code keyOf} returns 0, so nothing routes here and production is + * byte-identical. + * + *

    Disjoint from {@link #buckets} by construction: known-ness is global ({@code keyOf} is + * deterministic), so a known tag is ALWAYS dense and never bucketed, and vice-versa. That + * disjointness keeps read-through shadow checks within-region — a parent dense entry can only be + * shadowed by a local dense entry of the same id, a parent bucket entry only by a local bucket + * entry — so the existing bucket read-through code is unchanged. + * + *

    {@link #size} counts bucket entries only; {@link #knownCount} counts dense entries; the + * local total is {@code size + knownCount}. + */ + private long[] knownIds; + + private Object[] knownValues; + private int knownCount; + + private static final int KNOWN_INIT_CAP = + 12; // generous per-type max stopgap; exact per-type sizing comes with the tag registry + /** * Optional frozen parent for read-through (level-split phase 1). When non-null, reads that miss * the local buckets fall through to the parent; a local entry shadows the parent's (local-wins). @@ -1038,8 +1070,9 @@ public TagMap() { * can treat it as fixed. */ private TagMap(TagMap parent) { - // needs to be a power of 2 for bucket masking calculation to work as intended - this.buckets = new Object[1 << 4]; + // Start on the shared empty buckets; materializeBuckets() COWs to a private power-of-two array + // on the first custom-tag write. All-known maps never allocate buckets. + this.buckets = EMPTY_BUCKETS; this.size = 0; this.frozen = false; this.parent = parent; @@ -1060,17 +1093,24 @@ public boolean isOptimized() { @Override public int size() { // Exact (Map contract). Under read-through resolves the union; prefer estimateSize() for hints. + int local = this.size + this.knownCount; // buckets + dense TagMap parent = this.parent; - return parent == null ? this.size : this.size + this.visibleParentCount(); + return parent == null ? local : local + this.visibleParentCount(); } /** * Exact count of parent entries not shadowed locally or tombstoned (the read-through addition). */ private int visibleParentCount() { + int count = 0; + // parent dense entries not shadowed by a local dense entry / tombstoned + long[] parentIds = this.parent.knownIds; + int parentKnownCount = this.parent.knownCount; + for (int i = 0; i < parentKnownCount; ++i) { + if (!this.parentDenseHidden(parentIds[i])) count++; + } Object[] parentBuckets = this.parent.buckets; Object[] thisBuckets = this.buckets; - int count = 0; for (int i = 0; i < parentBuckets.length; ++i) { Object parentBucket = parentBuckets[i]; Object localBucket = thisBuckets[i]; @@ -1094,7 +1134,7 @@ private int visibleParentCount() { @Override public boolean isEmpty() { // Exact (Map contract). Under read-through resolves the parent; prefer isDefinitelyEmpty(). - if (this.size != 0) { + if (this.size != 0 || this.knownCount != 0) { return false; } TagMap parent = this.parent; @@ -1110,12 +1150,15 @@ public boolean isEmpty() { } public boolean isDefinitelyEmpty() { - return this.size == 0 && (this.parent == null || this.parent.isDefinitelyEmpty()); + return this.size == 0 + && this.knownCount == 0 + && (this.parent == null || this.parent.isDefinitelyEmpty()); } public int estimateSize() { - // Upper bound: local + parent, ignoring read-through shadowing/removals (over-counts). - return this.parent == null ? this.size : this.size + this.parent.estimateSize(); + // Upper bound: local (buckets + dense) + parent, ignoring shadowing/removals (over-counts). + int local = this.size + this.knownCount; + return this.parent == null ? local : local + this.parent.estimateSize(); } @Deprecated @@ -1241,8 +1284,15 @@ public Entry getEntry(String tag) { return parent.getEntry(tag); } - /** Looks up an entry in this map's own buckets only — no read-through to the parent. */ + /** Looks up an entry in this map's own storage only (dense then buckets) — no read-through. */ private Entry getLocalEntry(String tag) { + // Known tags live in the dense store; resolve identity and check there first. keyOf is a no-op + // (returns 0 -> isStored false) until a resolver is registered, so this is inert in production. + long id = KnownTagCodec.keyOf(tag); + if (KnownTagCodec.isStored(id)) { + Object known = this.knownRawValue(id); + return known == null ? null : Entry.newAnyEntry(tag, known); + } Object[] thisBuckets = this.buckets; int hash = TagMap.Entry._hash(tag); return findInBucket(thisBuckets[hash & (thisBuckets.length - 1)], hash, tag); @@ -1276,10 +1326,94 @@ private boolean parentEntryVisibleInBucket(Object localBucket, Entry parentEntry == null; // not shadowed by a local entry } + // ---- dense known-tag store (see the knownIds field doc) + // ---------------------------------------- + + /** + * Linear scan of the dense store for {@code tagId}, returning its index or -1. Ids are canonical + * (the only way one enters is {@link KnownTagCodec#keyOf} or a {@code KnownTags} constant, both + * canonical), so a full {@code long} compare is exact and cheaper than extracting globalSerial. + */ + private int knownIndexOf(long tagId) { + long[] ids = this.knownIds; + int n = this.knownCount; + for (int i = 0; i < n; ++i) { + if (ids[i] == tagId) return i; + } + return -1; + } + + private void ensureKnownCapacity() { + if (this.knownIds == null) { + this.knownIds = new long[KNOWN_INIT_CAP]; + this.knownValues = new Object[KNOWN_INIT_CAP]; + } else if (this.knownCount == this.knownIds.length) { + int newCap = this.knownIds.length << 1; + this.knownIds = Arrays.copyOf(this.knownIds, newCap); + this.knownValues = Arrays.copyOf(this.knownValues, newCap); + } + } + + /** + * Stores a known tag's value densely (no {@link Entry} alloc). Overwrites in place when present + * (returning the prior value materialized as an Entry, per the {@code Map} contract — usually + * discarded by {@code set}); otherwise appends, growing x2 as needed. + */ + private Entry putKnownValue(long tagId, Object value) { + int i = this.knownIndexOf(tagId); + if (i >= 0) { + Object prior = this.knownValues[i]; + this.knownValues[i] = value; + return materializeKnown(tagId, prior); + } + this.ensureKnownCapacity(); + int slot = this.knownCount++; + this.knownIds[slot] = tagId; + this.knownValues[slot] = value; + return null; + } + + /** Raw dense value for {@code tagId}, or {@code null} when absent (no Entry, no boxing). */ + private Object knownRawValue(long tagId) { + int i = this.knownIndexOf(tagId); + return i < 0 ? null : this.knownValues[i]; + } + + /** + * Removes a known tag from the dense store (swap-with-last), returning the prior Entry or null. + */ + private Entry removeKnown(long tagId) { + int i = this.knownIndexOf(tagId); + if (i < 0) return null; + Object prior = this.knownValues[i]; + int last = --this.knownCount; + this.knownIds[i] = this.knownIds[last]; + this.knownValues[i] = this.knownValues[last]; + this.knownIds[last] = 0L; + this.knownValues[last] = null; + return materializeKnown(tagId, prior); + } + + /** Materializes a transient Entry for a dense (id, value) pair — only on explicit get/iterate. */ + private static Entry materializeKnown(long tagId, Object value) { + return Entry.newAnyEntry(KnownTagCodec.nameOf(tagId), value); + } + + /** + * Whether a parent dense entry is hidden through this child: shadowed by a local dense entry of + * the same id, or tombstoned. (Disjointness means a parent dense entry can't be shadowed by a + * local bucket entry — known tags never bucket — so no bucket check is needed here.) + */ + private boolean parentDenseHidden(long tagId) { + if (this.knownIndexOf(tagId) >= 0) return true; // shadowed by a local dense entry + return this.removedFromParent != null + && this.removedFromParent.contains(KnownTagCodec.nameOf(tagId)); // tombstoned + } + @Deprecated @Override public Object put(@Nonnull String tag, Object value) { - TagMap.Entry entry = this.getAndSet(Entry.newAnyEntry(tag, value)); + TagMap.Entry entry = this.getAndSet(tag, value); return entry == null ? null : entry.objectValue(); } @@ -1291,32 +1425,35 @@ public void set(@Nullable TagMap.EntryReader newEntryReader) { this.getAndSet(newEntryReader.entry()); } + // The set(String, ...) family delegates to the matching getAndSet(String, ...) overload, which + // routes known tags to the dense store BEFORE constructing any Entry (so a known-tag set + // allocates no Entry). The discarded return is free on the common first-set path (prior == null). public void set(@Nonnull String tag, @Nonnull Object value) { - this.getAndSet(Entry.newAnyEntry(tag, value)); + this.getAndSet(tag, value); } public void set(@Nonnull String tag, @Nonnull CharSequence value) { - this.getAndSet(Entry.newObjectEntry(tag, value)); + this.getAndSet(tag, value); } public void set(@Nonnull String tag, boolean value) { - this.getAndSet(Entry.newBooleanEntry(tag, value)); + this.getAndSet(tag, value); } public void set(@Nonnull String tag, int value) { - this.getAndSet(Entry.newIntEntry(tag, value)); + this.getAndSet(tag, value); } public void set(@Nonnull String tag, long value) { - this.getAndSet(Entry.newLongEntry(tag, value)); + this.getAndSet(tag, value); } public void set(@Nonnull String tag, float value) { - this.getAndSet(Entry.newFloatEntry(tag, value)); + this.getAndSet(tag, value); } public void set(@Nonnull String tag, double value) { - this.getAndSet(Entry.newDoubleEntry(tag, value)); + this.getAndSet(tag, value); } /** @@ -1329,7 +1466,40 @@ public Entry getAndSet(@Nullable Entry newEntry) { if (newEntry == null) { return null; } + // Entry-based path (set(EntryReader), entry-sharing). The Entry is already constructed by the + // caller, so a known tag keeps its value densely and drops the Entry. The hot string/typed + // setters route to dense BEFORE constructing an Entry (see set/getAndSet(String, ...)) so a + // known-tag set allocates no Entry at all. + long id = KnownTagCodec.keyOf(newEntry.tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, newEntry.tag, newEntry.objectValue()) + : this.getAndSetBucket(newEntry); + } + + /** + * Stores a known tag's (resolved id, value) densely with NO Entry retained — the alloc win. + * Returns the prior value materialized as an Entry (Map contract); {@code set} discards it. + */ + private Entry getAndSetKnown(long id, String tag, Object value) { + this.checkWriteAccess(); + if (this.removedFromParent != null) { + this.removedFromParent.remove(tag); + } + return this.putKnownValue(id, value); + } + + /** Copy-on-write the shared empty buckets to a private array on the first bucket write. */ + private Object[] materializeBuckets() { + Object[] b = this.buckets; + if (b == EMPTY_BUCKETS) { + b = new Object[1 << 4]; + this.buckets = b; + } + return b; + } + /** Stores an entry in the hash buckets — the unknown/custom-tag path. */ + private Entry getAndSetBucket(Entry newEntry) { this.checkWriteAccess(); // Re-setting a key clears any read-through tombstone for it (the new value overrides the @@ -1338,7 +1508,7 @@ public Entry getAndSet(@Nullable Entry newEntry) { this.removedFromParent.remove(newEntry.tag); } - Object[] thisBuckets = this.buckets; + Object[] thisBuckets = this.materializeBuckets(); int newHash = newEntry.hash(); int bucketIndex = newHash & (thisBuckets.length - 1); @@ -1383,32 +1553,56 @@ public Entry getAndSet(@Nullable Entry newEntry) { return null; } + // Each getAndSet(String, ...) resolves keyOf FIRST: a known tag stores its value densely with no + // Entry (boxing the primitive only on this branch); a custom tag falls back to the typed Entry + // (no boxing for primitives, preserving the bucket store's no-box property). public Entry getAndSet(@Nonnull String tag, Object value) { - return this.getAndSet(Entry.newAnyEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, value) + : this.getAndSetBucket(Entry.newAnyEntry(tag, value)); } public Entry getAndSet(@Nonnull String tag, CharSequence value) { - return this.getAndSet(Entry.newObjectEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, value) + : this.getAndSetBucket(Entry.newObjectEntry(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, boolean value) { - return this.getAndSet(Entry.newBooleanEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, Boolean.valueOf(value)) + : this.getAndSetBucket(Entry.newBooleanEntry(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, int value) { - return this.getAndSet(Entry.newIntEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, Integer.valueOf(value)) + : this.getAndSetBucket(Entry.newIntEntry(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, long value) { - return this.getAndSet(Entry.newLongEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, Long.valueOf(value)) + : this.getAndSetBucket(Entry.newLongEntry(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, float value) { - return this.getAndSet(Entry.newFloatEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, Float.valueOf(value)) + : this.getAndSetBucket(Entry.newFloatEntry(tag, value)); } public TagMap.Entry getAndSet(@Nonnull String tag, double value) { - return this.getAndSet(Entry.newDoubleEntry(tag, value)); + long id = KnownTagCodec.keyOf(tag); + return KnownTagCodec.isStored(id) + ? this.getAndSetKnown(id, tag, Double.valueOf(value)) + : this.getAndSetBucket(Entry.newDoubleEntry(tag, value)); } public void putAll(Map map) { @@ -1442,7 +1636,9 @@ public void putAll(TagMap that) { } private void putAllOptimizedMap(TagMap that) { - if (this.size == 0) { + // "empty" must consider BOTH local regions — a map with only dense entries has size == 0 but is + // not empty, and putAllIntoEmptyMap would clobber its dense store. + if (this.size == 0 && this.knownCount == 0) { this.putAllIntoEmptyMap(that); } else { this.putAllMerge(that); @@ -1450,7 +1646,9 @@ private void putAllOptimizedMap(TagMap that) { } private void putAllMerge(TagMap that) { - Object[] thisBuckets = this.buckets; + // COW our buckets only if the source has bucket entries to merge in; otherwise the loop below + // writes nothing and the shared empty buckets stay shared. + Object[] thisBuckets = (that.size > 0) ? this.materializeBuckets() : this.buckets; Object[] thatBuckets = that.buckets; // Since TagMap-s don't support expansion, buckets are perfectly aligned @@ -1561,33 +1759,49 @@ private void putAllMerge(TagMap that) { } } } + + // merge the source's dense known-tag entries; incoming clobbers existing (same as buckets) + for (int i = 0; i < that.knownCount; ++i) { + this.putKnownValue(that.knownIds[i], that.knownValues[i]); + } } /* * Specially optimized version of putAll for the common case of destination map being empty */ private void putAllIntoEmptyMap(TagMap that) { - Object[] thisBuckets = this.buckets; - Object[] thatBuckets = that.buckets; - - // Check against both thisBuckets.length && thatBuckets.length is to help the JIT do bound check - // elimination - for (int i = 0; i < thisBuckets.length && i < thatBuckets.length; ++i) { - Object thatBucket = thatBuckets[i]; - - // faster to explicitly null check first, then do instanceof - if (thatBucket == null) { - // do nothing - } else if (thatBucket instanceof BucketGroup) { - // if it is a BucketGroup, then need to clone - BucketGroup thatGroup = (BucketGroup) thatBucket; + // Only copy buckets (and COW ours) when the source actually has bucket entries; an all-known + // source leaves us on the shared empty buckets. + if (that.size > 0) { + Object[] thisBuckets = this.materializeBuckets(); + Object[] thatBuckets = that.buckets; + + // Check against both thisBuckets.length && thatBuckets.length is to help the JIT do bound + // check elimination + for (int i = 0; i < thisBuckets.length && i < thatBuckets.length; ++i) { + Object thatBucket = thatBuckets[i]; + + // faster to explicitly null check first, then do instanceof + if (thatBucket == null) { + // do nothing + } else if (thatBucket instanceof BucketGroup) { + // if it is a BucketGroup, then need to clone + BucketGroup thatGroup = (BucketGroup) thatBucket; - thisBuckets[i] = thatGroup.cloneChain(); - } else { // if ( thatBucket instanceof Entry ) - thisBuckets[i] = thatBucket; + thisBuckets[i] = thatGroup.cloneChain(); + } else { // if ( thatBucket instanceof Entry ) + thisBuckets[i] = thatBucket; + } } + this.size = that.size; + } + + // clone the dense known-tag store (values are immutable boxes/objects -> safe to share refs) + if (that.knownCount > 0) { + this.knownIds = Arrays.copyOf(that.knownIds, that.knownIds.length); + this.knownValues = Arrays.copyOf(that.knownValues, that.knownValues.length); + this.knownCount = that.knownCount; } - this.size = that.size; } public void fillMap(Map map) { @@ -1606,6 +1820,9 @@ public void fillMap(Map map) { thisGroup.fillMapFromChain(map); } } + for (int i = 0; i < this.knownCount; ++i) { + map.put(KnownTagCodec.nameOf(this.knownIds[i]), this.knownValues[i]); + } } public void fillStringMap(Map stringMap) { @@ -1624,6 +1841,11 @@ public void fillStringMap(Map stringMap) { thisGroup.fillStringMapFromChain(stringMap); } } + for (int i = 0; i < this.knownCount; ++i) { + stringMap.put( + KnownTagCodec.nameOf(this.knownIds[i]), + TagValueConversions.toString(this.knownValues[i])); + } } @Override @@ -1665,8 +1887,13 @@ public Entry getAndRemove(String tag) { return localRemoved; } - /** Removes an entry from this map's own buckets only — no parent/tombstone handling. */ + /** Removes an entry from this map's own storage only — no parent/tombstone handling. */ private Entry removeLocal(String tag) { + long id = KnownTagCodec.keyOf(tag); + if (KnownTagCodec.isStored(id)) { + return this.removeKnown(id); + } + Object[] thisBuckets = this.buckets; int hash = TagMap.Entry._hash(tag); @@ -1734,6 +1961,15 @@ public Stream stream() { @Override public void forEach(Consumer consumer) { + // local dense known tags via a reused flyweight (no per-entry Entry alloc — the serialize win) + if (this.knownCount > 0) { + EntryReadingHelper reader = new EntryReadingHelper(); + for (int i = 0; i < this.knownCount; ++i) { + reader.set(KnownTagCodec.nameOf(this.knownIds[i]), this.knownValues[i]); + consumer.accept(reader); + } + } + Object[] thisBuckets = this.buckets; for (int i = 0; i < thisBuckets.length; ++i) { @@ -1758,6 +1994,21 @@ public void forEach(Consumer consumer) { } private void forEachParent(Consumer consumer) { + // parent dense known tags not shadowed by a local dense entry / tombstoned + long[] parentIds = this.parent.knownIds; + int parentKnownCount = this.parent.knownCount; + if (parentKnownCount > 0) { + Object[] parentValues = this.parent.knownValues; + EntryReadingHelper reader = new EntryReadingHelper(); + for (int i = 0; i < parentKnownCount; ++i) { + long id = parentIds[i]; + if (!this.parentDenseHidden(id)) { + reader.set(KnownTagCodec.nameOf(id), parentValues[i]); + consumer.accept(reader); + } + } + } + Object[] localBuckets = this.buckets; Object[] parentBuckets = this.parent.buckets; // leaf parent: same length, same bucket per key for (int i = 0; i < parentBuckets.length; ++i) { @@ -1781,6 +2032,14 @@ private void forEachParent(Consumer consumer) { } public void forEach(T thisObj, BiConsumer consumer) { + if (this.knownCount > 0) { + EntryReadingHelper reader = new EntryReadingHelper(); + for (int i = 0; i < this.knownCount; ++i) { + reader.set(KnownTagCodec.nameOf(this.knownIds[i]), this.knownValues[i]); + consumer.accept(thisObj, reader); + } + } + Object[] thisBuckets = this.buckets; for (int i = 0; i < thisBuckets.length; ++i) { @@ -1804,6 +2063,20 @@ public void forEach(T thisObj, BiConsumer con } private void forEachParent(T thisObj, BiConsumer consumer) { + long[] parentIds = this.parent.knownIds; + int parentKnownCount = this.parent.knownCount; + if (parentKnownCount > 0) { + Object[] parentValues = this.parent.knownValues; + EntryReadingHelper reader = new EntryReadingHelper(); + for (int i = 0; i < parentKnownCount; ++i) { + long id = parentIds[i]; + if (!this.parentDenseHidden(id)) { + reader.set(KnownTagCodec.nameOf(id), parentValues[i]); + consumer.accept(thisObj, reader); + } + } + } + Object[] localBuckets = this.buckets; Object[] parentBuckets = this.parent.buckets; // leaf parent: same length, same bucket per key for (int i = 0; i < parentBuckets.length; ++i) { @@ -1830,6 +2103,14 @@ private void forEachParent(T thisObj, BiConsumer void forEach( T thisObj, U otherObj, TriConsumer consumer) { + if (this.knownCount > 0) { + EntryReadingHelper reader = new EntryReadingHelper(); + for (int i = 0; i < this.knownCount; ++i) { + reader.set(KnownTagCodec.nameOf(this.knownIds[i]), this.knownValues[i]); + consumer.accept(thisObj, otherObj, reader); + } + } + Object[] thisBuckets = this.buckets; for (int i = 0; i < thisBuckets.length; ++i) { @@ -1854,6 +2135,20 @@ public void forEach( private void forEachParent( T thisObj, U otherObj, TriConsumer consumer) { + long[] parentIds = this.parent.knownIds; + int parentKnownCount = this.parent.knownCount; + if (parentKnownCount > 0) { + Object[] parentValues = this.parent.knownValues; + EntryReadingHelper reader = new EntryReadingHelper(); + for (int i = 0; i < parentKnownCount; ++i) { + long id = parentIds[i]; + if (!this.parentDenseHidden(id)) { + reader.set(KnownTagCodec.nameOf(id), parentValues[i]); + consumer.accept(thisObj, otherObj, reader); + } + } + } + Object[] localBuckets = this.buckets; Object[] parentBuckets = this.parent.buckets; // leaf parent: same length, same bucket per key for (int i = 0; i < parentBuckets.length; ++i) { @@ -1881,8 +2176,12 @@ private void forEachParent( public void clear() { this.checkWriteAccess(); - Arrays.fill(this.buckets, null); + // Drop the private bucket array back to the shared empty sentinel (also avoids mutating it). + this.buckets = EMPTY_BUCKETS; this.size = 0; + this.knownIds = null; + this.knownValues = null; + this.knownCount = 0; } public TagMap freeze() { @@ -1937,6 +2236,20 @@ void checkIntegrity() { } } + // dense store: ids must be unique (no tag stored twice) and the count within array bounds. + if (this.knownCount > 0) { + if (this.knownIds == null || this.knownCount > this.knownIds.length) { + throw new IllegalStateException("incorrect known count"); + } + for (int i = 0; i < this.knownCount; ++i) { + for (int j = i + 1; j < this.knownCount; ++j) { + if (this.knownIds[i] == this.knownIds[j]) { + throw new IllegalStateException("duplicate known id"); + } + } + } + } + if (this.size != this.computeSize()) { throw new IllegalStateException("incorrect size"); } @@ -2068,13 +2381,23 @@ abstract static class IteratorBase { private Object[] buckets; private boolean inParent = false; - private Entry nextEntry; + // Currency is EntryReader, not Entry: a BUCKET entry is its own (real, retain-safe) Entry, but + // a + // DENSE entry is emitted via the reused denseReader flyweight (alloc-free, "use now"). This is + // the contract of TagMap.iterator()/keySet()/values(). entrySet() (Iterator) sits on + // top and calls .entry() per next() to get a real retain-safe Entry (see EntriesIterator). + private EntryReader nextEntry; + private EntryReadingHelper denseReader; // lazily created on the first dense emit private int bucketIndex = -1; private BucketGroup group = null; private int groupIndex = 0; + // dense-store cursors: local known tags, then (read-through) parent known tags + private int knownIndex = 0; + private int parentKnownIndex = 0; + IteratorBase(TagMap map) { this.map = map; this.localBuckets = map.buckets; @@ -2088,9 +2411,9 @@ public final boolean hasNext() { return this.nextEntry != null; } - final Entry nextEntryOrThrowNoSuchElement() { + final EntryReader nextEntryOrThrowNoSuchElement() { if (this.nextEntry != null) { - Entry nextEntry = this.nextEntry; + EntryReader nextEntry = this.nextEntry; this.nextEntry = null; return nextEntry; } @@ -2102,9 +2425,9 @@ final Entry nextEntryOrThrowNoSuchElement() { } } - final Entry nextEntryOrNull() { + final EntryReader nextEntryOrNull() { if (this.nextEntry != null) { - Entry nextEntry = this.nextEntry; + EntryReader nextEntry = this.nextEntry; this.nextEntry = null; return nextEntry; } @@ -2112,7 +2435,14 @@ final Entry nextEntryOrNull() { return this.hasNext() ? this.nextEntry : null; } - private final Entry advance() { + private final EntryReader advance() { + // phase: local dense known tags (local entries always emit — no shadow check). Emitted via + // the + // reused denseReader flyweight — NO per-entry Entry alloc (the read/serialize alloc win). + if (this.knownIndex < this.map.knownCount) { + int i = this.knownIndex++; + return this.emitDense(this.map.knownIds[i], this.map.knownValues[i]); + } while (true) { Entry tagEntry = this.rawAdvance(); if (tagEntry != null) { @@ -2127,8 +2457,14 @@ private final Entry advance() { continue; // parent entry shadowed/tombstoned -> skip } - // current array exhausted; switch to the parent's buckets once (read-through union) + // current bucket array exhausted; before switching to parent buckets, drain parent dense + // (read-through union). Re-entrant: while inParent stays false, the exhausted local-bucket + // rawAdvance keeps returning null and funnels back here until parent dense is fully + // drained. if (!this.inParent && this.map.parent != null) { + EntryReader parentDense = this.advanceParentDense(); + if (parentDense != null) return parentDense; + this.inParent = true; this.buckets = this.map.parent.buckets; this.bucketIndex = -1; @@ -2140,6 +2476,33 @@ private final Entry advance() { } } + /** + * Next visible parent dense entry (not shadowed locally / tombstoned), or null when drained. + */ + private final EntryReader advanceParentDense() { + TagMap p = this.map.parent; + long[] parentIds = p.knownIds; + int parentKnownCount = p.knownCount; + while (this.parentKnownIndex < parentKnownCount) { + int i = this.parentKnownIndex++; + long id = parentIds[i]; + if (!this.map.parentDenseHidden(id)) { + return this.emitDense(id, p.knownValues[i]); + } + } + return null; + } + + /** Sets and returns the reused dense flyweight (lazily created); "use now", do not retain. */ + private EntryReader emitDense(long tagId, Object value) { + EntryReadingHelper reader = this.denseReader; + if (reader == null) { + reader = this.denseReader = new EntryReadingHelper(); + } + reader.set(KnownTagCodec.nameOf(tagId), value); + return reader; + } + /** Next raw entry in the current bucket array, ignoring shadowing/tombstones. */ private final Entry rawAdvance() { while (this.bucketIndex < this.buckets.length) { @@ -2669,9 +3032,26 @@ public boolean isEmpty() { @Override public Iterator> iterator() { - @SuppressWarnings({"rawtypes", "unchecked"}) - Iterator> iter = (Iterator) this.map.iterator(); - return iter; + return new EntriesIterator(this.map); + } + } + + /** + * entrySet() yields real, retain-safe {@code Map.Entry} objects. It sits on top of the + * EntryReader iterator and materializes each via {@code .entry()}: a bucket entry's reader IS the + * real stored Entry (returns {@code this}, free); a dense entry's flyweight materializes a fresh + * Entry. Deliberately NOT alloc-optimized for dense — bulk reads use {@code forEach}/EntryReader, + * and manual instrumentation does point get/set, not bulk entrySet iteration. + */ + static final class EntriesIterator extends IteratorBase + implements Iterator> { + EntriesIterator(TagMap map) { + super(map); + } + + @Override + public Map.Entry next() { + return this.nextEntryOrThrowNoSuchElement().entry(); } } diff --git a/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java b/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java new file mode 100644 index 00000000000..11d01bc7ec8 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/KnownTagsTest.java @@ -0,0 +1,152 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Parity test for the keyOf substrate (slice 1): the {@link KnownTags} registry + the {@link + * KnownTagCodec.Resolver} it registers. Verifies name ↔ id resolution without any dense store + * — {@code keyOf}/{@code nameOf} depend only on globalSerial + name, not on the (dormant) + * positional layout. + */ +class KnownTagsTest { + + /** (name, id) pairs — the full registry. keyOf returns the id verbatim (incl. INTERCEPTED). */ + static Stream knownTags() { + return Stream.of( + Arguments.of(Tags.ERROR, KnownTags.ERROR_ID), + Arguments.of(DDTags.PARENT_ID, KnownTags.PARENT_ID), + Arguments.of(DDTags.BASE_SERVICE, KnownTags.BASE_SERVICE_ID), + Arguments.of(Tags.VERSION, KnownTags.VERSION_ID), + Arguments.of(KnownTags.ENV, KnownTags.ENV_ID), + Arguments.of(DDTags.DJM_ENABLED, KnownTags.DJM_ENABLED_ID), + Arguments.of(DDTags.DSM_ENABLED, KnownTags.DSM_ENABLED_ID), + Arguments.of(DDTags.TRACER_HOST, KnownTags.TRACER_HOST_ID), + Arguments.of(DDTags.DD_INTEGRATION, KnownTags.INTEGRATION_ID), + Arguments.of(DDTags.DD_SVC_SRC, KnownTags.SVC_SRC_ID), + Arguments.of(Tags.PEER_SERVICE, KnownTags.PEER_SERVICE_ID), + Arguments.of(DDTags.PEER_SERVICE_REMAPPED_FROM, KnownTags.PEER_SERVICE_REMAPPED_FROM_ID), + Arguments.of(Tags.HTTP_METHOD, KnownTags.HTTP_METHOD_ID), + Arguments.of(Tags.HTTP_ROUTE, KnownTags.HTTP_ROUTE_ID), + Arguments.of(Tags.HTTP_URL, KnownTags.HTTP_URL_ID), + Arguments.of(Tags.PEER_HOSTNAME, KnownTags.PEER_HOSTNAME_ID), + Arguments.of(Tags.PEER_HOST_IPV4, KnownTags.PEER_HOST_IPV4_ID), + Arguments.of(Tags.PEER_HOST_IPV6, KnownTags.PEER_HOST_IPV6_ID), + Arguments.of(Tags.PEER_PORT, KnownTags.PEER_PORT_ID), + Arguments.of(Tags.COMPONENT, KnownTags.COMPONENT_ID), + Arguments.of(Tags.SPAN_KIND, KnownTags.SPAN_KIND_ID), + Arguments.of(DDTags.LANGUAGE_TAG_KEY, KnownTags.LANGUAGE_ID), + Arguments.of(Tags.DB_TYPE, KnownTags.DB_TYPE_ID), + Arguments.of(Tags.DB_INSTANCE, KnownTags.DB_INSTANCE_ID), + Arguments.of(Tags.DB_USER, KnownTags.DB_USER_ID), + Arguments.of(Tags.DB_OPERATION, KnownTags.DB_OPERATION_ID), + Arguments.of(Tags.DB_POOL_NAME, KnownTags.DB_POOL_NAME_ID)); + } + + /** + * The subset flagged INTERCEPTED (sign bit) — must agree with the interceptor's needsIntercept. + */ + static Stream interceptedTags() { + return Stream.of( + Arguments.of(KnownTags.ERROR_ID), + Arguments.of(KnownTags.PEER_SERVICE_ID), + Arguments.of(KnownTags.HTTP_METHOD_ID), + Arguments.of(KnownTags.HTTP_URL_ID), + Arguments.of(KnownTags.SPAN_KIND_ID)); + } + + @Test + void resolverIsActiveOnceReferenced() { + // referencing any constant triggers KnownTags. -> KnownTagCodec.register + assertTrue(KnownTags.ERROR_ID != 0L); + assertTrue(KnownTagCodec.isActive()); + assertEquals(KnownTags.SLOT_COUNT, KnownTagCodec.slotCount()); + } + + @ParameterizedTest + @MethodSource("knownTags") + void keyOfResolvesNameToId(String name, long id) { + assertEquals(id, KnownTagCodec.keyOf(name), "keyOf(" + name + ")"); + } + + @ParameterizedTest + @MethodSource("knownTags") + void nameOfResolvesIdToName(String name, long id) { + assertEquals(name, KnownTagCodec.nameOf(id), "nameOf(" + name + ")"); + } + + @ParameterizedTest + @MethodSource("knownTags") + void nameHashMatchesEntryHash(String name, long id) { + assertEquals( + (int) TagMap.Entry._hash(name), KnownTagCodec.nameHash(id), "nameHash(" + name + ")"); + } + + @ParameterizedTest + @MethodSource("interceptedTags") + void interceptedTagsCarryFlag(long id) { + assertTrue(KnownTagCodec.isIntercepted(id), "isIntercepted"); + } + + @Test + void nonInterceptedTagsDoNotCarryFlag() { + Set intercepted = new HashSet<>(); + interceptedTags().forEach(a -> intercepted.add((Long) a.get()[0])); + knownTags() + .forEach( + a -> { + long id = (Long) a.get()[1]; + if (!intercepted.contains(id)) { + assertFalse(KnownTagCodec.isIntercepted(id), "not intercepted: " + a.get()[0]); + } + }); + } + + @Test + void unknownNamesResolveToZero() { + assertEquals(0L, KnownTagCodec.keyOf("definitely.not.a.known.tag")); + assertEquals(0L, KnownTagCodec.keyOf("http.statuscode")); // close-but-not-listed + assertEquals(0L, KnownTagCodec.keyOf("")); + } + + @Test + void unknownIdsResolveToNullName() { + assertNull(KnownTagCodec.nameOf(0L)); + assertNull(KnownTagCodec.nameOf(KnownTagCodec.tagId(9999, "made.up"))); + } + + @Test + void errorIsReservedTheRestAreStored() { + assertTrue(KnownTagCodec.isReserved(KnownTags.ERROR_ID), "ERROR reserved"); + assertFalse(KnownTagCodec.isStored(KnownTags.ERROR_ID), "ERROR not stored"); + knownTags() + .forEach( + a -> { + long id = (Long) a.get()[1]; + if (id != KnownTags.ERROR_ID) { + assertTrue(KnownTagCodec.isStored(id), "stored: " + a.get()[0]); + assertFalse(KnownTagCodec.isReserved(id), "not reserved: " + a.get()[0]); + } + }); + } + + @Test + void globalSerialsAreUnique() { + List serials = new ArrayList<>(); + knownTags().forEach(a -> serials.add((long) KnownTagCodec.globalSerial((Long) a.get()[1]))); + assertEquals(serials.size(), new HashSet<>(serials).size(), "globalSerials must be unique"); + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapDenseForkedTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapDenseForkedTest.java new file mode 100644 index 00000000000..a82742c3acd --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapDenseForkedTest.java @@ -0,0 +1,273 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Exercises the dense known-tag store with a LIVE resolver. Registration ({@link KnownTagCodec}) is + * a global static with no un-register, so this lives in a {@code ForkedTest} (isolated JVM) to keep + * dense routing from leaking into the bucket-only tests in the shared JVM. The dense store is + * dormant in production (no resolver) — this is where it actually executes. + * + *

    Stored tags (globalSerial ≥ {@code FIRST_STORED_SERIAL}) route to the dense store; reserved + * tags (e.g. {@code error}) and arbitrary tags stay in the hash buckets. Behavior must be + * observationally identical to the bucket store. + */ +class TagMapDenseForkedTest { + + // stored (dense-routed) tags + static final String BASE_SERVICE = DDTags.BASE_SERVICE; + static final String COMPONENT = Tags.COMPONENT; + static final String DB_TYPE = Tags.DB_TYPE; + static final String HTTP_METHOD = Tags.HTTP_METHOD; // stored + intercepted + static final String DB_INSTANCE = Tags.DB_INSTANCE; + // arbitrary (bucket-routed) tags + static final String CUSTOM_A = "custom.tag.a"; + static final String CUSTOM_B = "custom.tag.b"; + + @BeforeAll + static void registerResolver() { + // referencing any KnownTags constant triggers its -> KnownTagCodec.register + assertTrue(KnownTags.BASE_SERVICE_ID != 0L); + assertTrue(KnownTagCodec.isActive(), "resolver must be live for the dense store to engage"); + assertTrue( + KnownTagCodec.isStored(KnownTagCodec.keyOf(BASE_SERVICE)), "base_service routes dense"); + assertFalse( + KnownTagCodec.isStored(KnownTagCodec.keyOf(CUSTOM_A)), "custom tag stays in buckets"); + assertFalse( + KnownTagCodec.isStored(KnownTagCodec.keyOf(Tags.ERROR)), "error is reserved, not stored"); + } + + private static TagMap map() { + return (TagMap) TagMap.create(); + } + + @Test + void knownTagRoundTripsThroughDenseStore() { + TagMap map = map(); + map.set(BASE_SERVICE, "billing"); + map.set(COMPONENT, "spring-web"); + + assertEquals("billing", map.getObject(BASE_SERVICE)); + assertEquals("spring-web", map.getString(COMPONENT)); + assertEquals("billing", map.getEntry(BASE_SERVICE).objectValue()); + assertTrue(map.containsKey(BASE_SERVICE)); + assertEquals(2, map.size()); + map.checkIntegrity(); + } + + @Test + void typedKnownValuesRoundTrip() { + TagMap map = map(); + map.set(DB_TYPE, "postgresql"); + map.set(HTTP_METHOD, "GET"); + map.set(Tags.PEER_PORT, 5432); + + assertEquals("postgresql", map.getString(DB_TYPE)); + assertEquals("GET", map.getString(HTTP_METHOD)); + assertEquals(5432, map.getInt(Tags.PEER_PORT)); + assertEquals(3, map.size()); + map.checkIntegrity(); + } + + @Test + void knownAndUnknownCoexist() { + TagMap map = map(); + map.set(BASE_SERVICE, "billing"); // dense + map.set(CUSTOM_A, "alpha"); // bucket + map.set(DB_TYPE, "h2"); // dense + map.set(CUSTOM_B, "beta"); // bucket + + assertEquals("billing", map.getObject(BASE_SERVICE)); + assertEquals("alpha", map.getObject(CUSTOM_A)); + assertEquals("h2", map.getObject(DB_TYPE)); + assertEquals("beta", map.getObject(CUSTOM_B)); + assertEquals(4, map.size()); + assertFalse(map.isEmpty()); + map.checkIntegrity(); + + Map collected = new HashMap<>(); + map.fillMap(collected); + assertEquals(4, collected.size()); + assertEquals("billing", collected.get(BASE_SERVICE)); + assertEquals("alpha", collected.get(CUSTOM_A)); + assertEquals("h2", collected.get(DB_TYPE)); + assertEquals("beta", collected.get(CUSTOM_B)); + } + + @Test + void overwriteKnownReplacesInPlace() { + TagMap map = map(); + map.set(COMPONENT, "first"); + assertEquals("first", map.getObject(COMPONENT)); + map.set(COMPONENT, "second"); + assertEquals("second", map.getObject(COMPONENT)); + assertEquals(1, map.size()); // overwrite, not append + map.checkIntegrity(); + } + + @Test + void removeKnownClearsIt() { + TagMap map = map(); + map.set(BASE_SERVICE, "billing"); + map.set(DB_TYPE, "h2"); + map.set(CUSTOM_A, "alpha"); + assertEquals(3, map.size()); + + TagMap.Entry removed = map.getAndRemove(BASE_SERVICE); + assertEquals("billing", removed.objectValue()); + assertNull(map.getObject(BASE_SERVICE)); + assertEquals("h2", map.getObject(DB_TYPE)); // sibling dense entry intact + assertEquals("alpha", map.getObject(CUSTOM_A)); + assertEquals(2, map.size()); + map.checkIntegrity(); + } + + @Test + void forEachAndIteratorEmitDenseAndBucketEntries() { + TagMap map = map(); + map.set(BASE_SERVICE, "billing"); + map.set(COMPONENT, "web"); + map.set(CUSTOM_A, "alpha"); + + Map viaForEach = new HashMap<>(); + map.forEach(reader -> viaForEach.put(reader.tag(), reader.objectValue())); + assertEquals(3, viaForEach.size()); + assertEquals("billing", viaForEach.get(BASE_SERVICE)); + assertEquals("web", viaForEach.get(COMPONENT)); + assertEquals("alpha", viaForEach.get(CUSTOM_A)); + + Map viaIterator = new HashMap<>(); + for (TagMap.EntryReader reader : map) { + viaIterator.put(reader.tag(), reader.objectValue()); + } + assertEquals(viaForEach, viaIterator); + } + + @Test + void copyPreservesDenseStore() { + TagMap map = map(); + map.set(BASE_SERVICE, "billing"); + map.set(CUSTOM_A, "alpha"); + + TagMap copy = (TagMap) map.copy(); + assertEquals("billing", copy.getObject(BASE_SERVICE)); + assertEquals("alpha", copy.getObject(CUSTOM_A)); + assertEquals(2, copy.size()); + + // independence: mutating the copy doesn't touch the original's dense store + copy.set(BASE_SERVICE, "shipping"); + assertEquals("shipping", copy.getObject(BASE_SERVICE)); + assertEquals("billing", map.getObject(BASE_SERVICE)); + copy.checkIntegrity(); + map.checkIntegrity(); + } + + @Test + void clearEmptiesDenseStore() { + TagMap map = map(); + map.set(BASE_SERVICE, "billing"); + map.set(CUSTOM_A, "alpha"); + map.clear(); + assertEquals(0, map.size()); + assertTrue(map.isEmpty()); + assertNull(map.getObject(BASE_SERVICE)); + map.checkIntegrity(); + } + + @Test + void putAllMergesDenseStore() { + TagMap src = map(); + src.set(BASE_SERVICE, "billing"); + src.set(DB_TYPE, "h2"); + src.set(CUSTOM_A, "alpha"); + + TagMap dst = map(); + dst.set(COMPONENT, "web"); // dense, distinct + dst.set(BASE_SERVICE, "old"); // dense, clobbered by src + dst.putAll((TagMap) src); + + assertEquals("billing", dst.getObject(BASE_SERVICE)); // src clobbers + assertEquals("h2", dst.getObject(DB_TYPE)); + assertEquals("web", dst.getObject(COMPONENT)); + assertEquals("alpha", dst.getObject(CUSTOM_A)); + assertEquals(4, dst.size()); + dst.checkIntegrity(); + } + + // ---- read-through union (dense parent + dense child) ---- + + private static TagMap frozenParent() { + TagMap parent = map(); + parent.set(BASE_SERVICE, "billing"); // dense + parent.set(COMPONENT, "web"); // dense + parent.set(CUSTOM_A, "alpha"); // bucket + parent.freeze(); + return parent; + } + + @Test + void childReadsThroughToParentDense() { + TagMap child = TagMap.createFromParent(frozenParent()); + child.set(DB_TYPE, "h2"); // child-only dense + child.set(CUSTOM_B, "beta"); // child-only bucket + + // inherited from parent + assertEquals("billing", child.getObject(BASE_SERVICE)); + assertEquals("web", child.getObject(COMPONENT)); + assertEquals("alpha", child.getObject(CUSTOM_A)); + // own + assertEquals("h2", child.getObject(DB_TYPE)); + assertEquals("beta", child.getObject(CUSTOM_B)); + // union size: 3 parent + 2 child + assertEquals(5, child.size()); + assertFalse(child.isEmpty()); + + Map union = new HashMap<>(); + child.forEach(reader -> union.put(reader.tag(), reader.objectValue())); + assertEquals(5, union.size()); + assertEquals("billing", union.get(BASE_SERVICE)); + assertEquals("h2", union.get(DB_TYPE)); + child.checkIntegrity(); + } + + @Test + void childDenseShadowsParentDense() { + TagMap child = TagMap.createFromParent(frozenParent()); + child.set(BASE_SERVICE, "shipping"); // shadows parent's dense base_service + + assertEquals("shipping", child.getObject(BASE_SERVICE)); // local wins + assertEquals("web", child.getObject(COMPONENT)); // still inherited + assertEquals(3, child.size()); // base_service counted once (shadowed, not doubled) + + Map union = new HashMap<>(); + child.forEach(reader -> union.put(reader.tag(), reader.objectValue())); + assertEquals(3, union.size()); + assertEquals("shipping", union.get(BASE_SERVICE)); // shadow value, parent suppressed + } + + @Test + void removingParentDenseKeyTombstonesIt() { + TagMap child = TagMap.createFromParent(frozenParent()); + + TagMap.Entry removed = child.getAndRemove(BASE_SERVICE); // parent-only dense key + assertEquals("billing", removed.objectValue()); // prior visible value was the parent's + assertNull(child.getObject(BASE_SERVICE)); // tombstoned: no read-through + assertEquals("web", child.getObject(COMPONENT)); // sibling still inherited + assertEquals(2, child.size()); // 3 parent - 1 tombstoned + + Map union = new HashMap<>(); + child.forEach(reader -> union.put(reader.tag(), reader.objectValue())); + assertEquals(2, union.size()); + assertFalse(union.containsKey(BASE_SERVICE)); + child.checkIntegrity(); + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapDenseFuzzForkedTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapDenseFuzzForkedTest.java new file mode 100644 index 00000000000..a29795c542a --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapDenseFuzzForkedTest.java @@ -0,0 +1,202 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.TagMapFuzzTest.MapAction; +import datadog.trace.api.TagMapFuzzTest.TestCase; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.concurrent.ThreadLocalRandom; +import java.util.function.Supplier; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Fuzz test for the dense store under a LIVE resolver, across three key regimes. Reuses {@link + * TagMapFuzzTest}'s oracle machinery ({@code test(TestCase)} replays a random action sequence + * against a {@code HashMap}, verifying each step + {@code checkIntegrity}). + * + *

    Uses a synthetic prefix resolver ({@code known-N} -> stored / dense, anything else -> bucket) + * rather than the real {@link KnownTags}: it gives an UNBOUNDED known key space, so the dense array + * actually grows past its initial capacity and the linear scan gets long, and it lets each test pin + * the known/custom ratio. The three regimes exercise paths the mixed run alone would miss: + * + *

      + *
    • known-only — the all-dense map (dense growth, dense-only putAll/copy/clear/iterate + * with no bucket phase, knownCount-only size). + *
    • custom-only — confirms the dense branches stay inert when nothing resolves, even + * with a resolver registered. + *
    • mixed — both regions and their interaction. + *
    + * + *

    Forked (isolated JVM) because resolver registration is a global static with no un-register. + */ +class TagMapDenseFuzzForkedTest { + static final int SINGLE_MAP_CASES = 1500; + static final int MERGE_CASES = 400; + static final int MAX_ACTIONS = 40; + static final int MIN_ACTIONS = 8; + + // unbounded synthetic key spaces — large enough to grow the dense array past cap-8 several times + static final int KNOWN_SPACE = 48; + static final int CUSTOM_SPACE = 48; + + enum Regime { + KNOWN_ONLY, + CUSTOM_ONLY, + MIXED + } + + /** + * Synthetic resolver: {@code known-N} -> stored id (serial = FIRST_STORED_SERIAL + N); else 0. + */ + static final KnownTagCodec.Resolver FUZZ_RESOLVER = + new KnownTagCodec.Resolver() { + @Override + public long keyOf(String name) { + if (name.startsWith("known-")) { + int n = Integer.parseInt(name.substring("known-".length())); + return KnownTagCodec.tagId(KnownTagCodec.FIRST_STORED_SERIAL + n, name); + } + return 0L; + } + + @Override + public String nameOf(long tagId) { + int serial = KnownTagCodec.globalSerial(tagId); + return serial >= KnownTagCodec.FIRST_STORED_SERIAL + ? "known-" + (serial - KnownTagCodec.FIRST_STORED_SERIAL) + : null; + } + + @Override + public int slotCount() { + return 0; // positional unused + } + }; + + @BeforeAll + static void registerResolver() { + KnownTagCodec.register(FUZZ_RESOLVER); + assertTrue(KnownTagCodec.isActive(), "resolver must be live"); + assertTrue(KnownTagCodec.isStored(KnownTagCodec.keyOf("known-0")), "known- routes dense"); + assertFalse( + KnownTagCodec.isStored(KnownTagCodec.keyOf("custom-0")), "custom- stays in buckets"); + // round-trip the synthetic encoding + long id = KnownTagCodec.keyOf("known-7"); + assertTrue("known-7".equals(KnownTagCodec.nameOf(id)), "name<->id round-trips"); + } + + @Test + void knownOnlyFuzz() { + runRegime(Regime.KNOWN_ONLY); + } + + @Test + void customOnlyFuzz() { + runRegime(Regime.CUSTOM_ONLY); + } + + @Test + void mixedFuzz() { + runRegime(Regime.MIXED); + } + + private static void runRegime(Regime regime) { + for (int i = 0; i < SINGLE_MAP_CASES; ++i) { + TagMapFuzzTest.test(generateTest(regime)); + } + for (int i = 0; i < MERGE_CASES; ++i) { + TagMap mapA = TagMapFuzzTest.test(generateTest(regime)); + TagMap mapB = TagMapFuzzTest.test(generateTest(regime)); + + HashMap hashA = new HashMap<>(mapA); + HashMap hashB = new HashMap<>(mapB); + + mapA.putAll(mapB); + hashA.putAll(hashB); + + TagMapFuzzTest.assertMapEquals(hashA, mapA); + } + } + + // --- action generation (mirrors TagMapFuzzTest.randomAction, regime-driven key pool) --- + + private static TestCase generateTest(Regime regime) { + ThreadLocalRandom r = ThreadLocalRandom.current(); + int numActions = r.nextInt(MAX_ACTIONS - MIN_ACTIONS) + MIN_ACTIONS; + List actions = new ArrayList<>(numActions); + for (int i = 0; i < numActions; ++i) { + actions.add(randomAction(regime)); + } + return new TestCase(actions); + } + + private static MapAction randomAction(Regime regime) { + switch (randomChoice(0.02, 0.1, 0.2)) { + case 0: + return TagMapFuzzTest.clear(); + case 1: + return choose( + () -> TagMapFuzzTest.putAll(randomKeysAndValues(regime)), + () -> TagMapFuzzTest.putAllTagMap(randomKeysAndValues(regime)), + () -> TagMapFuzzTest.putAllLedger(randomKeysAndValues(regime))); + case 2: + return choose( + () -> TagMapFuzzTest.remove(randomKey(regime)), + () -> TagMapFuzzTest.removeLight(randomKey(regime)), + () -> TagMapFuzzTest.getAndRemove(randomKey(regime))); + default: + return choose( + () -> TagMapFuzzTest.put(randomKey(regime), randomValue()), + () -> TagMapFuzzTest.set(randomKey(regime), randomValue()), + () -> TagMapFuzzTest.getAndSet(randomKey(regime), randomValue())); + } + } + + private static String randomKey(Regime regime) { + ThreadLocalRandom r = ThreadLocalRandom.current(); + boolean known; + switch (regime) { + case KNOWN_ONLY: + known = true; + break; + case CUSTOM_ONLY: + known = false; + break; + default: + known = r.nextBoolean(); + } + return known ? "known-" + r.nextInt(KNOWN_SPACE) : "custom-" + r.nextInt(CUSTOM_SPACE); + } + + private static String randomValue() { + return "values-" + ThreadLocalRandom.current().nextInt(); + } + + private static String[] randomKeysAndValues(Regime regime) { + int numEntries = ThreadLocalRandom.current().nextInt(KNOWN_SPACE + CUSTOM_SPACE); + String[] keysAndValues = new String[numEntries << 1]; + for (int i = 0; i < keysAndValues.length; i += 2) { + keysAndValues[i] = randomKey(regime); + keysAndValues[i + 1] = randomValue(); + } + return keysAndValues; + } + + private static int randomChoice(double... proportions) { + double selector = ThreadLocalRandom.current().nextDouble(); + for (int i = 0; i < proportions.length; ++i) { + if (selector < proportions[i]) return i; + selector -= proportions[i]; + } + return proportions.length; + } + + @SafeVarargs + private static MapAction choose(Supplier... choices) { + return choices[ThreadLocalRandom.current().nextInt(choices.length)].get(); + } +}