Skip to content

Address poor performance of Parquet string decoding#15304

Merged
rapids-bot[bot] merged 4 commits intorapidsai:branch-24.04from
etseidl:lumpy_strings
Mar 19, 2024
Merged

Address poor performance of Parquet string decoding#15304
rapids-bot[bot] merged 4 commits intorapidsai:branch-24.04from
etseidl:lumpy_strings

Conversation

@etseidl
Copy link
Copy Markdown
Contributor

@etseidl etseidl commented Mar 14, 2024

Description

See #15297. The Parquet string decoder can become a bottleneck in the presence of strings of widely varying sizes. This PR is an attempt to address this, at least as a stop gap solution. A more complete solution may be to rework the string decoder to work in a block-wide fashion, such as the new micro-kernels added in #15159.

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
Copy link
Copy Markdown

copy-pr-bot bot commented Mar 14, 2024

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.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Mar 14, 2024
@etseidl
Copy link
Copy Markdown
Contributor Author

etseidl commented Mar 14, 2024

cc @GregoryKimball

@davidwendt davidwendt added 2 - In Progress Currently a work in progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 14, 2024
@davidwendt
Copy link
Copy Markdown
Contributor

/ok to test

@etseidl
Copy link
Copy Markdown
Contributor Author

etseidl commented Mar 14, 2024

String decode current 24.04 top, this PR (using avg length per batch of 32 strings) bottom. Note that there are only 4 pages being decoded in this file.

Screenshot from 2024-03-14 12-33-45

@GregoryKimball
Copy link
Copy Markdown
Contributor

Thank you @etseidl for looking into this performance case!

@GregoryKimball
Copy link
Copy Markdown
Contributor

@etseidl would you please let me know if there are more changes you would like to make - or is this ready for review?

@etseidl
Copy link
Copy Markdown
Contributor Author

etseidl commented Mar 18, 2024

would you please let me know if there are more changes you would like to make - or is this ready for review?

@GregoryKimball If a quick fix is wanted for 24.04, then I think this is ready. It will take longer to evaluate more complicated solutions. My first attempt at going block wide didn't pan out :(

@etseidl etseidl changed the title [WIP] Address poor performance of Parquet string decoding Address poor performance of Parquet string decoding Mar 18, 2024
@etseidl etseidl marked this pull request as ready for review March 18, 2024 22:57
@etseidl etseidl requested a review from a team as a code owner March 18, 2024 22:57
@etseidl etseidl requested review from hyperbolic2346 and vyasr March 18, 2024 22:57
@ttnghia
Copy link
Copy Markdown
Contributor

ttnghia commented Mar 19, 2024

/ok to test

@vuule vuule self-requested a review March 19, 2024 21:02
Copy link
Copy Markdown
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

❤️

@vuule
Copy link
Copy Markdown
Contributor

vuule commented Mar 19, 2024

/merge

@rapids-bot rapids-bot bot merged commit 7cc02e5 into rapidsai:branch-24.04 Mar 19, 2024
@etseidl etseidl deleted the lumpy_strings branch March 19, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 - In Progress Currently a work in progress improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants