From e05e4c503cc099a69056584ccd89fa8941067af1 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 11 May 2023 17:45:51 -0700 Subject: [PATCH 1/6] Expose null_count API publicly --- cpp/include/cudf/detail/null_mask.hpp | 15 ++------------- cpp/include/cudf/null_mask.hpp | 16 ++++++++++++++++ cpp/src/bitmask/null_mask.cu | 8 +++++++- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/cpp/include/cudf/detail/null_mask.hpp b/cpp/include/cudf/detail/null_mask.hpp index 7f1b15893c5..8c10bbe416f 100644 --- a/cpp/include/cudf/detail/null_mask.hpp +++ b/cpp/include/cudf/detail/null_mask.hpp @@ -141,20 +141,9 @@ cudf::size_type valid_count(bitmask_type const* bitmask, rmm::cuda_stream_view stream); /** - * @brief Given a validity bitmask, counts the number of null elements (unset bits) - * in the range `[start, stop)`. - * - * If `bitmask == nullptr`, all elements are assumed to be valid and the - * function returns ``. - * - * @throws cudf::logic_error if `start > stop` - * @throws cudf::logic_error if `start < 0` + * @copydoc null_count(bitmask_type const* bitmask, size_type start, size_type stop) * - * @param[in] bitmask Validity bitmask residing in device memory. - * @param[in] start Index of the first bit to count (inclusive). - * @param[in] stop Index of the last bit to count (exclusive). - * @param[in] stream CUDA stream used for device memory operations and kernel launches. - * @return The number of null elements in the specified range. + * @param stream Stream view on which to allocate resources and queue execution. */ cudf::size_type null_count(bitmask_type const* bitmask, size_type start, diff --git a/cpp/include/cudf/null_mask.hpp b/cpp/include/cudf/null_mask.hpp index 360006c1eea..fc6af7f7bb4 100644 --- a/cpp/include/cudf/null_mask.hpp +++ b/cpp/include/cudf/null_mask.hpp @@ -168,5 +168,21 @@ std::pair bitmask_or( table_view const& view, rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource()); +/** + * @brief Given a validity bitmask, counts the number of null elements (unset bits) + * in the range `[start, stop)`. + * + * If `bitmask == nullptr`, all elements are assumed to be valid and the + * function returns ``. + * + * @throws cudf::logic_error if `start > stop` + * @throws cudf::logic_error if `start < 0` + * + * @param[in] bitmask Validity bitmask residing in device memory. + * @param[in] start Index of the first bit to count (inclusive). + * @param[in] stop Index of the last bit to count (exclusive). + * @return The number of null elements in the specified range. + */ +cudf::size_type null_count(bitmask_type const* bitmask, size_type start, size_type stop); /** @} */ // end of group } // namespace cudf diff --git a/cpp/src/bitmask/null_mask.cu b/cpp/src/bitmask/null_mask.cu index b98a2196748..a7a2423dc22 100644 --- a/cpp/src/bitmask/null_mask.cu +++ b/cpp/src/bitmask/null_mask.cu @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2022, NVIDIA CORPORATION. + * Copyright (c) 2019-2023, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -531,4 +531,10 @@ std::pair bitmask_or(table_view const& view, return detail::bitmask_or(view, cudf::get_default_stream(), mr); } +// Count non-zero bits in the specified range +cudf::size_type null_count(bitmask_type const* bitmask, size_type start, size_type stop) +{ + return detail::null_count(bitmask, start, stop, cudf::get_default_stream()); +} + } // namespace cudf From 49e753dabc3b9d9d15fe80b03dce3b6b18db96a9 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 11 May 2023 17:48:20 -0700 Subject: [PATCH 2/6] Implement minimal Python solution for computing the null count without using the lazy computation --- python/cudf/cudf/_lib/column.pyx | 38 +++++++++++++++++++++++-- python/cudf/cudf/_lib/cpp/null_mask.pxd | 16 +++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/python/cudf/cudf/_lib/column.pyx b/python/cudf/cudf/_lib/column.pyx index 428db210532..355c0bd19ed 100644 --- a/python/cudf/cudf/_lib/column.pyx +++ b/python/cudf/cudf/_lib/column.pyx @@ -17,13 +17,14 @@ from cudf.core.buffer import ( as_buffer, ) from cudf.utils.dtypes import _get_base_dtype + from cpython.buffer cimport PyObject_CheckBuffer from libc.stdint cimport uintptr_t from libcpp.memory cimport make_unique, unique_ptr from libcpp.utility cimport move from libcpp.vector cimport vector -from rmm._lib.device_buffer cimport DeviceBuffer +from rmm._lib.device_buffer cimport DeviceBuffer, device_buffer from cudf._lib.types cimport dtype_from_column_view, dtype_to_data_type @@ -37,6 +38,10 @@ from cudf._lib.cpp.column.column_factories cimport ( make_numeric_column, ) from cudf._lib.cpp.column.column_view cimport column_view +from cudf._lib.cpp.null_mask cimport ( + copy_bitmask as c_copy_bitmask, + null_count as c_null_count, +) from cudf._lib.cpp.scalar.scalar cimport scalar from cudf._lib.scalar cimport DeviceScalar @@ -307,8 +312,37 @@ cdef class Column: return other_col cdef libcudf_types.size_type compute_null_count(self) except? 0: + cdef device_buffer db + cdef unique_ptr[device_buffer] up_db + cdef DeviceBuffer rmm_db with acquire_spill_lock(): - return self._view(libcudf_types.UNKNOWN_NULL_COUNT).null_count() + if self.nullable: + if self.offset == 0: + mask = self.base_mask + else: + # Can't use the normal copy_bitmask function because that + # requires creating a view, which leads to infinite + # recursion + db = move(c_copy_bitmask( + ( + self.base_mask.get_ptr(mode="read") + ), + self.offset, + self.offset + self.size, + )) + up_db = make_unique[device_buffer](move(db)) + rmm_db = DeviceBuffer.c_from_unique_ptr(move(up_db)) + mask = as_buffer(rmm_db) + + return c_null_count( + ( + mask.get_ptr(mode="read") + ), + 0, + self.size + ) + else: + return 0 cdef mutable_column_view mutable_view(self) except *: if is_categorical_dtype(self.dtype): diff --git a/python/cudf/cudf/_lib/cpp/null_mask.pxd b/python/cudf/cudf/_lib/cpp/null_mask.pxd index 3050a9f3459..6f0e82973a7 100644 --- a/python/cudf/cudf/_lib/cpp/null_mask.pxd +++ b/python/cudf/cudf/_lib/cpp/null_mask.pxd @@ -1,4 +1,4 @@ -# Copyright (c) 2020-2022, NVIDIA CORPORATION. +# Copyright (c) 2020-2023, NVIDIA CORPORATION. from libc.stdint cimport int32_t from libcpp.pair cimport pair @@ -7,7 +7,7 @@ from rmm._lib.device_buffer cimport device_buffer from cudf._lib.cpp.column.column_view cimport column_view from cudf._lib.cpp.table.table_view cimport table_view -from cudf._lib.cpp.types cimport mask_state, size_type +from cudf._lib.cpp.types cimport bitmask_type, mask_state, size_type ctypedef int32_t underlying_type_t_mask_state @@ -17,6 +17,12 @@ cdef extern from "cudf/null_mask.hpp" namespace "cudf" nogil: column_view view ) except + + cdef device_buffer copy_bitmask "cudf::copy_bitmask" ( + const bitmask_type * mask, + size_type begin_bit, + size_type end_bit + ) except + + cdef size_t bitmask_allocation_size_bytes ( size_type number_of_bits, size_t padding_boundary @@ -38,3 +44,9 @@ cdef extern from "cudf/null_mask.hpp" namespace "cudf" nogil: cdef pair[device_buffer, size_type] bitmask_or( table_view view ) + + cdef size_type null_count( + const bitmask_type * bitmask, + size_type start, + size_type stop, + ) From 7ce77b54fb08cbc1cb248cb142422801e1d43810 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 11 May 2023 18:14:38 -0700 Subject: [PATCH 3/6] Assume that null count is 0 if None --- python/cudf/cudf/_lib/column.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/cudf/cudf/_lib/column.pyx b/python/cudf/cudf/_lib/column.pyx index 355c0bd19ed..ae6cd52d9dd 100644 --- a/python/cudf/cudf/_lib/column.pyx +++ b/python/cudf/cudf/_lib/column.pyx @@ -383,7 +383,7 @@ cdef class Column: null_count = self._null_count if null_count is None: - null_count = libcudf_types.UNKNOWN_NULL_COUNT + null_count = 0 cdef libcudf_types.size_type c_null_count = null_count self._mask = None @@ -403,7 +403,7 @@ cdef class Column: cdef column_view view(self) except *: null_count = self.null_count if null_count is None: - null_count = libcudf_types.UNKNOWN_NULL_COUNT + null_count = 0 cdef libcudf_types.size_type c_null_count = null_count return self._view(c_null_count) From e61bc12c300d8f60ab2dcd28f3601a891c0588bc Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 11 May 2023 18:15:04 -0700 Subject: [PATCH 4/6] Remove now unnecessary enum --- python/cudf/cudf/_lib/cpp/types.pxd | 3 --- 1 file changed, 3 deletions(-) diff --git a/python/cudf/cudf/_lib/cpp/types.pxd b/python/cudf/cudf/_lib/cpp/types.pxd index e4106ffb99d..11480d774ef 100644 --- a/python/cudf/cudf/_lib/cpp/types.pxd +++ b/python/cudf/cudf/_lib/cpp/types.pxd @@ -8,9 +8,6 @@ cdef extern from "cudf/types.hpp" namespace "cudf" nogil: ctypedef uint32_t bitmask_type ctypedef uint32_t char_utf8 - cdef enum: - UNKNOWN_NULL_COUNT = -1 - ctypedef enum mask_state: UNALLOCATED "cudf::mask_state::UNALLOCATED" UNINITIALIZED "cudf::mask_state::UNINITIALIZED" From d83ff84d70dce210539b69a14eacc205270e36fa Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 12 May 2023 16:44:39 -0700 Subject: [PATCH 5/6] Update cpp/include/cudf/null_mask.hpp Co-authored-by: Nghia Truong <7416935+ttnghia@users.noreply.github.com> --- cpp/include/cudf/null_mask.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/include/cudf/null_mask.hpp b/cpp/include/cudf/null_mask.hpp index fc6af7f7bb4..2d3bef66a44 100644 --- a/cpp/include/cudf/null_mask.hpp +++ b/cpp/include/cudf/null_mask.hpp @@ -178,9 +178,9 @@ std::pair bitmask_or( * @throws cudf::logic_error if `start > stop` * @throws cudf::logic_error if `start < 0` * - * @param[in] bitmask Validity bitmask residing in device memory. - * @param[in] start Index of the first bit to count (inclusive). - * @param[in] stop Index of the last bit to count (exclusive). + * @param bitmask Validity bitmask residing in device memory. + * @param start Index of the first bit to count (inclusive). + * @param stop Index of the last bit to count (exclusive). * @return The number of null elements in the specified range. */ cudf::size_type null_count(bitmask_type const* bitmask, size_type start, size_type stop); From b321e44a9f0d5c8ccd1e3b1457f8d33f55b0f359 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 16 May 2023 11:09:25 -0700 Subject: [PATCH 6/6] Remove now unnecessary copy --- python/cudf/cudf/_lib/column.pyx | 44 ++++++------------------- python/cudf/cudf/_lib/cpp/null_mask.pxd | 6 ---- 2 files changed, 10 insertions(+), 40 deletions(-) diff --git a/python/cudf/cudf/_lib/column.pyx b/python/cudf/cudf/_lib/column.pyx index ae6cd52d9dd..3a36ca65866 100644 --- a/python/cudf/cudf/_lib/column.pyx +++ b/python/cudf/cudf/_lib/column.pyx @@ -24,7 +24,7 @@ from libcpp.memory cimport make_unique, unique_ptr from libcpp.utility cimport move from libcpp.vector cimport vector -from rmm._lib.device_buffer cimport DeviceBuffer, device_buffer +from rmm._lib.device_buffer cimport DeviceBuffer from cudf._lib.types cimport dtype_from_column_view, dtype_to_data_type @@ -38,10 +38,7 @@ from cudf._lib.cpp.column.column_factories cimport ( make_numeric_column, ) from cudf._lib.cpp.column.column_view cimport column_view -from cudf._lib.cpp.null_mask cimport ( - copy_bitmask as c_copy_bitmask, - null_count as c_null_count, -) +from cudf._lib.cpp.null_mask cimport null_count as c_null_count from cudf._lib.cpp.scalar.scalar cimport scalar from cudf._lib.scalar cimport DeviceScalar @@ -312,37 +309,16 @@ cdef class Column: return other_col cdef libcudf_types.size_type compute_null_count(self) except? 0: - cdef device_buffer db - cdef unique_ptr[device_buffer] up_db - cdef DeviceBuffer rmm_db with acquire_spill_lock(): - if self.nullable: - if self.offset == 0: - mask = self.base_mask - else: - # Can't use the normal copy_bitmask function because that - # requires creating a view, which leads to infinite - # recursion - db = move(c_copy_bitmask( - ( - self.base_mask.get_ptr(mode="read") - ), - self.offset, - self.offset + self.size, - )) - up_db = make_unique[device_buffer](move(db)) - rmm_db = DeviceBuffer.c_from_unique_ptr(move(up_db)) - mask = as_buffer(rmm_db) - - return c_null_count( - ( - mask.get_ptr(mode="read") - ), - 0, - self.size - ) - else: + if not self.nullable: return 0 + return c_null_count( + ( + self.base_mask.get_ptr(mode="read") + ), + self.offset, + self.offset + self.size + ) cdef mutable_column_view mutable_view(self) except *: if is_categorical_dtype(self.dtype): diff --git a/python/cudf/cudf/_lib/cpp/null_mask.pxd b/python/cudf/cudf/_lib/cpp/null_mask.pxd index 6f0e82973a7..bd0eb684690 100644 --- a/python/cudf/cudf/_lib/cpp/null_mask.pxd +++ b/python/cudf/cudf/_lib/cpp/null_mask.pxd @@ -17,12 +17,6 @@ cdef extern from "cudf/null_mask.hpp" namespace "cudf" nogil: column_view view ) except + - cdef device_buffer copy_bitmask "cudf::copy_bitmask" ( - const bitmask_type * mask, - size_type begin_bit, - size_type end_bit - ) except + - cdef size_t bitmask_allocation_size_bytes ( size_type number_of_bits, size_t padding_boundary