-
Notifications
You must be signed in to change notification settings - Fork 891
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 nvCOMP for ZLIB decompression in ORC reader #11024
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-22.08 #11024 +/- ##
===============================================
Coverage ? 86.34%
===============================================
Files ? 144
Lines ? 22710
Branches ? 0
===============================================
Hits ? 19608
Misses ? 3102
Partials ? 0 Continue to review full report at Codecov.
|
Perf impact evaluation blocked on ZLIB compression issues in the writer. We can merge this PR without the perf evaluation, but will need to run the benchmarks before enabling this feature by default. |
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.
+1 this looks good to me.
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.
Everything looks good to me!
@@ -29,6 +29,14 @@ | |||
#define NVCOMP_HAS_ZSTD 0 | |||
#endif | |||
|
|||
#define NVCOMP_DEFLATE_HEADER <nvcomp/deflate.h> | |||
#if __has_include(NVCOMP_DEFLATE_HEADER) |
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.
Interesting. This is the first time I've seen __has_include
. It's a standardized feature since C++17.
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.
Yeah, when I was adding ZSTD support, I searched libcudf for existing uses and found none :D
I'm not sure if we covered it in the C++17 talks we did a while ago.
@gpucibot merge |
Managed to get the writer ZLIB compression up and running. |
Issue #11023
Adds
DEFLATE
compression type to the nvCOMP adapter.ORC reader uses the adapter when nvCOMP experimental integrations are enabled (for now). Otherwise behavior is unchanged and the internal implementation is still used.
No tests as no visible behavior change is expected.
Pending: Performance impact