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

vector-search: avoid unaligned reads #1810

Merged
merged 7 commits into from
Nov 24, 2024

Conversation

sivukhin
Copy link
Contributor

@sivukhin sivukhin commented Nov 11, 2024

Now vector search can produce unaligned reads because node metadata is not aligned to word boundary (it has size of 10 bytes) and if we will store F32/F64 vectors in the node - they will be be accessed with half-word offset.

You can test the issue like this:

$> make CC=clang CFLAGS='-fsanitize=alignment' testfixture
$> ./testfixture test/libsql_vector*
...
vector-f32-compress-bf16...sqlite3.c:212025:39: runtime error: load of misaligned address 0x5d89bb2b0b1a for type 'float', which requires 4 byte alignment
0x5d89bb2b0b1a: note: pointer points here
...

This can create real trouble on some architectures. For example, seems like Cortex-A series of ARM CPU supports unaligned reads but this capability must be enabled explicitly (see https://developer.arm.com/documentation/den0013/d/Porting/Alignment)

For the Cortex-A series of processors, unaligned accesses are supported, although you must enable this by setting the U bit in the CP15:SCTL register, indicating that unaligned accesses are permitted.

The complication in the fix lying in the fact that we reuse memory from sqlite blobs a lot in the vector search code and quite often instead of additional allocation we just initialize vector from blob memory (see vectorInitStatic).

So, due to the complication, this PR attack the problem from format perspective and align metadata to double-word boundary - making read to have proper alignment.

Note, that PR introduce V3 on-disk binary format with the fix and unaligned reads will be fixed only for newly created indices (old ones will continue to work as it is - but will have potential unaligned reads issue)

@sivukhin sivukhin force-pushed the vector-search-avoid-unaligned-reads branch from b6255ec to b11c0c5 Compare November 12, 2024 20:40
Copy link
Contributor

@haaawk haaawk left a comment

Choose a reason for hiding this comment

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

LGTM modulo my misunderstanding of the comment

@sivukhin sivukhin added this pull request to the merge queue Nov 24, 2024
Merged via the queue into main with commit a77f422 Nov 24, 2024
19 checks passed
@sivukhin sivukhin deleted the vector-search-avoid-unaligned-reads branch November 24, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants