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

Use nvcomp's snappy decompressor in avro reader #9181

Merged
merged 42 commits into from
Sep 27, 2021
Merged

Use nvcomp's snappy decompressor in avro reader #9181

merged 42 commits into from
Sep 27, 2021

Conversation

devavret
Copy link
Contributor

@devavret devavret commented Sep 7, 2021 •
edited
Loading

devavret added 27 commits May 7, 2021 17:26
@devavret
Initial changes to get nvcomp integrated
db23741 
Cmake changes (excluding changes needed in nvcomp's cmake)
Replace cuIO's snappy compressor with nvcomp
@devavret
Using nvcomp provided max compressed buffer size
a5f3363
@devavret
Recover from error in nvcomp compressing and encode uncompressed.
61018aa
@devavret
review changes
64d7d1c
@devavret
Use nvcomp's snappy decompressor in parquet reader
9787c7c
@devavret
Replace accidental vector with uvector.
27764e7
@devavret
Provide the actual max uncomp page size to nvcomp's temp size estimat…
95a57ec 
…or rather than a hardcoded value
@devavret
cmake changes requested in review
cc9500a
@devavret
Merge branch 'parquet-writer-nvcomp-snappy' into parquet-reader-nvcom…
679eae1 
…p-snappy
@devavret
Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy
7989b9c
@devavret
Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy
f90409c
@devavret
Merge branch 'parquet-writer-nvcomp-snappy' into parquet-reader-nvcom…
63d22b0 
…p-snappy
@devavret
Update parquet writer to use nvcomp 2.1
40ebd1e
@devavret
One more cmake change related to updating nvcomp
4a2cb24
@devavret
Merge branch 'parquet-writer-nvcomp-snappy' into parquet-reader-nvcom…
1e6a1be 
…p-snappy
@devavret
Update nvcomp to version with fix for snappy decompressor
6019b0f
@devavret
Merge branch 'parquet-writer-nvcomp-snappy' into parquet-reader-nvcom…
79953a8 
…p-snappy
@devavret
Update reader to use new nvcomp API
d7b86b7
@devavret
Fix allocation size bug
140d3d0 
When writing statistics, there's not enough space allocated in chunk's compressed buffer.
This results in the compressed buffer being written into another chunk's memory.
@devavret
Merge branch 'parquet-writer-nvcomp-snappy' into parquet-reader-nvcom…
004d50b 
…p-snappy
@devavret
Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy
05f5343
@devavret
Update cmake to find nvcomp in new manner
62d92b4
@devavret
Merge branch 'parquet-writer-nvcomp-snappy' into avro-reader-nvcomp-s…
4155a89 
…nappy
@devavret
First commit to add nvcomp snappy decomp to avro
a0824e6 
Decompression works and newly added pytest passes.
Needs cleanup and benchmarking
@devavret
Cleanups
8645c68
@devavret
Add begin and end accessors to hd_vec.
ddff6fb 
And more cleanups
@devavret
Collect deflate decomp code into a single branch and add pytest for d…
26d9bbf 
…eflate codec
@devavret devavret requested review from a team as code owners September 7, 2021 12:03
@github-actions github-actions bot added the conda label Sep 14, 2021
@devavret devavret requested a review from a team as a code owner September 16, 2021 21:59
@github-actions github-actions bot removed the CMake CMake build issue label Sep 16, 2021
nvdbaranec
nvdbaranec requested changes Sep 20, 2021
View reviewed changes
Copy link
Contributor

@nvdbaranec nvdbaranec left a comment

Choose a reason for hiding this comment

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

Just a couple of small things.

cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
nvdbaranec
nvdbaranec requested changes Sep 20, 2021
View reviewed changes
cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/avro/reader_impl.cu Outdated Show resolved Hide resolved
@devavret devavret mentioned this pull request Sep 20, 2021
Merged
@ajschmidt8
Copy link
Member

rerun tests

@codecov
Copy link

codecov bot commented Sep 21, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.12@0cf2c5f). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head ad66a24 differs from pull request most recent head 03a278b. Consider uploading reports for the commit 03a278b to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.12    #9181   +/-   ##
===============================================
  Coverage                ?   10.89%           
===============================================
  Files                   ?      115           
  Lines                   ?    19087           
  Branches                ?        0           
===============================================
  Hits                    ?     2080           
  Misses                  ?    17007           
  Partials                ?        0

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 0cf2c5f...03a278b. Read the comment docs.

ajschmidt8
ajschmidt8 approved these changes Sep 21, 2021
View reviewed changes
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

@devavret devavret changed the base branch from branch-21.10 to branch-21.12 September 22, 2021 18:50
@devavret
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 42ffe78 into rapidsai:branch-21.12 Sep 27, 2021
@vuule vuule added the cuIO cuIO issue label Sep 30, 2021
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuIO Reviewer labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe left review comments

@robertmaynard robertmaynard robertmaynard left review comments

@nvdbaranec nvdbaranec nvdbaranec approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@vuule vuule Awaiting requested review from vuule

@trxcllnt trxcllnt Awaiting requested review from trxcllnt

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@devavret @ajschmidt8 @robertmaynard @jlowe @galipremsagar @nvdbaranec @vyasr @vuule