feat: SSE4.2 SIMD path, double-precision interpolation, OpenMP auto-enable for AVX1-only CPUs - #1
Conversation
New C API that pre-populates the anchor table with evenly-spaced anchors from a sorted array. This "warms up" the interpolation search table so the first batch of lookups benefits from good anchor coverage without needing to learn anchors one-by-one from search misses. Useful for batch ingestion workflows where the full dataset is loaded before lookups begin. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…nable for AVX1-only CPUs Security fixes (P1/P2): - P1: tar.zst parser OOB read — bounded streaming integer parser - P1: CUDA cache race — reader-lease protocol with refcount + generation - P2: archive index min/max recomputed after sort - P2: auto-backend data races fixed with mutexes - P2: signed overflow in query-shape classifier using __int128 - P2: FNV hash collision verification against source bytes - QIHSE ingestion_principal security context through bridge Performance: - Archive index keys retention (eliminates repeat decompression) - LSD radix sort for hash index (O(n) vs qsort) - Zero-copy NumPy batch API (15.6x faster on 1M queries) Sandy Bridge / AVX1-only CPU support: - SSE4.2 branchless SIMD path (128-bit PCMPEQQ, 2x unrolled) - Double-precision interpolation (replaces __int128 div, 2x single-key speedup) - Software prefetch enabled for SSE4.2 (was #ifdef'd out on AVX1-only) - Branchless scalar fallback (enables auto-vectorization) - Wider SIMD scan window (64 on SSE4.2+, was 32) - OpenMP auto-enabled by default (2x batch speedup on 8-core) - Lowered parallel threshold to 4096 (was 16384) Benchmark (Xeon E5-2407, 2.2GHz, 8-core, AVX1+SSE4.2): Single-key: 317ns -> 157ns (2.0x) Batch+OpenMP: N/A -> 165ns (2.4x vs serial) Small-window: 125ns -> 82ns (1.5x) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b14d0ce330
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (size_t i = 0; i < count; i++) { | ||
| uint8_t bucket = (uint8_t)((uint64_t)keys[i] >> shift); | ||
| hist[bucket]++; |
There was a problem hiding this comment.
The radix passes order hashes by their unsigned bit patterns, but every KEYSTONE search and bounds comparison treats the array as signed int64_t. Once an index of at least 32 entries contains both positive and negative FNV hashes, positives precede negatives, so the array endpoints are reversed in signed order and keystone_search can reject every lookup at its bounds check. Transform the most-significant byte with the sign bit (or otherwise sort in signed order) before marking the index finalized.
Useful? React with 👍 / 👎.
| if (active_table->anchors[i].i == l->i || | ||
| active_table->anchors[i].i == r->i) { |
There was a problem hiding this comment.
When a successful irregular search triggers anchor learning, keystone_learn_anchor() can realloc the anchor array or shift it with memmove; the subsequent dereferences of l and r therefore read dangling pointers or anchors that no longer represent the original bounds. This can cause a use-after-free when the table grows and updates the wrong LRU entries otherwise. Save the two bounding indices before calling the mutating function, then use those saved values.
Useful? React with 👍 / 👎.
| for (i = 0u; i < anchor_count; i++) { | ||
| size_t idx = (n * i) / anchor_count; |
There was a problem hiding this comment.
For every anchor_count < n, the formula's final sample is floor(n * (anchor_count - 1) / anchor_count), never n - 1. Since keystone_search() rejects keys above the last anchor, a table seeded with, for example, 10 anchors over 1,000 elements reports indices 901–999 as absent. Ensure the seed set includes both array endpoints, such as by spacing over n - 1.
Useful? React with 👍 / 👎.
| if not isinstance(arr, np.ndarray) or arr.dtype != np.int64: | ||
| arr = np.ascontiguousarray(np.array(arr, dtype=np.int64)) | ||
| if not isinstance(keys, np.ndarray) or keys.dtype != np.int64: | ||
| keys = np.ascontiguousarray(np.array(keys, dtype=np.int64)) |
There was a problem hiding this comment.
When an input is already an int64 NumPy array but is a non-contiguous view such as np.arange(100, dtype=np.int64)[::2], these conditions skip conversion and pass its first-data pointer to C as if elements were contiguous. The native function consequently searches adjacent backing-buffer values rather than the logical array or reads the wrong query keys. Check flags.c_contiguous for both arrays and copy non-contiguous views before taking their pointers.
Useful? React with 👍 / 👎.
| if (idx->string_lens[result] != query_len || | ||
| memcmp(idx->strings[result], query_str, query_len) != 0) { | ||
| /* Hash collision — the key is not actually present. | ||
| * (If duplicate hashes with different strings are expected, | ||
| * a linear probe around this index would find the real match. | ||
| * For now, we treat collision as not-found, which is safe.) */ | ||
| return KEYSTONE_NOT_FOUND; |
There was a problem hiding this comment.
If two different indexed strings share an FNV-1a hash, KEYSTONE may return either equal-hash entry; returning NOT_FOUND as soon as that candidate's bytes differ creates a false negative even when another adjacent entry contains the requested string. Collision verification needs to scan the full contiguous run of the matching hash and compare each retained string before concluding absence.
Useful? React with 👍 / 👎.
…ache Adopted main's lock-free double-buffered cache design (CAS-based, 2 slots, per-slot streams) as the base — it provides better concurrency than the reader-lease mutex protocol. Preserved our additions on top: - dataset_version field in cache slots + cache hit check - keystone_search_batch_cuda_versioned() API - keystone_cuda_cache_invalidate() adapted to lock-free design - Removed pthread dependency (no longer needed) Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
Summary
Security fixes (P1/P2)
strtoll()with bounded streaming integer parserfirst_key/last_keyrecomputed after sort for unsorted source datag_backend_cacheandg_last_backend_decision__int128math for key deltasqihse_kv_set_user()Performance
Sandy Bridge / AVX1-only CPU support
__int128division (80-100 cycle libgcc call) with double fast path#ifdef'd out on AVX1-only CPUsBenchmark results (Xeon E5-2407, 2.2GHz, 8-core, AVX1+SSE4.2)
Test plan
-O3 -march=native -Wall -Wextra -Werror=implicit-function-declaration12 passed, 0 failed)Generated with Devin