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

Split up mixed-join kernels source files #10671

Merged
merged 8 commits into from
Apr 19, 2022

Conversation

davidwendt
Copy link
Contributor

@davidwendt davidwendt commented Apr 15, 2022

Split up mixed_join_kernels.cu and mixed_join_size_kernels.cu to improve overall build time.
Currently these take about 30 minutes each on the gpuCI build. Example of a recent build metrics report:
https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-22-06/job/cudf/job/prb/job/cudf-cpu-cuda-build/CUDA=11.5/164/Build_20Metrics_20Report/

The nulls and non-nulls definitions are placed into separate source files.

The kernel source used for mixed_join_kernels.cu (both null and non-null) is moved to mixed_join_kernel.cuh and the nulls definition is moved to mixed_join_kernel_nulls.cu. For consistency the mixed_join_kernels.cu name is changed to just mixed_join_kernel.cu since it now only contains one definition.
This same pattern applies to mixed_join_size_kernels.cu splitting into mixed_join_size_kernel.cuh, mixed_join_size_kernel_nulls.cu and mixed_join_size_kernel.cu

No function behavior or actual code generation has changed. The source code has just moved into more source files to help better parallelize and speed up the build process. This improves compile time by 10% for a release build and ~25% for a debug build.

@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 15, 2022
@davidwendt davidwendt self-assigned this Apr 15, 2022
@github-actions github-actions bot added the CMake CMake build issue label Apr 15, 2022
@codecov
Copy link

codecov bot commented Apr 15, 2022

Codecov Report

Merging #10671 (587cb1b) into branch-22.06 (94a5d41) will increase coverage by 0.02%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           branch-22.06   #10671      +/-   ##
================================================
+ Coverage         86.38%   86.41%   +0.02%     
================================================
  Files               142      142              
  Lines             22334    22334              
================================================
+ Hits              19294    19300       +6     
+ Misses             3040     3034       -6     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/string.py 89.22% <0.00%> (+0.12%) ⬆️
python/cudf/cudf/core/groupby/groupby.py 91.72% <0.00%> (+0.22%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <0.00%> (+0.30%) ⬆️
python/cudf/cudf/core/column/lists.py 92.79% <0.00%> (+1.27%) ⬆️

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 9409559...587cb1b. Read the comment docs.

@davidwendt
Copy link
Contributor Author

Here are the times for mixed_join_kernels.cu and mixed_join_size_kernels.cu with but nulls and non-nulls templated functions defined in single source files:
https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-22-06/job/cudf/job/prb/job/cudf-cpu-cuda-build/CUDA=11.5/323/Build_20Metrics_20Report/

Here are the times after splitting up each of these into nulls and non-nulls source files:
https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-22-06/job/cudf/job/prb/job/cudf-cpu-cuda-build/CUDA=11.5/329/Build_20Metrics_20Report/

This improved the overall build time by about 10 minutes. And though this may be only a 10% improvement for a release build, this change reduces a debug build from 2 hours to 1.5 hours (~25%).

@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Apr 18, 2022
@davidwendt davidwendt marked this pull request as ready for review April 18, 2022 14:29
@davidwendt davidwendt requested review from a team as code owners April 18, 2022 14:29
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

CMake changes LGTM

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Thanks @davidwendt!

@davidwendt
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 565f474 into rapidsai:branch-22.06 Apr 19, 2022
@davidwendt davidwendt deleted the split-up-mixed-join branch April 19, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CMake CMake build issue 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.

4 participants