From 66c3e8e92f9c37dd909b78936addb463f1bd6011 Mon Sep 17 00:00:00 2001
From: Suraj Aralihalli <suraj.ara16@gmail.com>
Date: Thu, 18 Jan 2024 10:40:15 -0800
Subject: [PATCH] Resolve degenerate performance in `create_structs_data`
 (#14761)

Resolves issue [#14716](https://github.com/rapidsai/cudf/issues/14716)

- Eliminated unnecessary recursive self-calls in the `superimpose_nulls_no_sanitize` function, addressing performance issues in `make_structs_column`.
- Introduced `STRUCT_CREATION_NVBENCH` to assess the performance of the `create_structs_data` function.

Authors:
  - Suraj Aralihalli (https://github.com/SurajAralihalli)
  - Nghia Truong (https://github.com/ttnghia)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - David Wendt (https://github.com/davidwendt)

URL: https://github.com/rapidsai/cudf/pull/14761
---
 cpp/benchmarks/CMakeLists.txt             |  7 ++++-
 cpp/benchmarks/structs/create_structs.cpp | 31 +++++++++++++++++++++++
 cpp/src/structs/utilities.cpp             | 14 +++-------
 3 files changed, 41 insertions(+), 11 deletions(-)
 create mode 100644 cpp/benchmarks/structs/create_structs.cpp

diff --git a/cpp/benchmarks/CMakeLists.txt b/cpp/benchmarks/CMakeLists.txt
index 9c3a05a2f5f..35b03fa33d0 100644
--- a/cpp/benchmarks/CMakeLists.txt
+++ b/cpp/benchmarks/CMakeLists.txt
@@ -1,5 +1,5 @@
 # =============================================================================
-# Copyright (c) 2018-2023, NVIDIA CORPORATION.
+# Copyright (c) 2018-2024, 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
@@ -183,6 +183,11 @@ ConfigureNVBench(
   sort/sort_lists.cpp sort/sort_structs.cpp
 )
 
+# ##################################################################################################
+# * structs benchmark
+# --------------------------------------------------------------------------------
+ConfigureNVBench(STRUCT_CREATION_NVBENCH structs/create_structs.cpp)
+
 # ##################################################################################################
 # * quantiles benchmark
 # --------------------------------------------------------------------------------
diff --git a/cpp/benchmarks/structs/create_structs.cpp b/cpp/benchmarks/structs/create_structs.cpp
new file mode 100644
index 00000000000..480a719461e
--- /dev/null
+++ b/cpp/benchmarks/structs/create_structs.cpp
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2024, 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 <benchmarks/common/generate_nested_types.hpp>
+
+#include <nvbench/nvbench.cuh>
+
+void nvbench_create_structs(nvbench::state& state)
+{
+  state.exec(nvbench::exec_tag::sync,
+             [&](nvbench::launch& launch) { auto const table_ptr = create_structs_data(state); });
+}
+
+NVBENCH_BENCH(nvbench_create_structs)
+  .set_name("create_structs")
+  .add_int64_power_of_two_axis("NumRows", {10, 18, 26})
+  .add_int64_axis("Depth", {1, 8, 16})
+  .add_int64_axis("Nulls", {0, 1});
diff --git a/cpp/src/structs/utilities.cpp b/cpp/src/structs/utilities.cpp
index acb153f28d6..f47d066852c 100644
--- a/cpp/src/structs/utilities.cpp
+++ b/cpp/src/structs/utilities.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020-2023, NVIDIA CORPORATION.
+ * Copyright (c) 2020-2024, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
 #include <cudf/column/column_factories.hpp>
 #include <cudf/detail/copy.hpp>
 #include <cudf/detail/null_mask.hpp>
+#include <cudf/detail/nvtx/ranges.hpp>
 #include <cudf/detail/structs/utilities.hpp>
 #include <cudf/detail/unary.hpp>
 #include <cudf/structs/structs_column_view.hpp>
@@ -229,6 +230,7 @@ std::unique_ptr<column> superimpose_nulls_no_sanitize(bitmask_type const* null_m
                                                       rmm::cuda_stream_view stream,
                                                       rmm::mr::device_memory_resource* mr)
 {
+  CUDF_FUNC_RANGE();
   if (input->type().id() == cudf::type_id::EMPTY) {
     // EMPTY columns should not have a null mask,
     // so don't superimpose null mask on empty columns.
@@ -258,19 +260,11 @@ std::unique_ptr<column> superimpose_nulls_no_sanitize(bitmask_type const* null_m
   // If the input is also a struct, repeat for all its children. Otherwise just return.
   if (input->type().id() != cudf::type_id::STRUCT) { return std::move(input); }
 
-  auto const current_mask   = input->view().null_mask();
   auto const new_null_count = input->null_count();  // this was just computed in the step above
   auto content              = input->release();
 
-  // Build new children columns.
-  std::for_each(content.children.begin(),
-                content.children.end(),
-                [current_mask, new_null_count, stream, mr](auto& child) {
-                  child = superimpose_nulls_no_sanitize(
-                    current_mask, new_null_count, std::move(child), stream, mr);
-                });
-
   // Replace the children columns.
+  // make_structs_column recursively calls superimpose_nulls
   return cudf::make_structs_column(num_rows,
                                    std::move(content.children),
                                    new_null_count,