From 9d7cf56d613436f21228627a6a0966790074f3f1 Mon Sep 17 00:00:00 2001 From: Alessandro Bellina Date: Sat, 11 Dec 2021 19:15:22 +0000 Subject: [PATCH 1/7] Fix fallback to sort aggregation for grouping only hash aggregate --- cpp/benchmarks/CMakeLists.txt | 1 + .../groupby/group_no_requests_benchmark.cu | 109 ++++++++++++++++++ cpp/src/groupby/hash/groupby.cu | 6 +- 3 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 cpp/benchmarks/groupby/group_no_requests_benchmark.cu diff --git a/cpp/benchmarks/CMakeLists.txt b/cpp/benchmarks/CMakeLists.txt index 71b452a1004..3be4104a77e 100644 --- a/cpp/benchmarks/CMakeLists.txt +++ b/cpp/benchmarks/CMakeLists.txt @@ -173,6 +173,7 @@ ConfigureBench(FILL_BENCH filling/repeat_benchmark.cpp) ConfigureBench( GROUPBY_BENCH groupby/group_sum_benchmark.cu groupby/group_nth_benchmark.cu groupby/group_shift_benchmark.cu groupby/group_struct_benchmark.cu + groupby/group_no_requests_benchmark.cu ) # ################################################################################################## diff --git a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu new file mode 100644 index 00000000000..835026d86db --- /dev/null +++ b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2021, NVIDIA CORPORATION. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +class Groupby : public cudf::benchmark { +}; + +// TODO: put it in a struct so `uniform` can be remade with different min, max +template +T random_int(T min, T max) +{ + static unsigned seed = 13377331; + static std::mt19937 engine{seed}; + static std::uniform_int_distribution uniform{min, max}; + + return uniform(engine); +} + +void BM_basic_no_requests(benchmark::State& state) +{ + using wrapper = cudf::test::fixed_width_column_wrapper; + + const cudf::size_type column_size{(cudf::size_type)state.range(0)}; + + auto data_it = cudf::detail::make_counting_transform_iterator( + 0, [=](cudf::size_type row) { return random_int(0, 100); }); + + wrapper keys(data_it, data_it + column_size); + wrapper vals(data_it, data_it + column_size); + + std::vector requests; + + for (auto _ : state) { + cuda_event_timer timer(state, true); + cudf::groupby::groupby gb_obj(cudf::table_view({keys})); + auto result = gb_obj.aggregate(requests); + } +} + +BENCHMARK_DEFINE_F(Groupby, BasicNoRequest)(::benchmark::State& state) { BM_basic_no_requests(state); } + +BENCHMARK_REGISTER_F(Groupby, BasicNoRequest) + ->UseManualTime() + ->Unit(benchmark::kMillisecond) + ->Arg(10000) + ->Arg(1000000) + ->Arg(10000000) + ->Arg(100000000); + +void BM_pre_sorted_no_requests(benchmark::State& state) +{ + using wrapper = cudf::test::fixed_width_column_wrapper; + + const cudf::size_type column_size{(cudf::size_type)state.range(0)}; + + auto data_it = cudf::detail::make_counting_transform_iterator( + 0, [=](cudf::size_type row) { return random_int(0, 100); }); + auto valid_it = cudf::detail::make_counting_transform_iterator( + 0, [=](cudf::size_type row) { return random_int(0, 100) < 90; }); + + wrapper keys(data_it, data_it + column_size); + wrapper vals(data_it, data_it + column_size, valid_it); + + auto keys_table = cudf::table_view({keys}); + auto sort_order = cudf::sorted_order(keys_table); + auto sorted_keys = cudf::gather(keys_table, *sort_order); + // No need to sort values using sort_order because they were generated randomly + + std::vector requests; + + for (auto _ : state) { + cuda_event_timer timer(state, true); + cudf::groupby::groupby gb_obj(*sorted_keys, cudf::null_policy::EXCLUDE, cudf::sorted::YES); + auto result = gb_obj.aggregate(requests); + } +} + +BENCHMARK_DEFINE_F(Groupby, PreSortedNoRequests)(::benchmark::State& state) { BM_pre_sorted_no_requests(state); } + +BENCHMARK_REGISTER_F(Groupby, PreSortedNoRequests) + ->UseManualTime() + ->Unit(benchmark::kMillisecond) + ->Arg(1000000) + ->Arg(10000000) + ->Arg(100000000); diff --git a/cpp/src/groupby/hash/groupby.cu b/cpp/src/groupby/hash/groupby.cu index 1474cfd8a19..585dea2dfbf 100644 --- a/cpp/src/groupby/hash/groupby.cu +++ b/cpp/src/groupby/hash/groupby.cu @@ -647,12 +647,12 @@ bool can_use_hash_groupby(table_view const& keys, host_span Date: Sun, 12 Dec 2021 14:05:59 +0000 Subject: [PATCH 2/7] Codestyle fixes --- cpp/benchmarks/groupby/group_no_requests_benchmark.cu | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu index 835026d86db..112bd1d31db 100644 --- a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu +++ b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu @@ -61,7 +61,10 @@ void BM_basic_no_requests(benchmark::State& state) } } -BENCHMARK_DEFINE_F(Groupby, BasicNoRequest)(::benchmark::State& state) { BM_basic_no_requests(state); } +BENCHMARK_DEFINE_F(Groupby, BasicNoRequest)(::benchmark::State& state) +{ + BM_basic_no_requests(state); +} BENCHMARK_REGISTER_F(Groupby, BasicNoRequest) ->UseManualTime() @@ -99,7 +102,10 @@ void BM_pre_sorted_no_requests(benchmark::State& state) } } -BENCHMARK_DEFINE_F(Groupby, PreSortedNoRequests)(::benchmark::State& state) { BM_pre_sorted_no_requests(state); } +BENCHMARK_DEFINE_F(Groupby, PreSortedNoRequests)(::benchmark::State& state) +{ + BM_pre_sorted_no_requests(state); +} BENCHMARK_REGISTER_F(Groupby, PreSortedNoRequests) ->UseManualTime() From 0334f81dc42a2b926bd0678492aec2f179a23f97 Mon Sep 17 00:00:00 2001 From: Alessandro Bellina Date: Sun, 12 Dec 2021 14:12:04 +0000 Subject: [PATCH 3/7] Codestyle missed a space --- cpp/benchmarks/groupby/group_no_requests_benchmark.cu | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu index 112bd1d31db..88c61301e94 100644 --- a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu +++ b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu @@ -63,7 +63,7 @@ void BM_basic_no_requests(benchmark::State& state) BENCHMARK_DEFINE_F(Groupby, BasicNoRequest)(::benchmark::State& state) { - BM_basic_no_requests(state); + BM_basic_no_requests(state); } BENCHMARK_REGISTER_F(Groupby, BasicNoRequest) @@ -104,7 +104,7 @@ void BM_pre_sorted_no_requests(benchmark::State& state) BENCHMARK_DEFINE_F(Groupby, PreSortedNoRequests)(::benchmark::State& state) { - BM_pre_sorted_no_requests(state); + BM_pre_sorted_no_requests(state); } BENCHMARK_REGISTER_F(Groupby, PreSortedNoRequests) From 10afade09bbee2b45c312a40c06fdb96054c3fd8 Mon Sep 17 00:00:00 2001 From: Alessandro Bellina Date: Sun, 12 Dec 2021 14:18:53 +0000 Subject: [PATCH 4/7] Codestyle missed another space --- cpp/benchmarks/groupby/group_no_requests_benchmark.cu | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu index 88c61301e94..7dbe1888cee 100644 --- a/cpp/benchmarks/groupby/group_no_requests_benchmark.cu +++ b/cpp/benchmarks/groupby/group_no_requests_benchmark.cu @@ -62,7 +62,7 @@ void BM_basic_no_requests(benchmark::State& state) } BENCHMARK_DEFINE_F(Groupby, BasicNoRequest)(::benchmark::State& state) -{ +{ BM_basic_no_requests(state); } @@ -103,7 +103,7 @@ void BM_pre_sorted_no_requests(benchmark::State& state) } BENCHMARK_DEFINE_F(Groupby, PreSortedNoRequests)(::benchmark::State& state) -{ +{ BM_pre_sorted_no_requests(state); } From 43ef6a0a63a5649ba8acd6c33c53205eea31e015 Mon Sep 17 00:00:00 2001 From: Alessandro Bellina Date: Sun, 12 Dec 2021 15:39:55 +0000 Subject: [PATCH 5/7] Apply cmake-format diff --- cpp/benchmarks/CMakeLists.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cpp/benchmarks/CMakeLists.txt b/cpp/benchmarks/CMakeLists.txt index 3be4104a77e..63f6857ee08 100644 --- a/cpp/benchmarks/CMakeLists.txt +++ b/cpp/benchmarks/CMakeLists.txt @@ -171,8 +171,11 @@ ConfigureBench(FILL_BENCH filling/repeat_benchmark.cpp) # ################################################################################################## # * groupby benchmark ----------------------------------------------------------------------------- ConfigureBench( - GROUPBY_BENCH groupby/group_sum_benchmark.cu groupby/group_nth_benchmark.cu - groupby/group_shift_benchmark.cu groupby/group_struct_benchmark.cu + GROUPBY_BENCH + groupby/group_sum_benchmark.cu + groupby/group_nth_benchmark.cu + groupby/group_shift_benchmark.cu + groupby/group_struct_benchmark.cu groupby/group_no_requests_benchmark.cu ) From c1113c39c7d1b6d64023e86ed8ba324ae73e3b9c Mon Sep 17 00:00:00 2001 From: Alessandro Bellina Date: Mon, 13 Dec 2021 05:23:15 +0000 Subject: [PATCH 6/7] Use std::any_of instead --- cpp/src/groupby/hash/groupby.cu | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/src/groupby/hash/groupby.cu b/cpp/src/groupby/hash/groupby.cu index 585dea2dfbf..cb4fc37487d 100644 --- a/cpp/src/groupby/hash/groupby.cu +++ b/cpp/src/groupby/hash/groupby.cu @@ -647,12 +647,12 @@ bool can_use_hash_groupby(table_view const& keys, host_span Date: Mon, 13 Dec 2021 18:43:05 +0000 Subject: [PATCH 7/7] Address code review comment --- cpp/src/groupby/hash/groupby.cu | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/cpp/src/groupby/hash/groupby.cu b/cpp/src/groupby/hash/groupby.cu index cb4fc37487d..4f2cb4de14b 100644 --- a/cpp/src/groupby/hash/groupby.cu +++ b/cpp/src/groupby/hash/groupby.cu @@ -636,23 +636,16 @@ std::unique_ptr groupby(table_view const& keys, */ bool can_use_hash_groupby(table_view const& keys, host_span requests) { - auto const all_hash_aggregations = - std::all_of(requests.begin(), requests.end(), [](aggregation_request const& r) { - return cudf::has_atomic_support(r.values.type()) and - std::all_of(r.aggregations.begin(), r.aggregations.end(), [](auto const& a) { - return is_hash_aggregation(a->kind); - }); - }); - - // Currently, structs are not supported in any of hash-based aggregations. - // Therefore, if any request contains structs then we must fallback to sort-based aggregations. - // TODO: Support structs in hash-based aggregations. - auto const has_structs = - std::any_of(requests.begin(), requests.end(), [](aggregation_request const& r) { - return r.values.type().id() == type_id::STRUCT; - }); - - return all_hash_aggregations && !has_structs; + return std::all_of(requests.begin(), requests.end(), [](aggregation_request const& r) { + // Currently, structs are not supported in any of hash-based aggregations. + // Therefore, if any request contains structs then we must fallback to sort-based aggregations. + // TODO: Support structs in hash-based aggregations. + return not(r.values.type().id() == type_id::STRUCT) and + cudf::has_atomic_support(r.values.type()) and + std::all_of(r.aggregations.begin(), r.aggregations.end(), [](auto const& a) { + return is_hash_aggregation(a->kind); + }); + }); } // Hash-based groupby