From 31c6dedb82068dd206612ca26a90c1122b66b9bf Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 22 Aug 2026 00:42:50 +0100 Subject: [PATCH 1/3] use SWAR in SmileParser --- .../jackson/dataformat/smile/SmileParser.java | 38 ++++------- .../dataformat/smile/SmileParserBase.java | 47 ++++++++++++++ .../dataformat/smile/SmileVarHandleUtil.java | 65 +++++++++++++++++++ .../async/NonBlockingByteArrayParser.java | 6 +- 4 files changed, 125 insertions(+), 31 deletions(-) create mode 100644 smile/src/main/java/tools/jackson/dataformat/smile/SmileVarHandleUtil.java diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java index e1dbfb523..b1f19a938 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java @@ -999,10 +999,8 @@ private final int _nextNameOptimized(PropertyNameMatcher matcher, int len) throw int inPtr = _inputPtr; // First quadbyte is easy - int q1 = (inBuf[inPtr++] & 0xFF); - q1 = (q1 << 8) | (inBuf[inPtr++] & 0xFF); - q1 = (q1 << 8) | (inBuf[inPtr++] & 0xFF); - q1 = (q1 << 8) | (inBuf[inPtr++] & 0xFF); + int q1 = _decodeQuad(inBuf, inPtr); + inPtr += 4; if (len < 9) { int q2 = (inBuf[inPtr++] & 0xFF); @@ -1021,10 +1019,8 @@ private final int _nextNameOptimized(PropertyNameMatcher matcher, int len) throw return matcher.matchByQuad(q1, q2); } - int q2 = (inBuf[inPtr++] & 0xFF); - q2 = (q2 << 8) | (inBuf[inPtr++] & 0xFF); - q2 = (q2 << 8) | (inBuf[inPtr++] & 0xFF); - q2 = (q2 << 8) | (inBuf[inPtr++] & 0xFF); + int q2 = _decodeQuad(inBuf, inPtr); + inPtr += 4; if (len < 13) { int q3 = (inBuf[inPtr++] & 0xFF); @@ -1070,11 +1066,8 @@ private final int _nextNameFromSymbolsLong(PropertyNameMatcher matcher, final byte[] inBuf = _inputBuffer; do { - int q = (inBuf[inPtr++] & 0xFF); - q = (q << 8) | inBuf[inPtr++] & 0xFF; - q = (q << 8) | inBuf[inPtr++] & 0xFF; - q = (q << 8) | inBuf[inPtr++] & 0xFF; - _quadBuffer[offset++] = q; + _quadBuffer[offset++] = _decodeQuad(inBuf, inPtr); + inPtr += 4; } while ((len -= 4) > 3); // and then leftovers if (len > 0) { @@ -2146,10 +2139,8 @@ private final String _findDecodedFromSymbols(final int len) throws JacksonExcept int inPtr = _inputPtr; // First quadbyte is easy - int q1 = (inBuf[inPtr++] & 0xFF); - q1 = (q1 << 8) | (inBuf[inPtr++] & 0xFF); - q1 = (q1 << 8) | (inBuf[inPtr++] & 0xFF); - q1 = (q1 << 8) | (inBuf[inPtr++] & 0xFF); + int q1 = _decodeQuad(inBuf, inPtr); + inPtr += 4; if (len < 9) { int q2 = _padQuadForNulls(inBuf[inPtr++]); @@ -2168,10 +2159,8 @@ private final String _findDecodedFromSymbols(final int len) throws JacksonExcept return _symbols.findName(q1, q2); } - int q2 = (inBuf[inPtr++] & 0xFF); - q2 = (q2 << 8) | (inBuf[inPtr++] & 0xFF); - q2 = (q2 << 8) | (inBuf[inPtr++] & 0xFF); - q2 = (q2 << 8) | (inBuf[inPtr++] & 0xFF); + int q2 = _decodeQuad(inBuf, inPtr); + inPtr += 4; if (len < 13) { int q3 = _padQuadForNulls(inBuf[inPtr++]); @@ -2215,11 +2204,8 @@ private final String _findDecodedFixed12(int len, int q1, int q2) throws Jackson final byte[] inBuf = _inputBuffer; do { - int q = (inBuf[inPtr++] & 0xFF); - q = (q << 8) | inBuf[inPtr++] & 0xFF; - q = (q << 8) | inBuf[inPtr++] & 0xFF; - q = (q << 8) | inBuf[inPtr++] & 0xFF; - _quadBuffer[offset++] = q; + _quadBuffer[offset++] = _decodeQuad(inBuf, inPtr); + inPtr += 4; } while ((len -= 4) > 3); // and then leftovers if (len > 0) { diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java index 5ebc80275..9b4f572a9 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java @@ -30,6 +30,27 @@ public abstract class SmileParserBase extends ParserMinimalBase protected final static JacksonFeatureSet SMILE_READ_CAPABILITIES = DEFAULT_READ_CAPABILITIES.with(StreamReadCapability.EXACT_FLOATS); + /** + * Whether {@code VarHandle}-based array access is usable on this runtime; + * probed once at class load. See {@link #_decodeQuad}. + * + * @since 3.3 + */ + private final static boolean _VARHANDLE_AVAILABLE = _checkVarHandleAvailable(); + + private static boolean _checkVarHandleAvailable() { + // NOTE: this call is what first loads `SmileVarHandleUtil`, and that class + // names `VarHandle` in its field/method signatures. On a runtime without + // `java.lang.invoke.VarHandle` (some Android builds) loading it raises + // `NoClassDefFoundError` -- an Error, not an Exception -- so `Throwable` + // is what has to be caught here. + try { + return SmileVarHandleUtil.isAvailable(); + } catch (Throwable t) { + return false; + } + } + /* /********************************************************************** /* Config @@ -720,6 +741,32 @@ protected void _reportMismatchedEndMarker(int actCh, char expCh) throws StreamRe (char) actCh, expCh, ctxt.typeDesc(), ctxt.startLocation(_sourceReference()))); } + /** + * Helper method for decoding 4 bytes of an Object property name into the + * "quad" (big-endian {@code int}) form used by {@link ByteQuadsCanonicalizer}. + *

