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.

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

Address poor performance of Parquet string decoding #15304

Merged
merged 4 commits into from
Mar 19, 2024

Conversation

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

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
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
Contributor

/ok to test

@etseidl
Copy link
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
Contributor

Thank you @etseidl for looking into this performance case!

@GregoryKimball
Copy link
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
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 vyasr and hyperbolic2346 March 18, 2024 22:57
@ttnghia
Copy link
Contributor

ttnghia commented Mar 19, 2024

/ok to test

@vuule vuule self-requested a review March 19, 2024 21:02
Copy link
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
Contributor

vuule commented Mar 19, 2024

/merge

@rapids-bot rapids-bot bot merged commit 7cc02e5 into rapidsai:branch-24.04 Mar 19, 2024
75 checks passed
@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