Repository navigation
Render nested scalars without encoding/json - #527
vuanhphung wants to merge 3 commits into
Conversation
Go 1.27 enables the jsonv2 experiment by default, so encoding/json.Marshal runs on the json/v2 engine and costs noticeably more per call. The nested ARRAY/MAP/STRUCT renderers call it once per scalar element, which made complex-type results 13-27% slower on Go 1.27 than on Go 1.26. Add internal/jsonfmt, which formats bools, integers, finite floats, and strings that need no escaping directly, byte-identical to json.Marshal, and falls back to json.Marshal for everything else. Use it from both the Thrift (arrowbased) and kernel (arrowscan) renderers. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
- Look up escape-free bytes in a table: the per-byte comparisons made long ASCII strings slower than json.Marshal (8 KiB: 7.6 vs 5.5 us; now 3.2 us). - Return json.Marshal's slice directly when dst is empty instead of copying it. - Silence gosec G404 for the seeded test RNG. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Looks good. appendFloat matches encoding/json's float encoder exactly, the type switch only fast-paths exact built-in types (named types and json.Marshaler types fall back), and both renderers keep the same output as before. One low note: strings containing any non-ASCII byte always take the slower json.Marshal fallback.
| // plainASCII marks the bytes json.Marshal copies verbatim. A table lookup keeps | ||
| // the scan faster than json.Marshal's own on long strings. | ||
| var plainASCII = func() (t [256]bool) { | ||
| for c := 0x20; c < 0x7f; c++ { |
There was a problem hiding this comment.
🔵 Low — plainASCII only marks 0x20–0x7e as safe, so a string with any non-ASCII byte (accented Latin, CJK, emoji) skips the fast path and goes through the full json.Marshal fallback. The output is still correct. The cost is that STRING elements in non-English data get none of the speedup this PR is after, and the CloudFetch benchmark (likely ASCII alpha0-style data) won't show it.
If you want to cover that case: encoding/json copies valid multi-byte UTF-8 through unchanged except U+2028/U+2029 (and it replaces invalid sequences with �). You could validate with utf8.DecodeRuneInString and fall back only when a sequence is invalid or is U+2028/U+2029. The existing every-single-byte and héllo/日本語/\xff\xfe parity tests would guard that change. This is optional; the current behaviour is safe.
- Copy valid multi-byte UTF-8 (and 0x7f) verbatim, as both json engines do; only invalid UTF-8 and U+2028/U+2029 still fall back to json.Marshal. - Thrift containers and kernel map keys append scalars into a stack scratch buffer instead of allocating per value. - Tests: every-rune and random Unicode parity, error-text parity, nested DOUBLE and UTF-8/HTML string cases in the backend parity test. - Add BenchmarkRenderNested for both backends; report allocs and cover fallback values in BenchmarkMarshalScalars. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
Go 1.27 enables the
jsonv2experiment by default, soencoding/json.Marshalnow runs on the json/v2 engine and costs more per call. The nested ARRAY / MAP / STRUCT renderers call it once per scalar element, which made complex-type results 13–27% slower on Go 1.27 than on Go 1.26. Since v1.18.0 requires Go 1.27.3, every user upgrading hits this; it showed up as the Extended Types jump on the Go nightly benchmark when its build image moved from Go 1.26 to 1.27.This adds
internal/jsonfmt, which formats bools, integers, finite floats, and strings (including valid UTF-8) directly, byte-identical tojson.Marshal, and falls back tojson.Marshalfor everything else (strings needing escapes or with invalid UTF-8,[]byte, NaN/Inf, other types). Both the Thrift (arrowbased) and kernel (arrowscan) renderers use it, appending into stack scratch buffers, so cross-backend parity is unchanged.Offline CloudFetch harness, Go 1.27.3, single-column results drained through
database/sql:This is also faster than main built with
GOEXPERIMENT=nojsonv2or with Go 1.26.BenchmarkRenderNested(internal/rows/arrowbased) reproduces this per backend in-repo: 1.7–2.9x faster on both, except binary leaves, which still go throughjson.Marshaland are unchanged. The remaining ~10% Go 1.27 slowdown on primitive columns comes fromdatabase/sql's newRowslock and isn't addressable in the driver.Testing: new tests check byte-for-byte parity with
json.Marshal(edge cases, every single-byte string, every rune, random Unicode mixes, random float bit patterns), under both the default json/v2 backend andGOEXPERIMENT=nojsonv2. The Thrift/kernel parity test gains nested DOUBLE and UTF-8/HTML string cases.go test -short ./...and golangci-lint pass.This pull request and its description were written by Isaac.