Skip to content

perf: lazily initialize ScanHashMap resize targets - #363

Closed
thweetkomputer wants to merge 1 commit into
eloqdata:mainfrom
thweetkomputer:perf/lazy-scan-hash-buckets
Closed

thweetkomputer wants to merge 1 commit into
eloqdata:mainfrom
thweetkomputer:perf/lazy-scan-hash-buckets

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Large ScanHashMap resizes currently initialize the entire destination bucket array synchronously before starting incremental rehash. The triggering mutation therefore pays an initialization cost proportional to the new table size.

Initialize resize targets larger than 64 KiB in 64 KiB blocks on first write. A bitmap makes untouched blocks read as empty across lookup, address validation, admission estimates, scans, deletion, and cleanup. Early inserts and migration share the initialization state, so migration cannot erase an early write. Migration prepares destinations even for empty source buckets; all buckets are constructed and the bitmap is discarded before the target becomes primary and concurrent reader admission reopens.

Small tables and recovery preallocation remain eager. Primary-table Find and candidate lookup compile out initialization checks, and lookup wrappers explicitly retain inlining. Resize metadata shares the optional overflow allocation, and the lazy flag fits in existing table padding (sizeof(ScanHashMap<uint64_t>) remains 128 bytes in the measured build). Full bucket-array allocation/accounting and final release remain synchronous. This bounds explicit initialization per block, not allocator or OS latency; total initialization work moves into writes and migration steps.

Validation:

  • Debug build with LAVIK_ENABLE_WERROR=ON.
  • All 1,030 registered lavik_unit_tests cases passed using CTest process isolation.
  • All 43 ScanHashMapTest cases passed in a standalone UBSan build (-fsanitize=undefined -fno-sanitize-recover=all).
  • Added coverage for early writes and overflow chains, untouched-block reads and scans, migration during SCAN, bitmap word boundaries, sparse shrinking, move/detach/clear accounting, failed resize admission rollback, and concurrent reader reopening.
  • Formatting hooks and git diff --check passed.
  • Running the entire unit binary in one process also exposed a TCP test failure outside the changed code: ReceiveModes/TcpStreamCloseTest.PendingReceiveDoesNotKeepPeerConnected/0 receives EINTR from recv. Its two parameterizations passed 10 focused repetitions, and both passed under CTest. No end-to-end Valkey compatibility or production throughput benchmark was run.

Local microbenchmark against 2d8a293dc on Linux ARM64, GCC 13.3, -O3 -DNDEBUG -march=native, pinned to CPU 7. Each map starts with nine entries per source bucket. Time the next insertion that starts growth, each subsequent Maintain() call, then two million successful const lookups after migration. Each cell below is the median of seven rounds; the step column is the median of each round's p99.9. These are synthetic local measurements, not an end-to-end latency claim.

Destination array Expansion-triggering insert, before → after Migration step p99.9, before → after Settled lookup, before → after
128 KiB 1.084 → 1.084 μs 0.126 → 0.375 μs 2.795 → 2.693 ns
256 KiB 3.958 → 0.709 μs 0.208 → 0.250 μs 3.756 → 3.582 ns
4 MiB 58.041 → 2.208 μs 0.208 → 1.333 μs 4.517 → 4.418 ns
32 MiB 971.419 → 5.917 μs 0.125 → 2.667 μs 20.544 → 21.937 ns

For the 32 MiB target, the slowest growth-triggering insertion across those seven rounds changed from 6,018.265 to 20.875 μs. The slowest individual migration step changed from 37.708 to 63.625 μs, reflecting the deferred first-touch work and system noise. The largest-table settled lookup was about 7% slower in this sample; the other sizes were similar or faster, and primary lookup performs no bitmap access.

Standalone benchmark source and run settings

Compile this source with src/memory.cpp and src/storage/format.cpp, the repository's mimalloc and Abseil static libraries, and the include directories for Lavik, mimalloc, and Abseil. Use C++23 and the compiler flags above. The baseline uses the header at 2d8a293dc; the changed executable uses this PR's header, with other source and library inputs identical. Run each executable with taskset -c 7 ./bench <exponent> 7 for exponents 10, 11, 15, and 18 (source bucket counts are powers of two). Both command arguments are required.

#include "lavik/storage/scan_hash_map.h"
#include <algorithm>
#include <chrono>
#include <cstdio>
#include <cstdlib>
#include <vector>

using Map = lavik::storage::ScanHashMap<std::uint64_t>;
using lavik::storage::Digest;
using Clock = std::chrono::steady_clock;

double Us(Clock::duration value) {
  return std::chrono::duration<double, std::micro>(value).count();
}

