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 CSV reader/writer #7805

Merged
merged 19 commits into from
Apr 22, 2021

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Apr 1, 2021

Issue #7287

Replaces device_vector with device_uvector and device_span. Removed the device_vector data members.

Performance impact:

  • Writer: None
  • Reader: up to 10% slower, will look into this. None

@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 Apr 1, 2021
@vuule vuule self-assigned this Apr 1, 2021
@vuule vuule added this to PR-WIP in v21.06 Release via automation Apr 1, 2021
@vuule vuule marked this pull request as ready for review April 1, 2021 05:24
@vuule vuule requested a review from a team as a code owner April 1, 2021 05:24
@vuule vuule requested review from trxcllnt and harrism April 1, 2021 05:24
@@ -180,7 +183,19 @@ std::vector<std::string> setColumnNames(std::vector<char> const &header,
return col_names;
}

table_with_metadata reader::impl::read(rmm::cuda_stream_view stream)
template <typename C>
void erase_except_last(C &container, rmm::cuda_stream_view stream)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pulled this into a separate function because a device lambda cannot be declared in a private function

@vuule vuule requested review from devavret and kaatish April 5, 2021 20:13
@vuule
Copy link
Contributor Author

vuule commented Apr 5, 2021

Added cuIO reviewers.

@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #7805 (d44e9bf) into branch-0.20 (51336df) will increase coverage by 0.05%.
The diff coverage is 88.23%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.20    #7805      +/-   ##
===============================================
+ Coverage        82.88%   82.94%   +0.05%     
===============================================
  Files              103      103              
  Lines            17668    17713      +45     
===============================================
+ Hits             14645    14692      +47     
+ Misses            3023     3021       -2     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/__init__.py 100.00% <ø> (ø)
python/cudf/cudf/io/orc.py 86.89% <ø> (ø)
python/cudf/cudf/utils/cudautils.py 57.75% <25.00%> (ø)
python/cudf/cudf/core/column/column.py 88.64% <71.42%> (ø)
python/cudf/cudf/core/column/numerical.py 94.43% <72.72%> (ø)
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% <90.90%> (+0.49%) ⬆️
python/cudf/cudf/core/column/datetime.py 89.91% <100.00%> (ø)
python/cudf/cudf/core/column/timedelta.py 88.66% <100.00%> (ø)
... and 22 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 d501d2c...d44e9bf. Read the comment docs.

cpp/src/io/csv/csv_gpu.cu Outdated Show resolved Hide resolved
cpp/src/io/csv/csv_gpu.cu Outdated Show resolved Hide resolved
cpp/src/io/csv/reader_impl.cu Outdated Show resolved Hide resolved
v21.06 Release automation moved this from PR-WIP to PR-Reviewer approved Apr 6, 2021
@vuule vuule added the 0 - Waiting on Author Waiting for author to respond to review label Apr 6, 2021
@vuule vuule requested a review from kaatish April 20, 2021 21:33
@vuule vuule added 4 - Needs cuIO Reviewer and removed 0 - Waiting on Author Waiting for author to respond to review labels Apr 20, 2021
cpp/src/io/csv/csv_gpu.cu Outdated Show resolved Hide resolved
@vuule vuule requested a review from kaatish April 21, 2021 05:45
@vuule vuule added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs cuIO Reviewer labels Apr 21, 2021
@kkraus14
Copy link
Collaborator

rerun tests

@kkraus14
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f8f9988 into rapidsai:branch-0.20 Apr 22, 2021
v21.06 Release automation moved this from PR-Reviewer approved to Done Apr 22, 2021
@vuule vuule deleted the avoid-device-vector-csv branch November 11, 2021 06:49
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants