Skip to content

Commit 577d3b9

Browse files
Reranko05pitrou
andauthored
GH-50901: [C++] Replace RapidJSON with simdjson in tensor extension types (#50874)
### Rationale for this change This PR continues the simdjson migration by replacing RapidJSON usage in the `FixedShapeTensorType` and `VariableShapeTensorType` extension types with simdjson's DOM API. ### Changes - Replace RapidJSON parsing in `FixedShapeTensorType::Deserialize` with simdjson. - Replace RapidJSON parsing in `VariableShapeTensorType::Deserialize` with simdjson. - Centralize the reusable simdjson DOM helpers in `arrow/util/simdjson_internal.h`. - Add helpers for parsing JSON objects, retrieving optional fields, and validating JSON arrays and their element types. - Remove the obsolete RapidJSON-specific helpers and includes from the tensor extension utilities. - Preserve existing deserialization behavior and error validation, including the ordering of metadata-level validation errors. - Update deserialization tests to match simdjson JSON type names. * GitHub Issue: #50901 Lead-authored-by: Aaditya Srinivasan <aadityasri03@gmail.com> Co-authored-by: Aaditya Srinivasan <156181482+Reranko05@users.noreply.github.com> Co-authored-by: Antoine Pitrou <pitrou@free.fr> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent a2a9dce commit 577d3b9

6 files changed

Lines changed: 215 additions & 138 deletions

File tree

cpp/src/arrow/extension/fixed_shape_tensor.cc

Lines changed: 29 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,22 @@
1919
#include <numeric>
2020
#include <sstream>
2121

22+
#include <simdjson.h>
23+
2224
#include "arrow/extension/fixed_shape_tensor.h"
2325
#include "arrow/extension/tensor_internal.h"
2426
#include "arrow/scalar.h"
2527

2628
#include "arrow/array/array_nested.h"
2729
#include "arrow/array/array_primitive.h"
2830
#include "arrow/json/json_writer_internal.h"
29-
#include "arrow/json/rapidjson_defs.h" // IWYU pragma: keep
3031
#include "arrow/tensor.h"
3132
#include "arrow/util/logging_internal.h"
3233
#include "arrow/util/print_internal.h"
34+
#include "arrow/util/simdjson_internal.h"
3335
#include "arrow/util/sort_internal.h"
3436
#include "arrow/util/string.h"
3537

36-
#include <rapidjson/document.h>
37-
38-
namespace rj = arrow::rapidjson;
3938
using ::arrow::json::JsonWriter;
4039

4140
namespace arrow::extension {
@@ -116,57 +115,39 @@ Result<std::shared_ptr<DataType>> FixedShapeTensorType::Deserialize(
116115
return Status::Invalid("Expected FixedSizeList storage type, got ",
117116
storage_type->ToString());
118117
}
118+
119119
auto fsl_type = internal::checked_pointer_cast<FixedSizeListType>(storage_type);
120120
auto value_type = fsl_type->value_type();
121-
rj::Document document;
122-
if (document.Parse(serialized_data.data(), serialized_data.length()).HasParseError() ||
123-
!document.IsObject() || !document.HasMember("shape") ||
124-
!document["shape"].IsArray()) {
125-
return Status::Invalid("Invalid serialized JSON data: ", serialized_data);
126-
}
127121

128-
std::vector<int64_t> shape;
129-
for (const auto& x : document["shape"].GetArray()) {
130-
if (!x.IsInt64()) {
131-
return Status::Invalid("shape must contain integers, got ",
132-
internal::JsonTypeName(x));
133-
}
134-
shape.emplace_back(x.GetInt64());
135-
}
122+
simdjson::dom::parser parser;
123+
ARROW_ASSIGN_OR_RAISE(auto object, internal::ParseJsonObject(parser, serialized_data));
124+
125+
ARROW_ASSIGN_OR_RAISE(auto shape_value,
126+
internal::ResolveSimdjsonResult(object.at_key("shape"),
127+
"Invalid serialized JSON data"));
128+
ARROW_ASSIGN_OR_RAISE(auto shape, internal::GetJsonIntArray(shape_value, "shape"));
129+
ARROW_ASSIGN_OR_RAISE(auto permutation_value,
130+
internal::GetOptionalJsonField(object, "permutation"));
136131

137132
std::vector<int64_t> permutation;
138-
if (document.HasMember("permutation")) {
139-
const auto& json_permutation = document["permutation"];
140-
if (!json_permutation.IsArray()) {
141-
return Status::Invalid("permutation must be an array, got ",
142-
internal::JsonTypeName(json_permutation));
143-
}
144-
for (const auto& x : json_permutation.GetArray()) {
145-
if (!x.IsInt64()) {
146-
return Status::Invalid("permutation must contain integers, got ",
147-
internal::JsonTypeName(x));
148-
}
149-
permutation.emplace_back(x.GetInt64());
150-
}
133+
if (permutation_value.has_value()) {
134+
ARROW_ASSIGN_OR_RAISE(permutation,
135+
internal::GetJsonIntArray(*permutation_value, "permutation"));
136+
151137
if (shape.size() != permutation.size()) {
152138
return Status::Invalid("Invalid permutation");
153139
}
154140
RETURN_NOT_OK(internal::IsPermutationValid(permutation));
155141
}
142+
143+
ARROW_ASSIGN_OR_RAISE(auto dim_names_value,
144+
internal::GetOptionalJsonField(object, "dim_names"));
145+
156146
std::vector<std::string> dim_names;
157-
if (document.HasMember("dim_names")) {
158-
const auto& json_dim_names = document["dim_names"];
159-
if (!json_dim_names.IsArray()) {
160-
return Status::Invalid("dim_names must be an array, got ",
161-
internal::JsonTypeName(json_dim_names));
162-
}
163-
for (const auto& x : json_dim_names.GetArray()) {
164-
if (!x.IsString()) {
165-
return Status::Invalid("dim_names must contain strings, got ",
166-
internal::JsonTypeName(x));
167-
}
168-
dim_names.emplace_back(x.GetString());
169-
}
147+
if (dim_names_value.has_value()) {
148+
ARROW_ASSIGN_OR_RAISE(dim_names,
149+
internal::GetJsonStringArray(*dim_names_value, "dim_names"));
150+
170151
if (shape.size() != dim_names.size()) {
171152
return Status::Invalid("Invalid dim_names");
172153
}
@@ -177,14 +158,18 @@ Result<std::shared_ptr<DataType>> FixedShapeTensorType::Deserialize(
177158
// (type mismatches, size mismatches) are reported first.
178159
ARROW_ASSIGN_OR_RAISE(auto ext_type, FixedShapeTensorType::Make(
179160
value_type, shape, permutation, dim_names));
161+
180162
const auto& fst_type = internal::checked_cast<const FixedShapeTensorType&>(*ext_type);
163+
181164
ARROW_ASSIGN_OR_RAISE(const int64_t expected_size,
182165
internal::ComputeShapeProduct(fst_type.shape()));
166+
183167
if (expected_size != fsl_type->list_size()) {
184168
return Status::Invalid("Product of shape dimensions (", expected_size,
185169
") does not match FixedSizeList size (", fsl_type->list_size(),
186170
")");
187171
}
172+
188173
return ext_type;
189174
}
190175

cpp/src/arrow/extension/tensor_extension_array_test.cc

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -223,15 +223,15 @@ TEST_F(TestFixedShapeTensorType, MetadataSerializationRoundtrip) {
223223
// Validate shape values must be integers. Error message should include the
224224
// JSON type name of the offending value.
225225
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":[3.5,4]})",
226-
"shape must contain integers, got Number");
226+
"shape must contain integers, got number");
227227
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":["3","4"]})",
228-
"shape must contain integers, got String");
228+
"shape must contain integers, got string");
229229
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":[null]})",
230-
"shape must contain integers, got Null");
230+
"shape must contain integers, got null");
231231
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":[true]})",
232-
"shape must contain integers, got True");
232+
"shape must contain integers, got boolean");
233233
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":[false]})",
234-
"shape must contain integers, got False");
234+
"shape must contain integers, got boolean");
235235

