-
Notifications
You must be signed in to change notification settings - Fork 108
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
Oversizing vectors in faiss querying functionality #497
Comments
Experimental setupI ran performance tests on the sift data set which has 1M 128D vectors with a 10K query set. I tested faiss HNSW with m=16, ef_search=64 and ef_construction=64. I setup cluster with 2.1 with 3 c5.xlarge leaders and 1 r5.2xlarge data nodes. The control cluster came from the official 2.1 release. The test cluster was based off of https://github.com/jmazanec15/k-NN-1/tree/faiss-oversize-fix. I ran 3 iterations of each. I used 1 primary shard and 0 replicas. Benchmark code can be found in https://github.com/opensearch-project/k-NN/tree/main/benchmarks/osb. ResultsControlExperiment 1
Experiment 2
Experiment 3
TestExperiment 1
Experiment 2
Experiment 3
ConclusionFrom these experiments, we see that removing the additional overhead might slightly improve performance at this scale. |
Removes overallocation of 2 c++ vectors in faiss querying functionality. Performance results can be viewed in [497](opensearch-project#497 (comment)). In general, this change could provide a small improvement in memory footprint during search workloads. Signed-off-by: John Mazanec <jmazane@amazon.com>
Removes overallocation of 2 c++ vectors in faiss querying functionality. Performance results can be viewed in [497](#497 (comment)). In general, this change could provide a small improvement in memory footprint during search workloads. Signed-off-by: John Mazanec <jmazane@amazon.com>
Removes overallocation of 2 c++ vectors in faiss querying functionality. Performance results can be viewed in [497](#497 (comment)). In general, this change could provide a small improvement in memory footprint during search workloads. Signed-off-by: John Mazanec <jmazane@amazon.com> (cherry picked from commit 507bafe)
Removes overallocation of 2 c++ vectors in faiss querying functionality. Performance results can be viewed in [497](#497 (comment)). In general, this change could provide a small improvement in memory footprint during search workloads. Signed-off-by: John Mazanec <jmazane@amazon.com> (cherry picked from commit 507bafe) Co-authored-by: John Mazanec <jmazane@amazon.com>
In our jni layer, in knn_jni::faiss_wrapper::QueryIndex, we execute a query against a faiss index.
We initialize 2 c++ vectors to have k*dimension elements:
"dis" refers to a vector of distances and "ids" refers to a vector of returned ids. Given that "kJ" results are returned, and that distance and ID only have a single value, multiplying by "dim" overallocates memory. This does not have correctness implications, however, it is an inefficiency that may impact performance.
This should be fixed, but Id also like to compare the metrics before and after the fix to see how much this impacts performance.
The text was updated successfully, but these errors were encountered: