Skip to content

.Net: Remove concurrent "batching" implementations in MEVD - #11864

Merged
Shay Rojansky (roji) merged 3 commits into
microsoft:feature-vector-data-preb3from
roji:RemoveConcurrency
May 2, 2025
Merged

Shay Rojansky (roji) merged 3 commits into
microsoft:feature-vector-data-preb3from
roji:RemoveConcurrency

Conversation

@roji

Copy link
Copy Markdown
Member
  • Add default non-concurrent implementations to VectorStoreCollection for GetAsync and DeleteAsync - but not for UpsertAsync. The reason is that when upserting, the provider (usually) needs to generate embeddings, and this is definitely not something that should happen separately for every record (IEmbeddingGenerators support batching, and a single call should be made for the vector property of all records).
  • Some providers were actually implementing batched UpsertAsync this way - fixed them to generate embeddings once before upserting. This just aligns those providers to use the same techniques as other providers which support native batching.

Closes #11854

Add default non-concurrent implementations for Get/Delete to VectorStoreCollection

Closes microsoft#11854
@roji
Shay Rojansky (roji) requested a review from a team as a code owner May 2, 2025 15:24
@markwallace-microsoft Mark Wallace (markwallace-microsoft) added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel memory labels May 2, 2025
@github-actions github-actions Bot changed the title Remove concurrent "batching" implementations in MEVD .Net: Remove concurrent "batching" implementations in MEVD May 2, 2025
{
Verify.NotNull(records);

(records, var generatedEmbeddings) = await ProcessEmbeddingsAsync(this._model, records, cancellationToken).ConfigureAwait(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement. The code feels much cleaner when it doesn't have these really long methods doing lots of different things.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long and complicated stuff is still there, but it's at least out of sight, yeah... I absolutely want to revisit and try to make this common across providers.

@roji
Shay Rojansky (roji) merged commit 39f9b82 into microsoft:feature-vector-data-preb3 May 2, 2025
@roji
Shay Rojansky (roji) deleted the RemoveConcurrency branch May 2, 2025 17:51

This branch was previously deployed

1 inactive deployment
integration — 98aa7134 Deployed May 2, 2025 by roji
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Issues or pull requests impacting the core kernel memory .NET Issue or Pull requests regarding .NET code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants