-
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
Make snappy decompress check more efficient #9995
Conversation
cheinger
commented
Jan 7, 2022
- Add status checking of decompressed snappy parquet pages when using cuDF snappy decompression.
- Improve the performance of checking decompressed snappy parquet pages when using nvCOMP snappy decompression by reducing the number of synchronizes.
Can one of the admins verify this patch? |
ok to test |
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.
I realized we allocate and pass comp_stat
to snappy_decompress()
but don't use it. We should use it for checking instead.
@cheinger Also, please don't force-push. It decouples the review comments from the code and makes it hard to see what changed in the most recent commit |
@cheinger is this ready for another review? Click the re-request review button next to my name in reviewers when ready. |
This should probably be retargeted to 22.04 at this point. |
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.
Nearly there.
a9ec722
to
db93bda
Compare
da4c811
to
17d1219
Compare
@cheinger what is the status of this? Looks like it might need to be merged up to head of branch-22.04. |
Rename pages to blocks Use util::div_rounding_up_safe
@jbrennan333 Hey Jim - yeah I would like to get this in as well, however I keep running into issues that don't seem to be related to these changes? 🤔 I've rebased several times but always run into a different issue. Maybe you could provide some guidance? Thanks |
I know there was a bug earlier this week that was impacting pre-commit tests which was fixed. This happens sometimes. It's a big project with a lot of changes going in - once you commit, you should keep an eye on those pre-commit checks and follow up quickly. I think this one has missed the opportunity to get into 22.04 as we are already in burn-down. Would be great to get it in early in 22.06. |
rerun tests |
2 similar comments
rerun tests |
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #9995 +/- ##
================================================
- Coverage 86.13% 86.13% -0.01%
================================================
Files 139 139
Lines 22438 22435 -3
================================================
- Hits 19328 19324 -4
- Misses 3110 3111 +1
Continue to review full report at Codecov.
|
@gpucibot merge |