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

Reduce cagra build binary size #334

Open
wants to merge 4 commits into
base: branch-24.10
Choose a base branch
from

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Sep 18, 2024

During cagra::build we use either ivf_pq or nn_descent. Until now we used these algorithm through the detail namespace, which resulted in recompilation. This PR changes to the public interface of ivf_pq and nn_descent to avoid recompilation.

@tfeher tfeher requested a review from a team as a code owner September 18, 2024 22:36
@github-actions github-actions bot added the cpp label Sep 18, 2024
@tfeher tfeher added improvement Improves an existing functionality non-breaking Introduces a non-breaking change and removed cpp labels Sep 18, 2024
@github-actions github-actions bot added the cpp label Sep 18, 2024
@tfeher
Copy link
Contributor Author

tfeher commented Sep 19, 2024

Currently we have around 13 MiB reduction in cagra_build, which is around 33% reduction for those binaries. The binaries are still larger than I hoped them to be, investigating.

@@ -357,8 +353,7 @@ void build_knn_graph(
raft::host_matrix_view<IdxT, int64_t, raft::row_major> knn_graph,
cuvs::neighbors::nn_descent::index_params build_params)
{
auto nn_descent_idx = cuvs::neighbors::nn_descent::index<IdxT>(res, knn_graph);
cuvs::neighbors::nn_descent::build<DataT, IdxT>(res, build_params, dataset, nn_descent_idx);
auto nn_descent_idx = cuvs::neighbors::nn_descent::build(res, build_params, dataset);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is relevant: #264 (comment)

@tfeher
Copy link
Contributor Author

tfeher commented Sep 28, 2024

Most of these changes were already contained in #264. We keep this open to test further improvements in build binary size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants