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

Improve IVF index mmap performance by cache policy #167

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

yah01
Copy link
Contributor

@yah01 yah01 commented Oct 31, 2023

/kind improvement
As we tested for HSNW index, MADV_RANDOM perform the best

@codecov
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #167 (72a9c2c) into main (a0953c3) will not change coverage.
The diff coverage is n/a.

❗ Current head 72a9c2c differs from pull request most recent head 7d8ea78. Consider uploading reports for the commit 7d8ea78 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@     Coverage Diff     @@
##   main   #167   +/-   ##
===========================
===========================

@mergify mergify bot added the ci-passed label Oct 31, 2023
@PwzXxm
Copy link
Collaborator

PwzXxm commented Oct 31, 2023

Please provide some results from benchmarking :)

@alexanderguzhva
Copy link
Collaborator

alexanderguzhva commented Oct 31, 2023

two comments

  1. It is an OS-specific change and it won't work in MSVC. Is it acceptable?
  2. Are there any other locations where the same optimization can be applied?
    Thanks

@alexanderguzhva
Copy link
Collaborator

Please provide some results from benchmarking :)

And the way of running benchmarks, please :)

@mergify mergify bot added ci-passed and removed ci-passed labels Oct 31, 2023
@yah01
Copy link
Contributor Author

yah01 commented Oct 31, 2023

We evaluate this with an end-to-end benchmark, the dataset is 8M rows with 128 dim vectors.
search param: nq=10, topk=10, nprobe=16

Before this PR: https://argo-workflows.zilliz.cc/workflows/qa/fouramf-85grz?tab=workflow&nodeId=fouramf-85grz-3384434876&nodePanelView=summary&sidePanel=logs:fouramf-85grz-3384434876:main
QPS=0.81, avg_latency=24759.91

After this PR: https://argo-workflows.zilliz.cc/workflows/qa/fouramf-nlhgl?tab=workflow&nodeId=fouramf-nlhgl-2020693927&sidePanel=logs:fouramf-nlhgl-2020693927:main
QPS=2.18, avg_latency=9143.93

You could start the test by argo like the links mentioned above.

NOTE: the result is based on nq=10, maybe we need a result with nq=1?

@yah01
Copy link
Contributor Author

yah01 commented Oct 31, 2023

two comments

  1. It is an OS-specific change and it won't work in MSVC. Is it acceptable?
  2. Are there any other locations where the same optimization can be applied?
    Thanks
  1. As I know Milvus stopped windows support, and now Knowhere can't be compiled on Windows (due to HNSW impl), is Knowhere targeting on Winodws support? @liliu-z
  2. For now we supports only IVF and HNSW index with mmap, HNSW has been optimized before this, so this is all

@PwzXxm
Copy link
Collaborator

PwzXxm commented Nov 1, 2023

/approve
/lgtm

@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

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

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 ebe8504 into zilliztech:main Nov 1, 2023
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.

4 participants