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

Set the default number of threads in KvikIO thread pool to 8 #17126

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Set the default number of threads in KvikIO thread pool to 8 #17126

merged 3 commits into from
Oct 22, 2024

Conversation

kingcrimsontianyu
Copy link
Contributor

@kingcrimsontianyu kingcrimsontianyu commented Oct 18, 2024 •
edited
Loading

Description

Recent benchmarks have shown that setting the environment variable KVIKIO_NTHREADS=8 in cuDF usually leads to optimal I/O performance. This PR internally sets the default KvikIO thread pool size to 8. The env KVIKIO_NTHREADS will still be honored if users explicitly set it.

Fixes #16718

Checklist

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

@kingcrimsontianyu kingcrimsontianyu self-assigned this Oct 18, 2024
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Oct 18, 2024
@kingcrimsontianyu kingcrimsontianyu added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. and removed libcudf Affects libcudf (C++/CUDA) code. labels Oct 18, 2024
@kingcrimsontianyu kingcrimsontianyu marked this pull request as ready for review October 21, 2024 19:35
@kingcrimsontianyu kingcrimsontianyu requested a review from a team as a code owner October 21, 2024 19:35
@GregoryKimball
Copy link
Contributor

Linking #16718 to this PR

@mythrocks
Copy link
Contributor

We might consider adding Fixes #16718 to this PR's description, since we're mentioning positive results from recent benchmarks. That way, the issues will be linked in the Github commit history.

The changes look good to me, FWIW.

vuule
vuule requested changes Oct 21, 2024
View reviewed changes
cpp/include/cudf/io/config_utils.hpp Outdated Show resolved Hide resolved
cpp/src/io/utilities/config_utils.cpp Outdated Show resolved Hide resolved
madsbk
madsbk approved these changes Oct 22, 2024
View reviewed changes
Copy link
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @kingcrimsontianyu

@kingcrimsontianyu
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 27c0c9d into rapidsai:branch-24.12 Oct 22, 2024
106 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madsbk madsbk madsbk approved these changes

@vuule vuule vuule approved these changes

@lamarrr lamarrr Awaiting requested review from lamarrr lamarrr is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@kingcrimsontianyu kingcrimsontianyu

Labels
improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
libcudf
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Increase the default thread count for kvikIO file reads
5 participants
@kingcrimsontianyu @GregoryKimball @mythrocks @madsbk @vuule