From 9afeb4e9f85a0f69158fbaaae7ccfc505982e687 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 21 Aug 2026 18:18:15 -0700 Subject: [PATCH 1/3] Fix #759: use direct String construction for Smile "long" ASCII text values `SmileParser` already builds `String` straight from input bytes for short ASCII values and names, but the two "long" (64+ byte) text paths still decoded byte-by-byte into a `char[]`. Add a shared fast path: when the end-of-string marker is found within the current input buffer and everything before it is ASCII, the value is contiguous and `TextBuffer.resetWithASCII()` can build the `String` directly, skipping the `char[]` round-trip. Falls back to existing handling when the value spans a buffer reload or contains actual non-ASCII content. Applies to `_decodeLongUnicodeValue()` as well, not just the ASCII token type: `SmileGenerator._writeNonSharedString()` cannot speculate on ASCII-ness once `3 * len + 2` exceeds the output buffer, so pure-ASCII text above ~2666 chars is written with `TOKEN_MISC_LONG_TEXT_UNICODE` and previously got no benefit at all. Measured (ns per decoded char, min of 3 JVM trials, best-of-10 each): len before after speedup token 100 0.8120 0.5901 1.38x long-ASCII 1500 1.0227 0.7811 1.31x long-ASCII 2600 0.9012 0.7536 1.20x long-ASCII 4000 0.8680 0.7094 1.22x forced-Unicode 8000 0.8633 0.7221 1.20x forced-Unicode 20000 0.8411 0.5985 1.41x forced-Unicode Output verified byte-identical to previous decoding across lengths 65-30000 with non-ASCII at every boundary position and surrogate pairs, read via byte[], full stream, and 1-/7-byte chunked streams. Co-Authored-By: Claude Opus 5 (1M context) --- release-notes/VERSION | 2 + .../jackson/dataformat/smile/SmileParser.java | 39 +++++ .../smile/parse/LongTextDecode759Test.java | 151 ++++++++++++++++++ 3 files changed, 192 insertions(+) create mode 100644 smile/src/test/java/tools/jackson/dataformat/smile/parse/LongTextDecode759Test.java diff --git a/release-notes/VERSION b/release-notes/VERSION index 1c56a169e..216e26a28 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -23,6 +23,8 @@ implementations) (contributed by @pjfanning) #752: (cbor) Use `VarHandle` for multi-byte primitive writes in `CBORGenerator` (contributed by @pjfanning) +#759: (smile) Use more efficient `String` construction wrt "Compact Strings" + for "long" text values 3.2.3 (not yet released) 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..8c823e5a5 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java @@ -2797,8 +2797,42 @@ protected final String _decodeShortUnicodeValue(int byteLen) throws JacksonExcep return _textBuffer.setCurrentAndReturn(outPtr); } + /** + * Fast path shared by "long" (64+ byte) text value decoding: if the + * end-of-string marker is found within the current input buffer, preceded + * only by ASCII bytes, the whole value is contiguous and the String can be + * constructed straight from input bytes -- skipping the char[] round-trip + * (which JDK 17+ "Compact Strings" makes measurably cheaper). + * + * @return {@code true} if value was fully decoded into {@code _textBuffer}; + * {@code false} if caller needs to use general (slower) handling + */ + private final boolean _tryDecodeLongContiguousAscii() throws JacksonException + { + final byte[] inBuf = _inputBuffer; + final int start = _inputPtr; + final int end = _inputEnd; + int ptr = start; + while (ptr < end) { + final byte b = inBuf[ptr]; + if (b < 0) { // end marker, or start of multi-byte UTF-8 sequence + if (b == SmileConstants.BYTE_MARKER_END_OF_STRING) { + _inputPtr = ptr + 1; + _textBuffer.resetWithASCII(inBuf, start, ptr - start); + return true; + } + return false; // actual non-ASCII content + } + ++ptr; + } + return false; // marker not within current buffer + } + private final void _decodeLongAsciiValue() throws JacksonException { + if (_tryDecodeLongContiguousAscii()) { + return; + } int outPtr = 0; char[] outBuf = _textBuffer.emptyAndGetCurrentSegment(); main_loop: @@ -2828,6 +2862,11 @@ private final void _decodeLongAsciiValue() throws JacksonException private final void _decodeLongUnicodeValue() throws JacksonException { + // 22-Aug-2026: note that generator is forced to use "Unicode" token type for + // any text too long to speculate on, so content here may well be all-ASCII + if (_tryDecodeLongContiguousAscii()) { + return; + } int outPtr = 0; char[] outBuf = _textBuffer.emptyAndGetCurrentSegment(); final int[] codes = SmileConstants.sUtf8UnitLengths; diff --git a/smile/src/test/java/tools/jackson/dataformat/smile/parse/LongTextDecode759Test.java b/smile/src/test/java/tools/jackson/dataformat/smile/parse/LongTextDecode759Test.java new file mode 100644 index 000000000..441346df7 --- /dev/null +++ b/smile/src/test/java/tools/jackson/dataformat/smile/parse/LongTextDecode759Test.java @@ -0,0 +1,151 @@ +package tools.jackson.dataformat.smile.parse; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; + +import org.junit.jupiter.api.Test; + +import tools.jackson.core.*; +import tools.jackson.dataformat.smile.*; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Tests for [dataformats-binary#759]: "long" (64+ byte) text values decoded + * straight from input bytes when contiguous and all-ASCII. Covers both + * `TOKEN_BYTE_LONG_STRING_ASCII` and `TOKEN_MISC_LONG_TEXT_UNICODE` token + * types -- generator is forced to use the latter for text too long to + * speculate on, even when content is pure ASCII. + */ +public class LongTextDecode759Test + extends BaseTestForSmile +{ + // Lengths straddling the point (~2666 chars) where the generator can no + // longer speculate on ASCII-ness and switches to the "Unicode" token type + private final static int[] LENGTHS = { 64, 65, 100, 1000, 2665, 2666, 2667, 3000, 9000 }; + + @Test + public void testLongAsciiValues() throws Exception + { + for (int len : LENGTHS) { + String text = _ascii(len); + _verifyRoundTrip(text); + } + } + + @Test + public void testLongValuesWithNonAsciiAtEveryBoundary() throws Exception + { + // non-ASCII char at start/middle/end must defeat the fast path cleanly + for (int len : LENGTHS) { + for (int pos : new int[] { 0, 1, len / 2, len - 1 }) { + StringBuilder sb = new StringBuilder(_ascii(len)); + sb.setCharAt(pos, 'δΈ­'); + _verifyRoundTrip(sb.toString()); + } + } + } + + @Test + public void testLongValuesWithSurrogatePair() throws Exception + { + for (int len : LENGTHS) { + StringBuilder sb = new StringBuilder(_ascii(len - 2)); + sb.appendCodePoint(0x1F601); + _verifyRoundTrip(sb.toString()); + } + } + + // Values spanning buffer reloads must fall back to general handling + @Test + public void testLongValuesFromChunkedStream() throws Exception + { + List texts = new ArrayList<>(); + for (int len : LENGTHS) { + texts.add(_ascii(len)); + } + byte[] doc = _doc(texts); + for (int chunkSize : new int[] { 1, 7, 999 }) { + _verifyDoc(texts, new ChunkedStream(doc, chunkSize)); + } + } + + private void _verifyRoundTrip(String text) throws Exception + { + List texts = new ArrayList<>(); + texts.add(text); + _verifyDoc(texts, null); // byte[] backed + _verifyDoc(texts, new ByteArrayInputStream(_doc(texts))); + } + + private void _verifyDoc(List expected, InputStream in) throws Exception + { + try (JsonParser p = (in == null) ? _smileParser(_doc(expected)) : _smileParser(in)) { + assertEquals(JsonToken.START_ARRAY, p.nextToken()); + for (String exp : expected) { + assertEquals(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals(exp, p.getString()); + } + assertEquals(JsonToken.END_ARRAY, p.nextToken()); + assertNull(p.nextToken()); + } + } + + private byte[] _doc(List texts) throws Exception + { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + SmileFactory f = smileFactory(false, false, false); + try (JsonGenerator g = f.createGenerator(ObjectWriteContext.empty(), out)) { + g.writeStartArray(); + for (String text : texts) { + g.writeString(text); + } + g.writeEndArray(); + } + return out.toByteArray(); + } + + private String _ascii(int len) + { + Random r = new Random(len); + StringBuilder sb = new StringBuilder(len); + for (int i = 0; i < len; ++i) { + sb.append((char) ('a' + r.nextInt(26))); + } + return sb.toString(); + } + + // Yields small chunks, forcing buffer reloads in the middle of values + static class ChunkedStream extends InputStream + { + private final byte[] _data; + private final int _chunkSize; + private int _ptr; + + public ChunkedStream(byte[] data, int chunkSize) { + _data = data; + _chunkSize = chunkSize; + } + + @Override + public int read() { + return (_ptr < _data.length) ? (_data[_ptr++] & 0xFF) : -1; + } + + @Override + public int read(byte[] buffer, int offset, int len) { + if (_ptr >= _data.length) { + return -1; + } + int count = Math.min(Math.min(len, _chunkSize), _data.length - _ptr); + System.arraycopy(_data, _ptr, buffer, offset, count); + _ptr += count; + return count; + } + } +} From 6a97b26f89f8822840a2f63486d316b1bc7f1771 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 21 Aug 2026 18:39:50 -0700 Subject: [PATCH 2/3] Address review: retain scanned ASCII prefix on fallback (#759) The initial version discarded the ASCII scan whenever the fast path did not apply, so both fallback paths re-scanned bytes already examined. Since Smile "long" text is marker-terminated rather than length-prefixed, whether a value is contiguous cannot be known up front, making that a real cost rather than a rare one: - streamed value larger than the read buffer: marker is never in the current buffer, so EVERY such value paid a full wasted buffer scan and none could benefit (measured 0.96x, i.e. slower than before) - byte[]-backed value that is ASCII until near its end: scan runs almost to the end, then the whole value is decoded again (measured 0.79-0.86x) `_scanLongContiguousAscii()` now returns the offset at which scanning stopped (or -1 when it completed), and `_copyScannedAsciiPrefix()` materializes the verified prefix so decoding resumes from there instead of restarting. Both regressions are gone; streamed values now gain as well: len byte[] InputStream 100 1.31x 1.51x 1500 1.30x 1.21x 2600 1.26x 1.08x 4000 1.18x 1.10x 8000 1.18x 1.05x 20000 1.39x 1.05x One trade-off remains by construction: a value that is ASCII until very near its end pays for a scan that cannot be turned into a String. Measured 0.94x at 20000 chars (was 0.79-0.86x); shorter values of that shape now gain (1.77x at 5000, was 1.10x). Also adds the release-notes attribution line to match the other 3.3.0 entries, and corrects a comment date. Co-Authored-By: Claude Opus 5 (1M context) --- release-notes/VERSION | 1 + .../jackson/dataformat/smile/SmileParser.java | 69 ++++++++++++++----- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/release-notes/VERSION b/release-notes/VERSION index 216e26a28..16ad1bb7b 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -25,6 +25,7 @@ implementations) (contributed by @pjfanning) #759: (smile) Use more efficient `String` construction wrt "Compact Strings" for "long" text values + (fix by @cowtowncoder, w/ Claude code) 3.2.3 (not yet released) 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 8c823e5a5..5a442c007 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java @@ -2798,16 +2798,20 @@ protected final String _decodeShortUnicodeValue(int byteLen) throws JacksonExcep } /** - * Fast path shared by "long" (64+ byte) text value decoding: if the - * end-of-string marker is found within the current input buffer, preceded - * only by ASCII bytes, the whole value is contiguous and the String can be - * constructed straight from input bytes -- skipping the char[] round-trip - * (which JDK 17+ "Compact Strings" makes measurably cheaper). + * Fast path shared by "long" (64+ byte) text value decoding: scans current + * input buffer for the end-of-string marker. If found with only ASCII bytes + * before it, the whole value is contiguous and the String can be constructed + * straight from input bytes -- skipping the char[] round-trip (which JDK 17+ + * "Compact Strings" makes measurably cheaper). + *

