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.

Sign up for GitHub

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

Jump to bottom

Implement zero-copy host buffer source instead of using an arrow implementation #15189

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Implement zero-copy host buffer source instead of using an arrow implementation #15189

merged 3 commits into from
Mar 5, 2024

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Feb 29, 2024

Description

Avoids an arrow dependency with a bit of simple code.

No real impact on performance.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vuule vuule added feature request New feature or request non-breaking Non-breaking change labels Feb 29, 2024
@vuule vuule self-assigned this Feb 29, 2024
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Feb 29, 2024
bdice
bdice approved these changes Feb 29, 2024
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This helps a ton to weaken the dependency on Arrow headers (which change quite frequently). Thank you!

GregoryKimball
GregoryKimball reviewed Feb 29, 2024
View reviewed changes
cpp/src/io/utilities/datasource.cpp
@@ -27,7 +26,6 @@

#include <rmm/device_buffer.hpp>

#include <arrow/io/memory.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

LOVE THIS

@GregoryKimball GregoryKimball mentioned this pull request Feb 29, 2024
Closed
@vuule vuule marked this pull request as ready for review March 1, 2024 17:30
@vuule vuule requested a review from a team as a code owner March 1, 2024 17:30
@vuule vuule requested review from mythrocks and PointKernel March 1, 2024 17:30
@vuule
Copy link
Contributor Author

vuule commented Mar 1, 2024

/merge

@vuule vuule added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Mar 1, 2024
@rapids-bot rapids-bot bot merged commit cd79fe5 into rapidsai:branch-24.04 Mar 5, 2024
74 checks passed
@vuule vuule deleted the fea-no-arrow-buffer-source branch March 5, 2024 04:04
@ZelboK ZelboK mentioned this pull request Jul 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GregoryKimball GregoryKimball GregoryKimball left review comments

@PointKernel PointKernel PointKernel approved these changes

@bdice bdice bdice approved these changes

@mythrocks mythrocks Awaiting requested review from mythrocks mythrocks is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@vuule vuule

Labels
5 - Ready to Merge Testing and reviews complete, ready to merge feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vuule @bdice @PointKernel @GregoryKimball