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

Remove unused stats in PQFastScan #180

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

chasingegg
Copy link
Collaborator

Remove unused stats in PQFastScan, where milvus could fail on global variable initialization on an sse4_2 machine.

@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chasingegg

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

@chasingegg
Copy link
Collaborator Author

/kind improvement

@alexanderguzhva
Copy link
Collaborator

@chasingegg Could you please elaborate why the failure can happen? Thanks

@chasingegg
Copy link
Collaborator Author

chasingegg commented Nov 7, 2023

@chasingegg Could you please elaborate why the failure can happen? Thanks

We will get _GLOBAL__sub_I_IndexPQFastScan.cpp on sse4_2, which means the initialization code for a global object or function in the file caused the problem, that is FastScan_stats. By removing this, the problem disappears.

@alexanderguzhva
Copy link
Collaborator

@chasingegg Could you please elaborate why the failure can happen? Thanks

We will get _GLOBAL__sub_I_IndexPQFastScan.cpp on sse4_2, which means the initialization code for a global object or function in the file caused the problem, that is FastScan_stats. By removing this, the problem disappears.

Also, as far I as remember, some of the veeeeeery recent commits to Faiss moved this stats from global to local variable, which is passed in a function. We can go this way as well.

@alexanderguzhva
Copy link
Collaborator

/lgtm

@sre-ci-robot sre-ci-robot merged commit a40f61d into zilliztech:main Nov 7, 2023
7 checks passed
@chasingegg chasingegg deleted the fix-sse branch November 7, 2023 02:11
chasingegg added a commit to chasingegg/Knowhere that referenced this pull request Nov 7, 2023
sre-ci-robot pushed a commit that referenced this pull request Nov 7, 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.

3 participants