Skip to content

Reduce binary size part2 - #5486

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

rapids-bot[bot] merged 8 commits into
rapidsai: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-pr-bot Bot commented Apr 8, 2026

Copy link
Copy Markdown

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

@seunghwak seunghwak left a comment

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.

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
@ChuckHastings
ChuckHastings deleted the reduce_binary_size_part2 branch September 11, 2026 16:08
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