Skip to content

[WIP] Reduce redundant compilation through explicit specialization of extern templates [skip-ci] - #3514

Closed
wphicks wants to merge 8 commits into
NVIDIA:branch-0.19from
wphicks:fea-extern_templates
Closed

wphicks wants to merge 8 commits into
NVIDIA:branch-0.19from
wphicks:fea-extern_templates

Conversation

@wphicks

@wphicks wphicks commented Feb 18, 2021

Copy link
Copy Markdown
Contributor

DO NOT MERGE YET: Posting this to facilitate discussion

Declare some of the most redundantly-compiled templates as extern and provide explicit specializations in order to reduce unnecessary compilation time

@wphicks wphicks added feature request New feature or request Build or Dep Issues related to building the code or dependencies BigConflictCauser Large refactors or similar that will touch many files non-breaking Non-breaking change CUDA/C++ labels Feb 18, 2021
@wphicks
wphicks requested review from a team as code owners February 18, 2021 15:59
@github-actions github-actions Bot added the CMake label Feb 18, 2021
Comment thread cpp/CMakeLists.txt

# single GPU components
add_library(${CUML_CPP_TARGET} SHARED
src/specializations/prims/selection/knn.cu

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I love the execution of how you have proposed and organized this!

The only things I can think off the top of my head are:

  • Add somewhere a readme that lists what functions have been specialized (maybe a markdown in the C++) to make it very easy and obvious how to use/reference/add specializations, unless you think this would be redundant. I think it could be valuable if we get a significantly large number of specializations.
  • Add a section to the C++ developer guide with usage guides and also guidelines of when a new specialization is preferred over importing the prims/raft functions directly.

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think both of those are great ideas! I've written scripts to help automate generation of the specializations, so I'll add something to them to start generating the markdown list as well.

@github-actions

Copy link
Copy Markdown

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

@dantegd dantegd added the 2 - In Progress Currenty a work in progress label Mar 26, 2021
@JohnZed

JohnZed commented Apr 15, 2021

Copy link
Copy Markdown
Contributor

Will re-post after other header cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 - In Progress Currenty a work in progress BigConflictCauser Large refactors or similar that will touch many files Build or Dep Issues related to building the code or dependencies CMake feature request New feature or request inactive-30d non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants