perf(java): copy plain runs between escapes when decoding json strings - #4058
Open
pavel-ptashyts wants to merge 1 commit into
Open
pavel-ptashyts wants to merge 1 commit into
pavel-ptashyts wants to merge 1 commit into
Conversation
Once a string contains its first escape, all three json readers decoded the rest of it one character at a time and never returned to the word scanner, so a string cost time proportional to the distance from its first escape to its end. The scanner that found the escape now finds the next stop character and the plain run between them is copied in one pass. clear() also replaced any decode buffer above the retained size with a fresh 8 KiB array after every parse, so a pooled reader re-grew it on every document whose string was larger. The readers now keep what the last document needed, bounded by a ceiling, and release it once a later document does not need it. Closes apache#4055 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
|
The one failing check, Other open PRs still show it green because their runs are from 12–13 September, before that pin left the ASF Everything that exercises this change is green: Java CI 8, 11, 17, 21, 25 and 26, Openj9 21, Windows Java 21, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4055.
What this changes
Once a JSON string contains its first escape, all three readers decode the rest of that string one character per
loop iteration and never return to the word scanner. The cost of a string is therefore proportional to the
distance from its first escape to its end, not to the number of escapes it contains.
This reuses the scanner that already found the escape to find the next stop character, and copies the plain run
between them in one pass -
System.arraycopyforUtf8JsonReaderandLatin1JsonReader, a packing loop forUtf16JsonReaderwhose characters are two bytes wide while the latin1 output is one byte each. Each reader keepsits own stop mask, so a run stops exactly where the character-at-a-time path would have handled the character
itself: the utf8 scan excludes high-bit bytes, the latin1 scan admits them because they are valid payload, and the
utf16 scan ors in
word & UTF16_NON_LATIN_BYTESso it stops at every character the latin1 output cannot hold.A second, independent change is in
clear(). A decode buffer larger thanRETAINED_STRING_DECODE_BUFFER_SIZEwas replaced by a fresh 8 KiB array after every parse. Readers are pooled, so a document with a string larger than
8 KiB re-grew the buffer by doubling on every single parse, and the growth arrays plus the replacement were
garbage each time. The readers now record the longest output decoded since the last
clear()- fromfinishDecodedStringfor aStringand fromdecodeQuotedTextfor theCharSequenceview, since both grow thesame buffer - and shrink only when the buffer is at least twice that. Consecutive large documents therefore stop
re-growing it, while
MAX_RETAINED_STRING_DECODE_BUFFER_SIZEbounds what any single document can leave pinned ina pooled reader; above that the buffer is released whatever the shrink threshold says.
Benchmark data
I do not have numbers from
benchmarks/java; these are from the application harness where the problem was found,and I am glad to run whatever you would rather see. JDK 25, min of 7 rounds of 300 iterations, both libraries
warmed over every document before anything is timed, rounds alternating between the libraries.
Six real OpenRTB bid responses, 12-110 KB, each roughly 95% one string holding VAST XML: pure ASCII, about 100
escaped quotes, the first of them at character 14 of 20006. Read from
byte[]into the same model.6.1x against the current code, and it turns a 2.9x loss against Jackson into a 2.3x win, with 1.7x less
allocation than Jackson rather than 1.8x more. Jackson drifts about 7% between the two runs, which is this
machine's noise floor; the Fory column moves by six times. The
fromStringpaths improve on the same documentstoo: 3.2-5.8x for
Latin1JsonReaderand 1.8-2.3x forUtf16JsonReader.The cause is isolated by changing only the escapes inside that one string, keeping its length at 20006
characters:
With no escape the reader is already 1.76x faster than Jackson, and one escape near the front costs the same as a
hundred of them.
Tests
JsonStringTest.readEscapeFollowedByNonAsciiTextis new: an escape followed by latin1 text, text outsidelatin1, a surrogate pair and mixtures of them, varying both the text before the escape and the gap between the
escape and the non-ascii character so the stop lands in every lane of a scanned word, driven through all three
readers.
JsonStringTest.rejectMalformedInputAfterAnEscapeis new: an unterminated string, a raw control character, abad escape and an unpaired high surrogate, each both inside a scanned word and in the scalar remainder. The new
block sits between the escape handling and those checks, so they have to keep firing.
JsonStringTest.readerDecodeBufferShrinkspinned the old unconditional shrink and is replaced byreaderDecodeBufferIsKeptWhileNeeded, which pins the new policy for all three readers and for thereadQuotedTextpath: the buffer survives aclear()while documents still need it, is released once one doesnot, and is released regardless once it exceeds the ceiling.
One gap I did not close: the test pins that a buffer grown only through
decodeQuotedTextis kept, but not thatit is released.
Human verification
On JDK 25.0.2, Windows, from
java/:mvn -pl fory-core -am -DskipTests install, which the fory-json java9 module step needs, thenmvn -pl fory-json clean test spotless:check checkstyle:check: 1146 tests, 0 failures, 0 errors, 0 skipped,spotless clean, 0 checkstyle violations.
main: 1144 tests, 0 failures.readEscapeFollowedByNonAsciiTextfails, then restored it and confirmed the suite is green again.
benchmarks/java.Final review results, both on commit d950b0b.
Fory-guided reviewer:
Independent reviewer:
🤖 Generated with Claude Code