[8.2] MOD-14916 Devirtualize HNSW / brute-force search hot path#940
Merged
[8.2] MOD-14916 Devirtualize HNSW / brute-force search hot path#940
Conversation
* MOD-14916 Devirtualize distance + getElement on HNSW search hot path
MOD-14916 / LTK perf investigation.
Two virtual dispatches per HNSW candidate added between v2.10.21 and
v8.2.6 account for a measurable share of the KNN regression observed
in the LTK benchmarks (-38% throughput on Intel). Both are removed
here with the minimum possible change.
V1 - distance computation:
Every calcDistance() call goes through IndexCalculatorInterface's
vtable to reach DistanceCalculatorCommon, which then calls the
underlying SIMD function pointer. The intermediate vtable hop is
pure indirection; the concrete calculator class is fixed for the
life of an index.
Expose the underlying dist_func via a new pure-virtual
getDistFunc() on IndexCalculatorInterface, implemented by
DistanceCalculatorCommon. Cache the returned function pointer in
VecSimIndexAbstract at construction time and call it directly in
calcDistance(), bypassing the virtual dispatch.
V2 - vector fetch:
HNSWIndex::getDataByInternalId and BruteForceIndex::getDataByInternalId
call this->vectors->getElement(id), which is virtual through the
RawDataContainer base. DataBlocksContainer is the only concrete
implementation, and this->vectors is always a DataBlocksContainer
(created and owned by VecSimIndexAbstract's constructor).
Use a static_cast to DataBlocksContainer* plus a qualified call to
DataBlocksContainer::getElement to skip the vtable lookup.
No behavior change; per-candidate distance and neighbor-fetch calls
on HNSW / brute force search paths become direct function-pointer /
direct-member calls. Headers in index_factories, hnsw_serializer,
and brute_force_factory compile cleanly.
* MOD-14916 Inline DataBlocksContainer::getElement on HNSW search hot path
Follow-up to the previous V1/V2 devirt commit. The static_cast+qualified
call in getDataByInternalId removed the vtable lookup but left the larger
cost on the table: DataBlocksContainer::getElement was still defined in
data_blocks_container.cpp, so every per-candidate neighbor fetch still
paid a real out-of-line function call and a bounds-checked blocks.at()
lookup. Without LTO the compiler could neither inline the body nor hoist
the div/mod in the HNSW hot loop.
Move the definition into the header as inline and drop the .at() bounds
check to match the v2.10.21 baseline, which used unchecked operator[] and
was fully inlined into processCandidate.
Also add a getDistFunc() override to DistanceCalculatorDummy in
test_components.cpp so BUILD_TESTS still compiles after the pure virtual
added in the previous commit.
(cherry picked from commit 4ca500a)
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
GuyAv46
approved these changes
Apr 20, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.2 #940 +/- ##
==========================================
- Coverage 96.98% 96.96% -0.03%
==========================================
Files 126 126
Lines 7366 7371 +5
==========================================
+ Hits 7144 7147 +3
- Misses 222 224 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport of #937 to
8.2.Pull Request opened by Augment Code with guidance from the PR author
Note
Medium Risk
Touches core HNSW/brute-force search hot paths by bypassing virtual dispatch and caching function pointers; incorrect assumptions (e.g.,
vectorstype or nullcachedDistFunc) could lead to crashes or subtle runtime issues.Overview
Devirtualizes VecSim search hot paths by bypassing
RawDataContainer/calculator vtables.HNSW and brute-force now fetch vector data via an explicit cast to
DataBlocksContaineringetDataByInternalId, andDataBlocksContainer::getElementis inlined (moved from.cppto header) to help the compiler fold indexing arithmetic.VecSimIndexAbstractnow caches the raw distance function pointer (cachedDistFunc) fromIndexCalculatorInterface::getDistFunc()and uses it incalcDistanceinstead of virtualindexCalculator->calcDistance; unit tests update the dummy calculator to returnnullptrfor this new API when a nonstandard signature is used.Reviewed by Cursor Bugbot for commit c02982a. Bugbot is set up for automated code reviews on this repo. Configure here.