+ * Uses a single unaligned load via {@code VarHandle} where supported, + * falling back to byte shifting otherwise. Since {@link #_VARHANDLE_AVAILABLE} + * is a {@code static final} the branch folds away at JIT time; and on the + * fallback path {@code SmileVarHandleUtil} is never resolved, which is what + * keeps this working on runtimes that lack {@code VarHandle} entirely. + *

+ * Exists on the base class so that subclasses in the {@code async} package + * can share the same implementation; caller MUST have verified that 4 bytes + * are readable at given offset. + * + * @since 3.3 + */ + protected final static int _decodeQuad(byte[] buffer, int offset) { + if (_VARHANDLE_AVAILABLE) { + return SmileVarHandleUtil.getIntBE(buffer, offset); + } + return ((buffer[offset] & 0xFF) << 24) + | ((buffer[offset+1] & 0xFF) << 16) + | ((buffer[offset+2] & 0xFF) << 8) + | (buffer[offset+3] & 0xFF); + } + /** * Helper method used to encapsulate logic of including (or not) of * "source reference" when constructing {@link TokenStreamLocation} instances. diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/SmileVarHandleUtil.java b/smile/src/main/java/tools/jackson/dataformat/smile/SmileVarHandleUtil.java new file mode 100644 index 000000000..511e802c0 --- /dev/null +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileVarHandleUtil.java @@ -0,0 +1,65 @@ +package tools.jackson.dataformat.smile; + +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.nio.ByteOrder; + +/** + * Utility class that provides {@link VarHandle}-based access for reading + * multi-byte primitives out of byte arrays. + *

+ * IMPORTANT: this class references {@link VarHandle} in field and method + * signatures, so on a runtime that does not provide {@code java.lang.invoke.VarHandle} + * at all it will fail to link, before any code here gets a chance to run. + * Callers MUST therefore both: + *

    + *
  1. load this class from within a {@code try}/{@code catch (Throwable)} block, + * so that {@link LinkageError} is caught, and
    2. + *
  2. keep the byte-shifting fallback in a class that does not reference + * {@link VarHandle}, so the fallback path never resolves this class.
    3. + *
+ * {@code SmileParserBase._decodeQuad()} does both; see it for the pattern. + * + * @since 3.3 + */ +final class SmileVarHandleUtil +{ + /** + * VarHandle for reading 4 big-endian bytes as an {@code int}. + * {@code null} if {@code byteArrayViewVarHandle()} is unsupported. + */ + private static final VarHandle INT_BE; + + static { + VarHandle intBe = null; + try { + intBe = MethodHandles.byteArrayViewVarHandle(int[].class, ByteOrder.BIG_ENDIAN); + } catch (Throwable t) { + // Byte-array views not supported: caller falls back to byte shifting + } + INT_BE = intBe; + } + + private SmileVarHandleUtil() { } + + /** + * @return {@code true} if {@link #getIntBE} may be called; if {@code false}, + * caller MUST use its own byte-shifting fallback + */ + static boolean isAvailable() { + return INT_BE != null; + } + + /** + * Reads 4 bytes starting at given offset as a big-endian {@code int}. + *

+ * Only to be called if {@link #isAvailable()} returned {@code true}: the + * handle is dereferenced unconditionally, and the fallback lives in the + * caller, not here. + * Caller MUST also have verified that {@code offset+4} is within bounds of + * given array. + */ + static int getIntBE(byte[] buffer, int offset) { + return (int) INT_BE.get(buffer, offset); + } +} diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java b/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java index 001cce336..05f140e85 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java @@ -702,11 +702,7 @@ private final JsonToken _finishLongFieldName(int outPtr) throws JacksonException int quadCount = 0; for (final int inEnd = (outPtr & ~3); in < inEnd; in += 4) { - int q = (copyBuffer[in] << 24) - | ((copyBuffer[in+1] & 0xFF) << 16) - | ((copyBuffer[in+2] & 0xFF) << 8) - | (copyBuffer[in+3] & 0xFF); - quads[quadCount++] = q; + quads[quadCount++] = _decodeQuad(copyBuffer, in); } // and possibly more... ? if (in < outPtr) { // at least 1 From 48c116ddb26e00bd258458fd60d7722bc21f65ac Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 21 Aug 2026 19:15:15 -0700 Subject: [PATCH 2/3] add release notes --- release-notes/CREDITS | 3 +++ release-notes/VERSION | 2 ++ 2 files changed, 5 insertions(+) diff --git a/release-notes/CREDITS b/release-notes/CREDITS index 19b9d67a8..71bf6021d 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -64,3 +64,6 @@ PJ Fanning (@pjfanning) * Contributed #752: (cbor) Use `VarHandle` for multi-byte primitive writes in `CBORGenerator` (3.3.0) +* Contributed #757: (smile) Use `VarHandle` for multi-byte primitive reads in + `SmileParser` + (3.3.0) diff --git a/release-notes/VERSION b/release-notes/VERSION index d4f0f6dcc..01511734b 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -20,6 +20,8 @@ implementations) (contributed by @pjfanning) #752: (cbor) Use `VarHandle` for multi-byte primitive writes in `CBORGenerator` (contributed by @pjfanning) +#757: (smile) Use `VarHandle` for multi-byte primitive reads in `SmileParser` + (contributed by @pjfanning) 3.2.3 (not yet released) From 8c1b1fc4303ef2af1b2f80f1392e3bd938dc0860 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 21 Aug 2026 19:32:07 -0700 Subject: [PATCH 3/3] Extend the fix, add testing --- .../jackson/dataformat/smile/SmileParser.java | 106 ++-------- .../dataformat/smile/SmileParserBase.java | 33 +++ .../async/NonBlockingByteArrayParser.java | 11 +- .../smile/async/NonBlockingParserBase.java | 48 +---- .../smile/parse/NameQuadDecodingTest.java | 190 ++++++++++++++++++ 5 files changed, 251 insertions(+), 137 deletions(-) create mode 100644 smile/src/test/java/tools/jackson/dataformat/smile/parse/NameQuadDecodingTest.java diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java index b1f19a938..e9a8153ed 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java @@ -979,18 +979,7 @@ private final int _nextNameOptimized(PropertyNameMatcher matcher, int len) throw } // First: maybe we already have this name decoded? if (len < 5) { - int inPtr = _inputPtr; - final byte[] inBuf = _inputBuffer; - int q = inBuf[inPtr] & 0xFF; - if (len > 1) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (len > 2) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (len > 3) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - } - } - } + int q = _decodePartialQuad(_inputBuffer, _inputPtr, len); _quad1 = q; return matcher.matchByQuad(q); } @@ -1003,17 +992,7 @@ private final int _nextNameOptimized(PropertyNameMatcher matcher, int len) throw inPtr += 4; if (len < 9) { - int q2 = (inBuf[inPtr++] & 0xFF); - int left = len - 5; - if (left > 0) { - q2 = (q2 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 1) { - q2 = (q2 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 2) { - q2 = (q2 << 8) + (inBuf[inPtr++] & 0xFF); - } - } - } + int q2 = _decodePartialQuad(inBuf, inPtr, len - 4); _quad1 = q1; _quad2 = q2; return matcher.matchByQuad(q1, q2); @@ -1023,17 +1002,7 @@ private final int _nextNameOptimized(PropertyNameMatcher matcher, int len) throw inPtr += 4; if (len < 13) { - int q3 = (inBuf[inPtr++] & 0xFF); - int left = len - 9; - if (left > 0) { - q3 = (q3 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 1) { - q3 = (q3 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 2) { - q3 = (q3 << 8) + (inBuf[inPtr++] & 0xFF); - } - } - } + int q3 = _decodePartialQuad(inBuf, inPtr, len - 8); _quad1 = q1; _quad2 = q2; _quad3 = q3; @@ -1071,14 +1040,7 @@ private final int _nextNameFromSymbolsLong(PropertyNameMatcher matcher, } while ((len -= 4) > 3); // and then leftovers if (len > 0) { - int q = inBuf[inPtr] & 0xFF; - if (len > 1) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (len > 2) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - } - } - _quadBuffer[offset++] = q; + _quadBuffer[offset++] = _decodePartialQuad(inBuf, inPtr, len); } return matcher.matchByQuad(_quadBuffer, offset); } @@ -2119,18 +2081,7 @@ private final String _findDecodedFromSymbols(final int len) throws JacksonExcept { // First: maybe we already have this name decoded? if (len < 5) { - int inPtr = _inputPtr; - final byte[] inBuf = _inputBuffer; - int q = _padQuadForNulls(inBuf[inPtr]); - if (len > 1) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (len > 2) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (len > 3) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - } - } - } + int q = _decodePartialQuadForNulls(_inputBuffer, _inputPtr, len); _quad1 = q; return _symbols.findName(q); } @@ -2143,17 +2094,7 @@ private final String _findDecodedFromSymbols(final int len) throws JacksonExcept inPtr += 4; if (len < 9) { - int q2 = _padQuadForNulls(inBuf[inPtr++]); - int left = len - 5; - if (left > 0) { - q2 = (q2 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 1) { - q2 = (q2 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 2) { - q2 = (q2 << 8) + (inBuf[inPtr++] & 0xFF); - } - } - } + int q2 = _decodePartialQuadForNulls(inBuf, inPtr, len - 4); _quad1 = q1; _quad2 = q2; return _symbols.findName(q1, q2); @@ -2163,17 +2104,7 @@ private final String _findDecodedFromSymbols(final int len) throws JacksonExcept inPtr += 4; if (len < 13) { - int q3 = _padQuadForNulls(inBuf[inPtr++]); - int left = len - 9; - if (left > 0) { - q3 = (q3 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 1) { - q3 = (q3 << 8) + (inBuf[inPtr++] & 0xFF); - if (left > 2) { - q3 = (q3 << 8) + (inBuf[inPtr++] & 0xFF); - } - } - } + int q3 = _decodePartialQuadForNulls(inBuf, inPtr, len - 8); _quad1 = q1; _quad2 = q2; _quad3 = q3; @@ -2209,14 +2140,7 @@ private final String _findDecodedFixed12(int len, int q1, int q2) throws Jackson } while ((len -= 4) > 3); // and then leftovers if (len > 0) { - int q = _padQuadForNulls(inBuf[inPtr]); - if (len > 1) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (len > 2) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - } - } - _quadBuffer[offset++] = q; + _quadBuffer[offset++] = _decodePartialQuadForNulls(inBuf, inPtr, len); } return _symbols.findName(_quadBuffer, offset); } @@ -2254,8 +2178,18 @@ private final static int _padLastQuad(int q, int bytes) { return (bytes == 4) ? q : (q | (-1 << (bytes << 3))); } - private final static int _padQuadForNulls(int firstByte) { - return (firstByte & 0xFF) | 0xFFFFFF00; + /** + * Variant of {@link #_decodePartialQuad} that pads the unused high bytes with + * 1s rather than 0s, which is what {@link ByteQuadsCanonicalizer} expects of a + * partial quad: without it a name ending in NULL bytes would collide with the + * shorter name that precedes those NULLs. + * + * @param len Number of bytes to decode; must be between 1 and 4 + * + * @since 3.3 + */ + private final static int _decodePartialQuadForNulls(byte[] buffer, int offset, int len) { + return _padLastQuad(_decodePartialQuad(buffer, offset, len), len); } /* diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java index 9b4f572a9..1061211f3 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParserBase.java @@ -767,6 +767,39 @@ protected final static int _decodeQuad(byte[] buffer, int offset) { | (buffer[offset+3] & 0xFF); } + /** + * Helper method for decoding the trailing 1 - 4 bytes of an Object property + * name into a right-aligned, zero-padded "quad": that is, produces the same + * value as accumulating {@code len} bytes with 8-bit shifts would. + *

+ * Reads all 4 bytes with a single load where it can, which may read up to 3 + * bytes past {@code offset+len}. Those bytes are shifted out and cannot + * affect the result, but the read still has to stay within the array: hence + * the length check, which also selects the byte-shifting path for a name + * that ends within 3 bytes of the end of the buffer. + *

+ * Caller MUST have verified that {@code len} bytes are readable at given + * offset, and that {@code len} is between 1 and 4. + * + * @since 3.3 + */ + protected final static int _decodePartialQuad(byte[] buffer, int offset, int len) { + if ((offset + 4) <= buffer.length) { + return _decodeQuad(buffer, offset) >>> ((4 - len) << 3); + } + int q = buffer[offset] & 0xFF; + if (len > 1) { + q = (q << 8) | (buffer[offset+1] & 0xFF); + if (len > 2) { + q = (q << 8) | (buffer[offset+2] & 0xFF); + if (len > 3) { + q = (q << 8) | (buffer[offset+3] & 0xFF); + } + } + } + return q; + } + /** * Helper method used to encapsulate logic of including (or not) of * "source reference" when constructing {@link TokenStreamLocation} instances. diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java b/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java index 05f140e85..ceb55b9b7 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java @@ -705,15 +705,8 @@ private final JsonToken _finishLongFieldName(int outPtr) throws JacksonException quads[quadCount++] = _decodeQuad(copyBuffer, in); } // and possibly more... ? - if (in < outPtr) { // at least 1 - int q = copyBuffer[in++] & 0xFF; - if (in < outPtr) { // at least 2 - q = (q << 8) | (copyBuffer[in++] & 0xFF); - if (in < outPtr) { // 3 (can't be more) - q = (q << 8) | (copyBuffer[in++] & 0xFF); - } - } - quads[quadCount++] = q; + if (in < outPtr) { // 1 - 3 bytes left over + quads[quadCount++] = _decodePartialQuad(copyBuffer, in, outPtr - in); } String name = _symbols.findName(quads, quadCount); diff --git a/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingParserBase.java b/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingParserBase.java index 5a56367d8..eb52e3e5a 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingParserBase.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/async/NonBlockingParserBase.java @@ -447,38 +447,14 @@ protected final String _findDecodedFromSymbols(byte[] inBuf, int inPtr, int len) { // First: maybe we already have this name decoded? if (len < 5) { - int q = inBuf[inPtr] & 0xFF; - if (--len > 0) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - } - } - } + int q = _decodePartialQuad(inBuf, inPtr, len); _quad1 = q; return _symbols.findName(q); } if (len < 9) { // First quadbyte is easy - int q1 = (inBuf[inPtr] & 0xFF) << 8; - q1 += (inBuf[++inPtr] & 0xFF); - q1 <<= 8; - q1 += (inBuf[++inPtr] & 0xFF); - q1 <<= 8; - q1 += (inBuf[++inPtr] & 0xFF); - int q2 = (inBuf[++inPtr] & 0xFF); - len -= 5; - if (len > 0) { - q2 = (q2 << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { - q2 = (q2 << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { - q2 = (q2 << 8) + (inBuf[++inPtr] & 0xFF); - } - } - } + int q1 = _decodeQuad(inBuf, inPtr); + int q2 = _decodePartialQuad(inBuf, inPtr+4, len - 4); _quad1 = q1; _quad2 = q2; return _symbols.findName(q1, q2); @@ -499,24 +475,12 @@ private final String _findDecodedLonger(byte[] inBuf, int inPtr, int len) throws // then decode, full quads first int offset = 0; do { - int q = (inBuf[inPtr++] & 0xFF) << 8; - q |= inBuf[inPtr++] & 0xFF; - q <<= 8; - q |= inBuf[inPtr++] & 0xFF; - q <<= 8; - q |= inBuf[inPtr++] & 0xFF; - _quadBuffer[offset++] = q; + _quadBuffer[offset++] = _decodeQuad(inBuf, inPtr); + inPtr += 4; } while ((len -= 4) > 3); // and then leftovers if (len > 0) { - int q = inBuf[inPtr] & 0xFF; - if (--len > 0) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { - q = (q << 8) + (inBuf[++inPtr] & 0xFF); - } - } - _quadBuffer[offset++] = q; + _quadBuffer[offset++] = _decodePartialQuad(inBuf, inPtr, len); } return _symbols.findName(_quadBuffer, offset); } diff --git a/smile/src/test/java/tools/jackson/dataformat/smile/parse/NameQuadDecodingTest.java b/smile/src/test/java/tools/jackson/dataformat/smile/parse/NameQuadDecodingTest.java new file mode 100644 index 000000000..64e8a28b2 --- /dev/null +++ b/smile/src/test/java/tools/jackson/dataformat/smile/parse/NameQuadDecodingTest.java @@ -0,0 +1,190 @@ +package tools.jackson.dataformat.smile.parse; + +import java.io.ByteArrayInputStream; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import tools.jackson.core.JsonParser; +import tools.jackson.core.JsonToken; +import tools.jackson.core.async.ByteArrayFeeder; + +import tools.jackson.dataformat.smile.BaseTestForSmile; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for the "quad" decoding shared by the blocking and non-blocking Smile + * parsers, which reads Object property names 4 bytes at a time for symbol table + * lookup. + *

+ * Trailing partial quads have two decoding paths -- a single wide load, taken + * only when 4 bytes are readable at the offset, and byte-at-a-time shifting + * otherwise -- so this covers every name length across the branch points (4, 8 + * and 12 bytes) and asserts that all three parser flavors agree. Parsing from a + * {@code byte[]} exercises the shifting path for the last name in a document + * (which ends within 3 bytes of the end of the array), while parsing the same + * bytes from an {@link java.io.InputStream} exercises the wide load. + */ +public class NameQuadDecodingTest extends BaseTestForSmile +{ + private final static int MAX_NAME_LEN = 40; + + private final static char UNICODE_2BYTES = (char) 167; // law symbol + private final static char UNICODE_3BYTES = (char) 0x4567; + + @Test + public void testAsciiNamesOfEveryLength() throws Exception + { + for (String name : _asciiNames()) { + _verifyName(name); + } + } + + @Test + public void testUnicodeNamesOfEveryLength() throws Exception + { + for (String name : _unicodeNames()) { + _verifyName(name); + } + } + + // Partial quads are padded with 1s, not 0s, so that a name with trailing + // NULLs does not collide with the shorter name that precedes them + @Test + public void testNamesWithNulls() throws Exception + { + for (int len = 1; len <= MAX_NAME_LEN; ++len) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < len; ++i) { + sb.append('a'); + } + // "a", "a\0", "a\0\0", "a\0\0\0" and so on must all stay distinct + for (int nulls = 1; nulls <= 3; ++nulls) { + _verifyName(sb.toString() + String.valueOf(new char[nulls])); + } + } + } + + // Same names within a single document, so that symbol table interning is + // exercised across all lengths and both decoding paths + @Test + public void testAllNamesInSingleDocument() throws Exception + { + List names = new ArrayList<>(); + names.addAll(_asciiNames()); + names.addAll(_unicodeNames()); + + Map input = new LinkedHashMap<>(); + for (int i = 0; i < names.size(); ++i) { + input.put(names.get(i), i); + } + byte[] doc = smileMapper().writeValueAsBytes(input); + + for (String read : new String[] { "bytes", "stream", "async" }) { + List actual = _readNames(doc, read); + assertEquals(names, actual, "Mismatch when reading via "+read); + } + } + + /* + /********************************************************************** + /* Helper methods + /********************************************************************** + */ + + private List _asciiNames() + { + List names = new ArrayList<>(); + for (int len = 1; len <= MAX_NAME_LEN; ++len) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < len; ++i) { + sb.append((char) ('a' + (i % 26))); + } + names.add(sb.toString()); + } + return names; + } + + // Multi-byte characters, so that byte length and character count differ + private List _unicodeNames() + { + List names = new ArrayList<>(); + for (int len = 1; len <= MAX_NAME_LEN; ++len) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < len; ++i) { + sb.append((i % 2 == 0) ? UNICODE_2BYTES : UNICODE_3BYTES); + } + names.add(sb.toString()); + } + return names; + } + + private void _verifyName(String name) throws Exception + { + Map input = new LinkedHashMap<>(); + input.put(name, 13); + byte[] doc = smileMapper().writeValueAsBytes(input); + + for (String read : new String[] { "bytes", "stream", "async" }) { + List actual = _readNames(doc, read); + assertEquals(1, actual.size(), "Mismatch when reading via "+read); + assertEquals(name, actual.get(0), "Mismatch when reading via "+read); + } + } + + private List _readNames(byte[] doc, String read) throws Exception + { + if (read.equals("async")) { + return _readNamesAsync(doc); + } + List names = new ArrayList<>(); + try (JsonParser p = read.equals("bytes") + ? _smileParser(doc) + : _smileParser(new ByteArrayInputStream(doc))) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + while (p.nextToken() == JsonToken.PROPERTY_NAME) { + names.add(p.currentName()); + p.nextToken(); // and past the value + } + assertToken(JsonToken.END_OBJECT, p.currentToken()); + assertNull(p.nextToken()); + } + return names; + } + + // Fed one byte at a time, so names are also split across input boundaries + private List _readNamesAsync(byte[] doc) throws Exception + { + List names = new ArrayList<>(); + try (JsonParser p = smileMapper().reader().createNonBlockingByteArrayParser()) { + final ByteArrayFeeder feeder = (ByteArrayFeeder) p.nonBlockingInputFeeder(); + int offset = 0; + JsonToken t; + + while (true) { + while ((t = p.nextToken()) == JsonToken.NOT_AVAILABLE) { + if (offset < doc.length) { + assertTrue(feeder.needMoreInput()); + feeder.feedInput(doc, offset, offset+1); + ++offset; + } else { + feeder.endOfInput(); + } + } + if (t == null) { + break; + } + if (t == JsonToken.PROPERTY_NAME) { + names.add(p.currentName()); + } + } + } + return names; + } +}