fix(orchestrators): route stream assemble through the loader-based assemble by ahuber21 · Pull Request #417 · intel/ScalableVectorSearch

ahuber21 · 2026-10-09T13:13:25Z

Loading a Vamana-family index from a stream now goes through the same assemble(config_proto, graph_loader, data_loader, ...) entry point as loading from disk, so a stream is just one more kind of config source rather than its own code path.

Stream save/load was added after the loader-based interface already existed, and it took a shortcut. Instead of handing assemble a config, a graph loader and a data loader, the stream overload took the stream plus a pack of arguments for the data, and decided internally how the graph was read. The graph allocator was fixed, and debug_load_from_static was not exposed. That was fine while one default allocator was enough. It stopped being fine once the C API needed to put the graph and the data in different allocators. #402 and the first version of this PR tried to close that gap by adding parameters and overloads to the stream path, and every attempt made the stream path more of a parallel copy of the loader path. This PR stops patching the copy and removes it.

This PR goes back to the original design. The loader-based assemble on Vamana, DynamicVamana and ConcurrentDynamicVamana now accepts a std::istream as config_proto. In that case it runs the existing stream logic unchanged: it detects native vs. directory-archive format, reads the native stream in place, or unpacks the archive and loads from paths. The graph and data allocators come from the loaders, exactly as they do for file loads. The released assemble<Q, Data>(stream, distance, pool, data_args...) stays as a thin wrapper with its v0.4.0 signature and default allocators, so existing callers compile and behave as before. The C API and the C++ runtime now call the loader-based form directly, and neither public binding header changes.

svs::StreamLoader<T, Args...> lives in svs/core/loading.h, next to dispatch_load. It plays the role on the stream side that GraphLoader and VectorDataLoader play for files: it fixes the type to load and carries the arguments for it, typically an allocator. Unlike those two it holds no source. The orchestrator hands it either the stream or the unpacked component directory, because only the orchestrator knows which format arrived. One generic template is enough, since the caller always knows the exact type and graphs, datasets and configs all already load from both a stream and a path. A file-side loader like VectorDataLoader also deduces element type and dimensions, which a stream load never needs. static_asserts reject mixing a StreamLoader with a non-stream config, in either direction. A new public alias, Vamana::default_graph_type<Data>, names the default graph type. For view-backed data it rebinds the data's allocator to uint32_t; otherwise it uses HugepageAllocator. The wrapper and the C++ runtime share that one rule instead of each keeping a copy.

A few things a reviewer should know:

The stream path now honours logger and debug_load_from_static. That is a behaviour change, though only for callers who were passing values that were being ignored.
StreamLoader stores copies of its arguments and passes them on as const lvalues. A loader that needs to consume an rvalue would fail to compile; it would not misbehave silently.
Flat, DynamicFlat, IVF and DynamicIVF keep their dedicated stream overloads. Flat's loader-based assemble has no config slot, and IVF has no loader-based assemble at all, so bringing them in line is separate work.
The index-level stream auto_assemble functions in svs::index::* are untouched.

rfsaliev · 2026-10-12T10:49:24Z

