Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize some low efficient code #1223

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

liliu-z
Copy link
Member

@liliu-z liliu-z commented Nov 15, 2022

Signed-off-by: Li Liu [email protected]

This PR fix some low efficient code:

  1. Call to_dict() of a collection at construction time and store it for search use.
  2. Use a map instead of endless if else to check param.
  3. For sync search, simply use serial execution instead of async + get
  4. Remove deep copy in search path

For performance enhancement, I did some test in my 2C8M container:
Pymilvus: Set grpc to return empty response immediately, the QPS of Pymilvus increase from ~1200 to ~2100, increased about 75%
End 2 End: For nq1 serial execution, latency improved ~10%.

Notice: Still lots of similar style in other parts of the code, like deep copy, to_dict() call, if else param check and mixed sync/async call. We need to fix them all if needed.

@liliu-z liliu-z force-pushed the fix_some_low_effienct_code branch from 8b06225 to 9b7d462 Compare November 15, 2022 12:26
@mergify mergify bot added the ci-passed label Nov 15, 2022
@XuanYang-cn XuanYang-cn self-assigned this Nov 16, 2022
@XuanYang-cn
Copy link
Contributor

Marvelous!
/lgtm
/approve

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liliu-z, XuanYang-cn

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:

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

@sre-ci-robot sre-ci-robot merged commit 966ad86 into milvus-io:master Nov 16, 2022
XuanYang-cn pushed a commit to XuanYang-cn/pymilvus that referenced this pull request Nov 24, 2022
- Modify bulkinsert example (milvus-io#1218)
- Fix the partition example (milvus-io#1219)
- Fix error message for primary key type (milvus-io#1222)
- Optimize some low efficient code (milvus-io#1223)
- Change doc string of Collection init and property (milvus-io#1220)
- Fix mutation results print error (milvus-io#1226)
- Update docs for collection (milvus-io#1225)
- Modify bulkinsert example (milvus-io#1228)
- Add the example comments for rbac methods (milvus-io#1235)
- Add documentation for flush and set_properties (milvus-io#1236)
- Enhance expression of search (milvus-io#1237)
- Update docs for collection/partition search (milvus-io#1238)
- Update docs for Collection (milvus-io#1242)

Co-Authored-By: yhmo <[email protected]>
Co-Authored-By: SimFG <[email protected]>
Co-Authored-By: yangxuan <[email protected]>
Co-Authored-By: Li Liu <[email protected]>
Co-Authored-By: yhmo <[email protected]>

Signed-off-by: yangxuan <[email protected]>
Signed-off-by: yhmo <[email protected]>
@XuanYang-cn XuanYang-cn mentioned this pull request Nov 24, 2022
sre-ci-robot pushed a commit that referenced this pull request Nov 24, 2022
- Modify bulkinsert example (#1218)
- Fix the partition example (#1219)
- Fix error message for primary key type (#1222)
- Optimize some low efficient code (#1223)
- Change doc string of Collection init and property (#1220)
- Fix mutation results print error (#1226)
- Update docs for collection (#1225)
- Modify bulkinsert example (#1228)
- Add the example comments for rbac methods (#1235)
- Add documentation for flush and set_properties (#1236)
- Enhance expression of search (#1237)
- Update docs for collection/partition search (#1238)
- Update docs for Collection (#1242)

Co-Authored-By: yhmo <[email protected]>
Co-Authored-By: SimFG <[email protected]>
Co-Authored-By: yangxuan <[email protected]>
Co-Authored-By: Li Liu <[email protected]>
Co-Authored-By: yhmo <[email protected]>

Signed-off-by: yangxuan <[email protected]>
Signed-off-by: yhmo <[email protected]>

Signed-off-by: yangxuan <[email protected]>
Signed-off-by: yhmo <[email protected]>
Co-authored-by: groot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants