Jackie-Jiang commented on code in PR #20153:
URL: https://github.com/apache/pinot/pull/20153#discussion_r4475640624
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/Utf8Utils.java:
##########
@@ -18,57 +18,71 @@
*/
package org.apache.pinot.spi.utils;
-import com.google.common.base.Utf8;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+import java.nio.ByteOrder;
import java.nio.charset.StandardCharsets;
+/// UTF-8 helpers whose sizing and encoding agree: [#encodedLength] returns
exactly the length of the array that
+/// [#encode] produces for the same string.
public class Utf8Utils {
private Utf8Utils() {
}
+ // Byte order is irrelevant: isAscii() only ORs the bytes together
+ private static final VarHandle LONG_VIEW =
+ MethodHandles.byteArrayViewVarHandle(long[].class,
ByteOrder.nativeOrder());
+
public static byte[] encode(String s) {
return s.getBytes(StandardCharsets.UTF_8);
}
+ /// Returns the number of bytes in the UTF-8 encoding of `s`, which is
exactly `encode(s).length`. An unpaired
+ /// surrogate counts as the single replacement byte that [String#getBytes]
writes for it, so this never throws.
public static int encodedLength(String s) {
Review Comment:
Keeping the removal rather than adding a deprecated delegate:
`encodedLengthWithReplacement` shipped only in 1.6.0, and after this PR no
caller remains in Pinot. The Binary Compatibility Check failure is expected and
limited to that one method.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/Utf8Utils.java:
##########
@@ -18,57 +18,71 @@
*/
package org.apache.pinot.spi.utils;
-import com.google.common.base.Utf8;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+import java.nio.ByteOrder;
import java.nio.charset.StandardCharsets;
+/// UTF-8 helpers whose sizing and encoding agree: [#encodedLength] returns
exactly the length of the array that
+/// [#encode] produces for the same string.
public class Utf8Utils {
private Utf8Utils() {
}
+ // Byte order is irrelevant: isAscii() only ORs the bytes together
+ private static final VarHandle LONG_VIEW =
+ MethodHandles.byteArrayViewVarHandle(long[].class,
ByteOrder.nativeOrder());
+
public static byte[] encode(String s) {
return s.getBytes(StandardCharsets.UTF_8);
}
+ /// Returns the number of bytes in the UTF-8 encoding of `s`, which is
exactly `encode(s).length`. An unpaired
+ /// surrogate counts as the single replacement byte that [String#getBytes]
writes for it, so this never throws.
public static int encodedLength(String s) {
- return Utf8.encodedLength(s);
- }
-
- /// Returns the UTF-8 byte length using the same malformed UTF-16
replacement behavior as [String#getBytes()].
- /// Unlike [#encodedLength(String)], this method does not reject unpaired
surrogate code units.
- public static int encodedLengthWithReplacement(String s) {
- int length = 0;
- int index = 0;
- while (index < s.length()) {
- char character = s.charAt(index);
- if (character <= 0x7f) {
- length++;
- } else if (character <= 0x7ff) {
- length += 2;
- } else if (Character.isHighSurrogate(character) && index + 1 < s.length()
- && Character.isLowSurrogate(s.charAt(index + 1))) {
- length += 4;
- index++;
- } else if (Character.isSurrogate(character)) {
- length++;
+ int utf16Length = s.length();
+ // Fast path for the common all-ASCII prefix, one byte per char
+ int i = 0;
+ while (i < utf16Length && s.charAt(i) < 0x80) {
+ i++;
+ }
+ int utf8Length = i;
+ for (; i < utf16Length; i++) {
+ char c = s.charAt(i);
+ if (c < 0x80) {
+ utf8Length++;
+ } else if (c < 0x800) {
Review Comment:
Rewrote the loop branch-free as suggested, starting from the UTF-16 length
and adding only the extra bytes. That alone still left CJK about 1.6x behind
Guava, and the remaining cost turned out to be `Character.isSurrogate` itself:
C2's `fold_compares` merges two adjacent trap-guarded range checks into one
unsigned comparison only when both are in the same method, so with the helper
inlined the loop kept two compare-and-branch pairs per char (`cmp #0x800; b.lt;
mov #0xd800; cmp; b.ge` vs `sub #0x800; cmp #0xd000; b.hs`). The surrogate
ranges are now compared inline.
JMH on JDK 25, 1024 strings of 64 chars, 2 forks x 5 iterations, Guava vs
this PR in us/op: ASCII 13.6 vs 13.8, Latin with 1 in 8 `é` 19.4 vs 19.4, CJK
17.1 vs 17.1, emoji 33.5 vs 26.3. The count was fuzzed against
`String.getBytes(UTF_8).length` on 2M random surrogate-heavy strings and every
UTF-16 code unit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
