enhance: speed up search iterator stage 1 #37947

PwzXxm · 2024-11-22T09:53:22Z

mergify · 2024-11-22T10:39:01Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

codecov · 2024-11-22T11:56:18Z

Codecov Report

Attention: Patch coverage is 87.56757% with 46 lines in your changes missing coverage. Please review.

Project coverage is 80.98%. Comparing base (5394f47) to head (9016c4a).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/core/src/query/SearchOnSealed.cpp	36.84%	12 Missing ⚠️
internal/core/src/query/CachedSearchIterator.cpp	95.85%	7 Missing ⚠️
internal/proxy/search_util.go	89.55%	5 Missing and 2 partials ⚠️
internal/core/src/query/PlanProto.cpp	16.66%	5 Missing ⚠️
internal/core/src/query/SearchBruteForce.cpp	80.95%	4 Missing ⚠️
internal/core/src/query/SearchOnGrowing.cpp	42.85%	4 Missing ⚠️
internal/core/src/query/SearchOnIndex.cpp	42.85%	4 Missing ⚠️
internal/core/src/index/Utils.cpp	88.23%	2 Missing ⚠️
internal/core/src/query/CachedSearchIterator.h	95.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #37947       +/-   ##
===========================================
+ Coverage   69.32%   80.98%   +11.65%     
===========================================
  Files         292     1381     +1089     
  Lines       26183   194670   +168487     
===========================================
+ Hits        18152   157644   +139492     
- Misses       8031    31463    +23432     
- Partials        0     5563     +5563

Components	Coverage Δ
Client	`75.27% <ø> (∅)`
Core	`69.44% <85.50%> (+0.11%)`	⬆️
Go	`83.02% <93.06%> (∅)`

Files with missing lines	Coverage Δ
internal/core/src/common/QueryInfo.h	`100.00% <100.00%> (ø)`
internal/core/src/index/Utils.h	`88.88% <ø> (ø)`
internal/core/src/index/VectorDiskIndex.cpp	`76.96% <100.00%> (-1.05%)`	⬇️
internal/core/src/index/VectorMemIndex.cpp	`64.80% <100.00%> (-0.35%)`	⬇️
internal/proxy/proxy.go	`71.30% <100.00%> (ø)`
internal/proxy/task.go	`80.69% <ø> (ø)`
internal/proxy/task_search.go	`76.34% <100.00%> (ø)`
internal/core/src/query/CachedSearchIterator.h	`95.00% <95.00%> (ø)`
internal/core/src/index/Utils.cpp	`40.90% <88.23%> (+3.96%)`	⬆️
internal/core/src/query/SearchBruteForce.cpp	`79.72% <80.95%> (-0.85%)`	⬇️
... and 6 more

... and 1084 files with indirect coverage changes

PwzXxm · 2024-11-25T02:22:21Z

rerun cpp-unit-test

mergify · 2024-11-25T02:24:25Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

mergify · 2024-11-25T11:32:09Z

@PwzXxm E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-11-25T12:15:57Z

@PwzXxm go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-11-25T12:17:20Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

mergify · 2024-11-27T08:03:00Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

mergify · 2024-11-27T08:27:47Z

@PwzXxm E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-11-29T04:55:10Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

PwzXxm · 2024-11-29T07:18:09Z

/hold

PwzXxm · 2024-11-29T07:40:01Z

/unhold
Rename iterator token to iterator id

mergify · 2024-11-29T08:19:12Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

MrPresent-Han

NIT: just some questions

MrPresent-Han · 2024-11-29T08:37:19Z

internal/core/src/query/CachedSearchIterator.cpp

