diff --git a/docs/demos/ClassNameTrieDemo.html b/docs/demos/ClassNameTrieDemo.html index 6acf7d1..e2d6318 100644 --- a/docs/demos/ClassNameTrieDemo.html +++ b/docs/demos/ClassNameTrieDemo.html @@ -662,12 +662,12 @@

Mappings

const BUD_MARKER = 0x4000; const GLOB_MARKER = 0x2000; const MAX_NODE_VALUE = 0x1FFF; -const LONG_JUMP_MARKER = 0x8000; // same bit as LEAF_MARKER — safe because jump slots and value slots occupy disjoint positions +const LONG_JUMP_MARKER = 0xF000; // 4096 long-jumps, each >= 61440 const BRANCH_CONTROL_CHARS = 3; const FIRST_JUMP_OMITTED = 1; function resolveJump(longJumps, c) { - return (c & LONG_JUMP_MARKER) === 0 ? c : longJumps[c & ~LONG_JUMP_MARKER]; + return (c & LONG_JUMP_MARKER) === LONG_JUMP_MARKER ? longJumps[c & ~LONG_JUMP_MARKER] : c; } function roleFor(v) { if ((v & LEAF_MARKER) !== 0) return (v & GLOB_MARKER) !== 0 ? "value-leaf-glob" : "value-leaf"; @@ -1042,6 +1042,7 @@

Mappings

