perf(brute_force): drop the duplicate bitset/bitmap→CSR conversion on the SDDMM path

[maxwbuckley]

The problem

On the sparse (SDDMM) branch of brute_force_search_filtered, the filter is converted to CSR twice per search:

auto csr = raft::make_device_csr_matrix<DistanceT, IdxT>(res, n_queries, n_dataset, nnz_h);
std::visit([&](const auto& actual_view) { actual_view.to_csr(res, csr); }, *filter_view);   // (1)
...
raft::sparse::linalg::masked_matmul(res, queries, dataset_view, actual_view, csr_view);     // (2)

masked_matmul builds its own CSR over the caller's structure view and calls bitset_to_csr / bitmap_to_csr on it:

auto C_matrix = raft::make_device_csr_matrix<output_t, index_t>(handle, compressed_C_view);
raft::sparse::convert::bitset_to_csr(handle, mask, C_matrix);

Because C_matrix is sparsity-preserving, it shares indptr/indices with the CSR cuVS just filled. So (2) recomputes the identical structure over the top of (1). The second conversion is not an implementation detail we are relying on by accident — it is masked_matmul's documented job, and RAFT's own masked_matmul test passes a C whose structure is pre-populated from a CPU reference and depends on it being overwritten.

This removes cuVS's call and lets masked_matmul do the conversion once.

Evidence that both conversions really ran

nsys over 21 SDDMM-path searches (1M x 128, 0.5% selectivity, RTX 5090, sm_120), launch counts before → after:

kernel	before	after
calc_nnz_by_rows_kernel	42	21
fill_indices_by_rows_kernel<check_nnz=false>	21	0
fill_indices_by_rows_kernel<check_nnz=true>	21	21
repeat_csr_kernel	42	21
exclusive-scan (DeviceScanKernel)	42	21
CSR value fill	42	21
cusparse::sddmm_ker	21	21
csr_to_coo_kernel	21	21

Every conversion kernel halves. sddmm_ker and csr_to_coo are unchanged, i.e. no actual work is removed. The check_nnz=false instantiation is the sparsity-owning one, which only cuVS's to_csr call could produce; it disappears entirely, leaving masked_matmul's sparsity-preserving conversion.

A host sync goes with it

to_csr on a sparsity-owning matrix copies the computed nnz back to the host and syncs the stream before sizing the matrix. The conversion inside masked_matmul runs on a sparsity-preserving matrix and takes neither branch. 8-byte device-to-host copies over 29 filtered searches drop from 50 to 29 — exactly the 21 removed readbacks.

Measurements

Paired A/B on a rebuilt libcuvs.so, 10 alternating rounds x 200 reps per config, dedicated idle RTX 5090, fp16 / L2Expanded / k=64 / 100 queries / shared bitset. CIs are Student-t over the 10 paired round-medians.

N	dim	selectivity	baseline	this PR	speedup	saving (95% CI)	t
1M	128	0.1%	0.383 ms	0.238 ms	1.61x	+136 us [93, 180]	7.1
1M	128	0.5%	0.548 ms	0.407 ms	1.35x	+138 us [106, 170]	10.9
1M	128	1.0%	0.708 ms	0.579 ms	1.22x	+133 us [115, 151]	12.7
1M	128	3.0%	1.409 ms	1.272 ms	1.11x	+139 us [123, 155]	18.4
1M	1024	0.1%	0.468 ms	0.323 ms	1.45x	+146 us [109, 183]	9.3
1M	1024	0.5%	0.783 ms	0.663 ms	1.18x	+129 us [105, 153]	12.3
1M	1024	1.0%	1.239 ms	1.082 ms	1.15x	+147 us [125, 168]	14.8
1M	1024	3.0%	4.643 ms	4.467 ms	1.04x	+169 us [124, 214]	6.7
10M	128	0.1%	0.750 ms	0.589 ms	1.27x	+158 us [134, 182]	15.9
10M	128	0.5%	2.491 ms	2.338 ms	1.07x	+152 us [139, 165]	26.5
10M	128	1.0%	4.707 ms	4.531 ms	1.04x	+201 us [140, 261]	8.2
10M	128	3.0%	14.084 ms	13.760 ms	1.02x	+335 us [252, 419]	7.9

All 12 SDDMM-path configs improve with the interval excluding zero.

The saving is a roughly fixed ~150 us per search, not a ratio, so it is largest where the search is cheapest — which is where the SDDMM path is supposed to live. Post-#2321 the sparse path is entered below ~2000 passing rows, i.e. squarely in the region where a fixed 150 us is a large fraction of the total.

Null control. Dense-path configs cannot be affected by this change and are flat, as they must be:

N	dim	selectivity	delta	95% CI
1M	128	25%	+1.8 us	[-11.3, +15.0]
1M	1024	25%	-57.6 us	[-134.0, +18.9]
10M	128	25%	+96.7 us	[-277.8, +471.1]

Every interval spans zero. An earlier 3-round pass showed apparent ±2 ms swings at 10M x 1024 in both arms including the null control, so that config is drift-dominated on this host and is excluded rather than reported.

What the saving is not

I initially read the ~150 us as the removed host sync. It is not. Feeding the popcount from an environment variable to remove the other per-search sync (bitset_view::count), with results bit-identical, is worth median +0.0 us, 0 of 15 configs significant. A sync on an already-idle stream costs nothing — count() runs before anything is queued. The win here is the removed conversion work plus the pipeline bubble from draining the stream immediately before a block of host-side cuSPARSE setup, which the sync in to_csr sits right in front of.

Correctness

Top-k ids and distances are bit-identical to the unpatched build across 14 configs (dim ∈ {128, 1024} x selectivity ∈ {0.1%, 0.5%, 1%, 3%, 5%, 10%, 25%}), spanning both dispatch paths.

NEIGHBORS_TEST --gtest_filter='*Prefiltered*': 156/156 pass (4 suites x 39 parameterisations). To be precise about what that run was: it is the branch I benchmarked on, whose checkout predates #2321, so it has 39 parameterisations where main has 45. The 6 extra ones on main are the ones #2321 added to reach the gather and dense paths, which this change does not touch. The change itself compiles clean against main.

Compiles clean against main with the build's own flags, which include -Werror=all-warnings.

Related, not included

masked_matmul also allocates nnz * sizeof(output_t) for C_matrix's element buffer that nothing ever reads — both sddmm and faster_dot_on_csr write through C.get_elements(). That is a RAFT-side fix and is memory-only (no measurable latency change), so it is not part of this PR; happy to open it against RAFT if useful.

Method

RTX 5090 (sm_120), CUDA 12.9, WSL2, dedicated idle GPU. Each arm is a separately built libcuvs.so swapped via LD_LIBRARY_PATH; arms alternate within every round and every (N, dim) block so drift hits both equally.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[maxwbuckley]

@lowener would you mind taking a look? This is the same corner of knn_brute_force.cuh as #2321 and #2128, which you reviewed.

It sits inside the sddmm branch whose entry conditions #2321 rewrote, and it does not touch that dispatch — the gather and dense paths are unchanged, and the three dense configs in the table are there as a null control precisely to show that.

The one thing worth a reviewer's eye is the assumption the change rests on: that masked_matmul populating the CSR structure from the mask is contract, not incidental. RAFT's own masked_matmul test passes a C whose structure is pre-filled from a CPU reference and relies on it being overwritten, which is what convinced me, but you would know better than I do whether that is something cuVS should depend on or whether the conversion belongs on the cuVS side with the RAFT one removed instead. Either direction removes the duplicate; I picked the one that needs no RAFT release.

(No permission to add reviewers from a fork, hence the mention.)