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 f8f33f1c023..cdf68653c11 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; @@ -16,6 +17,8 @@ import java.util.function.Function; import java.util.stream.Stream; import java.util.stream.StreamSupport; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * A super simple hash map designed for... @@ -42,19 +45,22 @@ *
  • 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; + // 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 */ - 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 */ - static TagMap fromMapImmutable(Map map) { + public static final TagMap fromMapImmutable(Map map) { if (map.isEmpty()) { return TagMap.EMPTY; } else { @@ -62,223 +68,38 @@ static TagMap fromMapImmutable(Map map) { } } - static TagMap create() { - return new OptimizedTagMap(); + public static final TagMap create() { + return new TagMap(); } - static TagMap create(int size) { - return new OptimizedTagMap(); + 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 */ - static Ledger ledger() { + public static final Ledger ledger() { return new Ledger(); } /** Creates a new TagMap.Ledger which handles size modifications before expansion */ - static Ledger ledger(int size) { + public static final 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(String tag, Object value); - - /** Sets value without returning prior value - more efficient than {@link Map#put} */ - void set(String tag, 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(String tag, CharSequence value); - - void set(String tag, boolean value); - - void set(String tag, int value); - - void set(String tag, long value); - - void set(String tag, float value); - - void set(String tag, double value); - - void set(EntryReader newEntry); - - /** sets the value while returning the prior Entry */ - Entry getAndSet(String tag, Object value); - - Entry getAndSet(String tag, CharSequence value); - - Entry getAndSet(String tag, boolean value); - - Entry getAndSet(String tag, int value); - - Entry getAndSet(String tag, long value); - - Entry getAndSet(String tag, float value); - - Entry getAndSet(String tag, double value); - - /** - * TagMap specific method that places an Entry directly into an optimized TagMap avoiding need to - * allocate a new Entry object - */ - Entry getAndSet(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); } @@ -300,7 +121,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); } @@ -311,7 +132,7 @@ public boolean isRemoval() { } } - interface EntryReader { + public interface EntryReader { public static final byte OBJECT = 1; /* @@ -361,23 +182,33 @@ 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. */ static final byte ANY = 0; - /** If value is non-null, returns a new TagMap.Entry If value is null, returns null */ - public static final Entry create(String tag, Object value) { - // NOTE: From the static typing, it is possible that value is a primitive box, so need to call - // Any variant - - return (value == null) ? null : TagMap.Entry.newAnyEntry(tag, value); + /** + * Entry for {@code (tag, value)}, or null when {@code value} is null or an empty {@code + * CharSequence} -- checked by runtime type, so an empty String passed as {@code Object} skips + * the same as via the {@link #create(String, CharSequence)} overload. + */ + @Nullable + public static final Entry create(@Nonnull String tag, Object value) { + if (value == null) { + return null; + } + if (value instanceof CharSequence && ((CharSequence) value).length() == 0) { + return null; + } + return TagMap.Entry.newAnyEntry(tag, value); } /** If value is non-null, returns a new TagMap.Entry If value is null or empty, returns null */ - public static final Entry create(String tag, CharSequence value) { + @Nullable + public static final Entry create(@Nonnull String tag, CharSequence value) { // NOTE: From the static typing, we know that value is not a primitive box return (value == null || value.length() == 0) @@ -385,23 +216,23 @@ public static final Entry create(String tag, CharSequence value) { : TagMap.Entry.newObjectEntry(tag, value); } - public static final Entry create(String tag, boolean value) { + public static final Entry create(@Nonnull String tag, boolean value) { return TagMap.Entry.newBooleanEntry(tag, value); } - public static final Entry create(String tag, int value) { + public static final Entry create(@Nonnull String tag, int value) { return TagMap.Entry.newIntEntry(tag, value); } - public static final Entry create(String tag, long value) { + public static final Entry create(@Nonnull String tag, long value) { return TagMap.Entry.newLongEntry(tag, value); } - public static final Entry create(String tag, float value) { + public static final Entry create(@Nonnull String tag, float value) { return TagMap.Entry.newFloatEntry(tag, value); } - public static final Entry create(String tag, double value) { + public static final Entry create(@Nonnull String tag, double value) { return TagMap.Entry.newDoubleEntry(tag, value); } @@ -968,7 +799,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; @@ -1153,71 +984,138 @@ 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. - 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); - } + /* + * 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. + */ private final Object[] buckets; private int size; private boolean frozen; - public OptimizedTagMap() { + /** + * 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 */ - private OptimizedTagMap(Object[] buckets, int size) { + private TagMap(Object[] buckets, int size) { this.buckets = buckets; this.size = size; this.frozen = true; + this.parent = null; } - @Override public boolean isOptimized() { return true; } @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 @@ -1306,7 +1204,6 @@ public Set keySet() { return new Keys(this); } - @Override public Iterator tagIterator() { return new KeysIterator(this); } @@ -1316,7 +1213,6 @@ public Collection values() { return new Values(this); } - @Override public Iterator valueIterator() { return new ValuesIterator(this); } @@ -1326,28 +1222,60 @@ public Set> entrySet() { return new Entries(this); } - @Override 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(String tag, Object value) { @@ -1355,50 +1283,54 @@ public Object put(String tag, Object value) { return entry == null ? null : entry.objectValue(); } - @Override public void set(TagMap.EntryReader newEntryReader) { + if (newEntryReader == null) { + return; + } 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; + } + 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(); @@ -1444,37 +1376,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)); } @@ -1482,8 +1407,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); } @@ -1506,14 +1431,10 @@ private void putAllUnoptimizedMap(Map that) 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 { @@ -1521,7 +1442,7 @@ private void putAllOptimizedMap(OptimizedTagMap that) { } } - private void putAllMerge(OptimizedTagMap that) { + private void putAllMerge(TagMap that) { Object[] thisBuckets = this.buckets; Object[] thatBuckets = that.buckets; @@ -1638,7 +1559,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; @@ -1710,10 +1631,35 @@ public boolean remove(String tag) { return (this.getAndRemove(tag) != null); } - @Override 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); @@ -1750,10 +1696,15 @@ public Entry getAndRemove(String tag) { return null; } - @Override public TagMap copy() { - OptimizedTagMap copy = new OptimizedTagMap(); + // 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; } @@ -1770,7 +1721,6 @@ public Iterator iterator() { return new EntryReaderIterator(this); } - @Override public Stream stream() { return StreamSupport.stream(spliterator(), false); } @@ -1792,9 +1742,37 @@ 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); + } + } + } + } } - @Override public void forEach(T thisObj, BiConsumer consumer) { Object[] thisBuckets = this.buckets; @@ -1811,9 +1789,38 @@ 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); + } + } + } + } + } } - @Override public void forEach( T thisObj, U otherObj, TriConsumer consumer) { Object[] thisBuckets = this.buckets; @@ -1831,6 +1838,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() { @@ -1840,7 +1878,7 @@ public void clear() { this.size = 0; } - public OptimizedTagMap freeze() { + public TagMap freeze() { this.frozen = true; return this; @@ -1895,7 +1933,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"); } } @@ -1939,7 +1980,7 @@ public Object compute( String key, BiFunction remappingFunction) { this.checkWriteAccess(); - return TagMap.super.compute(key, remappingFunction); + return Map.super.compute(key, remappingFunction); } @Override @@ -1947,7 +1988,7 @@ public Object computeIfAbsent( String key, Function mappingFunction) { this.checkWriteAccess(); - return TagMap.super.computeIfAbsent(key, mappingFunction); + return Map.super.computeIfAbsent(key, mappingFunction); } @Override @@ -1955,7 +1996,7 @@ public Object computeIfPresent( String key, BiFunction remappingFunction) { this.checkWriteAccess(); - return TagMap.super.computeIfPresent(key, remappingFunction); + return Map.super.computeIfPresent(key, remappingFunction); } @Override @@ -2013,7 +2054,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; @@ -2022,19 +2068,17 @@ abstract static class IteratorBase { private BucketGroup group = null; private int groupIndex = 0; - IteratorBase(OptimizedTagMap map) { + 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() { @@ -2062,6 +2106,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) { @@ -2096,7 +2169,7 @@ private final Entry advance() { } static final class EntryReaderIterator extends IteratorBase implements Iterator { - EntryReaderIterator(OptimizedTagMap map) { + EntryReaderIterator(TagMap map) { super(map); } @@ -2571,20 +2644,20 @@ 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; } @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 @@ -2596,20 +2669,20 @@ public Iterator> iterator() { } static final class Keys extends AbstractSet { - final OptimizedTagMap map; + final TagMap map; - Keys(OptimizedTagMap map) { + Keys(TagMap map) { this.map = map; } @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 @@ -2624,7 +2697,7 @@ public Iterator iterator() { } static final class KeysIterator extends IteratorBase implements Iterator { - KeysIterator(OptimizedTagMap map) { + KeysIterator(TagMap map) { super(map); } @@ -2635,20 +2708,20 @@ public String next() { } static final class Values extends AbstractCollection { - final OptimizedTagMap map; + final TagMap map; - Values(OptimizedTagMap map) { + Values(TagMap map) { this.map = map; } @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 @@ -2663,7 +2736,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/TagMapNullToleranceTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapNullToleranceTest.java new file mode 100644 index 00000000000..b8764472847 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapNullToleranceTest.java @@ -0,0 +1,66 @@ +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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** + * Pins the Entry-pathway null-tolerance contract: {@code create(...)} returns null for a null or + * empty value, and the {@code Entry} sinks ({@code set(Entry)} / {@code getAndSet(Entry)}) treat + * null as a no-op -- so a null/empty value flows through as "no tag" without any caller guarding. + */ +class TagMapNullToleranceTest { + + @Test + void createReturnsNullForNullOrEmptyValue() { + // CharSequence overload: null or empty -> null + assertNull(TagMap.Entry.create("k", (CharSequence) null)); + assertNull(TagMap.Entry.create("k", "")); + + // Object overload: null -> null, and an empty String typed as Object -> null (runtime check, + // so the convention holds regardless of the static type at the call site) + assertNull(TagMap.Entry.create("k", (Object) null)); + assertNull(TagMap.Entry.create("k", (Object) "")); + + // Non-empty values still produce an entry, whatever the static type + assertNotNull(TagMap.Entry.create("k", "v")); + assertNotNull(TagMap.Entry.create("k", (Object) "v")); + // Non-CharSequence Object has no notion of "empty" -> always an entry + assertNotNull(TagMap.Entry.create("k", (Object) Integer.valueOf(0))); + } + + @Test + void getAndSetToleratesNullEntry() { + TagMap map = TagMap.create(); + assertNull(map.getAndSet((TagMap.Entry) null), "null entry is a no-op returning null"); + assertEquals(0, map.size(), "no tag added"); + } + + @Test + void setToleratesNullReader() { + TagMap map = TagMap.create(); + map.set((TagMap.EntryReader) null); // must not throw + assertEquals(0, map.size(), "no tag added"); + } + + @Test + void nullOrEmptyFlowsThroughTheEntryPathwayAsNoTag() { + TagMap map = TagMap.create(); + + // The seamless case: create(...) -> set(Entry) with a null/empty value, no caller guard. + map.set(TagMap.Entry.create("empty", "")); + map.set(TagMap.Entry.create("emptyObj", (Object) "")); + map.set(TagMap.Entry.create("nul", (CharSequence) null)); + assertEquals(0, map.size(), "null/empty values leave no tags behind"); + assertFalse(map.containsKey("empty")); + + // A real value still lands. + map.set(TagMap.Entry.create("present", "v")); + assertEquals(1, map.size()); + assertTrue(map.containsKey("present")); + } +} 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 + } +} 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..da179553f56 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,9 +1064,7 @@ 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()); - } + 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 OptimizedTagMap) { - assertFalse(((OptimizedTagMap) map).checkIfEmpty()); - } + assertFalse(map.checkIfEmpty()); assertFalse(map.isEmpty()); } static void assertEmpty(TagMap map) { - if (map instanceof OptimizedTagMap) { - assertTrue(((OptimizedTagMap) 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 OptimizedTagMap) { - OptimizedTagMap optMap = (OptimizedTagMap) map; - optMap.checkIntegrity(); - } + map.checkIntegrity(); } }