int main(int argc, char** argv) {
  if (argc != 3) return 2;
  const auto buckets = std::size_t{1} << std::atoi(argv[1]);
  const int rounds = std::atoi(argv[2]);
  if (!lavik::InitMemoryLimit(4ULL << 30, 1).ok()) return 3;
  lavik::BindMemoryAccountingShard(0);
  std::printf("round,new_table_bytes,expand_us,step_p999_us,step_max_us,settled_lookup_ns,map_bytes\n");
  for (int round = 0; round < rounds; ++round) {
    Map map;
    const std::size_t size = 9 * buckets;
    if (!map.PreallocateForExpectedSize(size)) return 4;
    for (std::size_t i = 0; i < size; ++i) {
      if (!map.InsertNew(Digest{i}, "external", i, false)) return 5;
    }
    const auto begin = Clock::now();
    if (!map.InsertNew(Digest{size}, "external", size, false)) return 6;
    const double expansion = Us(Clock::now() - begin);
    std::vector<double> steps;
    steps.reserve(buckets);
    while (map.rehashing()) {
      const auto start = Clock::now();
      if (!map.Maintain()) return 7;
      steps.push_back(Us(Clock::now() - start));
    }
    std::sort(steps.begin(), steps.end());
    constexpr std::size_t lookups = 2000000;
    std::uint64_t sum = 0;
    const auto lookup_begin = Clock::now();
    for (std::uint64_t i = 0; i < lookups; ++i) {
      const std::uint64_t key = (i * 0x9e3779b97f4a7c15ULL) & (buckets - 1);
      const auto* entry = std::as_const(map).Find(Digest{key}, "external");
      if (!entry) return 8;
      sum += entry->value();
    }
    const double lookup_ns = Us(Clock::now() - lookup_begin) * 1000 / lookups;
    if (sum == 0) return 9;
    std::printf("%d,%zu,%.3f,%.3f,%.3f,%.3f,%zu\n", round,
                2 * buckets * 64, expansion, steps[steps.size()*999/1000],
                steps.back(), lookup_ns, sizeof(Map));
    std::fflush(stdout);
  }
}

Summary by CodeRabbit

  • Performance
    • Improved hash map resizing by deferring initialization of large bucket ranges, reducing unnecessary work during growth and shrink operations.
    • Preserved safe, consistent lookups and scans while resizing is in progress.
  • Reliability
    • Strengthened handling of concurrent reads, overflow chains, migrations, and rollback scenarios during capacity changes.
  • Capacity Management
    • Improved memory accounting and tracking for partially initialized storage during resize operations.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b39d852e-49f2-4b3e-81a7-a45954a9ed8b
📥 Commits

Reviewing files that changed from the base of the PR and between 2d8a293 and 0328b96.

📒 Files selected for processing: 3
  • include/lavik/storage/scan_hash_map.h
  • tests/record_location_test.cpp
  • tests/scan_hash_map_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

ScanHashMap now defers bucket construction for sufficiently large resize targets. Reads and scans skip uninitialized blocks, while writes construct a block before mutation. Rehash completion finalizes the target table. New tests cover growth, shrink, concurrent reads, metadata ownership, and allocation rollback.

Changes

Lazy ScanHashMap bucket initialization

Layer / File(s) Summary
Deferred bucket storage and accounting
include/lavik/storage/scan_hash_map.h
Large resize targets can defer bucket construction in blocks. The map tracks initialized blocks, reports uninitialized bucket capacity, and accounts for bitmap metadata in allocation estimates.
Resize and rehash lifecycle
include/lavik/storage/scan_hash_map.h
Expansion and shrink targets use deferred initialization. Rehash operations access target buckets through initialization-aware helpers and finalize the target before promotion.
Bucket access and migration coverage
include/lavik/storage/scan_hash_map.h, tests/scan_hash_map_test.cpp, tests/record_location_test.cpp
Lookup, mutation, scanning, prefetch, and destruction paths use initialization-aware bucket access. Tests cover resize behavior, concurrent reads, ownership changes, bitmap accounting, and allocation rollback.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant ScanHashMap
  participant Table
  participant OverflowBuckets
  ScanHashMap->>Table: Allocate resize target with deferred initialization
  Table->>OverflowBuckets: Record uninitialized blocks
  ScanHashMap->>Table: ReadBucket for an uninitialized block
  Table-->>ScanHashMap: Return null
  ScanHashMap->>Table: WriteBucket for a target block
  Table->>OverflowBuckets: Mark the constructed block initialized
  ScanHashMap->>Table: Finish bucket initialization after rehash
  Table->>OverflowBuckets: Verify all blocks and clear lazy state
Loading
Merge Risk: ⚪ Minimal · up to 0328b

No identified issue currently blocks merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 21.95% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: lazy initialization of ScanHashMap resize targets.
Description check Passed The description is detailed and covers the context, behavior changes, implementation, design decisions, testing, performance results, and risks. It does not use the template headings and does not expl…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and covers the context, behavior changes, implementation, design decisions, testing, performance results, and risks. It does not use the template headings and does not explicitly provide rollback, reviewer guide, or follow-up sections, but the required implementation and validation information is mostly complete.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

I’m a rabbit watching buckets grow,
New blocks wait until writes say go.
Reads skip gaps along the way,
Rehash finishes; blocks join the array.
I twitch my nose and hop away.

Comment @coderabbitai help to get the list of available commands.

@thweetkomputer
thweetkomputer deleted the perf/lazy-scan-hash-buckets branch October 20, 2026 10:42
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.

1 participant