Remove metadata type in favor of just std::vector<uint8_t>

rapidsai · Apr 24, 2023 · d8dd746 · d8dd746
1 parent 71d4645
commit d8dd746
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 32 deletions.
diff --git a/cpp/include/cudf/contiguous_split.hpp b/cpp/include/cudf/contiguous_split.hpp
@@ -40,15 +40,9 @@ namespace cudf {
  * table metadata and one on device which contains the table data.
  */
 struct packed_columns {
-  /**
-   * @brief Host-side metadata buffer used for reconstructing columns via unpack.
-   *
-   * @ingroup copy_split
-   */
-  using metadata = std::vector<uint8_t>;
-
   packed_columns()
-    : metadata_(std::make_unique<metadata>()), gpu_data(std::make_unique<rmm::device_buffer>())
+    : metadata_(std::make_unique<std::vector<uint8_t>>()), 
+      gpu_data(std::make_unique<rmm::device_buffer>())
   {
   }
 
@@ -58,12 +52,13 @@ struct packed_columns {
    * @param md Host-side metadata buffer
    * @param gd Device-side data buffer
    */
-  packed_columns(std::unique_ptr<metadata>&& md, std::unique_ptr<rmm::device_buffer>&& gd)
+  packed_columns(std::unique_ptr<std::vector<uint8_t>>&& md, 
+                 std::unique_ptr<rmm::device_buffer>&& gd)
     : metadata_(std::move(md)), gpu_data(std::move(gd))
   {
   }
 
-  std::unique_ptr<metadata> metadata_;           ///< Host-side metadata buffer
+  std::unique_ptr<std::vector<uint8_t>> metadata_;           ///< Host-side metadata buffer
   std::unique_ptr<rmm::device_buffer> gpu_data;  ///< Device-side data buffer
 };
 
@@ -160,9 +155,9 @@ packed_columns pack(cudf::table_view const& input,
  * @param buffer_size The size of `contiguous_buffer`
  * @return Vector of bytes representing the metadata used to `unpack` a packed_columns struct
  */
-packed_columns::metadata pack_metadata(table_view const& table,
-                                       uint8_t const* contiguous_buffer,
-                                       size_t buffer_size);
+std::vector<uint8_t> pack_metadata(table_view const& table,
+                                   uint8_t const* contiguous_buffer,
+                                   size_t buffer_size);
 
 /**
  * @brief Deserialize the result of `cudf::pack`.

diff --git a/cpp/src/copying/contiguous_split.cu b/cpp/src/copying/contiguous_split.cu
@@ -1001,7 +1001,7 @@ std::vector<packed_table> contiguous_split(cudf::table_view const& input,
                    [&empty_inputs](int partition_index) {
                      return packed_table{
                        empty_inputs,
-                       packed_columns{std::make_unique<packed_columns::metadata>(pack_metadata(
+                       packed_columns{std::make_unique<std::vector<uint8_t>>(pack_metadata(
                                         empty_inputs, static_cast<uint8_t const*>(nullptr), 0)),
                                       std::make_unique<rmm::device_buffer>()}};
                    });
@@ -1251,7 +1251,7 @@ std::vector<packed_table> contiguous_split(cudf::table_view const& input,
     result.push_back(packed_table{
       t,
       packed_columns{
-        std::make_unique<packed_columns::metadata>(cudf::pack_metadata(
+        std::make_unique<std::vector<uint8_t>>(cudf::pack_metadata(
           t, reinterpret_cast<uint8_t const*>(out_buffers[idx].data()), out_buffers[idx].size())),
         std::make_unique<rmm::device_buffer>(std::move(out_buffers[idx]))}});
 

diff --git a/cpp/src/copying/pack.cpp b/cpp/src/copying/pack.cpp
@@ -151,10 +151,10 @@ packed_columns pack(cudf::table_view const& input,
 }
 
 template <typename ColumnIter>
-packed_columns::metadata pack_metadata(ColumnIter begin,
-                                       ColumnIter end,
-                                       uint8_t const* contiguous_buffer,
-                                       size_t buffer_size)
+std::vector<uint8_t> pack_metadata(ColumnIter begin,
+                                   ColumnIter end,
+                                   uint8_t const* contiguous_buffer,
+                                   size_t buffer_size)
 {
   std::vector<serialized_column> metadata;
 
@@ -174,7 +174,7 @@ packed_columns::metadata pack_metadata(ColumnIter begin,
             metadata_begin + (metadata.size() * sizeof(serialized_column)),
             std::back_inserter(metadata_bytes));
 
-  return packed_columns::metadata{std::move(metadata_bytes)};
+  return metadata_bytes;
 }
 
 /**
@@ -222,13 +222,13 @@ packed_columns pack(cudf::table_view const& input, rmm::mr::device_memory_resour
 /**
  * @copydoc cudf::pack_metadata
  */
-packed_columns::metadata pack_metadata(table_view const& table,
+std::vector<uint8_t> pack_metadata(table_view const& table,
                                        uint8_t const* contiguous_buffer,
                                        size_t buffer_size)
 {
   CUDF_FUNC_RANGE();
   return table.is_empty()
-           ? packed_columns::metadata{}
+           ? std::vector<uint8_t>()
            : detail::pack_metadata(table.begin(), table.end(), contiguous_buffer, buffer_size);
 }
 

diff --git a/java/src/main/native/src/ContiguousTableJni.cpp b/java/src/main/native/src/ContiguousTableJni.cpp
@@ -144,7 +144,7 @@ JNIEXPORT jlong JNICALL Java_ai_rapids_cudf_ContiguousTable_createPackedMetadata
     auto data_addr = reinterpret_cast<uint8_t const *>(j_buffer_addr);
     auto data_size = static_cast<size_t>(j_buffer_length);
     auto metadata_ptr =
-        new cudf::packed_columns::metadata(cudf::pack_metadata(*table, data_addr, data_size));
+        new std::vector<uint8_t>(cudf::pack_metadata(*table, data_addr, data_size));
     return reinterpret_cast<jlong>(metadata_ptr);
   }
   CATCH_STD(env, 0);
@@ -154,7 +154,7 @@ JNIEXPORT jobject JNICALL Java_ai_rapids_cudf_ContiguousTable_createMetadataDire
     JNIEnv *env, jclass, jlong j_metadata_ptr) {
   JNI_NULL_CHECK(env, j_metadata_ptr, "metadata is null", nullptr);
   try {
-    auto metadata = reinterpret_cast<cudf::packed_columns::metadata *>(j_metadata_ptr);
+    auto metadata = reinterpret_cast<std::vector<uint8_t>*>(j_metadata_ptr);
     return env->NewDirectByteBuffer(const_cast<uint8_t *>(metadata->data()), metadata->size());
   }
   CATCH_STD(env, nullptr);
@@ -164,7 +164,7 @@ JNIEXPORT void JNICALL Java_ai_rapids_cudf_ContiguousTable_closeMetadata(JNIEnv
                                                                          jlong j_metadata_ptr) {
   JNI_NULL_CHECK(env, j_metadata_ptr, "metadata is null", );
   try {
-    auto metadata = reinterpret_cast<cudf::packed_columns::metadata *>(j_metadata_ptr);
+    auto metadata = reinterpret_cast<std::vector<uint8_t>*>(j_metadata_ptr);
     delete metadata;
   }
   CATCH_STD(env, );

diff --git a/python/cudf/cudf/_lib/copying.pyx b/python/cudf/cudf/_lib/copying.pyx
@@ -772,7 +772,7 @@ cdef class _CPackedColumns:
 
         cdef cpp_contiguous_split.packed_columns data
         data.metadata_ = move(
-            make_unique[cpp_contiguous_split.metadata](
+            make_unique[vector[uint8_t]](
                 move(<vector[uint8_t]>header.get("metadata", []))
             )
         )

diff --git a/python/cudf/cudf/_lib/cpp/contiguous_split.pxd b/python/cudf/cudf/_lib/cpp/contiguous_split.pxd
@@ -9,14 +9,9 @@ from rmm._lib.device_buffer cimport device_buffer
 from cudf._lib.cpp.table.table_view cimport table_view
 from cudf._lib.cpp.types cimport size_type
 
-
-cdef extern from "cudf/contiguous_split.hpp" namespace \
-        "cudf::packed_columns" nogil:
-    ctypedef vector[uint8_t] metadata 
-
 cdef extern from "cudf/contiguous_split.hpp" namespace "cudf" nogil:
     cdef cppclass packed_columns:
-        unique_ptr[metadata] metadata_
+        unique_ptr[vector[uint8_t]] metadata_
         unique_ptr[device_buffer] gpu_data
 
     cdef struct contiguous_split_result: