You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Reranko05 (via GitHub)" <gi...@apache.org> on 2026/08/18 18:09:37 UTC
[PR] GH-50904: [C++] Replace RapidJSON with simdjson in OpaqueType [arrow]
Reranko05 opened a new pull request, #50905:
URL: https://github.com/apache/arrow/pull/50905
### Rationale for this change
This PR continues the simdjson migration by replacing the remaining RapidJSON usage in `OpaqueType` under `cpp/src/arrow/extension` with the existing simdjson helpers and On-Demand API.
The existing deserialization validation and behavior are preserved, with tests updated for parser-specific error messages.
### Changes
- Replace RapidJSON parsing in `OpaqueType::Deserialize()` with simdjson.
- Use the existing `ResolveSimdjsonResult()` helper.
- Remove the RapidJSON dependency from `opaque.cc`.
- Update OpaqueType deserialization tests for simdjson parsing errors.
Fixes: #50904
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50904: [C++] Replace RapidJSON with simdjson in OpaqueType [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #50905:
URL: https://github.com/apache/arrow/pull/50905#issuecomment-5393735829
@kou @Reranko05 Can we please revisit this and use an approach that adds less boilerplate? For example we could use the DOM API rather than the on-demand API. We could also build some higher-level helpers around the simdjson APIs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50904: [C++] Replace RapidJSON with simdjson in OpaqueType [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #50905:
URL: https://github.com/apache/arrow/pull/50905#issuecomment-5350973625
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit df824c10a7429bbed6de04715d8e008e28674074.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/96303481799) has more details. It also includes information about 106 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50904: [C++] Replace RapidJSON with simdjson in OpaqueType [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #50905:
URL: https://github.com/apache/arrow/pull/50905
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50904: [C++] Replace RapidJSON with simdjson in OpaqueType [arrow]
Posted by "Reranko05 (via GitHub)" <gi...@apache.org>.
Reranko05 commented on code in PR #50905:
URL: https://github.com/apache/arrow/pull/50905#discussion_r3811249514
##########
cpp/src/arrow/extension/opaque.cc:
##########
@@ -65,34 +64,83 @@ std::string OpaqueType::Serialize() const {
Result<std::shared_ptr<DataType>> OpaqueType::Deserialize(
std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
- rapidjson::Document document;
- const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
- if (parsed.HasParseError()) {
+ simdjson::padded_string padded_json(serialized_data);
+ simdjson::ondemand::parser parser;
+ simdjson::ondemand::document document;
+
+ if (auto error = parser.iterate(padded_json).get(document);
+ error != simdjson::SUCCESS) {
Review Comment:
Addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-50904: [C++] Replace RapidJSON with simdjson in OpaqueType [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #50905:
URL: https://github.com/apache/arrow/pull/50905#discussion_r3809057594
##########
cpp/src/arrow/extension/opaque.cc:
##########
@@ -65,34 +64,83 @@ std::string OpaqueType::Serialize() const {
Result<std::shared_ptr<DataType>> OpaqueType::Deserialize(
std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
- rapidjson::Document document;
- const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
- if (parsed.HasParseError()) {
+ simdjson::padded_string padded_json(serialized_data);
+ simdjson::ondemand::parser parser;
+ simdjson::ondemand::document document;
+
+ if (auto error = parser.iterate(padded_json).get(document);
+ error != simdjson::SUCCESS) {
Review Comment:
Can we use `internal::ResolveSimdjsonResult()` for this case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org