Repository navigation
perf(storage): prefetch flush lookups and vectorize address filtering - #312
Closed
thweetkomputer wants to merge 1 commit into
Closed
thweetkomputer wants to merge 1 commit into
thweetkomputer wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thweetkomputer
added a commit
to thweetkomputer/lavik
that referenced
this pull request
Oct 12, 2026
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
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.
Context
Draft: the first direct main A/B does not demonstrate a throughput improvement. Please do not merge on the basis of the earlier combined-branch result.
Random SET workloads leave a batch of flush receipts that must each locate their still-live index entry. On main, these independent random metadata/bucket loads are consumed serially, and tagged address lookup scans every bucket slot.
Behavior before and after
The SET and persistence semantics are unchanged. Flush overlaps future index metadata and bucket loads with earlier receipts, and tagged address lookup checks only the existing SIMD fingerprint candidates before validating occupancy and exact address.
Implementation
ScanHashMap.FlushPendingBlocks.MatchingTagsfor taggedFindAddress; preserve the untagged path, both rehash tables, overflow chains, live-handle validation, and exact address comparison.Design decisions and alternatives
This is based directly on main and contains no connection-worker GET or reader-admission changes. Prefetch distances are fixed hints; missing addresses and stale index generations are skipped. No entry pointer is retained by the hints. SIMD matches are only a filter: occupied-slot and exact-address checks remain mandatory.
Documentation and comments
Ownership and hint-only semantics are documented beside the helpers and flush loop. No architecture claim, durable format, ownership boundary, or lifecycle changes.
Test plan
cmake --build build --target lavik lavik_set_admission_tests --parallel 16passed. The temporary focused target contains record-location, scan-hash-map, transaction-lock and device-affinity tests.build/lavik_set_admission_tests: 54 tests passed, including all 256 fingerprints, holes/collisions, stale addresses, address reuse, overflow chains, and both rehash tables.PYTHONPATH="$PWD/tests" LAVIK_TEST_DATA_DIR=/mnt/dev/benchmark/get-update-build-tests python3 /tmp/main-redis-protocol-e2e.py build/lavik: passed GET/SET correctness, concurrent transactions, FLUSHDB and cold recovery. This fixture was copied from the connection-worker branch; main uses its normal owner-worker GET path.clang-format23.1.1 dry run andgit diff --check: passed. Full repository test suite was not run.Direct main SET A/B on Azure L16aos_v4 (16 data workers, six NVMe SPDK, kernel/io_uring networking; 32-thread F32als_v6 client): 1 billion keys, 1 KiB values, pipeline=1, 1,280 connections, 30-second warmup and 3×60-second measured runs per version.
1eacae54Observed throughput difference: -1.59%, not an improvement. No command/connection errors or VF drop/discard increases; dataset size and sampled value lengths remained valid. The versions ran sequentially on a reused, continually updated dataset, so background/storage-state effects remain. Subsequent main recheck: [869059.49, 868567.3, 867362.7] QPS, median 868,567.30, p99 median 8.511 ms. This patch is -1.10% versus that recheck. This run does not establish a throughput improvement, so the PR remains draft. The earlier +6.29% result on a different combined patch is not evidence for this PR.
Risk assessment
Prefetch effectiveness depends on CPU, cache pressure, and workload. Address filtering retains all membership and physical-boundary checks. Performance observations are limited to the tested hardware and sequential reused dataset.
Rollback plan
Revert this commit; no data migration or configuration change is needed.
Reviewer guide
Review the tagged lookup filter in
scan_hash_map.h, then the lookahead hints inflush.cpp. Existing fingerprint/holes/collision/stale-address/rehash tests exercise the changed lookup.Follow-up work
Measure representative workloads and other hardware before tuning the fixed lookahead distances.
Checkpoint verification: every group cleanly restarted with
--shutdown-checkpoint. Logs confirm successful loads of exactly 1,000,000,000 entries before warmup: main=generation 15, main-opt=generation 16, get-base=generation 17, get-opt=generation 18, get-base-recheck=generation 19, main-recheck=generation 20. Checkpoint recovery preallocates indexes from their stored entry counts; these were not cold-scan starts. We did not collect a runtime rehash counter and do not claim a direct measurement of zero rehash activity.