Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert cudf::rank to use device_uvector #8029

Merged
merged 7 commits into from
Apr 26, 2021

Conversation

harrism
Copy link
Member

@harrism harrism commented Apr 22, 2021

Converts cudf::rank to use device_uvector instead of device_vector.
Also adds a benchmark for cudf::rank.

Contributes to #7287

Performance is at most 1.7% slower and up to 12% faster.

Comparing /home/mharris/rapids/cudf/cpp/build/rank_before.json to /home/mharris/rapids/cudf/cpp/build/rank_after.json
Benchmark                                            Time             CPU      Time Old      Time New       CPU Old       CPU New
---------------------------------------------------------------------------------------------------------------------------------
Rank/no_nulls/1024/manual_time                    +0.0170         +0.0140             0             0             0             0
Rank/no_nulls/4096/manual_time                    +0.0092         +0.0076             0             0             0             0
Rank/no_nulls/32768/manual_time                   +0.0034         +0.0042             0             0             0             0
Rank/no_nulls/262144/manual_time                  -0.0542         -0.0574             0             0             0             0
Rank/no_nulls/2097152/manual_time                 -0.0493         -0.0509             1             1             1             1
Rank/no_nulls/16777216/manual_time                -0.1217         -0.1289             7             6             7             6
Rank/no_nulls/67108864/manual_time                +0.0007         +0.0005            24            24            24            24
Rank/nulls/1024/manual_time                       -0.0392         -0.0370             0             0             0             0
Rank/nulls/4096/manual_time                       -0.0173         -0.0160             0             0             0             0
Rank/nulls/32768/manual_time                      +0.0021         +0.0027             0             0             0             0
Rank/nulls/262144/manual_time                     -0.0027         -0.0025             0             0             0             0
Rank/nulls/2097152/manual_time                    +0.0002         -0.0003             7             7             7             7
Rank/nulls/16777216/manual_time                   -0.0409         -0.0334            59            56            58            56
Rank/nulls/67108864/manual_time                   -0.0003         -0.0004           232           232           232           232

@harrism harrism requested review from a team as code owners April 22, 2021 06:39
@github-actions github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Apr 22, 2021
@harrism harrism added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 22, 2021
@harrism harrism added this to PR-WIP in v21.06 Release via automation Apr 22, 2021
@harrism harrism self-assigned this Apr 22, 2021
@codecov
Copy link

codecov bot commented Apr 22, 2021

Codecov Report

Merging #8029 (4892858) into branch-0.20 (51336df) will decrease coverage by 0.05%.
The diff coverage is 89.88%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.20    #8029      +/-   ##
===============================================
- Coverage        82.88%   82.83%   -0.06%     
===============================================
  Files              103      103              
  Lines            17668    17796     +128     
===============================================
+ Hits             14645    14742      +97     
- Misses            3023     3054      +31     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/__init__.py 100.00% <ø> (ø)
python/cudf/cudf/io/orc.py 86.89% <ø> (ø)
python/cudf/cudf/utils/dtypes.py 81.87% <ø> (-1.57%) ⬇️
python/cudf/cudf/utils/cudautils.py 57.75% <25.00%> (ø)
python/cudf/cudf/core/column/numerical.py 94.43% <72.72%> (ø)
python/cudf/cudf/core/column/column.py 88.64% <75.00%> (ø)
python/cudf/cudf/core/column/decimal.py 90.08% <78.26%> (-2.85%) ⬇️
python/dask_cudf/dask_cudf/backends.py 89.51% <80.00%> (-0.08%) ⬇️
python/cudf/cudf/core/dataframe.py 90.87% <83.33%> (+0.01%) ⬆️
python/cudf/cudf/utils/utils.py 90.00% <95.00%> (+0.49%) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e385180...4892858. Read the comment docs.

v21.06 Release automation moved this from PR-WIP to PR-Needs review Apr 22, 2021
cpp/src/sort/rank.cu Outdated Show resolved Hide resolved
cpp/src/sort/rank.cu Outdated Show resolved Hide resolved
cpp/benchmarks/sort/rank_benchmark.cpp Outdated Show resolved Hide resolved
cpp/src/sort/rank.cu Outdated Show resolved Hide resolved
cpp/src/sort/rank.cu Outdated Show resolved Hide resolved
@nvdbaranec nvdbaranec self-requested a review April 23, 2021 14:45
v21.06 Release automation moved this from PR-Needs review to PR-Reviewer approved Apr 26, 2021
@harrism
Copy link
Member Author

harrism commented Apr 26, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 1894aeb into rapidsai:branch-0.20 Apr 26, 2021
v21.06 Release automation moved this from PR-Reviewer approved to Done Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants