Implement batch_null_count to count nulls for multiple null masks by a single kernel call, and application in groupby aggregations - #20872
Conversation
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a batch_null_count API to efficiently count null elements across multiple validity bitmasks using a single kernel launch, reducing overhead from repeated kernel calls. The implementation is immediately applied to groupby aggregations, yielding performance improvements up to nearly 50% for operations with multiple output columns.
Key Changes
- Adds
cudf::batch_null_countAPI and internalbatch_count_set_bit_kernelto process multiple bitmasks in a single kernel call - Refactors groupby aggregation output finalization to use batch processing instead of per-column null counting
- Includes test coverage for the new batch null counting functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cpp/include/cudf/null_mask.hpp | Adds public API declaration for batch_null_count with documentation |
| cpp/src/bitmask/null_mask.cu | Implements batch_count_set_bit_kernel and batch_null_count function |
| cpp/src/groupby/hash/output_utils.cu | Refactors to use batch_null_count instead of per-column null_count calls |
| cpp/tests/bitmask/bitmask_tests.cpp | Adds test case covering nullable and non-nullable columns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Seems the improvement is introduced by reducing the scalar creations. cudf::detail::device_scalar<size_type> non_zero_count(0, stream);Refer to the Your batch kernel allocates an array to hold all the result count values. It only allocates one time. But the original approach allocates multiple times for each count value(store in scalar). If you have time, could you double check this? cudf::size_type count_set_bits(bitmask_type const* bitmask,
size_type start,
size_type stop,
rmm::cuda_stream_view stream)to: cudf::size_type count_set_bits(bitmask_type const* bitmask,
size_type start,
size_type stop,
size_type* global_count, // memory on GPU to pass to kernel.
rmm::cuda_stream_view stream)Create an array of count values on GPU, then pass a count pointer to |
I ran a benchmark and it is likely that your guess is correct. When launching the kernel multiple times but creating the array of null count in device memory once, the performance is almost the same as using batch kernel. Probably here the kernel is small and doesn't have much overhead thus we don't observe its benefit. Nevertheless, I still want to have a unified (batch) kernel for doing bulk work instead. Batch kernel vs individual kernel launches (but writing to a shared pre-allocated device array): |
|
So, one option is: Need to add a variant of cudf::size_type null_count_with_pre_allocated_count_storage(bitmask_type const* bitmask,
size_type start,
size_type stop,
+ cudf::detail::device_scalar<size_type> & gpu_count_storage,
rmm::cuda_stream_view stream = cudf::get_default_stream());Compared to batch kernel, it reduces the code changes. |
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
Let's prefer the batch kernel model as it consolidate the GPU work and minimize any overhead/delay/latency. We have more of such batch kernel coming. |
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
…e input Signed-off-by: Nghia Truong <nghiat@nvidia.com>
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
Signed-off-by: Nghia Truong <nghiat@nvidia.com>
PointKernel
left a comment
There was a problem hiding this comment.
One small suggestion but non-blocking.
Thanks for the iterations addressing all my comments @ttnghia
Co-authored-by: Yunsong Wang <wangyunsong89@gmail.com>
|
/merge |
This implements
batch_null_countAPI to count nulls by a batch processing kernel, allowing to process multiple null masks at the same time using a single kernel call to reduce overhead :cudf::batch_null_countto accept multiple bitmask pointers and a single bit range.batch_count_set_bit_kernelto accumulate unset bits (non-zero) counts per bitmask.bitmask_tests.cppwith a simple test case covering nullable and non-nullable masks.Closes #19878.
Benchmark