236236
// Validate shape values must be non-negative
237237
CheckDeserializationRaises(ext_type_, fixed_size_list(int64(), 1), R"({"shape":[-1]})",
@@ -244,16 +244,20 @@ TEST_F(TestFixedShapeTensorType, MetadataSerializationRoundtrip) {
244244
// Validate permutation member must be an array with integer values
245245
CheckDeserializationRaises(ext_type_, storage_type,
246246
R"({"shape":[3,4],"permutation":"invalid"})",
247-
"permutation must be an array, got String");
247+
"permutation must be an array, got string");
248248
CheckDeserializationRaises(ext_type_, storage_type,
249249
R"({"shape":[3,4],"permutation":{"a":1}})",
250-
"permutation must be an array, got Object");
250+
"permutation must be an array, got object");
251251
CheckDeserializationRaises(ext_type_, storage_type,
252252
R"({"shape":[3,4],"permutation":[1.5,0.5]})",
253-
"permutation must contain integers, got Number");
253+
"permutation must contain integers, got number");
254254
CheckDeserializationRaises(ext_type_, storage_type,
255255
R"({"shape":[3,4],"permutation":["a","b"]})",
256-
"permutation must contain integers, got String");
256+
"permutation must contain integers, got string");
257+
// Validate permutation member must be an array with integer values
258+
CheckDeserializationRaises(ext_type_, storage_type,
259+
R"({"shape":[3,4],"permutation":[]})",
260+
"Invalid permutation");
257261

