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 88.62434% with 43 lines in your changes missing coverage. Please review.

Project coverage is 81.11%. Comparing base (9c3f59d) to head (3044496).
Report is 8 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.90%	7 Missing ⚠️
internal/proxy/search_util.go	90.14%	5 Missing and 2 partials ⚠️
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/PlanProto.cpp	75.00%	2 Missing ⚠️
internal/core/src/query/CachedSearchIterator.h	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #37947      +/-   ##
==========================================
- Coverage   82.88%   81.11%   -1.78%     
==========================================
  Files        1089     1383     +294     
  Lines      169079   195578   +26499     
==========================================
+ Hits       140147   158638   +18491     
- Misses      23336    31367    +8031     
+ Partials     5596     5573      -23

Components	Coverage Δ
Client	`78.26% <ø> (ø)`
Core	`69.43% <86.81%> (∅)`
Go	`83.10% <93.33%> (+0.04%)`	⬆️

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%> (ø)`
internal/core/src/index/VectorMemIndex.cpp	`64.80% <100.00%> (ø)`
internal/proxy/proxy.go	`72.18% <100.00%> (+1.49%)`	⬆️
internal/proxy/task.go	`80.69% <ø> (ø)`
internal/proxy/task_search.go	`76.34% <100.00%> (+0.54%)`	⬆️
internal/core/src/query/CachedSearchIterator.h	`95.00% <95.00%> (ø)`
internal/core/src/index/Utils.cpp	`40.90% <88.23%> (ø)`
internal/core/src/query/PlanProto.cpp	`88.44% <75.00%> (ø)`
... and 6 more

... and 322 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.

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

rerun go-sdk

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

mergify · 2024-12-25T03:29:17Z

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

PwzXxm · 2024-12-25T03:33:14Z

rerun go-sdk

mergify · 2024-12-25T08:27:34Z

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

Signed-off-by: Patrick Weizhi Xu <weizhi.xu@zilliz.com>

mergify · 2024-12-25T11:50:48Z

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

PwzXxm · 2024-12-25T12:07:09Z

rerun cpp-unit-test

MrPresent-Han · 2024-12-26T02:24:45Z

/lgtm

congqixia · 2024-12-26T02:27:24Z

/approve

sre-ci-robot · 2024-12-26T02:27:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia, PwzXxm

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [congqixia]

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

PwzXxm · 2024-12-26T02:30:41Z

/unhold

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

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

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

PwzXxm mentioned this pull request Dec 24, 2024

enhance: [2.5][cp] speed up search iterator stage 1 #38678

Open

PwzXxm force-pushed the search_iter_v2_s1 branch from 9016c4a to 9981e47 Compare December 25, 2024 02:22

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

PwzXxm force-pushed the search_iter_v2_s1 branch 2 times, most recently from 4167f19 to 00d8642 Compare December 25, 2024 07:35

PwzXxm force-pushed the search_iter_v2_s1 branch 2 times, most recently from 91556bb to bf22e21 Compare December 25, 2024 10:13

enhance: speed up search iter stage 1

3044496

Signed-off-by: Patrick Weizhi Xu <weizhi.xu@zilliz.com>

PwzXxm force-pushed the search_iter_v2_s1 branch from bf22e21 to 3044496 Compare December 25, 2024 11:29

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

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

sre-ci-robot added the approved label Dec 26, 2024

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

sre-ci-robot merged commit 85f462b into milvus-io:master Dec 26, 2024
20 checks passed

		@@ -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

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

PwzXxm commented Dec 13, 2024

mergify bot commented Dec 18, 2024

PwzXxm commented Dec 18, 2024

mergify bot commented Dec 25, 2024

PwzXxm commented Dec 25, 2024

mergify bot commented Dec 25, 2024

mergify bot commented Dec 25, 2024

PwzXxm commented Dec 25, 2024

MrPresent-Han commented Dec 26, 2024

congqixia commented Dec 26, 2024

sre-ci-robot commented Dec 26, 2024

PwzXxm commented Dec 26, 2024

codecov bot commented Nov 22, 2024 •

edited

Loading

PwzXxm commented Nov 29, 2024 •

edited

Loading

PwzXxm Nov 29, 2024 •

edited

Loading