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

Use rmm::device_uvector in place of rmm::device_vector for ORC reader/writer and cudf::io::column_buffer #7614

Merged
merged 3 commits into from
Mar 20, 2021

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Mar 16, 2021

Issue #7287

Replaces device_vector with device_uvector and device_span. Because device_uvector does not have a default constructor, some additional changes were required for device_uvector data members.

Performance impact: this change makes a measurable difference in reader benchmarks. Most string column cases are sped up around 5%, with other cases having a measurable, but less consistent improvement.

@vuule vuule added libcudf Affects libcudf (C++/CUDA) code. cuIO cuIO issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 16, 2021
@vuule vuule self-assigned this Mar 16, 2021
@vuule vuule marked this pull request as ready for review March 16, 2021 19:48
@vuule vuule requested a review from a team as a code owner March 16, 2021 19:48
@vuule vuule requested review from trxcllnt, jrhemstad, rgsl888prabhu and kaatish and removed request for trxcllnt and jrhemstad March 16, 2021 19:48
@vuule vuule added the 0 - Waiting on Author Waiting for author to respond to review label Mar 16, 2021
@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #7614 (12b52d1) into branch-0.19 (7871e7a) will increase coverage by 0.52%.
The diff coverage is 93.75%.

❗ Current head 12b52d1 differs from pull request most recent head 63228f6. Consider uploading reports for the commit 63228f6 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #7614      +/-   ##
===============================================
+ Coverage        81.86%   82.39%   +0.52%     
===============================================
  Files              101      101              
  Lines            16884    17350     +466     
===============================================
+ Hits             13822    14295     +473     
+ Misses            3062     3055       -7     
Impacted Files Coverage Δ
python/cudf/cudf/core/index.py 93.34% <ø> (+0.48%) ⬆️
python/cudf/cudf/core/column/numerical.py 94.83% <87.50%> (-0.20%) ⬇️
python/cudf/cudf/core/frame.py 89.09% <89.47%> (+0.08%) ⬆️
python/cudf/cudf/core/column/column.py 87.86% <90.00%> (+0.10%) ⬆️
python/cudf/cudf/core/column/decimal.py 92.75% <90.32%> (-2.12%) ⬇️
python/cudf/cudf/core/dataframe.py 90.58% <95.65%> (+0.11%) ⬆️
python/cudf/cudf/core/series.py 91.57% <95.83%> (+0.79%) ⬆️
python/cudf/cudf/core/column/categorical.py 91.97% <100.00%> (+0.58%) ⬆️
python/cudf/cudf/core/column/datetime.py 89.63% <100.00%> (+0.54%) ⬆️
python/cudf/cudf/core/column/string.py 86.76% <100.00%> (+0.26%) ⬆️
... and 58 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 2f5901f...63228f6. Read the comment docs.

@vuule
Copy link
Contributor Author

vuule commented Mar 17, 2021

Rerun tests

@vuule vuule added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 0 - Waiting on Author Waiting for author to respond to review labels Mar 20, 2021
@vuule
Copy link
Contributor Author

vuule commented Mar 20, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit bb05ddc into rapidsai:branch-0.19 Mar 20, 2021
@vuule vuule deleted the cuio-orc-col_buffer-uvector branch March 20, 2021 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants