-
Notifications
You must be signed in to change notification settings - Fork 915
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 cuFile for Parquet IO when available #7444
Conversation
reassigned review to @devavret since it's cuIO related |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obvious partial review but I wanted to say this sooner.
cpp/src/io/parquet/writer_impl.cu
Outdated
cudaFreeHost}; | ||
} | ||
}(); | ||
auto host_bfr = pinned_buffer<uint8_t>{[](size_t size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cudaMallocHost
is expensive. There was an optimization to not do it when using device write. Why was it removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not intentional, this is probably because this change was here before the optimization and I messed up a merge. Will restore the behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, now the buffer is lazily allocated on first use.
Updating the |
@kkraus14 @vuule FYI I just merged rapidsai/gpuci-build-environment#166 - it should take ~1hr for the images to build and be available for use in gpuCI |
Rerun tests |
@gpucibot merge |
@vuule awesome!!! So we should be able to try on nightly's tmw+? |
Nightly's available now should be able to be tested. You'll need to install GDS from here: https://developer.nvidia.com/gpudirect-storage Then there's the environment variable of
By default cuFile / GDS will not be used and our existing IO code will be used. |
Amazing, thanks all :) Starting on the build process here, and will hopefully have our first benchmarks over the weekend / early next. Will share on Slack when up :) |
Adds optional cuFile integration:
cufile.h
is included in the build when available.libcufile.so
is loaded at runtime ifLIBCUDF_CUFILE_POLICY
environment variable is set to "ALWAYS" or "GDS".device_read
.