258262
// Validate permutation values must be unique integers in [0, N-1]
259263
CheckDeserializationRaises(ext_type_, storage_type,
@@ -269,13 +273,15 @@ TEST_F(TestFixedShapeTensorType, MetadataSerializationRoundtrip) {
269273
// Validate dim_names member must be an array with string values
270274
CheckDeserializationRaises(ext_type_, storage_type,
271275
R"({"shape":[3,4],"dim_names":"invalid"})",
272-
"dim_names must be an array, got String");
276+
"dim_names must be an array, got string");
273277
CheckDeserializationRaises(ext_type_, storage_type,
274278
R"({"shape":[3,4],"dim_names":[1,2]})",
275-
"dim_names must contain strings, got Number");
279+
"dim_names must contain strings, got number");
276280
CheckDeserializationRaises(ext_type_, storage_type,
277281
R"({"shape":[3,4],"dim_names":[null,null]})",
278-
"dim_names must contain strings, got Null");
282+
"dim_names must contain strings, got null");
283+
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":[3,4],"dim_names":[]})",
284+
"Invalid dim_names");
279285
}
280286

281287
TEST_F(TestFixedShapeTensorType, MakeValidatesShape) {
@@ -858,35 +864,41 @@ TEST_F(TestVariableShapeTensorType, MetadataSerializationRoundtrip) {
858864
CheckDeserializationRaises(ext_type_, storage_type, R"({"shape":(3,4)})",
859865
"Invalid serialized JSON data");
860866
CheckDeserializationRaises(ext_type_, storage_type, R"({"permutation":[1,0]})",
861-
"Invalid: permutation");
867+
"Invalid permutation");
862868
CheckDeserializationRaises(ext_type_, storage_type, R"({"dim_names":["x","y"]})",
863-
"Invalid: dim_names");
869+
"Invalid dim_names");
864870

865871
// Validate permutation member must be an array with integer values. Error
866872
// message should include the JSON type name of the offending value.
867873
CheckDeserializationRaises(ext_type_, storage_type, R"({"permutation":"invalid"})",
868-
"permutation must be an array, got String");
874+
"permutation must be an array, got string");
869875
CheckDeserializationRaises(ext_type_, storage_type, R"({"permutation":[1.5,0.5,2.5]})",
870-
"permutation must contain integers, got Number");
876+
"permutation must contain integers, got number");
871877
CheckDeserializationRaises(ext_type_, storage_type,
872878
R"({"permutation":[null,null,null]})",
873-
"permutation must contain integers, got Null");
879+
"permutation must contain integers, got null");
880+
CheckDeserializationRaises(ext_type_, storage_type, R"({"permutation":[]})",
881+
"Invalid permutation");
874882

875883
// Validate dim_names member must be an array with string values
876884
CheckDeserializationRaises(ext_type_, storage_type, R"({"dim_names":"invalid"})",
877-
"dim_names must be an array, got String");
885+
"dim_names must be an array, got string");
878886
CheckDeserializationRaises(ext_type_, storage_type, R"({"dim_names":[1,2,3]})",
879-
"dim_names must contain strings, got Number");
887+
"dim_names must contain strings, got number");
888+
CheckDeserializationRaises(ext_type_, storage_type, R"({"dim_names":[]})",
889+
"Invalid dim_names");
880890

881891
// Validate uniform_shape member must be an array with integer-or-null values
882892
CheckDeserializationRaises(ext_type_, storage_type, R"({"uniform_shape":"invalid"})",
883-
"uniform_shape must be an array, got String");
893+
"uniform_shape must be an array, got string");
884894
CheckDeserializationRaises(ext_type_, storage_type,
885895
R"({"uniform_shape":[1.5,null,null]})",
886-
"uniform_shape must contain integers or nulls, got Number");
896+
"uniform_shape must contain integers or nulls, got number");
887897
CheckDeserializationRaises(ext_type_, storage_type,
888898
R"({"uniform_shape":["x",null,null]})",
889-
"uniform_shape must contain integers or nulls, got String");
899+
"uniform_shape must contain integers or nulls, got string");
900+
CheckDeserializationRaises(ext_type_, storage_type, R"({"uniform_shape":[]})",
901+
"Invalid uniform_shape");
890902
}
891903

892904
TEST_F(TestVariableShapeTensorType, RoundtripBatch) {

cpp/src/arrow/extension/tensor_internal.cc

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,6 @@
3030

3131
namespace arrow::internal {
3232

33-
namespace {
34-
35-
// Names indexed by rapidjson::Type enum value:
36-
// kNullType=0, kFalseType=1, kTrueType=2, kObjectType=3,
37-
// kArrayType=4, kStringType=5, kNumberType=6.
38-
constexpr const char* kJsonTypeNames[] = {"Null", "False", "True", "Object",
39-
"Array", "String", "Number"};
40-
41-
} // namespace
42-
43-
const char* JsonTypeName(const ::arrow::rapidjson::Value& v) {
44-
return kJsonTypeNames[v.GetType()];
45-
}
46-
4733
Result<int64_t> ComputeShapeProduct(std::span<const int64_t> shape) {
4834
int64_t product = 1;
4935
for (const auto dim : shape) {

cpp/src/arrow/extension/tensor_internal.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,11 @@
2121
#include <span>
2222
#include <vector>
2323

24-
#include "arrow/json/rapidjson_defs.h" // IWYU pragma: keep
2524
#include "arrow/result.h"
2625
#include "arrow/type_fwd.h"
2726

28-
#include <rapidjson/document.h>
29-
3027
namespace arrow::internal {
3128

32-
/// \brief Return the name of a RapidJSON value's type (e.g., "Null", "Array", "Number").
33-
ARROW_EXPORT
34-
const char* JsonTypeName(const ::arrow::rapidjson::Value& v);
35-
3629
/// \brief Compute the product of the given shape dimensions.
3730
///
3831
/// Returns Status::Invalid if the product would overflow int64_t.

0 commit comments

Comments
 (0)