Skip to content

perf(java): copy plain runs between escapes when decoding json strings - #4058

Open
pavel-ptashyts wants to merge 1 commit into
apache:mainfrom
pavel-ptashyts:escaped-string-bulk-copy
Open

pavel-ptashyts wants to merge 1 commit into
apache:mainfrom
pavel-ptashyts:escaped-string-bulk-copy

Conversation

@pavel-ptashyts

Copy link
Copy Markdown

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.arraycopy for Utf8JsonReader and Latin1JsonReader, a packing loop for
Utf16JsonReader whose characters are two bytes wide while the latin1 output is one byte each. Each reader keeps
its 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_BYTES so it stops at every character the latin1 output cannot hold.

A second, independent change is in clear(). A decode buffer larger than RETAINED_STRING_DECODE_BUFFER_SIZE
was 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() - from
finishDecodedString for a String and from decodeQuotedText for the CharSequence view, since both grow the
same 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_SIZE bounds what any single document can leave pinned in
a 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.

before after
ns/document, total 609691 99550
allocated bytes/document, total 934976 295664
Jackson 3 on the same documents, same run 213791 ns / 511184 B 229863 ns / 511191 B

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 fromString paths improve on the same documents
too: 3.2-5.8x for Latin1JsonReader and 1.8-2.3x for Utf16JsonReader.

The cause is isolated by changing only the escapes inside that one string, keeping its length at 20006
characters:

variant before Jackson 3
no escapes at all 7036 12385
one escape, at the very end 10104 10169
one escape, at character 1 41833 13562
the real document, 106 escapes 42533 14610

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.readEscapeFollowedByNonAsciiText is new: an escape followed by latin1 text, text outside
    latin1, 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.rejectMalformedInputAfterAnEscape is new: an unterminated string, a raw control character, a
    bad 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.readerDecodeBufferShrinks pinned the old unconditional shrink and is replaced by
    readerDecodeBufferIsKeptWhileNeeded, which pins the new policy for all three readers and for the
    readQuotedText path: the buffer survives a clear() while documents still need it, is released once one does
    not, and is released regardless once it exceeds the ceiling.

One gap I did not close: the test pins that a buffer grown only through decodeQuotedText is kept, but not that
it 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, then
    mvn -pl fory-json clean test spotless:check checkstyle:check: 1146 tests, 0 failures, 0 errors, 0 skipped,
    spotless clean, 0 checkstyle violations.
  • Baseline for comparison, same commands on a pristine worktree of main: 1144 tests, 0 failures.
  • I reverted the utf16 scan to a form without the non-latin stop and confirmed readEscapeFollowedByNonAsciiText
    fails, then restored it and confirmed the suite is green again.
  • I have not run benchmarks/java.
AI Usage Disclosure
- substantial_ai_assistance: yes
- scope: design drafting, code drafting, tests
- affected_files_or_subsystems: java/fory-json/src/main/java/org/apache/fory/json/reader (three readers),
  java/fory-json/src/test/java/org/apache/fory/json/JsonStringTest.java
- ai_review: line-by-line self-review first, then a two-reviewer loop repeated until clean - one reviewer guided
  by AGENTS.md and .agents/ci-and-pr.md, one independent reviewer in a separate clean-context session outside
  this repository, given no project guidance. The loop found six blocking defects, all in what this change adds.
  Four in the code: the utf16 run accepted characters above 0xFF and packed them into one byte; a bound
  computation could overflow near Integer.MAX_VALUE; the quoted-text decode path grew the shared buffer without
  recording it; and the retention ceiling was applied to the shrink size but not to the shrink condition, so a
  buffer up to twice the ceiling survived clear(). Two in the tests: the ceiling case did not actually depend on
  the ceiling condition, and it used a reader constructor that throws on jdk 8. Ten further optional findings
  were addressed; one was not and is named above. Both reviewers report no further actionable findings on commit
  d950b0b, which is the head of this pull request.
- ai_review_artifacts: the final verdicts are quoted below; full transcripts available on request.
- human_verification: the commands and results in the section above; I read the diff line by line and can
  explain and defend every line of it.
- performance_verification: the before/after table above, taken on the application harness that found the
  problem, not on benchmarks/java. The independent reviewer additionally ran a differential fuzz of this tree
  against the same tree with the three readers reverted: 30000 random documents (ascii, latin1 high bytes,
  U+0100-U+FFFF, supplementary pairs, every escape form, lengths 1-70) through fromJson(String), fromJson(byte[])
  and each reader directly, all exact, plus a transcript over 94981 truncated prefixes recording result, string
  coder, length and hash or exception class, message and position - sha256 identical between patched and
  baseline.
- provenance_license_confirmation: Apache-2.0-compatible provenance confirmed; no third-party code introduced.

Final review results, both on commit d950b0b.

Fory-guided reviewer:

No further actionable findings in commit d950b0b; this read-only review relies on your reported JDK 25
validation results.

Independent reviewer:

I have no further actionable findings in the revised patch.

🤖 Generated with Claude Code

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>
@pavel-ptashyts

Copy link
Copy Markdown
Author

The one failing check, asf-allowlist-check, is unrelated to this change: it flags swatinem/rust-cache@v2
in .github/workflows/ci.yml, which this PR does not touch — the diff is four Java files under fory-json.

Other open PRs still show it green because their runs are from 12–13 September, before that pin left the ASF
allowlist. The same log also warns that graalvm/setup-graalvm@6f3fa03 expires on 2026-09-27, so this will
start failing more widely; the job output prints the commands for the apache/infrastructure-actions PR.

Everything that exercises this change is green: Java CI 8, 11, 17, 21, 25 and 26, Openj9 21, Windows Java 21,
all GraalVM variants including the JSON ones, and Code Style Check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java] Escaped strings are decoded one character at a time from the first escape to the end of the string (java, fory-json)

1 participant