fix: Use a byte-class table for the string escape scan in jwriter - #57
Merged
kinyoklion merged 1 commit intoAug 12, 2026
Conversation
Backport of the v4 change: writeQuotedString scans for the next byte needing an escape with a 256-entry byte-class table instead of a range check and two equality comparisons per byte. A table lookup is a single always-L1-resident load plus one branch, which raises the throughput of the one-byte-per-iteration scan loop. Output is byte-identical. The table is deliberately not shared with jreader's: the predicates differ (this writer copies multi-byte characters through verbatim and must stop below 0x20 to escape control characters).
joker23
approved these changes
Aug 12, 2026
kinyoklion
merged commit Aug 12, 2026
18671e7
into
rlamb/sdk-2878/append-based-jwriter-v3
4 checks passed
kinyoklion
added a commit
that referenced
this pull request
Aug 12, 2026
**SDK-2878** — backport of #51 to v3. Rewrites the default token writer's internals around an append-based byte slice, the same design `encoding/json` adopted in Go 1.19–1.24: `streamableBuffer` holds a plain `[]byte` written with inlinable appends, the string escape scan appends clean segments in a single pass, numbers append via `strconv` directly into the buffer, and the streaming chunk-size check runs once per token instead of once per fragment. Buffer growth at least doubles capacity so reallocations stay logarithmic; `Flush` returns and records destination errors, detects short writes, and never retains undelivered data; `MarshalJSONWithWriter` starts from a pre-sized buffer. Public API and encoded output are unchanged (chunk boundaries in streaming mode move to token granularity; total output is identical). ## Adaptations for v3 The change is otherwise verbatim from v4; three things differ because v3 still ships the easyjson build-tag variant: - The default-implementation build tags and header comment are preserved on `token_writer_default.go`. - The two buffer-internals tests (growth amortization, marshal allocations) live in a new default-tagged file, because v3 compiles the shared test files under both tags and they reference default-only internals. - `token_writer_easyjson.go` gains the equivalent pre-sized constructor (`EnsureSpace`) so the shared marshal entry point compiles under both tags. ## Validation Full suite green under both build tags, plus `-race`; lint clean (default tags, matching CI). Benchmarks on v3 (linux/amd64, interleaved binaries, benchstat n=4, listed deltas p=0.029; `encoding/json` comparatives in the same runs were flat): ``` WriteString -27.1% WriteArrayOfBools -21.3% WriteArrayOfStrings -46.9% WriteObject -31.6% StreamingWriterArrayOfStrings -33.3% ``` On a real 3,228-flag / 3.2 MB LaunchDarkly payload marshaled through `ldmodel` (go-server-sdk-evaluation v3, which consumes this module natively): in-memory whole-environment marshal **10.2 ms → 8.4 ms (-17.6%)**, streaming with an 8 KiB chunk size **10.3 ms → 6.9 ms (-33.5%)**. The byte-class-table scan backport (#57) builds on this branch and merges after it. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **Rewrites the default jwriter token path** around an append-based `[]byte` `streamableBuffer` instead of `bytes.Buffer`, with at-least-doubling growth, pre-sized writers (64-byte default, 1000 for `MarshalJSONWithWriter`), and direct `strconv` appends for numbers. > > **String escaping** uses a `plainStringChars` lookup table and segment appends (no per-rune UTF-8 walk); streaming mode flushes at chunk boundaries while scanning so escape-heavy strings do not grow the buffer with input length. > > **Streaming reliability**: `Flush` detects short writes (`io.ErrShortWrite`), sticks destination errors, drops undelivered buffered data, and `Writer.Flush` records those errors on the writer; chunk flush timing aligns with completed tokens (total output unchanged). > > **v3 easyjson build**: adds matching `newTokenWriterWithCapacity` / `EnsureSpace`; buffer-internals tests live in default-tagged files only. > > **Tests** cover growth panics, reallocation bounds, marshal allocation budget, destination failures, short writes, parity across chunk sizes, and escape-heavy streaming bounds. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 18671e7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
kinyoklion
added a commit
that referenced
this pull request
Aug 13, 2026
…tions (#61) v3 counterpart of #60 — the same one-line change: `AllocsPerRun(1, ...)` becomes `AllocsPerRun(100, ...)`, with the test body otherwise unchanged. The easyjson-conditional expectation is preserved: that build's exactly-4-allocations-per-parse count is deterministic and holds under the averaging (the integer division yields 400/100 = 4). Verified under both build tags, including `-race`. See #60 for the analysis: `AllocsPerRun` samples the process-global malloc counter, so `runs=1` makes the assertion "nothing anywhere in the process allocates during the window" — one stray timer or finalizer allocation on a slow runner reads as a failure. Averaging over 100 runs absorbs strays through the integer division, while a genuine allocation in the code under test occurs in every run and still fails. No interaction with the open backport PRs (#56/#57/#58) — none of them touch this file. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **Hardens `TestReaderSkipValueAllocations`** so flaky CI failures from unrelated process allocations are less likely. > > The test still parses the same JSON and skips the nested `b` object while reading `a` and `c`, and still expects **0** allocs (or **4** under the easyjson build tag). The only behavioral change is **`testing.AllocsPerRun(1, …)` → `testing.AllocsPerRun(100, …)`**, with comments explaining that `AllocsPerRun` uses a process-wide counter, so a single run can fail if another goroutine allocates; averaging 100 runs smooths stray timer/finalizer noise while real per-run allocs in the code under test still fail the assertion. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 121ae6c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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
2 participants
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.
SDK-2888 — backport of #53 to v3, verbatim. Builds on the append-based writer branch (its base), since it converts the scan loop that change introduced.
writeQuotedStringscans for the next byte needing an escape with a 256-entry byte-class table instead of a range check and two equality comparisons per byte. A table lookup is a single always-L1-resident load plus one branch, which raises the throughput of the one-byte-per-iteration scan loop. Output is byte-identical.The table is deliberately not shared with jreader's: the predicates differ (this writer copies multi-byte characters through verbatim and must stop below 0x20 to escape control characters).
Validation
Full suite green under both build tags; lint clean. On v3 vs the append-based branch (interleaved, n=4):
WriteArrayOfStrings-4.5%,StreamingWriterArrayOfStrings-8.0%,WriteObjectToNoOpWriterNoAllocs-16.4% (all p=0.029); real-payload whole-environment marshal a further -3.6% geomean. Matches the v4 measurement (-3.9% geomean).Note
Overview
Speeds up JSON string encoding by replacing the per-byte range and equality checks in
writeQuotedStringwith a 256-entryplainStringCharslookup table.The scan still escapes only control characters, quotes, and backslashes; multi-byte characters continue to pass through verbatim. Output is byte-identical, with measurable gains on string-heavy marshal paths.
Reviewed by Cursor Bugbot for commit bbd1d48. Bugbot is set up for automated code reviews on this repo. Configure here.