Skip to content

Commit 9f8b226

Browse files
dougqhclaude
andcommitted
Add KnownTags.tagId(serial, fieldPos, name) encoder; dedupe callers
The tagId bit-packing was duplicated in four places (fuzz/unit/Entry tests and the insertion benchmark, which even hand-rolled the name hash). Add a single KnownTags.tagId(globalSerial, fieldPos, name) factory — the inverse of the existing globalSerial/fieldPos/nameHash extractors — that computes nameHash via the runtime's TagMap.Entry._hash so the low 32 bits always match Entry.hash(). Intended for the code generator and tests. Route all callers through it and add an encoder/decoder round-trip test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c89fad9 commit 9f8b226

5 files changed

Lines changed: 26 additions & 22 deletions

File tree

internal-api/src/jmh/java/datadog/trace/api/TagMapInsertionBenchmark.java

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.trace.api;
22

3-
import datadog.trace.api.TagMap.Entry;
43
import java.util.concurrent.TimeUnit;
54
import org.openjdk.jmh.annotations.Benchmark;
65
import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -21,9 +20,8 @@
2120
* Compares tag insertion / lookup by generated tag id vs by string name, with a {@link
2221
* KnownTags.Resolver} registered (the production configuration once code generation is live).
2322
*
24-
* <p>Placed in {@code datadog.trace.api} so it can build tag ids with the same {@code nameHash} the
25-
* runtime uses ({@link TagMap.Entry#_hash}); a mismatch would only matter on the bucket-fallback
26-
* path, but keeping it exact makes the comparison faithful.
23+
* <p>Tag ids are built via {@link KnownTags#tagId} (which uses the runtime's own name hash), so the
24+
* comparison is faithful even on the bucket-fallback path.
2725
*
2826
* <p>The tags use distinct {@code fieldPos} values (no collisions), so every known tag lands in its
2927
* positional slot. byId skips string hashing and the keyOf round-trip entirely; byString pays
@@ -65,16 +63,11 @@ public class TagMapInsertionBenchmark {
6563
// a pre-populated (slotted) map for the read benchmarks; built in setup once IDS exist
6664
TagMap readMap;
6765

68-
static int nameHash(String tag) {
69-
int hash = tag.hashCode();
70-
return hash == 0 ? 0xDD06 : hash ^ (hash >>> 16);
71-
}
72-
7366
@Setup(Level.Trial)
7467
public void setup() {
7568
for (int i = 0; i < NAMES.length; ++i) {
7669
// globalSerial = i + 1 (unique, non-zero); fieldPos = i (distinct - no collisions)
77-
IDS[i] = ((long) (i + 1) << 48) | ((long) i << 32) | (nameHash(NAMES[i]) & 0xFFFFFFFFL);
70+
IDS[i] = KnownTags.tagId(i + 1, i, NAMES[i]);
7871
VALUES[i] = "value-" + i;
7972
}
8073
// Representative resolver: nameOf is a dense array index by globalSerial; keyOf is a hash-table
@@ -100,11 +93,6 @@ public long keyOf(String name) {
10093
return id == null ? 0L : id;
10194
}
10295
});
103-
// sanity: assert _hash matches our nameHash so string lookups hit the same bucket if they ever
104-
// fall through (they shouldn't here, but keep the comparison honest)
105-
if (Entry._hash(NAMES[0]) != nameHash(NAMES[0])) {
106-
throw new IllegalStateException("nameHash mismatch with TagMap.Entry._hash");
107-
}
10896

10997
// pre-populate the read map by id (entries land in their slots)
11098
this.readMap = TagMap.create();

internal-api/src/main/java/datadog/trace/api/KnownTags.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ public static int nameHash(long tagId) {
3939
return (int) tagId;
4040
}
4141

42+
/**
43+
* Builds a tagId from its parts: {@code globalSerial} (globally unique per known tag), {@code
44+
* fieldPos} (the tag's slot within its span type's positional table), and the tag {@code name}
45+
* (whose hash is computed via the same function the runtime uses, so the low 32 bits match {@link
46+
* TagMap.Entry#hash()}). Inverse of {@link #globalSerial}/{@link #fieldPos}/{@link #nameHash}.
47+
* Intended for the code generator and tests.
48+
*/
49+
public static long tagId(int globalSerial, int fieldPos, String name) {
50+
long nameHash = TagMap.Entry._hash(name) & 0xFFFFFFFFL;
51+
return ((long) globalSerial << 48) | ((long) (fieldPos & 0xFFFF) << 32) | nameHash;
52+
}
53+
4254
public interface Resolver {
4355
String nameOf(long tagId);
4456

internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,7 @@ public void removalChange() {
562562
static final String[] TAG_NAMES = {"tag.alpha", "tag.beta", "tag.gamma"};
563563

564564
static long tagId(int serial, int fieldPos, String name) {
565-
long nameHash = TagMap.Entry._hash(name) & 0xFFFFFFFFL;
566-
return ((long) serial << 48) | ((long) fieldPos << 32) | nameHash;
565+
return KnownTags.tagId(serial, fieldPos, name);
567566
}
568567

569568
@BeforeAll

internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,10 @@ static boolean isFuzzKey(String name) {
102102

103103
static long tagIdOf(String key) {
104104
int n = Integer.parseInt(key.substring("key-".length()));
105-
long nameHash = TagMap.Entry._hash(key) & 0xFFFFFFFFL;
106105
// globalSerial = n + 1 (non-zero, unique per key); fieldPos spreads keys across the slot array
107106
// (n % CAPACITY), so distinct keys occupy distinct slots AND keys that share a fieldPos collide
108107
// (first-writer-wins -> the rest fall to buckets), exercising both paths.
109-
int fieldPos = n % OptimizedTagMap.KNOWN_ENTRIES_CAPACITY;
110-
return ((long) (n + 1) << 48) | ((long) fieldPos << 32) | nameHash;
108+
return KnownTags.tagId(n + 1, n % OptimizedTagMap.KNOWN_ENTRIES_CAPACITY, key);
111109
}
112110

113111
// Number of random sequences per @Test run. Default 1 (fast CI); crank via

internal-api/src/test/java/datadog/trace/api/TagMapTagIdTest.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,15 @@ public class TagMapTagIdTest {
3434
static final long DB_SYSTEM_ID = tagId(3, 0, DB_SYSTEM);
3535

3636
static long tagId(int globalSerial, int fieldPos, String name) {
37-
long nameHash = Entry._hash(name) & 0xFFFFFFFFL;
38-
return ((long) globalSerial << 48) | ((long) fieldPos << 32) | nameHash;
37+
return KnownTags.tagId(globalSerial, fieldPos, name);
38+
}
39+
40+
@Test
41+
public void tagId_roundTripsThroughExtractors() {
42+
long id = KnownTags.tagId(7, 13, "some.tag.name");
43+
assertEquals(7, KnownTags.globalSerial(id));
44+
assertEquals(13, KnownTags.fieldPos(id));
45+
assertEquals(Entry._hash("some.tag.name"), KnownTags.nameHash(id));
3946
}
4047

4148
@BeforeEach

0 commit comments

Comments
 (0)