_newJump(jump) { if (jump < LONG_JUMP_MARKER) return jump & 0xFFFF; + if (this.longJumpCount > (~LONG_JUMP_MARKER & 0xFFFF)) throw new Error("Class-name does not fit in the trie"); if (this.longJumps === null) this.longJumps = new Array(16).fill(0); else if (this.longJumpCount === this.longJumps.length) { const grown = this.longJumps.concat(new Array(this.longJumpCount >> 1).fill(0)); @@ -1052,9 +1053,11 @@

Mappings

} _updateJump(jump, delta) { - if ((jump & LONG_JUMP_MARKER) === 0) return this._newJump(jump + delta); - this.longJumps[jump & ~LONG_JUMP_MARKER] += delta; - return jump; + if ((jump & LONG_JUMP_MARKER) === LONG_JUMP_MARKER) { + this.longJumps[jump & ~LONG_JUMP_MARKER] += delta; + return jump; + } + return this._newJump(jump + delta); } finalize() { @@ -1194,7 +1197,7 @@

Mappings

const jumpIdx = valueIndex + branchCount - FIRST_JUMP_OMITTED; const rawJump = data[jumpIdx]; const jump = resolveJump(longJumps, rawJump); - const isLong = (rawJump & LONG_JUMP_MARKER) !== 0; + const isLong = (rawJump & LONG_JUMP_MARKER) === LONG_JUMP_MARKER; snap(`Branch ${branchIndex - keysLo} ≥ 1, so apply its jump offset at data[${jumpIdx}] = ${isLong ? "long-jump → " + jump : jump}.`, { highlight: { data: [{ i: jumpIdx, kind: "read" }] @@ -1382,7 +1385,7 @@

Mappings

return { val: kind + ((c & GLOB_MARKER) ? "*" : "") + " " + (c & MAX_NODE_VALUE), role: roleFor(c) }; } } - if ((c & LONG_JUMP_MARKER) !== 0) { + if ((c & LONG_JUMP_MARKER) === LONG_JUMP_MARKER) { return { val: "L" + (c & ~LONG_JUMP_MARKER), role: "jump" }; } if (c >= 0x20 && c < 0x80 && (role === "branch-key" || role === "segment-char")) { diff --git a/utils/src/jmh/java/datadog/instrument/utils/ClassNameTrieBenchmark.java b/utils/src/jmh/java/datadog/instrument/utils/ClassNameTrieBenchmark.java index 6a9a02c..22cad91 100644 --- a/utils/src/jmh/java/datadog/instrument/utils/ClassNameTrieBenchmark.java +++ b/utils/src/jmh/java/datadog/instrument/utils/ClassNameTrieBenchmark.java @@ -1,7 +1,7 @@ package datadog.instrument.utils; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.openjdk.jmh.annotations.Mode.SingleShotTime; +import static org.openjdk.jmh.annotations.Mode.AverageTime; import java.io.IOException; import java.io.UncheckedIOException; @@ -16,7 +16,6 @@ import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Level; -import org.openjdk.jmh.annotations.Measurement; import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.Setup; @@ -26,10 +25,8 @@ import org.openjdk.jmh.infra.Blackhole; @State(Scope.Benchmark) -@BenchmarkMode(SingleShotTime) +@BenchmarkMode(AverageTime) @OutputTimeUnit(MILLISECONDS) -@Warmup(iterations = 0) -@Measurement(iterations = 1) @SuppressWarnings("unused") public class ClassNameTrieBenchmark { @@ -49,23 +46,47 @@ public void setup() { } @Benchmark - @Fork(value = 20) + @Fork(value = 1) @Threads(value = 1) - public void classNameTrie(Blackhole blackhole) { + public void classNameTrieCompiled(Blackhole blackhole) { testClassNameTrie(blackhole); } @Benchmark - @Fork(value = 20) + @Fork(value = 1) @Threads(value = 1) - public void radixTrie(Blackhole blackhole) { + public void radixTrieCompiled(Blackhole blackhole) { testRadixTrie(blackhole); } @Benchmark - @Fork(value = 20) + @Fork(value = 1) @Threads(value = 1) - public void legacyCode(Blackhole blackhole) { + public void legacyCodeCompiled(Blackhole blackhole) { + testLegacyCode(blackhole); + } + + @Benchmark + @Warmup(iterations = 0) + @Fork(value = 1, jvmArgsPrepend = "-Xint") + @Threads(value = 1) + public void classNameTrieInterpreted(Blackhole blackhole) { + testClassNameTrie(blackhole); + } + + @Benchmark + @Warmup(iterations = 0) + @Fork(value = 1, jvmArgsPrepend = "-Xint") + @Threads(value = 1) + public void radixTrieInterpreted(Blackhole blackhole) { + testRadixTrie(blackhole); + } + + @Benchmark + @Warmup(iterations = 0) + @Fork(value = 1, jvmArgsPrepend = "-Xint") + @Threads(value = 1) + public void legacyCodeInterpreted(Blackhole blackhole) { testLegacyCode(blackhole); } diff --git a/utils/src/main/java/datadog/instrument/utils/ClassNameTrie.java b/utils/src/main/java/datadog/instrument/utils/ClassNameTrie.java index eee4e6a..c426b73 100644 --- a/utils/src/main/java/datadog/instrument/utils/ClassNameTrie.java +++ b/utils/src/main/java/datadog/instrument/utils/ClassNameTrie.java @@ -106,8 +106,8 @@ public final class ClassNameTrie { /** Maximum value that can be held in a single node of the trie. */ private static final char MAX_NODE_VALUE = 0x1FFF; - /** Marks a long jump that was replaced by an index into the long jump table. */ - private static final char LONG_JUMP_MARKER = 0x8000; + /** Boundary marker where a jump is replaced by an index into the long jump table. */ + private static final char LONG_JUMP_MARKER = 0xF000; // 4096 long-jumps, each >= 61440 /** A branch has at most 3 control characters: key, value, and optional jump offset/id. */ private static final int BRANCH_CONTROL_CHARS = 3; @@ -191,7 +191,7 @@ public static int apply(char[] data, int[] longJumps, String key, int fromIndex) // jump to the segment/node for the matched branch (branch 0 requires no jump) if (branchIndex > dataIndex) { int branchJump = data[valueIndex + branchCount - FIRST_JUMP_OMITTED]; - if ((branchJump & LONG_JUMP_MARKER) != 0) { + if ((branchJump & LONG_JUMP_MARKER) == LONG_JUMP_MARKER) { branchJump = longJumps[branchJump & ~LONG_JUMP_MARKER]; } dataIndex += branchJump; @@ -838,7 +838,10 @@ private void makeHole(int dataIndex, int hole) { /** Checks a new jump value; long jumps are replaced with an index into the long-jump table. */ private char newJump(int jump) { if (jump < LONG_JUMP_MARKER) { - return (char) jump; // jump is small enough to fit into the trie + return (char) jump; // jump is small enough to fit in the trie + } + if (longJumpCount > (char) ~LONG_JUMP_MARKER) { + throw new IllegalArgumentException("Class-name does not fit in the trie"); } if (longJumps == null) { longJumps = new int[16]; // create table on first use @@ -852,17 +855,22 @@ private char newJump(int jump) { /** Retrieves the true jump value, consulting the long-jump table if necessary. */ private int getJump(char jump) { - return (jump & LONG_JUMP_MARKER) == 0 ? jump : longJumps[jump & ~LONG_JUMP_MARKER]; + return isLongJump(jump) ? longJumps[jump & ~LONG_JUMP_MARKER] : jump; } /** Increases jump by the given delta, this may result in it becoming a long-jump. */ private char updateJump(char jump, int delta) { - if ((jump & LONG_JUMP_MARKER) == 0) { + if (isLongJump(jump)) { + // just need to update the table entry + longJumps[jump & ~LONG_JUMP_MARKER] += delta; + return jump; + } else { return newJump(jump + delta); } - // already a long-jump, just update table value - longJumps[jump & ~LONG_JUMP_MARKER] += delta; - return jump; + } + + private boolean isLongJump(char jump) { + return (jump & LONG_JUMP_MARKER) == LONG_JUMP_MARKER; } /** Records a jump index to be updated once we know how much content was added. */ diff --git a/utils/src/test/java/datadog/instrument/utils/ClassNameTrieTest.java b/utils/src/test/java/datadog/instrument/utils/ClassNameTrieTest.java index 83661c3..263d6cb 100644 --- a/utils/src/test/java/datadog/instrument/utils/ClassNameTrieTest.java +++ b/utils/src/test/java/datadog/instrument/utils/ClassNameTrieTest.java @@ -2,6 +2,7 @@ import static java.util.Collections.singletonList; import static java.util.function.Function.identity; +import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toMap; import static javax.tools.ToolProvider.getSystemJavaCompiler; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -26,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; import javax.tools.JavaCompiler; @@ -205,7 +205,7 @@ void badData() { assertThrows(IllegalArgumentException.class, () -> builder.put("test", 8192)); assertThrows( IllegalArgumentException.class, - () -> builder.put(Stream.generate(() -> "_").limit(8192).collect(Collectors.joining()), 1)); + () -> builder.put(Stream.generate(() -> "_").limit(8192).collect(joining()), 1)); // check that a bad magic header fails assertThrows( @@ -249,6 +249,30 @@ void overflow() { } } + @Test + void tooManyLongJumps() { + // 65 groups x 64 long-jumps/group > 4096-entry limit; each group's sub-trie spans + // ~61600 chars (8 padded entries) forcing 64 small leaf entries to use long-jumps + String padding = Stream.generate(() -> " ").limit(7700).collect(joining()); + ClassNameTrie.Builder builder = new ClassNameTrie.Builder(); + for (int g = 1; g <= 65; g++) { + char groupChar = (char) (g <= 41 ? g : g + 1); // skip '*' = chr(42) + for (int j = 1; j <= 8; j++) { + builder.put("" + groupChar + (char) 1 + (char) j + padding, 1); + } + for (int k = 2; k <= 65; k++) { + char smallChar = (char) (k <= 41 ? k : k + 1); // skip '*' = chr(42) + builder.put("" + groupChar + smallChar, 1); + } + } + for (int j = 1; j <= 8; j++) { + builder.put("" + ((char) 67) + '\u0001' + (char) j + padding, 1); + } + // last put should exceed the number of allowed long-jumps + assertThrows( + IllegalArgumentException.class, () -> builder.put("" + ((char) 67) + ((char) 2), 1)); + } + @Test void overwriting() { Map data =