diff --git a/spectator-api/src/jmh/java/com/netflix/spectator/perf/AsciiSetBench.java b/spectator-api/src/jmh/java/com/netflix/spectator/perf/AsciiSetBench.java new file mode 100644 index 000000000..fb855d480 --- /dev/null +++ b/spectator-api/src/jmh/java/com/netflix/spectator/perf/AsciiSetBench.java @@ -0,0 +1,131 @@ +/* + * Copyright 2014-2026 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.netflix.spectator.perf; + +import com.netflix.spectator.impl.AsciiSet; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.infra.Blackhole; + +import java.util.Optional; + +/** + * Measures {@link AsciiSet} on the inputs that matter in practice: {@code containsAll} and + * {@code replaceNonMembers} cover the meter lookup path, the rest cover pattern compilation. + * + *

To A/B a change here, build two jmh jars from two git states of {@code AsciiSet.java} and + * run this benchmark against each, rather than reproducing an old implementation in this file: + * which class a method compiles as part of can shift its measured throughput more than the + * algorithm difference being isolated.

+ */ +@State(Scope.Thread) +public class AsciiSetBench { + + /** The character set the Atlas registry applies to every tag key and value. */ + private static final String VALID = "-._A-Za-z0-9~^"; + + private final AsciiSet set = AsciiSet.fromPattern(VALID); + private final AsciiSet other = AsciiSet.fromPattern("a-z"); + private final AsciiSet setCopy = AsciiSet.fromPattern(VALID); + + // Built in setup, not assigned from literals inline, to avoid constant folding. + + /** Typical tag value: short and already valid. */ + private String shortClean; + + /** Typical metric name: longer, still valid. */ + private String longClean; + + /** A value that needs fixing, with the first bad character near the end. */ + private String dirtyLate; + + /** A value that needs fixing from the very first character. */ + private String dirtyEarly; + + /** Text to build a set from, kept separate from {@code VALID} for the same reason as above. */ + private String patternText; + + @Setup + public void setup() { + shortClean = new String("us-east-1c".toCharArray()); + longClean = new String("ipc.server.call.duration.percentile".toCharArray()); + dirtyLate = new String("deploy.prod.us-east-1/cluster".toCharArray()); + dirtyEarly = new String("/api/v1/users?id=42&name=bob".toCharArray()); + patternText = new String(VALID.toCharArray()); + } + + @Benchmark public AsciiSet fromPattern() { + return AsciiSet.fromPattern(patternText); + } + + // containsAll on already valid input: the dominant case on the meter lookup path. + + @Benchmark public boolean shortClean() { + return set.containsAll(shortClean); + } + + @Benchmark public boolean longClean() { + return set.containsAll(longClean); + } + + // containsAll that fails, so the scan exits early. + + @Benchmark public boolean dirty() { + return set.containsAll(dirtyLate); + } + + // replaceNonMembers: the allocating path. + + @Benchmark public String replaceClean() { + return set.replaceNonMembers(longClean, '_'); + } + + @Benchmark public String replaceLate() { + return set.replaceNonMembers(dirtyLate, '_'); + } + + @Benchmark public String replaceEarly() { + return set.replaceNonMembers(dirtyEarly, '_'); + } + + // Single character membership, as used by the pattern matcher. + + @Benchmark public void contains(Blackhole bh) { + bh.consume(set.contains('a')); + bh.consume(set.contains('/')); + bh.consume(set.contains('é')); + } + + // Set algebra and identity, as used while compiling a pattern. + + @Benchmark public AsciiSet union() { + return set.union(other); + } + + @Benchmark public boolean equalsSet() { + return set.equals(setCopy); + } + + @Benchmark public boolean isEmpty() { + return set.isEmpty(); + } + + @Benchmark public Optional character() { + return set.character(); + } +} diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/AsciiSet.java b/spectator-api/src/main/java/com/netflix/spectator/impl/AsciiSet.java index f39776e9a..638ac3edf 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/impl/AsciiSet.java +++ b/spectator-api/src/main/java/com/netflix/spectator/impl/AsciiSet.java @@ -16,7 +16,6 @@ package com.netflix.spectator.impl; import java.io.Serializable; -import java.util.Arrays; import java.util.Optional; /** @@ -30,7 +29,16 @@ */ public final class AsciiSet implements Serializable { - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = 2L; + + /** Golden ratio. A small odd multiplier like 31 leaves a raw bit mask's structure intact. */ + private static final long HASH_MIXER = 0x9E3779B97F4A7C15L; + + private static void checkAscii(char c) { + if (c >= 128) { + throw new IllegalArgumentException("invalid pattern, '" + c + "' is not ascii"); + } + } /** * Create a set containing ascii characters using a simple pattern. The pattern is similar @@ -45,73 +53,90 @@ public final class AsciiSet implements Serializable { * Set containing the characters specified in {@code pattern}. */ public static AsciiSet fromPattern(String pattern) { - final boolean[] members = new boolean[128]; + long b0 = 0L; + long b1 = 0L; final int n = pattern.length(); for (int i = 0; i < n; ++i) { final char c = pattern.charAt(i); - if (c >= members.length) { - throw new IllegalArgumentException("invalid pattern, '" + c + "' is not ascii"); - } final boolean isStartOrEnd = i == 0 || i == n - 1; if (isStartOrEnd || c != '-') { - members[c] = true; + checkAscii(c); + if (c < 64) { + b0 |= 1L << c; + } else { + b1 |= 1L << c; + } } else { final char s = pattern.charAt(i - 1); final char e = pattern.charAt(i + 1); for (char v = s; v <= e; ++v) { - members[v] = true; + checkAscii(v); + if (v < 64) { + b0 |= 1L << v; + } else { + b1 |= 1L << v; + } } } } - return new AsciiSet(members); + return new AsciiSet(b0, b1); } /** Returns a set that matches no characters. */ public static AsciiSet none() { - final boolean[] members = new boolean[128]; - return new AsciiSet(members); + return new AsciiSet(0L, 0L); } /** Returns a set that matches all ascii characters. */ public static AsciiSet all() { - final boolean[] members = new boolean[128]; - Arrays.fill(members, true); - return new AsciiSet(members); + // -1L has every bit set, so both words match every character. + return new AsciiSet(-1L, -1L); } /** Returns a set that matches ascii control characters. */ public static AsciiSet control() { - final boolean[] members = new boolean[128]; - for (char c = 0; c < members.length; ++c) { - members[c] = Character.isISOControl(c); + long b0 = 0L; + long b1 = 0L; + for (char c = 0; c < 128; ++c) { + if (Character.isISOControl(c)) { + if (c < 64) { + b0 |= 1L << c; + } else { + b1 |= 1L << c; + } + } } - return new AsciiSet(members); + return new AsciiSet(b0, b1); } - /** - * Converts the members array to a pattern string. Used to provide a user friendly toString - * implementation for the set. - */ - private static String toPattern(boolean[] members) { + private static boolean memberBit(long b0, long b1, char c) { + final long word = (c < 64) ? b0 : b1; + // Relies on JLS 15.19: a long shift masks its distance to the low 6 bits, so `c` correctly + // selects the bit within whichever word was chosen even though c can be up to 127. Masking + // explicitly (`c & 0x3F`) is slower, since x86's shrx already masks the count in hardware. + return c < 128 && ((word >>> c) & 1L) != 0L; + } + + private String toPattern() { StringBuilder buf = new StringBuilder(); - if (members['-']) { + if (contains('-')) { buf.append('-'); } boolean previous = false; char s = 0; - for (int i = 0; i < members.length; ++i) { - if (members[i] && !previous) { + for (int i = 0; i < 128; ++i) { + final boolean member = contains((char) i); + if (member && !previous) { s = (char) i; - } else if (!members[i] && previous) { + } else if (!member && previous) { final char e = (char) (i - 1); append(buf, s, e); } - previous = members[i]; + previous = member; } if (previous) { - final char e = (char) (members.length - 1); - append(buf, s, e); + append(buf, s, (char) 127); } return buf.toString(); } @@ -124,12 +149,15 @@ private static void append(StringBuilder buf, char s, char e) { } } - private final String pattern; - private final boolean[] members; + /** Membership bits for characters 0-63. */ + private final long bits0; - private AsciiSet(boolean[] members) { - this.members = Preconditions.checkNotNull(members, "members array cannot be null"); - this.pattern = toPattern(members); + /** Membership bits for characters 64-127. */ + private final long bits1; + + private AsciiSet(long bits0, long bits1) { + this.bits0 = bits0; + this.bits1 = bits1; } /** @@ -137,7 +165,7 @@ private AsciiSet(boolean[] members) { * operation. */ public boolean contains(char c) { - return c < 128 && members[c]; + return memberBit(bits0, bits1, c); } /** @@ -146,7 +174,7 @@ public boolean contains(char c) { public boolean containsAll(CharSequence str) { final int n = str.length(); for (int i = 0; i < n; ++i) { - if (!contains(str.charAt(i))) { + if (!memberBit(bits0, bits1, str.charAt(i))) { return false; } } @@ -156,7 +184,7 @@ public boolean containsAll(CharSequence str) { private int indexOfNonMember(CharSequence str) { final int n = str.length(); for (int i = 0; i < n; ++i) { - if (!contains(str.charAt(i))) { + if (!memberBit(bits0, bits1, str.charAt(i))) { return i; } } @@ -168,7 +196,7 @@ private int indexOfNonMember(CharSequence str) { */ public String replaceNonMembers(String input, char replacement) { if (!contains(replacement)) { - throw new IllegalArgumentException(replacement + " is not a member of " + pattern); + throw new IllegalArgumentException(replacement + " is not a member of " + toString()); } int i = indexOfNonMember(input); return i < input.length() ? replaceNonMembersImpl(input, i, replacement) : input; @@ -179,11 +207,7 @@ public String replaceNonMembers(String input, char replacement) { * set that is provided. */ public AsciiSet union(AsciiSet set) { - final boolean[] unionMembers = new boolean[128]; - for (int i = 0; i < unionMembers.length; ++i) { - unionMembers[i] = members[i] || set.members[i]; - } - return new AsciiSet(unionMembers); + return new AsciiSet(bits0 | set.bits0, bits1 | set.bits1); } /** @@ -191,11 +215,7 @@ public AsciiSet union(AsciiSet set) { * set that is provided. */ public AsciiSet intersection(AsciiSet set) { - final boolean[] intersectionMembers = new boolean[128]; - for (int i = 0; i < intersectionMembers.length; ++i) { - intersectionMembers[i] = members[i] && set.members[i]; - } - return new AsciiSet(intersectionMembers); + return new AsciiSet(bits0 & set.bits0, bits1 & set.bits1); } /** @@ -203,32 +223,24 @@ public AsciiSet intersection(AsciiSet set) { * set that is provided. */ public AsciiSet diff(AsciiSet set) { - final boolean[] diffMembers = new boolean[128]; - for (int i = 0; i < diffMembers.length; ++i) { - diffMembers[i] = members[i] && !set.members[i]; - } - return new AsciiSet(diffMembers); + return new AsciiSet(bits0 & ~set.bits0, bits1 & ~set.bits1); } /** * Returns a new set that will match characters that are not included this set. */ public AsciiSet invert() { - final boolean[] invertMembers = new boolean[128]; - for (int i = 0; i < invertMembers.length; ++i) { - invertMembers[i] = !members[i]; - } - return new AsciiSet(invertMembers); + // Every bit maps to a real ascii character, so there are no padding bits to mask off. + return new AsciiSet(~bits0, ~bits1); } private String replaceNonMembersImpl(String input, int start, char replacement) { - final int n = input.length(); final char[] buf = input.toCharArray(); buf[start] = replacement; - for (int i = start + 1; i < n; ++i) { - final char c = input.charAt(i); - if (!contains(c)) + for (int i = start + 1; i < buf.length; ++i) { + if (!memberBit(bits0, bits1, input.charAt(i))) { buf[i] = replacement; + } } return new String(buf); } @@ -238,39 +250,38 @@ private String replaceNonMembersImpl(String input, int start, char replacement) * Otherwise return an empty optional. */ public Optional character() { - char c = 0; - int count = 0; - for (int i = 0; i < members.length; ++i) { - if (members[i]) { - c = (char) i; - ++count; - } + if (bits1 == 0L && Long.bitCount(bits0) == 1) { + return Optional.of((char) Long.numberOfTrailingZeros(bits0)); + } + if (bits0 == 0L && Long.bitCount(bits1) == 1) { + return Optional.of((char) (64 + Long.numberOfTrailingZeros(bits1))); } - return (count == 1) ? Optional.of(c) : Optional.empty(); + return Optional.empty(); } /** Returns true if this set is isEmpty. */ public boolean isEmpty() { - for (boolean b : members) { - if (b) { - return false; - } - } - return true; + return (bits0 | bits1) == 0L; } @Override public String toString() { - return pattern; + return toPattern(); } @Override public int hashCode() { - return pattern.hashCode() + 31 * Arrays.hashCode(members); + // Folding the words straight through Long.hashCode would hash every set like its + // complement, since ~hi ^ ~lo == hi ^ lo. The xor-shift between them is load bearing too: + // 2^63 is a fixed point of odd multiplication, so char 63 would hash like char 127. + long h = bits0 * HASH_MIXER; + h ^= h >>> 32; + h = (h + bits1) * HASH_MIXER; + return (int) (h ^ (h >>> 32)); } @Override public boolean equals(Object obj) { if (this == obj) return true; - if (obj == null || !(obj instanceof AsciiSet)) return false; + if (!(obj instanceof AsciiSet)) return false; AsciiSet other = (AsciiSet) obj; - return pattern.equals(other.pattern) && Arrays.equals(members, other.members); + return bits0 == other.bits0 && bits1 == other.bits1; } } diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/AsciiSetTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/AsciiSetTest.java index f5d3acab4..602ca960a 100644 --- a/spectator-api/src/test/java/com/netflix/spectator/impl/AsciiSetTest.java +++ b/spectator-api/src/test/java/com/netflix/spectator/impl/AsciiSetTest.java @@ -20,6 +20,9 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.HashMap; +import java.util.Map; + public class AsciiSetTest { @Test @@ -158,4 +161,27 @@ public void equalsContractTest() { .suppress(Warning.NULL_FIELDS) .verify(); } + + @Test + public void hashCodeDistinctForEverySingleCharacter() { + // Exhaustive rather than a few chosen pairs: which pairs collide depends on how the two + // words are combined, so a fixed pair stops covering the defect as soon as that changes. + Map seen = new HashMap<>(); + for (char c = 0; c < 128; ++c) { + Character previous = seen.put(AsciiSet.fromPattern(String.valueOf(c)).hashCode(), c); + if (previous != null) { + Assertions.fail("chars " + (int) previous.charValue() + " and " + (int) c + + " have the same hashCode"); + } + } + } + + @Test + public void hashCodeDistinguishesSetFromComplement() { + AsciiSet lower = AsciiSet.fromPattern("a-z"); + Assertions.assertNotEquals(lower.hashCode(), lower.invert().hashCode()); + Assertions.assertNotEquals(AsciiSet.none().hashCode(), AsciiSet.all().hashCode()); + Assertions.assertNotEquals( + lower.hashCode(), AsciiSet.fromPattern("A-Z").hashCode()); + } }