bench(lite): align reproducible acceptance tools with Lite feature baseline

[nw9663644-eng]

Purpose

Provide reproducible benchmarks and acceptance evidence for VSAG Lite.
Closes #2888. Depends on #2904, which owns the Lite implementation, API, architecture and functional tests.

Contents

• Data preparation for SIFT, GIST and Cohere.
• Lite/native Full benchmarks, real-change CRUD, CPU/memory, package-size and recall checks, with regression tests.
• A comprehensive report linking configurations, measured commits and raw results. Positive and negative results are retained.

Historical exploration tools are archived at pinned commits rather than included in the current review tree. Until #2904 merges, the main-target diff also includes its feature dependency.

Validation

• Lite Release: RaBitQ ON 5/5, OFF 4/4; benchmark targets compile.
• Three native Full consumers compile against the recorded historical SDK.
• Python guards: 26 passed, 2 skipped with optional dependencies, including four normal Cohere unittest cases. CI installs these dependencies and checks imports before discovery.
• Latest SIFT/GIST resident run: 270,000 mutation calls and 270,000 interleaved queries; raw verification passed.

These are recorded local results, not a claim that hosted CI has passed. Restoring Cohere tools did not rerun Cohere performance.

Results and Limits

Defined package profiles and some load/memory comparisons show clear benefits. RaBitQ is not consistently faster than FP16; GIST dynamic quality, a unified same-quality/SIMD-aligned Full comparison and long-duration 100k/Cohere validation remain open. Historical and candidate results are explicitly separated from current production results.

Report

Read the comprehensive acceptance report.

Review follow-up: stream-position validation, consistent percentile bounds, compiled helper regressions and file-hash naming were addressed. Native Full was smoke-tested with dimension 16 and k=3 to confirm its result-layout contract; 13 representative inline threads received evidence-based replies. Existing historical measurements and Lite implementation are unchanged. Other historical/duplicate comments have not all been resolved.

[vsag-bot]

/label status/waiting-for-review
/waiting-on reviewer
/request-review @jiaweizone
/request-review @wxyucs
/request-review @inabao

[mergify]

Merge Protections

🔴 1 of 2 protections blocking · waiting on 🙋 you

	Protection	Waiting on
🔴	Require kind label	🙋 you
🟢	Require version label	—

🔴 Require kind label

Waiting for

• label~=^kind/

This rule is failing.

• label~=^kind/

---

Show 1 satisfied protection

🟢 Require version label

• label~=^version/

[LHT129]

I have completed a thorough review of all 16 changed files (2,040 additions). The code quality is high across the board.

Summary of findings:

The existing inline comments from @LHT129 already cover the notable points:

• src/lite/index.cpp: float value copy vs const auto& consistency in Save loop
• lite/benchmark/main.cpp: percentile() pass-by-value copies the vector on every call
• lite/benchmark/main.cpp: peak_rss_kib() uses getrusage(RUSAGE_SELF) — ru_maxrss is in KiB on Linux but bytes on macOS (documented as Linux-only, so not a practical issue)
• lite/benchmark/run_baseline.sh: lscpu is Linux-specific (consistent with the documented Linux x86_64 scope)

Additional observations (no new inline comments needed):

1. The make_vector refactoring in this commit (from XOR-combined mix calls to chained mix calls) improves clarity and matches full_main.cpp. Good change.

2. src/lite/index.cpp — The Index::Load function uses return std::move(*created); where the move is redundant (the compiler would move automatically from the dereferenced unique_ptr). This is harmless but unnecessary.

3. lite/benchmark/main.cpp — run_stability calls peak_rss_kib() per round, which reports the cumulative process-wide ru_maxrss. For a stability experiment this is actually the desired behavior (it detects monotonic memory growth), so the semantics are correct for the use case.

4. The snapshot format (VSAGLT01) is well-designed with explicit little-endian byte encoding, bounds validation before allocation, and rejection of truncation, trailing bytes, duplicate IDs, and non-finite values. The test coverage for malformed snapshots is thorough.

5. The Add implementation correctly uses geometric capacity growth and publishes the slot map entry only after all potentially-throwing vector operations succeed — this is proper exception safety.

6. The CI workflow (.github/workflows/lite.yml) correctly verifies that libvsag-lite.so does not link against libvsag.so, which is the key isolation guarantee.

No blocking issues found. The existing inline comments are all [note] or [suggestion] level and none should block merging.

[LHT129]

[suggestion] The loop below iterates k times unconditionally, reading GetIds()[i] and GetDistances()[i] without verifying that the result actually contains k elements. Combined with the GetDim() check on line 151 (which validates the vector dimension, not the result count), this can read out-of-bounds if KnnSearch returns fewer than k results.

While the benchmark currently operates on indices that always have enough vectors to satisfy k, a future change or an unexpected internal error could trigger undefined behavior here.

Consider either:

• Fixing line 151 to use GetNumElements() (as noted in the critical comment above), which would make this loop safe by failing early, or
• Adding an explicit bounds check before the loop:

require(static_cast<uint64_t>((*result)->GetNumElements()) >= k, "too few search results");

Current:

    for (uint64_t i = 0; i < k; ++i) {
        neighbors.push_back({(*result)->GetIds()[i], (*result)->GetDistances()[i]});
    }

[LHT129]

[critical] The percentile function in dataset_main.cpp is missing the std::min guard present in the same function in main.cpp and full_main.cpp. When fraction=0 and values is non-empty, rank = ceil(0) - 1 = 0 - 1 underflows to UINT64_MAX, causing an out-of-bounds access on values[rank].

Compare with the safe version in main.cpp:

return values[std::min<uint64_t>(rank, values.size() - 1)];

Current (dataset_main.cpp):

double
percentile(std::vector<double> values, double fraction) {
    std::sort(values.begin(), values.end());
    const auto rank =
        static_cast<uint64_t>(std::ceil(fraction * static_cast<double>(values.size()))) - 1;
    return values[rank];
}

Suggested fix:

double
percentile(std::vector<double> values, double fraction) {
    std::sort(values.begin(), values.end());
    const auto rank =
        static_cast<uint64_t>(std::ceil(fraction * static_cast<double>(values.size()))) - 1;
    return values[std::min<uint64_t>(rank, values.size() - 1)];
}

Note: fraction=0 is a valid input for P0 percentile, and while the current callers always pass 0.50 or 0.99, this is a latent bug that could be triggered by future changes or refactoring.

[nw9663644-eng]

Addressed in e37ea30. The current helpers already reject empty inputs/non-finite or out-of-range fractions and use max(1, ceil(f*n))-1, so P0 cannot underflow. Added the same upper clamp to all dataset variants for consistency. The new compiled-helper regression executes all five real implementations for P0/P50/P99/P100, singleton/empty input and invalid fractions; input samples remain unchanged. No historical measurements were recomputed.

[LHT129]

[suggestion] In read_records, const auto size = input.tellg() deduces size as std::streampos, which is a signed type. The subsequent size % (sizeof(int32_t) + sizeof(T) * expected_dim) performs a signed modulo with an unsigned operand, and size > 0 compares a signed value. While this works correctly on typical platforms for valid inputs, using an explicit static_cast<uint64_t> would make the intent clearer and avoid signed/unsigned comparison warnings.

Current:

const auto size = input.tellg();
require(size > 0 and size % (sizeof(int32_t) + sizeof(T) * expected_dim) == 0,
        "invalid input file length");
const auto count = static_cast<uint64_t>(size / (sizeof(int32_t) + sizeof(T) * expected_dim));

Suggested:

const auto size = static_cast<uint64_t>(input.tellg());
require(size > 0 and size % (sizeof(int32_t) + sizeof(T) * expected_dim) == 0,
        "invalid input file length");
const auto count = size / (sizeof(int32_t) + sizeof(T) * expected_dim);

[nw9663644-eng]

Fixed in e37ea30: reject non-positive tellg() and invalid dimensions before converting to uint64_t, then use unsigned record-byte arithmetic. Applied to both dataset consumers. Compiled-reader regressions cover valid rows, empty/truncated files and invalid dimensions.