Skip to content

Commit c285951

Browse files
committed
Address partitioning review suggestions
Remove redundant histogram-only and reordered-output block barriers while retaining synchronization before shared storage is reused. Reuse the warp histogram helper for local ranks, declare identity-copy dependencies, and stage trivial partition boundaries through pinned host memory. Dense-null and wide-column exact-result coverage was added in eac9bee. Validation: 2140 partition tests, 7 stream tests, stream interposition, 15 memcheck cases, and 5 synccheck/racecheck cases passed. Fresh 348-case before/after matrices and three paired repeat rounds found no consistent slowdown above 5%; tracked GPU allocation peaks were unchanged.
1 parent 8c93537 commit c285951

3 files changed

Lines changed: 15 additions & 29 deletions

File tree

‎cpp/src/partitioning/partition_identity.cuh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,16 @@
99
#include <cudf/detail/utilities/buffer_factories.hpp>
1010
#include <cudf/detail/utilities/vector_factories.hpp>
1111
#include <cudf/null_mask.hpp>
12+
#include <cudf/strings/detail/gather.cuh>
1213
#include <cudf/strings/detail/utilities.hpp>
1314
#include <cudf/strings/utilities.hpp>
1415
#include <cudf/table/table.hpp>
1516
#include <cudf/utilities/traits.hpp>
1617

1718
#include <rmm/device_buffer.hpp>
19+
#include <rmm/exec_policy.hpp>
1820

21+
#include <cuda/iterator>
1922
#include <thrust/transform.h>
2023

2124
namespace cudf::detail::partition_identity {

‎cpp/src/partitioning/partition_map_plan.cuh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ CUDF_KERNEL void count_tiles(MapType const* map, int n, int p, uint16_t* saved,
7979
}
8080
total += sum;
8181
}
82-
__syncthreads();
8382
if constexpr (SaveRanks) {
83+
__syncthreads();
8484
#pragma unroll
8585
for (int j = 0; j < 4; ++j) {
8686
int i = start + (threadIdx.x / 32) * 128 + j * 32 + threadIdx.x % 32;
@@ -155,7 +155,7 @@ __device__ void emit_reordered(int const (&keys)[4],
155155
int position = threadIdx.x + j * threads;
156156
if (position < min(tile_rows, n - start)) sink(source_rows[position], destinations[position]);
157157
}
158-
__syncthreads();
158+
// The caller synchronizes before reusing the shared storage.
159159
}
160160

161161
template <int Tiles, bool Reorder = false, typename Sink, typename MapType, typename Offsets>

‎cpp/src/partitioning/partitioning.cu‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -841,34 +841,16 @@ CUDF_KERNEL void partition_map_local_ranks(MapType const* partition_map,
841841
__shared__ size_type histogram[num_warps][MAP_PARTITION_MAX_BINS];
842842
auto const lane = threadIdx.x % warp_size;
843843
auto const warp = threadIdx.x / warp_size;
844-
for (auto p = lane; p < num_partitions; p += warp_size) {
845-
histogram[warp][p] = 0;
846-
}
847-
__syncwarp();
848-
849844
size_type ranks[MAP_PARTITION_ITEMS_PER_THREAD];
850845
size_type partitions[MAP_PARTITION_ITEMS_PER_THREAD];
851-
#pragma unroll
852-
for (size_type item = 0; item < MAP_PARTITION_ITEMS_PER_THREAD; ++item) {
853-
auto const row = static_cast<cuda::std::size_t>(blockIdx.x) * MAP_PARTITION_TILE_SIZE +
854-
warp * warp_size * MAP_PARTITION_ITEMS_PER_THREAD + item * warp_size + lane;
855-
auto const valid = row < num_rows;
856-
auto const active = __ballot_sync(0xffffffffu, valid);
857-
if (valid) {
858-
auto const partition = static_cast<size_type>(partition_map[row]);
859-
partitions[item] = partition;
860-
auto const peers = __match_any_sync(active, partition);
861-
auto const base = histogram[warp][partition];
862-
ranks[item] = base + cuda::std::popcount(peers & ((1u << lane) - 1u));
863-
// All readers of a bin finish before its elected writer updates it.
864-
__syncwarp(active);
865-
if (lane == __ffs(peers) - 1) {
866-
histogram[warp][partition] = base + cuda::std::popcount(peers);
867-
}
868-
}
869-
__syncwarp();
870-
}
871-
__syncthreads();
846+
partition_map_warp_histogram<true>(
847+
partition_map,
848+
num_rows,
849+
num_partitions,
850+
static_cast<cuda::std::size_t>(blockIdx.x) * MAP_PARTITION_TILE_SIZE,
851+
histogram,
852+
ranks,
853+
partitions);
872854

873855
for (auto partition = threadIdx.x; partition < num_partitions;
874856
partition += MAP_PARTITION_BLOCK_SIZE) {
@@ -1198,7 +1180,8 @@ std::vector<size_type> make_map_partition_offsets(column_view const& partition_m
11981180
offsets.begin(),
11991181
offsets.end(),
12001182
offsets.begin());
1201-
return cudf::detail::make_std_vector(offsets, stream);
1183+
auto const pinned_offsets = cudf::detail::make_pinned_vector(offsets, stream);
1184+
return {pinned_offsets.begin(), pinned_offsets.end()};
12021185
}
12031186

12041187
/**

0 commit comments

Comments
 (0)