Skip to content

Conversation

@BubbleCal
Copy link
Contributor

@BubbleCal BubbleCal commented Dec 29, 2025

No description provided.

@github-actions
Copy link
Contributor

Code Review

Summary: This PR optimizes HNSW index building by removing atomic operations from the hot path. Instead of incrementing level counts atomically during each parallel insertion, the counts are computed once after the build completes.

Assessment: LGTM ✅

The change is correct and the optimization is sound:

  1. Correctness: num_nodes() is not called during the parallel build phase, so deferring level count computation doesn't affect build behavior. The final compute_level_count() correctly iterates over all nodes.

  2. Performance: Eliminates atomic fetch_add operations from the hot insertion path, which should reduce contention in the parallel loop.

  3. Testing: The new test test_level_offsets_match_batch_rows properly validates that level offsets are correctly computed and match the final batch size.

No issues found.

Signed-off-by: BubbleCal <[email protected]>
@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-index/src/vector/hnsw/builder.rs 97.14% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

Look good!

@wjones127 wjones127 merged commit f20e0a9 into main Jan 1, 2026
30 of 31 checks passed
@wjones127 wjones127 deleted the hnsw-levelcount-fs branch January 1, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants