Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/cpp/benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ ConfigureBench(BLOOM_FILTER_BENCH
ConfigureBench(GET_JSON_OBJECT_BENCH
get_json_object.cu)

ConfigureBench(FROM_JSON_TO_STRUCTS_BENCH
from_json_to_structs.cu)

ConfigureBench(PARSE_URI_BENCH
parse_uri.cpp)

Expand Down
105 changes: 105 additions & 0 deletions src/main/cpp/benchmarks/from_json_to_structs.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright (c) 2026, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "json_utils.hpp"

#include <cudf_test/column_wrapper.hpp>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 SHOULD FIX

Can the new benchmark include the owning cudf::column header directly and keep its project-local header in the first include group? src/main/cpp/benchmarks/from_json_to_structs.cu:31 names cudf::column, and src/main/cpp/benchmarks/from_json_to_structs.cu:81 calls cudf::column::view(), but src/main/cpp/benchmarks/from_json_to_structs.cu:17-23 has no <cudf/column/column.hpp> and places the internal src/main/cpp/src/json_utils.hpp:17 header after other RAPIDS headers using angle brackets. This makes the translation unit depend on another header exposing the complete cudf::column definition and diverges from the project's local-first/internal-header include convention.

Suggested fix: Move the project-local header to the first group using quotes and directly include <cudf/column/column.hpp> in the cuDF group.

Confidence: 🟣❗ CERTAIN; reviewer scope: compliant.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated.


#include <cudf/column/column.hpp>
#include <cudf/strings/strings_column_view.hpp>
#include <cudf/types.hpp>

#include <nvbench/nvbench.cuh>

#include <memory>
#include <string>
#include <vector>

namespace {

[[nodiscard]] std::unique_ptr<cudf::column> make_input(cudf::size_type num_rows,
cudf::size_type mismatch_percent)
{
std::string const valid = R"({"data":{"c2":[{"c3":19,"c4":"x"}],"c1":1},"id":10})";
std::string const mismatched = R"({"data":{"c2":[19],"c1":2},"id":20})";

std::vector<std::string> rows;
rows.reserve(num_rows);
for (cudf::size_type row = 0; row < num_rows; ++row) {
rows.push_back(mismatch_percent > 0 && row % 100 < mismatch_percent ? mismatched : valid);
}
return cudf::test::strings_column_wrapper(rows.begin(), rows.end()).release();
}

std::vector<std::string> nested_schema_names()
{
return {"data", "c1", "c2", "element", "c3", "c4", "id"};
}

std::vector<int> nested_schema_num_children() { return {2, 0, 1, 2, 0, 0, 0}; }

std::vector<int> nested_schema_types()
{
return {static_cast<int>(cudf::type_id::STRUCT),
static_cast<int>(cudf::type_id::INT32),
static_cast<int>(cudf::type_id::LIST),
static_cast<int>(cudf::type_id::STRUCT),
static_cast<int>(cudf::type_id::INT32),
static_cast<int>(cudf::type_id::STRING),
static_cast<int>(cudf::type_id::INT32)};
}

std::vector<int> nested_schema_scales() { return {0, 0, 0, 0, 0, 0, 0}; }

std::vector<int> nested_schema_precisions() { return {-1, -1, -1, -1, -1, -1, -1}; }

} // namespace

void BM_from_json_to_structs(nvbench::state& state)
{
auto const num_rows = static_cast<cudf::size_type>(state.get_int64("num_rows"));
auto const mismatch_percent = static_cast<cudf::size_type>(state.get_int64("mismatch_percent"));
auto const input = make_input(num_rows, mismatch_percent);

auto const col_names = nested_schema_names();
auto const num_children = nested_schema_num_children();
auto const types = nested_schema_types();
auto const scales = nested_schema_scales();
auto const precisions = nested_schema_precisions();

state.set_cuda_stream(nvbench::make_cuda_stream_view(cudf::get_default_stream().value()));
state.exec(nvbench::exec_tag::sync, [&](nvbench::launch&) {
[[maybe_unused]] auto const output =
spark_rapids_jni::from_json_to_structs(cudf::strings_column_view{input->view()},
col_names,
num_children,
types,
scales,
precisions,
/*normalize_single_quotes=*/true,
/*allow_leading_zeros=*/true,
/*allow_nonnumeric_numbers=*/true,
/*allow_unquoted_control=*/true,
/*is_us_locale=*/true);
});

state.add_buffer_size(num_rows, "rows", "Rows");
}

NVBENCH_BENCH(BM_from_json_to_structs)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 SHOULD FIX

Could the regression include two mismatched top-level columns with different row-index vectors? The new lookup at src/main/cpp/src/from_json_to_structs.cu:974 builds one map entry per diagnostic column and selects rows by each schema name at src/main/cpp/src/from_json_to_structs.cu:990. The only current regression schema has one mismatch-capable top-level STRUCT (data) and a valid scalar sibling (id) at src/test/java/com/nvidia/spark/rapids/jni/FromJsonToStructsTest.java:45, so it supplies one diagnostic entry. A mutation that reuses the first row vector for every diagnostic key, associates rows by vector order instead of name, or nullifies only the first mismatched column would leave every current assertion unchanged.

Suggested fix: Add a two-column schema with a mismatched only in row 0 and b mismatched only in row 1, then assert the complete output. That verifies independent name-to-row association rather than only the single-entry case.

Confidence: 🟣❗ CERTAIN; reviewer scope: test-coverage.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ca0366b: added a two-column regression with independent mismatch rows for a and b. FromJsonToStructsTest passed 5/5.

.set_name("from_json_to_structs")
.add_int64_axis("num_rows", {10'000, 100'000})
.add_int64_axis("mismatch_percent", {0, 1, 10, 50, 100});
Loading