Uh oh!
There was an error while loading. Please reload this page.
GH-50944: [C++] Replace RapidJSON with simdjson in JSON chunker - #50945
GH-50944: [C++] Replace RapidJSON with simdjson in JSON chunker#50945Reranko05 wants to merge 7 commits into
Conversation
Reranko05
commented
Aug 21, 2026
e9fc9fe to
be4c2a1Compare
pitrou
left a comment
There was a problem hiding this comment.
I don't understand why this is parsing JSON by hand?
It seems that we might be able to use simdjson::ondemand::parser::iterate_many.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Reranko05
commented
Aug 24, 2026
I initially tried using That said, I agree that parsing JSON manually here is not ideal. I'll revisit this using |
Reranko05
commented
Aug 25, 2026
@pitrou I tried using Manual structural parsing approach preserved the existing test behavior. @rok, since your earlier implementation was helpful here, do you have any suggestions on how to preserve the current error behavior with |
pitrou
commented
Aug 25, 2026
As long as an error is reported while reading the JSON stream, I don't think we care if it's reported by the chunker or the parser. |
Reranko05
commented
Aug 26, 2026
@pitrou I gave |
pitrou
commented
Aug 26, 2026
Hmm, I see. Thanks for trying anyway :-)
Well, as a last resort, yes. The problem:
|
cyb70289
commented
Aug 27, 2026
Try to understand the issue. Is it that json strings legal for rapidjson may fail on simdjson, makes future Arrow release potentially incompatible to old version? Writing our own optimized version looks not ideal. Can we just use simdjson? It's state-of-the-art, and even with self written object delimiter, there's still incompatibility risk I'm afraid. |
Reranko05
commented
Aug 27, 2026
@cyb70289 I don't think the issue is JSON compatibility between RapidJSON and simdjson. The main issue I ran into is the boundary/streaming semantics. The previous RapidJSON implementation uses With I agree that writing our own optimized JSON parser would not be ideal. The manual approach I used, and which @rok also implemented in rok#47, only scans for the boundary of the first complete object or array while respecting strings and escapes, and then lets simdjson perform the actual JSON validation. But I agree this still introduces complexity and needs careful testing. If there is a way to use simdjson directly while preserving the old stop-after-one-value semantics, that would definitely be preferable. |
pitrou
commented
Aug 27, 2026
Is that a problem? We want to keep compatibility when parsing valid JSON streams. The failure mode for an invalid JSON stream can change. |
cyb70289
commented
Aug 27, 2026
A discussion about ignoring trailing garbage in simdjson. Looks there're real use cases lenient parsing can be useful. |
pitrou
commented
Aug 27, 2026
Trailing garbage is not the problem here. We are parsing a stream of valid JSON documents. We are happy to error out on trailing garbage. |
227961f to
3f15314Compare626bb0d to
b9af8ceCompare
Rationale for this change
This PR continues the simdjson migration by replacing the RapidJSON-based JSON boundary detection used by the JSON chunker.
The existing implementation uses RapidJSON's streaming parser to identify complete JSON values. This change replaces that logic with structural boundary detection and simdjson validation.
Changes
simdjson::dom::parser.Fixes: #50944