+        heap.pop();
+
+        // last_bound may change between NextBatch calls, discard any invalid results
+        if (!IsValid(cur_rst, last_bound, radius, range_filter)) {


so v2 iterator will not return better results compared to former iterations page?

Not in this stage, the next one will try to take care of this.

MrPresent-Han · 2024-11-29T08:38:32Z

internal/core/src/query/CachedSearchIterator.h

+        const float dist = result.first;
+        const bool is_valid =
+            !last_bound.has_value() || dist > last_bound.value();
+


question: no need to consider the positive or negative metrics for dist and last_bound?

The distances are converted when entering this class, no need to worry about it here

MrPresent-Han · 2024-11-29T08:43:22Z

internal/core/src/query/SearchOnGrowing.cpp

@@ -124,6 +125,19 @@ SearchOnGrowing(const segcore::SegmentGrowingImpl& segment,

        // step 3: brute force search where small indexing is unavailable
        auto vec_ptr = record.get_data_base(vecfield_id);
+


cached itrator will be created every time, so what is 'cached'?

It will introduce a pool of results in the next stage, as commented in https://github.com/milvus-io/milvus/pull/37947/files/9f6b88743198a575eb84cb427bcd41a7631676b7#diff-7344957165f4632a9363de767323618b7db0bd2d0f7cf7165965d3fb2612f18b. This class tries to provide a framework for the further implementation. If you think this name is confusing, I will change the naming if necessary.

no need to bother, just follow your scheme

MrPresent-Han · 2024-11-29T08:45:08Z

internal/core/src/query/CachedSearchIterator.cpp

+    search_result.distances_.resize(nq_ * batch_size_);
+
+    for (size_t query_idx = 0; query_idx < nq_; ++query_idx) {
+        auto rst = GetBatchedNextResults(query_idx, search_info);


NIT: seems that offsets and distances data retrieved are copied twice

The distance-id pairs need to be sorted before copy to the search_result. Knowhere needs to provide the ability to give batched results via iterator to eliminate this copy.

mergify · 2024-12-02T05:07:30Z

@PwzXxm go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-02T07:54:38Z

@PwzXxm cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

PwzXxm · 2024-12-03T02:07:11Z

/hold
V2.5.1

PwzXxm · 2024-12-03T02:09:02Z

/unhold

MrPresent-Han · 2024-12-03T06:48:29Z

/lgtm

PwzXxm · 2024-12-04T06:54:05Z

/hold
v.2.5.1

sre-ci-robot · 2024-12-13T03:11:26Z

New changes are detected. LGTM label has been removed.

mergify · 2024-12-13T03:58:46Z

@PwzXxm go-sdk check failed, comment rerun go-sdk can trigger the job again.

PwzXxm · 2024-12-13T06:14:58Z

rerun go-sdk

Signed-off-by: Patrick Weizhi Xu <[email protected]>

sre-ci-robot · 2024-12-18T05:57:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: PwzXxm
To complete the pull request process, please assign tedxu after the PR has been reviewed.
You can assign the PR to them by writing /assign @tedxu in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mergify · 2024-12-18T06:51:17Z

@PwzXxm go-sdk check failed, comment rerun go-sdk can trigger the job again.

PwzXxm · 2024-12-18T07:01:59Z

rerun go-sdk

sre-ci-robot added area/dependency Pull requests that update a dependency file size/XXL Denotes a PR that changes 1000+ lines. labels Nov 22, 2024

sre-ci-robot requested review from congqixia, czs007 and xiaocai2333 November 22, 2024 09:53

mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Nov 22, 2024

PwzXxm mentioned this pull request Nov 22, 2024

[Enhancement]: Speed up Search Iterator and Cleanup SDKs #37548

Open

1 task

PwzXxm force-pushed the search_iter_v2_s1 branch from 17da513 to a6bd30c Compare November 22, 2024 10:13

PwzXxm force-pushed the search_iter_v2_s1 branch 2 times, most recently from 00a762f to 97467bf Compare November 25, 2024 11:30

PwzXxm force-pushed the search_iter_v2_s1 branch from 97467bf to e74fbfd Compare November 27, 2024 07:13

PwzXxm force-pushed the search_iter_v2_s1 branch from e74fbfd to de74b0c Compare November 29, 2024 04:08

PwzXxm mentioned this pull request Nov 29, 2024

enhance: add search iterator v2 milvus-io/pymilvus#2395

Open

sre-ci-robot added the do-not-merge/hold label Nov 29, 2024

sre-ci-robot removed the do-not-merge/hold label Nov 29, 2024

MrPresent-Han reviewed Nov 29, 2024

View reviewed changes

PwzXxm force-pushed the search_iter_v2_s1 branch from bd0b450 to 8774ff5 Compare December 2, 2024 04:13

PwzXxm force-pushed the search_iter_v2_s1 branch from 8774ff5 to 9e649c7 Compare December 2, 2024 06:18

sre-ci-robot added the area/compilation label Dec 2, 2024

PwzXxm force-pushed the search_iter_v2_s1 branch 2 times, most recently from 5b68678 to 1995a47 Compare December 2, 2024 10:02

mergify bot added the ci-passed label Dec 2, 2024

sre-ci-robot added the do-not-merge/hold label Dec 3, 2024

sre-ci-robot removed the do-not-merge/hold label Dec 3, 2024

sre-ci-robot added the lgtm label Dec 3, 2024

sre-ci-robot added the do-not-merge/hold label Dec 4, 2024

PwzXxm force-pushed the search_iter_v2_s1 branch from 1995a47 to 0552b7d Compare December 13, 2024 03:11

sre-ci-robot removed the lgtm label Dec 13, 2024

mergify bot removed the ci-passed label Dec 13, 2024

mergify bot added the ci-passed label Dec 13, 2024

enhance: speed up search iter stage 1

9016c4a

Signed-off-by: Patrick Weizhi Xu <[email protected]>

PwzXxm force-pushed the search_iter_v2_s1 branch from 0552b7d to 9016c4a Compare December 18, 2024 05:57

mergify bot removed the ci-passed label Dec 18, 2024

mergify bot added the ci-passed label Dec 18, 2024

		@@ -124,6 +125,19 @@ SearchOnGrowing(const segcore::SegmentGrowingImpl& segment,

		// step 3: brute force search where small indexing is unavailable
		auto vec_ptr = record.get_data_base(vecfield_id);

enhance: speed up search iterator stage 1 #37947

Are you sure you want to change the base?

enhance: speed up search iterator stage 1 #37947

Conversation

PwzXxm commented Nov 22, 2024

mergify bot commented Nov 22, 2024

codecov bot commented Nov 22, 2024 • edited Loading

Codecov Report

PwzXxm commented Nov 25, 2024

mergify bot commented Nov 25, 2024

mergify bot commented Nov 25, 2024

mergify bot commented Nov 25, 2024

mergify bot commented Nov 25, 2024

mergify bot commented Nov 27, 2024

mergify bot commented Nov 27, 2024

mergify bot commented Nov 29, 2024

PwzXxm commented Nov 29, 2024

PwzXxm commented Nov 29, 2024 • edited Loading

mergify bot commented Nov 29, 2024

MrPresent-Han left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PwzXxm Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Dec 2, 2024

mergify bot commented Dec 2, 2024

PwzXxm commented Dec 3, 2024

PwzXxm commented Dec 3, 2024

MrPresent-Han commented Dec 3, 2024

PwzXxm commented Dec 4, 2024

sre-ci-robot commented Dec 13, 2024

mergify bot commented Dec 13, 2024

PwzXxm commented Dec 13, 2024

sre-ci-robot commented Dec 18, 2024

mergify bot commented Dec 18, 2024

PwzXxm commented Dec 18, 2024

codecov bot commented Nov 22, 2024 •

edited

Loading

PwzXxm commented Nov 29, 2024 •

edited

Loading

PwzXxm Nov 29, 2024 •

edited

Loading