From 4873780f9a11604b96627082822f538f3149be04 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 15 Jul 2026 13:24:06 -0400 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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 {