Skip to content

Reduce binary size part2#5486

Merged
rapids-bot[bot] merged 8 commits intorapidsai:mainfrom
ChuckHastings:reduce_binary_size_part2
Apr 23, 2026
Merged

Reduce binary size part2#5486
rapids-bot[bot] merged 8 commits intorapidsai:mainfrom
ChuckHastings:reduce_binary_size_part2

Conversation

@ChuckHastings
Copy link
Copy Markdown
Collaborator

Add some constexpr keywords to some multi_gpu if blocks to reduce library size of libcugraph_mg. Restructured Leiden refinement code to mirror other algorithms template parameters, then a few changes to reduce library size in Leiden.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 8, 2026

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@ChuckHastings
Copy link
Copy Markdown
Collaborator Author

/ok to test 5735572

@ChuckHastings ChuckHastings added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 8, 2026
@ChuckHastings
Copy link
Copy Markdown
Collaborator Author

/ok to test e888c3b

@ChuckHastings ChuckHastings marked this pull request as ready for review April 10, 2026 18:27
@ChuckHastings ChuckHastings requested review from a team as code owners April 10, 2026 18:27
Copy link
Copy Markdown
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

LGTM except for minor cosmetic issues.

rmm::device_uvector<vertex_t>&& d_srcs,
rmm::device_uvector<vertex_t>&& d_dsts)
{
using edge_t = vertex_t;
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.

Should we better avoid this?

We currently don't build for edge_t != vertex_t cases in our regular release but we still allow users to manually build for these cases.

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.

Or is there a guarantee that # edges <= # vertices in this use case? In that case, we should add a comment here to avoid future confusion.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I've added a comment. The number of entries in d_srcs/d_dsts starts as the number of vertices in the graph and then gets filtered down, so it's guaranteed to fit in a vertex_t.


cugraph::graph_t<vertex_t, edge_t, decision_store_transposed, multi_gpu> decision_graph(handle);

std::optional<rmm::device_uvector<vertex_t>> renumber_map{std::nullopt};
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.

Something very tedious, but in general, it is better to minimize the scope of a variable. We can define this after the shuffle operation below.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I moved it down.

@ChuckHastings
Copy link
Copy Markdown
Collaborator Author

Fix is coming in #5483

@ChuckHastings
Copy link
Copy Markdown
Collaborator Author

/merge

@rapids-bot rapids-bot Bot merged commit ccc5bda into rapidsai:main Apr 23, 2026
94 of 97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants