Repository navigation
perf(java): copy plain runs between escapes when decoding json strings - #4058
Merged
Merged
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>
Contributor
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, |
5 tasks
chaokunyang
added a commit
that referenced
this pull request
Sep 20, 2026
## Why? ## What does this PR do? Reuse buffer between writer/reader to reduce memory footprint ## Related issues #4058 ## AI Contribution Checklist - [ ] Substantial AI assistance was used in this PR: `yes` / `no` - [ ] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [ ] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence or equivalent persisted links of the final clean AI review results from both fresh reviewers described in `AI_POLICY.md`, the Fory-guided reviewer and the independent general reviewer, on the current PR diff or current HEAD after the latest code changes. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark
This was referenced Sep 28, 2026
chaokunyang
added a commit
that referenced
this pull request
Oct 6, 2026
…strings (#4122) ## Why? This is the writing side of #4055 / #4058. Once a string being written contains its first character that needs escaping (a quote, a backslash, a control character, or non-ASCII text with `escapeNonAscii`), both JSON writers write the rest of it one character at a time through `writeEscapedChar` - a switch, a capacity check and one store per character - and never return to a bulk copy. The cost of a string therefore follows the distance from that character to its end, not the number of characters to escape: one quote near the front of a 20 KB string makes it 13 times slower to write than with no quote at all. In `Utf8JsonWriter` a compact latin1 string longer than the fixed-size fast paths is also rescanned with `charAt` up to that character, so even a quote at the very end costs 5 times. Details and measurements in #4121. ## What does this PR do? - `Utf8JsonWriter`: a compact latin1 `String` that the fast paths reject now goes from `writeStringChars` to `writeLatin1StringEscaped`, which copies each plain run a word at a time with the existing `isJsonAsciiWord` predicate, writes only the byte that stopped it through `writeEscapedChar`, and resumes. The character tails (`writeStringSlow` for `String` and `CharSequence`) copy the plain ASCII run up to the next stop with one capacity check per run, then handle the stop character exactly as before. - `StringJsonWriter`: the same in all four tails. `writeLatin1StringSlow` and `writeLatin1StringUtf16Slow` copy plain words and resume the word copy after a byte above 0x7F, running the word predicate at most once per eight bytes. In `writeStringSlow` the run follows the current output coder, because a character above 0xFF upgrades the output to UTF16 in the middle of a string; the UTF16 runs (`writeStringSlow`, `writeStringUtf16Slow`) clear `latin1Output` when they copy such a character. - Every run copy reserves capacity for at most `RUN_CHUNK` (8192) characters at a time, the way `writeEscapedUtf16` already bounds its reservation, so `(length - i) << 1` or `pos + (length - i)` cannot overflow for a huge string and skip `grow` before the word stores. A run that ends at a chunk boundary rather than at a stop just continues. The copy is inlined into each tail: a helper call per stop measurably slowed strings with many escapes. - The hot `writeString(String)` methods and their fixed-size fast paths are untouched; all new code is in the cold paths, following the C2-layout notes in both classes. The output is byte-identical. - Tests in `JsonStringTest`: - `writeEscapeFollowedByOtherText` compares both writers with `JsonStringEscaper` (a character-at-a-time reference) for a stop character (quote, backslash, newline, control character, latin1 above 0x7F) followed by latin1 text, 0x7F/0x80, text outside latin1, a surrogate pair and further escapes, with 0-20 characters before it and 0-8 after it so stops land in every lane of a word, with and without trailing text, for `String` and `CharSequence` input (each on a fresh writer), utf8 output and LATIN1 and UTF16 string output, with and without `escapeNonAscii`. Accepting a quote inside the latin1 run, or ignoring `escapeNonAscii` in the UTF16 run, makes it fail. - `writeRunsAcrossReservationChunks`: a quote, latin1 above 0x7F, text outside latin1 and a surrogate pair at 8192 +/- 9 characters after an escape, so they land on and around a chunk boundary. - `writeRejectsUnpairedSurrogateAfterAnEscape`: an unpaired high or low surrogate after an escape, inside a long plain run and at the end, is still rejected by both writers, with and without `escapeNonAscii`. - `writeEscapeThenWideTextAfterUtf16Reset`: a writer reset after a UTF16 result starts in UTF16 while assuming its output fits latin1; the run copy must withdraw that assumption. Removing the `latin1Output` clear makes it fail. - The old code was correct but slow, so these tests pin behaviour and also pass on it. ## Related issues - Fixes #4121 - Writing counterpart of #4055 / #4058 ## AI Contribution Checklist - [x] Substantial AI assistance was used in this PR: `yes` - [x] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [x] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence or equivalent persisted links of the final clean AI review results from both fresh reviewers described in `AI_POLICY.md`, the Fory-guided reviewer and the independent general reviewer, on the current PR diff or current HEAD after the latest code changes. Completed checklist from `AI_POLICY.md` section 9: - [x] Substantial AI assistance was used in this PR: `yes` - [x] I included the standardized `AI Usage Disclosure` block below. - [x] I can explain and defend all important changes without AI help. - [x] I reviewed AI-assisted code changes line by line before submission. - [x] I completed line-by-line self-review first and fixed issues before requesting AI review. - [x] I ran two fresh AI review agents on the current PR diff or current HEAD after the latest code changes: one Fory-guided reviewer using `AGENTS.md` and `.agents/ci-and-pr.md`, and one independent general reviewer in a separate clean-context review session that was not pointed to `.agents/ci-and-pr.md` or any copied Fory-specific review checklist. - [x] I addressed all AI review comments and repeated the review loop until both ai reviewers reported no further actionable comments. - [x] I attached equivalent persisted links of the final clean AI review results from both fresh reviewers on the current PR head in this PR body. - [x] I ran adequate human verification and recorded evidence (checks run locally, pass/fail summary, and confirmation I reviewed results). - [x] I added/updated tests and specs where required. - [x] I validated protocol/performance impacts with evidence when applicable. - [x] I verified licensing and provenance compliance. ```text AI Usage Disclosure - substantial_ai_assistance: yes - scope: investigation, code drafting, tests, benchmark harness, PR text - affected_files_or_subsystems: java/fory-json writers (Utf8JsonWriter, StringJsonWriter) and JsonStringTest - ai_review: line-by-line self-review of the AI-drafted change is pending by the contributor (see unchecked items above). Two-reviewer loop on fresh sessions for every round, six rounds: Fory-guided reviewer Codex gpt-6-astra (xhigh), independent general reviewer Claude Fable 5.1. Fixed from review: each input type written by a fresh writer in the test so a CharSequence reaches the mid-string LATIN1-to-UTF16 upgrade itself; 0x7F/0x80 tails and strings ending right after a stop; surrogate test with and without escapeNonAscii; the StringJsonWriter latin1 tails resume the word copy after a byte above 0x7F, run the word predicate at most once per eight bytes and document that they are only reached without non-ASCII escaping; a test for the latin1Output clear after a UTF16 reset; every run copy reserves at most RUN_CHUNK characters at a time so the capacity arithmetic cannot overflow for huge strings (found by the Fory-guided reviewer), with a chunk-boundary test; the run copies are inlined into each tail after a helper call per stop measurably slowed strings with many escapes. Kept by decision: hot writeString fast paths untouched; String and CharSequence overloads stay duplicated like the existing code; tests check behaviour against JsonStringEscaper rather than speed. Final round on the current head: both reviewers report no further actionable comments. - ai_review_artifacts: Fory-guided reviewer: #4122 (comment) ; independent general reviewer: #4122 (comment) (both on commit 72008b9, the current head, final round, no further actionable comments) - human_verification: see "Verification" below - performance_verification: see "Benchmark" below - provenance_license_confirmation: all code written for this PR against the Apache Fory sources; no third-party code introduced ``` ## Verification JDK 25.0.2, Windows 11 x86_64, from `java/`: - `mvn -pl fory-json -am install -DskipTests`: success - `mvn -pl fory-json test`: 1276 tests, 0 failures, 0 errors, 0 skipped - `JsonStringTest#writeEscapeFollowedByOtherText` with a quote accepted inside the latin1 run of `Utf8JsonWriter`, and with `escapeNonAscii` ignored in the `StringJsonWriter` UTF16 run copy: fails, as intended - `JsonStringTest#writeEscapeThenWideTextAfterUtf16Reset` with the `latin1Output` clear removed from the UTF16 run copy: fails (`中` comes back as `-`), as intended - `mvn -pl fory-json spotless:check checkstyle:check`: success, 0 Checkstyle violations ## Does this PR introduce any user-facing change? - [x] Does this PR introduce any public API change? No. - [x] Does this PR introduce any binary protocol compatibility change? No. The JSON output is byte-identical. ## Benchmark Standalone harness from #4121 (not JMH): a bid whose `adm` field is a 19998-character VAST XML string, pure ASCII, 612 quotes and 345 newlines, the first quote at character 15. Variants change only the characters that need escaping inside `adm`, keeping its length. `ForyJson.builder().build()`, best of 7 rounds of 5000 writes after 30000 warm-up writes, one JVM per build, builds alternating three times on an otherwise idle machine; JDK 25.0.2. Range over the three runs, microseconds per write: | variant | `toJsonBytes` main f85877f | `toJsonBytes` this PR | `toJson` main | `toJson` this PR | | --- | ---: | ---: | ---: | ---: | | nothing to escape | 3.6-3.8 | 3.6-3.7 | 3.4-3.5 | 3.4-3.5 | | one quote at the very end | 18.7-19.0 | 6.0-6.4 | 3.3-3.4 | 3.3-3.4 | | one quote at character 1 | 44.5-45.0 | 4.2-4.6 | 33.8-36.8 | 3.7-4.0 | | the real string | 44.3-45.0 | 9.9-10.4 | 36.5-40.7 | 14.6-15.8 | A bean of eight short strings, four of them with one escape each: 286-291 ns -> 181-191 ns (`toJsonBytes`), 255-272 ns -> 231-235 ns (`toJson`). The output digest is identical between the builds in every case. For reference, Jackson 3.1 `writeValueAsString` takes 12.5 us on the string without escapes and 25.0 us on the real one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: chaokunyang <shawn.ck.yang@gmail.com>
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