Use page statistics in Parquet reader by etseidl · Pull Request #14973 · rapidsai/cudf

etseidl · 2024-02-06T01:38:29Z

Description

#14000 added the ability to write new page statistics to the Parquet writer. This PR uses these new statistics to avoid some string size computations. Benchmarks show an improvement in read times of up to 20%.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2024-02-06T01:38:32Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

etseidl · 2024-02-06T01:40:11Z

parquet_read_decode benchmark

## [0] NVIDIA RTX A6000

|  data_type  |    io_type    |  cardinality  |  run_length  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |         Diff |   %Diff |
|-------------|---------------|---------------|--------------|------------|-------------|------------|-------------|--------------|---------|
|  INTEGRAL   | DEVICE_BUFFER |       0       |      1       |  14.942 ms |       0.74% |  14.989 ms |       0.76% |    46.778 us |   0.31% |
|  INTEGRAL   | DEVICE_BUFFER |     1000      |      1       |  14.115 ms |       0.50% |  14.201 ms |       0.46% |    85.853 us |   0.61% |
|  INTEGRAL   | DEVICE_BUFFER |       0       |      32      |  12.908 ms |       0.95% |  13.012 ms |       0.41% |   103.184 us |   0.80% |
|  INTEGRAL   | DEVICE_BUFFER |     1000      |      32      |  12.272 ms |       1.04% |  12.323 ms |       0.49% |    51.350 us |   0.42% |
|   STRING    | DEVICE_BUFFER |       0       |      1       |  46.516 ms |       0.45% |  45.672 ms |       0.50% |  -844.320 us |  -1.82% |
|   STRING    | DEVICE_BUFFER |     1000      |      1       |  13.762 ms |       0.68% |  13.015 ms |       0.71% |  -746.030 us |  -5.42% |
|   STRING    | DEVICE_BUFFER |       0       |      32      |  46.612 ms |       0.50% |  45.692 ms |       0.35% |  -920.004 us |  -1.97% |
|   STRING    | DEVICE_BUFFER |     1000      |      32      |  12.815 ms |       1.11% |  10.469 ms |       1.45% | -2345.521 us | -18.30% |
|    LIST     | DEVICE_BUFFER |       0       |      1       |  76.023 ms |       0.11% |  75.662 ms |       0.05% |  -361.155 us |  -0.48% |
|    LIST     | DEVICE_BUFFER |     1000      |      1       |  64.130 ms |       0.20% |  63.906 ms |       0.22% |  -224.179 us |  -0.35% |
|    LIST     | DEVICE_BUFFER |       0       |      32      |  55.348 ms |       0.13% |  55.170 ms |       0.09% |  -177.805 us |  -0.32% |
|    LIST     | DEVICE_BUFFER |     1000      |      32      |  56.574 ms |       0.21% |  56.434 ms |       0.15% |  -139.456 us |  -0.25% |
|   STRUCT    | DEVICE_BUFFER |       0       |      1       |  48.973 ms |       2.36% |  49.407 ms |       2.21% |   433.937 us |   0.89% |
|   STRUCT    | DEVICE_BUFFER |     1000      |      1       |  30.172 ms |       0.78% |  30.140 ms |       0.73% |   -32.744 us |  -0.11% |
|   STRUCT    | DEVICE_BUFFER |       0       |      32      |  49.314 ms |       2.83% |  50.072 ms |       2.74% |   758.959 us |   1.54% |
|   STRUCT    | DEVICE_BUFFER |     1000      |      32      |  28.299 ms |       0.37% |  27.150 ms |       0.37% | -1148.298 us |  -4.06% |

etseidl · 2024-02-06T01:42:04Z

parquet_read_chunks benchmark

## [0] NVIDIA RTX A6000

|   T    |    io_type    |  cardinality  |  run_length  |  byte_limit  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |          Diff |   %Diff |
|--------|---------------|---------------|--------------|--------------|------------|-------------|------------|-------------|---------------|---------|
| STRING | DEVICE_BUFFER |       0       |      1       |      0       |  45.542 ms |       0.13% |  45.354 ms |       0.52% |   -187.811 us |  -0.41% |
| STRING | DEVICE_BUFFER |     1000      |      1       |      0       |  13.455 ms |       0.43% |  13.059 ms |       0.47% |   -396.648 us |  -2.95% |
| STRING | DEVICE_BUFFER |       0       |      32      |      0       |  45.564 ms |       0.17% |  45.433 ms |       0.31% |   -131.213 us |  -0.29% |
| STRING | DEVICE_BUFFER |     1000      |      32      |      0       |  12.537 ms |       1.16% |  10.671 ms |       4.28% |  -1865.298 us | -14.88% |
| STRING | DEVICE_BUFFER |       0       |      1       |    500000    | 225.866 ms |       0.04% | 184.424 ms |       0.09% | -41442.361 us | -18.35% |
| STRING | DEVICE_BUFFER |     1000      |      1       |    500000    |  77.381 ms |       0.35% |  74.072 ms |       0.27% |  -3309.740 us |  -4.28% |
| STRING | DEVICE_BUFFER |       0       |      32      |    500000    | 225.814 ms |       0.05% | 183.758 ms |       0.18% | -42056.079 us | -18.62% |
| STRING | DEVICE_BUFFER |     1000      |      32      |    500000    |  61.544 ms |       0.51% |  52.393 ms |       0.20% |  -9151.281 us | -14.87% |

and the same but forcing PLAIN encoding

|   T    |    io_type    |  cardinality  |  run_length  |  byte_limit  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |          Diff |   %Diff |
|--------|---------------|---------------|--------------|--------------|------------|-------------|------------|-------------|---------------|---------|
| STRING | DEVICE_BUFFER |       0       |      1       |      0       |  45.449 ms |       0.32% |  45.168 ms |       0.56% |   -281.006 us |  -0.62% |
| STRING | DEVICE_BUFFER |     1000      |      1       |      0       |  46.893 ms |       0.69% |  46.812 ms |       0.79% |    -80.940 us |  -0.17% |
| STRING | DEVICE_BUFFER |       0       |      32      |      0       |  45.666 ms |       0.47% |  45.352 ms |       0.25% |   -314.615 us |  -0.69% |
| STRING | DEVICE_BUFFER |     1000      |      32      |      0       |  24.542 ms |       0.16% |  24.602 ms |       0.38% |     59.229 us |   0.24% |
| STRING | DEVICE_BUFFER |       0       |      1       |    500000    | 227.223 ms |       0.04% | 183.026 ms |       0.13% | -44196.857 us | -19.45% |
| STRING | DEVICE_BUFFER |     1000      |      1       |    500000    | 230.145 ms |       0.11% | 185.712 ms |       0.13% | -44432.935 us | -19.31% |
| STRING | DEVICE_BUFFER |       0       |      32      |    500000    | 227.210 ms |       0.07% | 183.010 ms |       0.07% | -44200.238 us | -19.45% |
| STRING | DEVICE_BUFFER |     1000      |      32      |    500000    | 208.225 ms |       0.13% | 164.224 ms |       0.11% | -44001.523 us | -21.13% |

davidwendt · 2024-02-06T18:50:38Z

/ok to test

cpp/src/io/parquet/reader_impl_preprocess.cu

vuule

I already review the previous version of this feature, but going over it again since I didn't quite approve it back then (I almost did, IIRC).
Few nitpicks so far.

cpp/src/io/parquet/reader_impl_preprocess.cu

vuule

Looks good, just one comment that reads a bit odd to me.

vuule · 2024-02-27T18:38:05Z

cpp/src/io/parquet/reader_impl_helpers.cpp