+/// allocator type.
+///
+template <typename Idx = uint32_t, typename Allocator = HugepageAllocator<Idx>>
+struct StreamGraphLoader {


IMO, there is unnecessary code duplication.
It would be better to reuse existing GraphLoader class constructed from (stream, allocator) rather than multiply entities.

Thanks. Good idea. I will try it this way.

StreamLoader carries the element type and loader arguments (e.g. an allocator) for one component of an index saved to a stream. The orchestrator hands it either the stream (native format) or the unpacked component directory (directory archive). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…semble Vamana, DynamicVamana and ConcurrentDynamicVamana now load from a stream through the existing assemble(config_proto, graph_loader, data_loader, ...) when config_proto is a std::istream and both loaders are StreamLoader. The stream body moved unchanged into a private helper. - The released assemble<Q, Data>(stream, distance, pool, data_args...) on Vamana and DynamicVamana stays as a thin wrapper with its v0.4.0 signature and default graph allocators. - Removed, never released: Vamana's view-backed overload and the (data_allocator, graph_allocator) overloads from #402, and ConcurrentDynamicVamana's stream overload from #413. - The stream path used to drop the logger and debug_load_from_static; both are now passed through. - The C API and C++ runtime call the loader-based form. Public binding headers are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ahuber21 · 2026-10-13T10:29:14Z

@rfsaliev yet another attempts to harmonize assemble() calls - PR description updated.
As usual, static lib CI tests require uploading new artifacts. I'll fix that before merging, and the changes are ready for comments nonetheless.

razdoburdin · 2026-10-13T10:48:30Z

-            svs::ConcurrentDynamicVamana::assemble<float, ConcurrentData>(
-                stream, svs::distance::DistanceL2(), num_threads
-            );
+        svs::DynamicVamana loaded = svs::ConcurrentDynamicVamana::assemble<float>(


Consider adding the same tests for other affected orchestrators.

…loads Vamana, DynamicVamana and ConcurrentDynamicVamana now have a dedicated assemble(std::istream&, StreamLoader graph, StreamLoader data, ...) overload instead of branching inside the file-layout assemble. The file-layout overload is constrained to exclude streams and StreamLoader, so a mismatched call finds no viable overload rather than hitting a static_assert. The stream body moved unchanged; call sites are unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rfsaliev · 2026-10-13T13:43:16Z

/intelci

The static-library C API job builds against the headers shipped in the prebuilt archive. The previous archive predates `svs::StreamLoader` and the StreamLoader assemble overloads, so the dispatchers failed to compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The static-library C++ runtime job builds against the headers shipped in the prebuilt archive. The previous archive predates `svs::StreamLoader` and `Vamana::default_graph_type`, so the runtime impls failed to compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ahuber21 · 2026-10-14T11:51:46Z

/intelci

rfsaliev

There are some suggestions.
And seems like the complex refactoring is needed for save/load index cases.
But functionally this PR looks good for me.
Approving.

rfsaliev · 2026-10-14T15:38:19Z

+
+    T load(std::istream& stream) const {
+        return std::apply(
+            [&](const Args&... args) { return lib::load_from_stream<T>(stream, args...); },


what if one of Args is not copyable, i.e. has deleted copy constructor?

It works so long it's passed as an rvalue. Will add a test case showcasing it. StreamLoader can of course always be extended if more scenarios are needed. For now, everything compiles though.

The static-library C API job builds against the headers shipped in the prebuilt archive. The previous archive predates `svs::make_stream_loader`, which the stream dispatchers now call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The static-library C++ runtime job builds against the headers shipped in the prebuilt archive. The previous archive predates `svs::make_stream_loader`, which the runtime impls now call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rfsaliev

LGFM, just nitpick

…ble-loaders

ethanglaser

LGTM, additional test coverage etc can be added as follow-up but this fixes CI and should be part of release

ahuber21 force-pushed the fix/stream-assemble-data-loader-args branch from a2f6a28 to 416b0f9 Compare October 9, 2026 14:20

ahuber21 marked this pull request as ready for review October 9, 2026 14:20

ahuber21 requested review from ethanglaser, mihaic, rfsaliev and yuejiaointel as code owners October 9, 2026 14:20

ahuber21 changed the title ~~fix(orchestrators): forward data loader arguments in stream assemble~~ fix(orchestrators): StreamGraphLoader to forward template args in stream assemble Oct 12, 2026

rfsaliev reviewed Oct 12, 2026

View reviewed changes

ahuber21 and others added 2 commits October 13, 2026 02:37

ahuber21 force-pushed the fix/stream-assemble-data-loader-args branch from bc42712 to 4f2975f Compare October 13, 2026 09:59

ahuber21 changed the title ~~fix(orchestrators): StreamGraphLoader to forward template args in stream assemble~~ fix(orchestrators): route stream assemble through the loader-based assemble Oct 13, 2026

ahuber21 requested a review from rfsaliev October 13, 2026 10:29

razdoburdin reviewed Oct 13, 2026

View reviewed changes

ahuber21 and others added 2 commits October 14, 2026 00:37

rfsaliev approved these changes Oct 14, 2026

View reviewed changes

ahuber21 and others added 5 commits October 15, 2026 00:17

next set of review comments

acde405

Merge branch 'main' into fix/stream-assemble-data-loader-args

17e71f1

Merge branch 'main' into fix/stream-assemble-data-loader-args

abd6cce

rfsaliev approved these changes Oct 16, 2026

View reviewed changes

Comment thread include/svs/orchestrators/dynamic_vamana.h Outdated

ahuber21 and others added 3 commits October 16, 2026 04:50

Use make_stream_loader() factory

dc3287b

Merge remote-tracking branch 'origin/main' into refactor/stream-assem…

dea70e8

…ble-loaders

Merge branch 'main' into fix/stream-assemble-data-loader-args

efedbed

ethanglaser approved these changes Oct 16, 2026

View reviewed changes

ethanglaser merged commit cde3771 into main Oct 16, 2026
22 checks passed

ethanglaser deleted the fix/stream-assemble-data-loader-args branch October 16, 2026 21:06

ethanglaser mentioned this pull request Oct 16, 2026

bench fixes and SVS rc10 RedisAI/VectorSimilarity#1077

Draft

2 tasks

eshenayo mentioned this pull request Oct 16, 2026

build(svs): bump SVS to main cde3771 and drop the OpenMP shim eshenayo/valkey-search#35

Merged

Conversation

ahuber21 commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rfsaliev Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

ahuber21 Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

ahuber21 commented Oct 13, 2026

Uh oh!

razdoburdin Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rfsaliev commented Oct 13, 2026

Uh oh!

ahuber21 commented Oct 14, 2026

Uh oh!

rfsaliev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rfsaliev Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahuber21 Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rfsaliev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ethanglaser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ahuber21 commented Oct 9, 2026 •

edited

Loading

rfsaliev Oct 14, 2026 •

edited

Loading

rfsaliev left a comment •

edited

Loading