The GitHub Actions job "ASF GitHub Actions Allowlist" on fory.git/main has failed. Run started by GitHub user chaokunyang (triggered by chaokunyang).
Head commit for run: 1b9ca11d1de4c2bb42ea8f3d9e395eded47e69e0 / Pavel Ptashyts <[email protected]> perf(java): copy plain runs between escapes when decoding json strings (#4058) 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`. ```text 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](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <[email protected]> Report URL: https://github.com/apache/fory/actions/runs/35484782740 With regards, GitHub Actions via GitBox --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
