chore: Revert micro-optimizations for reader & RFC compliance - #54
Merged
Conversation
This reverts commit a74e968.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 40125f4. Configure here.
| } | ||
| return val | ||
| } | ||
|
|
There was a problem hiding this comment.
Breaking removal mislabeled as chore
Medium Severity
This PR removes the exported Reader.RawValue and Reader.Offset APIs but the title uses chore: instead of a breaking-change conventional commit prefix (feat!: or fix!:). That violates the rule that removing exported Go symbols must be labeled with ! so release-please cuts a major version bump.
Triggered by learned rule: Removing exported functions requires feat!: breaking change prefix
Reviewed by Cursor Bugbot for commit 40125f4. Configure here.
kinyoklion
approved these changes
Aug 11, 2026
kinyoklion
added a commit
that referenced
this pull request
Aug 12, 2026
**SDK-2884** — backport of #52 to v3. Independent of the two jwriter backports (disjoint files). Rewrites the default tokenizer as a single-pass scanner over the input byte slice: strings scan in place (unescaped strings keep the existing zero-copy subslice behavior and need no UTF-8 decoding, since only the quote and backslash end the scan), escaped strings decode in one forward pass into a buffer sized up front, scalar reads dispatch on the first non-whitespace byte, and tokens parse into a reused field instead of shuttling token structs through call layers. `bytes.Reader` and per-rune appends are gone. Observable behavior is unchanged, including the deliberately lenient baseline v3 and v4 share since their symmetric reverts (#54/#55): per-byte `unicode.IsSpace` whitespace, verbatim pass-through of control characters and invalid UTF-8 in unescaped strings, U+FFFD re-encoding on the escaped path, no surrogate combining, lenient number scanning with exact failure positions, and int64 wraparound. A lock-step differential harness ran this port against the v3 baseline directly: 1,020,486 paired runs over 145,464 inputs, zero mismatches (values bit-exact, errors compared by type, message, value, and offset), with the harness's sensitivity re-proven by a seeded whitespace mutation. The same harness had proven the v4 change against the shared baseline. The table-driven tests pinning those behaviors — including the external-conformance-sweep cases — are part of the port. `token_reader_default.go` is at 100% statement coverage. ## Adaptations for v3 Verbatim from v4 apart from: the default-implementation build tags and header comment are preserved, the ported test file keeps v3's `isEasyJSON` constant, and the benchmark's import uses the v3 module path. ## 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): ``` ReadString -59.6% ReadNumberIntNoAlloc -46.8% ReadArrayOfBools -49.7% ReadArrayOfStrings -53.4% ReadObjectNoAlloc -50.6% ReadArrayOfObjects -50.1% ReadObjectWithRequiredPropsNoAlloc -53.9% ``` All `NoAlloc` benchmarks remain at 0 allocs/op. On a real 3,228-flag / 3.2 MB LaunchDarkly payload parsed through `ldmodel` (go-server-sdk-evaluation v3, which consumes this module natively): direct jreader parse **28.2 ms → 15.1 ms (-46.4%)**; through the `encoding/json`-dispatch path **53.0 ms → 36.4 ms (-31.3%)**. This also closes the gap that previously made the easyjson build the faster read path on this payload (~19%), with margin. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Rewrites the default (non-easyjson) JSON tokenizer for substantially higher throughput while keeping observable parsing behavior unchanged. > > The scanner now walks the input byte slice in a single pass: unescaped strings stay zero-copy, escaped strings decode in one forward pass into a pre-sized buffer, and scalar reads (`Bool`/`Number`/`String`) fast-path on the first non-whitespace byte. Tokens are stored in a reused `tok` field instead of being returned through call layers, and `bytes.Reader` / per-rune appends are removed. Lookup tables replace per-byte `unicode.IsSpace` and plain-ASCII string checks. > > Adds table-driven edge-case tests that pin lenient number/string handling, pushed-back token interactions, whitespace classification, and error offsets, plus a `BenchmarkReadStringKinds` covering ASCII, multi-byte, and escaped paths. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3a40e1e. 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.
Note
Overview
This PR reverts two prior features:
Reader.RawValue/Reader.Offset(and the default tokenizer’sRawValue, container boundary scan, anderrMsgInvalidValue) plus all dedicated tests (unit, differential, fuzz, offset).It also rolls back RFC 8259–oriented default-backend behavior in
jreaderandjwriter: whitespace handling again usesunicode.IsSpaceinstead of JSON-only space/tab/LF/CR; number parsing returns to the simpler byte-scan path without strict leading-zero/overflow rules; string decoding drops surrogate-pair combining, invalid-UTF-8 substitution, and unescaped control-character rejection;jwriter.Float64no longer errors on NaN/Inf. The RFC/non-finite writer tests are removed.Reviewed by Cursor Bugbot for commit 40125f4. Bugbot is set up for automated code reviews on this repo. Configure here.