+ * If the marker is NOT reached (either non-ASCII content, or end of buffer), + * the offset at which scanning stopped is returned so that the caller can + * retain the already-verified ASCII prefix instead of scanning it again. * - * @return {@code true} if value was fully decoded into {@code _textBuffer}; - * {@code false} if caller needs to use general (slower) handling + * @return {@code -1} if value was fully decoded into {@code _textBuffer}; + * otherwise offset in {@code _inputBuffer} at which ASCII scan stopped */ - private final boolean _tryDecodeLongContiguousAscii() throws JacksonException + private final int _scanLongContiguousAscii() throws JacksonException { final byte[] inBuf = _inputBuffer; final int start = _inputPtr; @@ -2819,22 +2823,47 @@ private final boolean _tryDecodeLongContiguousAscii() throws JacksonException if (b == SmileConstants.BYTE_MARKER_END_OF_STRING) { _inputPtr = ptr + 1; _textBuffer.resetWithASCII(inBuf, start, ptr - start); - return true; + return -1; } - return false; // actual non-ASCII content + break; // actual non-ASCII content } ++ptr; } - return false; // marker not within current buffer + return ptr; // marker not reached: caller decodes rest, keeping prefix + } + + /** + * Copies range of already-verified ASCII bytes into given output segment, + * starting new segments as necessary; used to retain the prefix scanned by + * {@link #_scanLongContiguousAscii()} when the fast path does not apply. + * Advances {@code _inputPtr} past the copied range. + * + * @return Number of chars in the (possibly new) current segment + */ + private final int _copyScannedAsciiPrefix(int endOffset) + { + final byte[] inBuf = _inputBuffer; + int outPtr = 0; + char[] outBuf = _textBuffer.emptyAndGetCurrentSegment(); + for (int i = _inputPtr; i < endOffset; ++i) { + if (outPtr >= outBuf.length) { + outBuf = _textBuffer.finishCurrentSegment(); + outPtr = 0; + } + outBuf[outPtr++] = (char) inBuf[i]; + } + _inputPtr = endOffset; + return outPtr; } private final void _decodeLongAsciiValue() throws JacksonException { - if (_tryDecodeLongContiguousAscii()) { + final int scanned = _scanLongContiguousAscii(); + if (scanned < 0) { // fully decoded from input bytes return; } - int outPtr = 0; - char[] outBuf = _textBuffer.emptyAndGetCurrentSegment(); + int outPtr = _copyScannedAsciiPrefix(scanned); + char[] outBuf = _textBuffer.getBufferWithoutReset(); main_loop: while (true) { if (_inputPtr >= _inputEnd) { @@ -2862,13 +2891,15 @@ private final void _decodeLongAsciiValue() throws JacksonException private final void _decodeLongUnicodeValue() throws JacksonException { - // 22-Aug-2026: note that generator is forced to use "Unicode" token type for - // any text too long to speculate on, so content here may well be all-ASCII - if (_tryDecodeLongContiguousAscii()) { + // 21-Aug-2026, tatu: [dataformats-binary#759] Note that generator is forced to + // use "Unicode" token type for any text too long to speculate on, so content + // here may well be all-ASCII + final int scanned = _scanLongContiguousAscii(); + if (scanned < 0) { // fully decoded from input bytes return; } - int outPtr = 0; - char[] outBuf = _textBuffer.emptyAndGetCurrentSegment(); + int outPtr = _copyScannedAsciiPrefix(scanned); + char[] outBuf = _textBuffer.getBufferWithoutReset(); final int[] codes = SmileConstants.sUtf8UnitLengths; int c; final byte[] inputBuffer = _inputBuffer; From 34b6f848c84e0290f474f0a00efdf35027df2454 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 21 Aug 2026 18:51:58 -0700 Subject: [PATCH 3/3] Advance input pointer only after successful decode (#759) `_scanLongContiguousAscii()` advanced `_inputPtr` past the value before calling `resetWithASCII()`, which validates against `maxStringLength` and can throw. On a violation the parser was left pointing past the offending value, so the reported location was useless for working out which value tripped the limit: for a 5000-char value under a 1000-char limit it reported byte offset 5007 (end of value) rather than 6 (its start). Swapping the two lines makes the location point at the value that failed. Parser state after an exception remains formally undefined; this is about not making it needlessly misleading. Note that `_decodeShortAsciiValue()` / `_decodeShortAsciiName()` have the same ordering, but are left alone here: pre-existing, and only reachable if `maxStringLength` is configured below 64. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/tools/jackson/dataformat/smile/SmileParser.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 5a442c007..e3b62c645 100644 --- a/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/tools/jackson/dataformat/smile/SmileParser.java @@ -2821,8 +2821,12 @@ private final int _scanLongContiguousAscii() throws JacksonException final byte b = inBuf[ptr]; if (b < 0) { // end marker, or start of multi-byte UTF-8 sequence if (b == SmileConstants.BYTE_MARKER_END_OF_STRING) { - _inputPtr = ptr + 1; + // NOTE: only advance input pointer AFTER decoding succeeds; + // `resetWithASCII()` validates against `maxStringLength` and + // may throw, and leaving the pointer past the offending value + // would be needlessly confusing for anyone inspecting state _textBuffer.resetWithASCII(inBuf, start, ptr - start); + _inputPtr = ptr + 1; return -1; } break; // actual non-ASCII content