Use nvcomp's snappy decompression in ORC reader #9235

devavret · 2021-09-15T22:24:16Z

Cmake changes (excluding changes needed in nvcomp's cmake) Replace cuIO's snappy compressor with nvcomp

…or rather than a hardcoded value

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.

…appy

hyperbolic2346

Looks good in general. Just a single question regarding error handling.

hyperbolic2346 · 2021-09-15T23:12:35Z

cpp/src/io/orc/reader_impl.cu

+    num_blocks,
+    [=, actual_uncomp_sizes = actual_uncompressed_data_sizes.data()] __device__(auto i) {
+      comp_stat[i].bytes_written = actual_uncomp_sizes[i];
+      comp_stat[i].status        = 0;


I find it odd that we don't return the actual status and let the caller handle it. I assume things are in a terrible state at this point and there isn't anything useful the caller can do with this information. Is that correct?

This is intended as an improvement. Previously, if status was 0 for any compression block, the reader would give up in the kernel but wouldn't throw any error.

cudf/cpp/src/io/orc/stripe_init.cu

Lines 160 to 163 in 99e4f80

if (shuffle((lane_id == 0) ? dec_out[num_compressed_blocks].status : 0) != 0) {

// Decompression failed, not much point in doing anything else

break;

}

We discussed that it's better to fail loudly in case we encounter a corrupt compressed chunk.

codecov · 2021-09-16T01:06:39Z

Codecov Report

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

@@               Coverage Diff               @@
##             branch-21.10    #9235   +/-   ##
===============================================
  Coverage                ?   10.49%           
===============================================
  Files                   ?      115           
  Lines                   ?    19828           
  Branches                ?        0           
===============================================
  Hits                    ?     2080           
  Misses                  ?    17748           
  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...354e229. Read the comment docs.

devavret · 2021-09-16T10:02:13Z

rerun tests

vuule · 2021-09-17T06:31:31Z

cpp/src/io/orc/reader_impl.cu

@@ -549,6 +551,68 @@ class aggregate_orc_metadata {
  }
 };

+void snappy_decompress(device_span<gpu_inflate_input_s> comp_in,


feels like we could merge this with snappy_decompress in Parquet reader, but I'm fine with leaving this for another PR.

I realized that but once cuIO's internal decompressors are removed completely, there's a lot more to refactor around this that I deferred it for then.

robertmaynard · 2021-09-17T14:36:03Z

Removed CMake code owners as the PR now doesn't have any CMake changes

vuule · 2021-09-18T00:38:00Z

@gpucibot merge

Issue #9205 depends on #9235 Authors: - Devavret Makkar (https://github.com/devavret) Approvers: - Vukasin Milovanovic (https://github.com/vuule) - Elias Stehle (https://github.com/elstehle) - https://github.com/nvdbaranec - Mike Wilson (https://github.com/hyperbolic2346) URL: #9242

devavret added 24 commits May 7, 2021 17:26

Initial changes to get nvcomp integrated

db23741

Cmake changes (excluding changes needed in nvcomp's cmake) Replace cuIO's snappy compressor with nvcomp

Using nvcomp provided max compressed buffer size

a5f3363

Recover from error in nvcomp compressing and encode uncompressed.

61018aa

review changes

64d7d1c

Replace accidental vector with uvector.

27764e7

Provide the actual max uncomp page size to nvcomp's temp size estimat…

95a57ec

…or rather than a hardcoded value

cmake changes requested in review

cc9500a

Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy

7989b9c

Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy

f90409c

Update parquet writer to use nvcomp 2.1

40ebd1e

One more cmake change related to updating nvcomp

4a2cb24

Update nvcomp to version with fix for snappy decompressor

6019b0f

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.

Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy

05f5343

Update cmake to find nvcomp in new manner

62d92b4

Make nvcomp private in cmake and update get_nvcomp

3c73be3

Add an env var flip switch to choose b/w nvcomp and inbuilt compressor

e0a013d

Merge branch 'branch-21.10' into parquet-writer-nvcomp-snappy

7501b11

Static linking nvcomp into libcudf

bfa1366

Review changes

203cf15

Working orc reader with nvcomp

99e4f80

Merge changes from nvcomp -fPIC

6721fb8

Merge branch 'parquet-writer-nvcomp-snappy' into orc-reader-nvcomp-sn…

5391e13

…appy

Merge branch 'branch-21.10' into orc-reader-nvcomp-snappy

354e229

devavret requested review from a team as code owners September 15, 2021 22:24

devavret requested review from hyperbolic2346 and mythrocks September 15, 2021 22:24

github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Sep 15, 2021

github-actions bot removed the CMake CMake build issue label Sep 15, 2021

devavret added 4 - Needs cuIO Reviewer cuIO cuIO issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change 3 - Ready for Review Ready for review by team labels Sep 15, 2021

hyperbolic2346 approved these changes Sep 15, 2021

View reviewed changes

devavret requested review from vuule and rgsl888prabhu and removed request for mythrocks September 16, 2021 17:13

devavret mentioned this pull request Sep 16, 2021

Use nvcomp's snappy compressor in ORC writer #9242

Merged

vuule reviewed Sep 17, 2021

View reviewed changes

robertmaynard removed the request for review from a team September 17, 2021 14:35

vuule approved these changes Sep 17, 2021

View reviewed changes

rapids-bot bot merged commit f08d6f1 into rapidsai:branch-21.10 Sep 18, 2021

vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuIO Reviewer labels Feb 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use nvcomp's snappy decompression in ORC reader #9235

Use nvcomp's snappy decompression in ORC reader #9235

devavret commented Sep 15, 2021 •

edited by vuule

Loading

hyperbolic2346 left a comment

hyperbolic2346 Sep 15, 2021

devavret Sep 15, 2021

codecov bot commented Sep 16, 2021 •

edited

Loading

devavret commented Sep 16, 2021

vuule Sep 17, 2021

devavret Sep 17, 2021

robertmaynard commented Sep 17, 2021

vuule commented Sep 18, 2021

	if (shuffle((lane_id == 0) ? dec_out[num_compressed_blocks].status : 0) != 0) {
	// Decompression failed, not much point in doing anything else
	break;
	}

Use nvcomp's snappy decompression in ORC reader #9235

Use nvcomp's snappy decompression in ORC reader #9235

Conversation

devavret commented Sep 15, 2021 • edited by vuule Loading

hyperbolic2346 left a comment

Choose a reason for hiding this comment

hyperbolic2346 Sep 15, 2021

Choose a reason for hiding this comment

devavret Sep 15, 2021

Choose a reason for hiding this comment

codecov bot commented Sep 16, 2021 • edited Loading

Codecov Report

devavret commented Sep 16, 2021

vuule Sep 17, 2021

Choose a reason for hiding this comment

devavret Sep 17, 2021

Choose a reason for hiding this comment

robertmaynard commented Sep 17, 2021

vuule commented Sep 18, 2021

devavret commented Sep 15, 2021 •

edited by vuule

Loading

codecov bot commented Sep 16, 2021 •

edited

Loading