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.

Sign up for GitHub

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

Fixing the arithmetic to find the number of vectors to stream from java to jni layer. #1804

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Fixing the arithmetic to find the number of vectors to stream from java to jni layer. #1804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fixing the arithmetic to find the number of vectors to stream from ja… 
…va to jni layer

Signed-off-by: Navneet Verma <navneev@amazon.com>
  • Loading branch information
@navneet1v
navneet1v committed Jul 9, 2024
commit ac56da85ad38376739bd2b0ff7e679bd76bfb054
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
* Adds dynamic query parameter ef_search in radial search faiss engine [#1790](https://github.com/opensearch-project/k-NN/pull/1790)
### Enhancements
### Bug Fixes
* Fixing the arithmetic to find the number of vectors to stream from java to jni layer.[#1804](https://github.com/opensearch-project/k-NN/pull/1804)
### Infrastructure
### Documentation
* Update dev guide to fix clang linking issue on arm [#1746](https://github.com/opensearch-project/k-NN/pull/1746)
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/org/opensearch/knn/index/codec/util/KNNCodecUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ public static KNNCodecUtil.Pair getFloats(BinaryDocValues values) throws IOExcep
dimension = vector.length;

if (vectorsPerTransfer == Integer.MIN_VALUE) {
vectorsPerTransfer = (dimension * Float.BYTES * totalLiveDocs) / vectorsStreamingMemoryLimit;
// This condition comes if vectorsStreamingMemoryLimit is higher than total number floats to transfer
// Doing this will reduce 1 extra trip to JNI layer.
// if vectorsStreamingMemoryLimit is 100 bytes and we have 50 vectors with 5 dimension, then per
navneet1v marked this conversation as resolved.
Show resolved Hide resolved
// transfer we have to send 100/(5 * 4) => 20 vectors.
vectorsPerTransfer = vectorsStreamingMemoryLimit / ((long) dimension * Float.BYTES);
// If vectorsPerTransfer comes out to be 0, then we set number of vectors per transfer to 1, to ensure that
// we are sending minimum number of vectors.
if (vectorsPerTransfer == 0) {
vectorsPerTransfer = totalLiveDocs;
vectorsPerTransfer = 1;
}
}
if (vectorList.size() == vectorsPerTransfer) {
Expand Down
Loading