+
+        // calculate num_nulls if not available from column index
+        if (not pg_info.num_nulls.has_value()) {
+          pg_info.num_nulls = std::reduce(h, h + max_def_level);


the least verbose use of a standard algorithm I've ever seen :D

vuule · 2024-02-27T20:03:11Z

cpp/src/io/parquet/reader_impl_helpers.cpp

+      // info is missing or insufficient, then just return without modifying the row_group_info.
+      if (not pg_info.num_nulls.has_value() or not pg_info.num_valid.has_value()) { return; }
+
+      // if using older page indexes that lack size info, don't use


Suggested change

// if using older page indexes that lack size info, don't use

// if using older page indexes that lack size info, don't use `pg_info`

Cleaned this up and added a more detailed TODO...will push once CI finishes

looking at a few other PRs, this might be an "if CI finishes" situation 😬

vuule · 2024-02-27T20:04:13Z

/ok to test

vuule · 2024-03-01T02:52:17Z

/ok to test

etseidl · 2024-03-01T16:59:10Z

Looks like #15020 broke this (db 3 ets 0 😮 😭 🤣). Regrouping...

cpp/src/io/parquet/reader_impl_helpers.cpp

Co-authored-by: Yunsong Wang <yunsongw@nvidia.com>

PointKernel · 2024-03-06T21:09:22Z

/ok to test

PointKernel · 2024-03-07T03:57:45Z

/merge

etseidl added 5 commits February 5, 2024 13:23

use page statistics

a76d7a1

Merge remote-tracking branch 'origin/branch-24.04' into page_stats

679c67c

remove fixme

b2952d5

add read test

94aa6d6

tweak metadata reading

66ed34d

etseidl requested a review from a team as a code owner February 6, 2024 01:38

etseidl requested review from davidwendt and harrism February 6, 2024 01:38

github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Feb 6, 2024

GregoryKimball added the cuIO cuIO issue label Feb 6, 2024

GregoryKimball added this to the Parquet continuous improvement milestone Feb 6, 2024

Merge branch 'branch-24.04' into page_stats

0616ff4

davidwendt added 2 - In Progress Currently a work in progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 6, 2024

davidwendt requested a review from vuule February 6, 2024 18:51

Merge branch 'rapidsai:branch-24.04' into page_stats

c8f07e5

GregoryKimball removed this from the Parquet continuous improvement milestone Feb 16, 2024

etseidl added 6 commits February 17, 2024 11:14

Merge branch 'branch-24.04' into page_stats

6bd23d2

Merge branch 'branch-24.04' into page_stats

09c4a06

Merge branch 'rapidsai:branch-24.04' into page_stats

ef83bf3

Merge branch 'rapidsai:branch-24.04' into page_stats

642103b

Merge branch 'rapidsai:branch-24.04' into page_stats

2688627

Merge branch 'branch-24.04' into page_stats

a840850

etseidl commented Feb 26, 2024

View reviewed changes

cpp/src/io/parquet/reader_impl_preprocess.cu Outdated Show resolved Hide resolved

vuule reviewed Feb 27, 2024

View reviewed changes

cpp/src/io/parquet/reader_impl_preprocess.cu Outdated Show resolved Hide resolved

cpp/src/io/parquet/reader_impl_preprocess.cu Outdated Show resolved Hide resolved

cpp/src/io/parquet/reader_impl_preprocess.cu Outdated Show resolved Hide resolved

vuule approved these changes Feb 27, 2024

View reviewed changes

Merge branch 'branch-24.04' into page_stats

c23783b

etseidl and others added 6 commits February 27, 2024 12:28

clean up comment

96175d4

Merge branch 'page_stats' of github.com:etseidl/cudf into page_stats

d24a6f8

Merge branch 'rapidsai:branch-24.04' into page_stats

3de4261

Merge branch 'branch-24.04' into page_stats

cc9929c

Merge branch 'branch-24.04' into page_stats

76fc5a7

Merge remote-tracking branch 'origin/branch-24.04' into page_stats

b71fc63

GregoryKimball requested review from PointKernel and nvdbaranec March 1, 2024 04:40

etseidl and others added 3 commits March 1, 2024 10:28

need to push chunks to device when using page index

cd65580

Merge branch 'branch-24.04' into page_stats

5fc0186

Merge branch 'branch-24.04' into page_stats

b4d863a

etseidl mentioned this pull request Mar 4, 2024

Add microkernels for fixed-width and fixed-width dictionary in Parquet decode #15159

Merged

3 tasks

etseidl added 2 commits March 4, 2024 21:21

Merge branch 'branch-24.04' into page_stats

2ee6ee2

Merge branch 'branch-24.04' into page_stats

7c69d4f

PointKernel reviewed Mar 6, 2024

View reviewed changes

cpp/src/io/parquet/reader_impl_helpers.cpp Outdated Show resolved Hide resolved

etseidl and others added 2 commits March 6, 2024 12:53

implement suggestion from review

4570e2f

Co-authored-by: Yunsong Wang <yunsongw@nvidia.com>

Merge branch 'branch-24.04' into page_stats

ca078e1

PointKernel approved these changes Mar 7, 2024

View reviewed changes

vuule added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 2 - In Progress Currently a work in progress labels Mar 7, 2024

rapids-bot bot merged commit efae666 into rapidsai:branch-24.04 Mar 7, 2024

etseidl deleted the page_stats branch March 7, 2024 05:48

GregoryKimball mentioned this pull request Apr 16, 2024

Adding parquet transcoding example #15420

Merged

3 tasks

	// if using older page indexes that lack size info, don't use
	// if using older page indexes that lack size info, don't use `pg_info`

Conversation

etseidl commented Feb 6, 2024

Description

Checklist

Uh oh!

copy-pr-bot bot commented Feb 6, 2024

Uh oh!

etseidl commented Feb 6, 2024

Uh oh!

etseidl commented Feb 6, 2024

Uh oh!

davidwendt commented Feb 6, 2024

Uh oh!

Uh oh!

vuule left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vuule left a comment

Choose a reason for hiding this comment

Uh oh!

vuule Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

vuule Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

etseidl Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

vuule Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

vuule commented Feb 27, 2024

Uh oh!

vuule commented Mar 1, 2024

Uh oh!

etseidl commented Mar 1, 2024

Uh oh!

Uh oh!

PointKernel commented Mar 6, 2024

Uh oh!

PointKernel commented Mar 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants