Introduce temp_mr. Comment edits and some more initializer list and c…

…onstness issues
rapidsai · May 17, 2023 · dae5ceb · dae5ceb
1 parent 7a73d57
commit dae5ceb
Show file tree

Hide file tree

Showing 2 changed files with 129 additions and 86 deletions.
diff --git a/cpp/include/cudf/contiguous_split.hpp b/cpp/include/cudf/contiguous_split.hpp
@@ -141,8 +141,8 @@ struct contiguous_split_state;
  * all thrust and scratch memory allocations are using the passed-in memory resource exclusively,
  * not a per-device memory resource.
  *
- * The caller has two methods it can use to carry out the chunked_pack: has_next and next.
- * Here is an example:
+ * This class defines two methods that must be used in concert to carry out the chunked_pack:
+ * has_next and next. Here is an example:
  *
  * @code{.pseudo}
  * // Create a table_view
@@ -156,7 +156,7 @@ struct contiguous_split_state;
  * // Define a buffer size for each chunk: the larger the buffer is, the more SMs can be
  * // occupied by this algorithm.
  * //
- * // Internally, the GPU unit-of-work is a 1MB batch. When we instantiate `cudf::chunked_pack`,
+ * // Internally, the GPU unit of work is a 1MB batch. When we instantiate `cudf::chunked_pack`,
  * // all the 1MB batches for the source table_view are computed up front. Additionally,
  * // chunked_pack calculates the number of iterations that are required to go through all those
  * // batches given a `user_buffer_size` buffer. The number of 1MB batches in each iteration (chunk)
@@ -196,15 +196,16 @@ class chunked_pack {
    * @param input source `table_view` to pack
    * @param user_buffer_size buffer size (in bytes) that will be passed on `next`. Must be
    *                         at least 1MB
-   * @param mr An optional memory resource to be used for temporary and scratch allocations only
+   * @param temp_mr An optional memory resource to be used for temporary and scratch allocations
+   * only
    */
   explicit chunked_pack(
     cudf::table_view const& input,
     std::size_t user_buffer_size,
-    rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
+    rmm::mr::device_memory_resource* temp_mr = rmm::mr::get_current_device_resource());
 
   /**
-   * @brief Destructor that will be implemented as default, required because
+   * @brief Destructor that will be implemented as default. Declared with definition here because
    * contiguous_split_state is incomplete at this stage.
    */
   ~chunked_pack();
@@ -224,7 +225,7 @@ class chunked_pack {
   [[nodiscard]] bool has_next() const;
 
   /**
-   * @brief Packs the next chunk into `user_buffer`. This should be call as long as
+   * @brief Packs the next chunk into `user_buffer`. This should be called as long as
    * `has_next` returns true. If `next` is called when `has_next` is false, an exception
    * is thrown.
    *
@@ -255,12 +256,13 @@ class chunked_pack {
    * @param input source `table_view` to pack
    * @param user_buffer_size buffer size (in bytes) that will be passed on `next`. Must be
    *                         at least 1MB
-   * @param mr RMM memory resource to be used for temporary and scratch allocations only
+   * @param temp_mr RMM memory resource to be used for temporary and scratch allocations only
    * @return a unique_ptr of chunked_pack
    */
-  [[nodiscard]] static std::unique_ptr<chunked_pack> create(cudf::table_view const& input,
-                                                            std::size_t user_buffer_size,
-                                                            rmm::mr::device_memory_resource* mr);
+  [[nodiscard]] static std::unique_ptr<chunked_pack> create(
+    cudf::table_view const& input,
+    std::size_t user_buffer_size,
+    rmm::mr::device_memory_resource* temp_mr);
 
  private:
   // internal state of contiguous split