runtime: expose batch_size_cap as an overridable LeanVec build parameter (SVS-164) - #393
Draft
yuejiaointel wants to merge 2 commits into
Draft
yuejiaointel wants to merge 2 commits into
yuejiaointel wants to merge 2 commits into
Conversation
…ter (SVS-164) Adds one new, additive overload of DynamicVamanaIndexLeanVec::build (the training-data variant used for OOD LeanVec builds) that threads batch_size_cap through as a caller-settable argument, instead of it only being reachable by recompiling libsvs with a different default. All 4 existing overloads are untouched -- ABI-compatible, matching this repo's append-only-virtuals convention (see #385, #388). batch_size_cap itself (default 100'000, lowered from 1'000'000) was already validated end-to-end this session; this change only adds a way to override it without a rebuild. Milvus/knowhere config-schema exposure is a natural follow-up, not included here. New regression test (LeanVecWithTrainingDataPointerCustomBatchSizeCap) uses batch_size_cap=7 against test_n=100 to force reduce() through multiple, uneven-remainder batches -- the code path every other LeanVec test in this file never exercises, since their built-in dataset size is smaller than any cap value previously in use. Verified locally: full [runtime] suite passes (1452 assertions, 40 cases, zero regressions vs the unpatched build), new test passes (4 assertions). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d (SVS-164) Adds the same additive batch_size_cap overload to the static VamanaIndexLeanVec::build (training-data variant) and threads it through VamanaIndexLeanVecImpl into StorageFactory. Existing overloads are unchanged. Current knowhere builds static Vamana by default (svs_is_static), so this is the path Milvus uses. Adds StaticIndexLeanVecWithTrainingDataCustomBatchSizeCap, mirroring the dynamic test. Also clang-formats the dynamic #else stub. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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.
Summary
SVS-164 found that Milvus's SVS LeanVec index build holds a
batch_size_cap-sized combined transform buffer duringreduce(). Lowering the default from 1,000,000 to 100,000 meaningfully reduces build-time peak memory (companion change in the innersourcelibraries.ai.vector-search.svsrepo, not in this repo).This PR adds additive, ABI-compatible overloads that take
batch_size_capas a caller-settable argument, for both the static and the dynamic build (training-data variants):VamanaIndexLeanVec::build(..., training_data, params, default_search_params, batch_size_cap): static Vamana. This is the path current Milvus uses, since knowhere #1749 addedsvs_is_static(default true).DynamicVamanaIndexLeanVec::build(..., training_data, params, default_search_params, dynamic_index_params, batch_size_cap): dynamic Vamana.Both are threaded through the respective LeanVec impl into
StorageFactory<LeanVecStorageType>. All existing overloads are untouched, and each new overload has a matching#else(no LVQ/LeanVec) stub. This follows the repo's append-only ABI convention (see #385, #388).Evidence (from the companion default-value change)
Note: the e2e numbers below were measured on Milvus's old dynamic Vamana path (knowhere v3.0.1, which predates
svs_is_static). Static-path e2e numbers on current Milvus master are pending. The bare-metal sweep builds a static index directly, so it is not affected.svs_benchmarksweep (n=2/arm): peak memory reaches its floor at cap=100,000 and stays flat down to 10K. Build time reaches its floor at cap=500,000 and stays flat down to 10K. Only cap=1,000,000 is a slow outlier (+21%).How to verify
Both new tests use
batch_size_cap=7againsttest_n=100. That forcesreduce()through multiple uneven-remainder batches, a path no existing LeanVec test exercises because their dataset is smaller than any previously used cap. The#elsestubs were also checked with a-fsyntax-onlycompile withoutSVS_RUNTIME_HAVE_LVQ_LEANVEC.Not in scope
Exposing this as a knowhere index-build param (
svs_leanvec_batch_size_capinSvsVamanaLeanVecConfig, passed to both static and dynamicIndexSVSVamanaLeanVec::create_implbranches) is a follow-up that depends on this PR.🤖 Generated with Claude Code