perf(hgraph): parallelize cold build encoding

[jac0626]

Summary

• Pre-encode persistent RaBitQ base and SQ8 precise codes before parallel NSW graph insertion.
• Use a fixed number of contiguous worker blocks instead of per-vector encoding tasks.
• Skip the per-row pre-probe code insertion after the batch has been prepared.
• Preserve the existing path for incremental Add, deduplicated storage, force-remove, raw-vector, non-memory, and other quantization configurations.

Why

The reference SIFT1M build configured 96 threads but averaged 41.452 active cores. Profiling and ablation identified cold-build RaBitQ/SQ8 encoding as a material serialized stage before graph insertion.

Benchmark

Reference configuration: SIFT1M, L2, RaBitQ 3-bit base + SQ8 precise reorder, NSW, flat graph storage, max_degree=64, ef_construction=400, alpha=1.0, 96 pinned build threads.

Metric	Upstream baseline	Parallel pre-encoding ablation
Build wall	64.457 s (3-run mean)	39.204 s (confirmation run)
Average active cores	41.452	89.158
Whole-machine CPU utilization	43.180%	92.873%
Throughput	15,515 vec/s	25,508 vec/s
Memory	0.582 GiB	0.582 GiB
Recall@10	0.980	0.981
Self-nearest checks	passed	10/10

The confirmation run reduces wall time by 39.2% with unchanged reported memory and recall within 0.001. It is reported as a single retained-optimization ablation rather than a three-run mean.

Validation

• clang-format-15 --dry-run --Werror on all changed C++ files
• clang-tidy-15 -p build --quiet on hgraph_build.cpp and hgraph_fast_build.cpp
• build/tests/unittests "[ut][hgraph]" -r compact: 41 cases, 389 assertions passed
• New RaBitQ 3-bit + SQ8 parallel cold-build regression: 1 case, 18 assertions passed
• Existing MRLE optimized-build regression passed
• git diff --check

Scope and compatibility

No public API, serialization format, search path, Pyramid code, or third-party dependency changes. Unsupported configurations retain the existing encoding/insertion behavior.

Closes: #2749

[vsag-bot]

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

[mergify]

Merge Protections

🟢 All 2 merge protections satisfied — ready to merge.

Show 2 satisfied protections

🟢 Require kind label

• label~=^kind/

🟢 Require version label

• label~=^version/

[Copilot]

Pull request overview

This pull request parallelizes RaBitQ/SQ8 encoding during HGraph cold builds while preserving existing fallback paths.

Changes:

• Adds fixed-block parallel encoding.
• Avoids redundant per-row encoding.
• Updates internal interfaces and adds regression coverage.

A critical exception-safety issue remains in hgraph_fast_build.cpp: queued workers may outlive add_lock and codes_lock if GeneralEnqueue throws.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/algorithm/hgraph/hgraph.h	Updates internal build and insertion interfaces.
src/algorithm/hgraph/hgraph_fast_build.cpp	Implements parallel code preparation.
src/algorithm/hgraph/hgraph_build.cpp	Integrates prepared-code handling.
src/algorithm/hgraph/hgraph_add_test.cpp	Adds cold-build regression coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[LHT129]

[critical] The declaration order of task_guard relative to add_lock and codes_lock creates an exception-safety bug that was previously flagged and remains unfixed.

If GeneralEnqueue throws after one or more blocks have already been queued, stack unwinding destroys add_lock and codes_lock first (reverse declaration order), then task_guard. When task_guard's destructor calls wait_all_futures, the already-queued workers will execute insert_persistent_codes_unlocked without the locks held, while another thread could concurrently resize or mutate the code storage.

The fix is to declare the locks before task_guard so they outlive it:

std::shared_lock<std::shared_mutex> add_lock(this->add_mutex_);
std::unique_lock<std::shared_mutex> codes_lock(this->persistent_codes_mutex_);
std::vector<std::future<void>> futures;
HGraphBuildTaskGuard task_guard(futures, worker_count);

This ensures wait_all_futures completes (or the futures are abandoned) before the locks are released.

[LHT129]

[suggestion] The persistent_codes_prepared boolean parameter added to insert_one_logical_point is used to skip prepare_codes_before_probe_if_needed when codes were already pre-encoded. However, this creates a subtle coupling: the correctness of insert_one_logical_point now depends on the caller correctly passing this flag, and the flag's meaning is not self-documenting at the call site.

Consider an alternative approach: instead of threading a boolean through the call chain, move the conditional check into prepare_codes_before_probe_if_needed itself by checking whether codes already exist for the given inner_id. This would make insert_one_logical_point self-contained and eliminate the risk of a future caller forgetting to pass the flag correctly.

If the current approach is preferred for performance (avoiding a lookup), consider renaming the parameter to something more descriptive like skip_prepare_codes_before_probe to make the intent clearer at call sites.

[LHT129]

[note] The persistent_codes_prepared flag is captured by value in the lambda at line 483 (add_func). Since prepare_build_codes returns before any parallel add_func invocation starts (due to wait_all_futures in the parallel-add path), this is safe. However, if the parallel-add dispatch logic ever changes to overlap prepare_build_codes with add_func execution, this value capture would become stale. Consider documenting this ordering dependency explicitly.