Add the interface for streaming the vectors from java to jni layer with initial capacity #1586

navneet1v · 2024-03-29T07:18:48Z

Description

Add the interface for streaming the vectors from java to jni layer with initial capacity. The PR will be added in a feature branch.

Along with this change I deprecated the methods in FaissService. Will remove those methods going forward. I also removed the transferVectorsV2 function as it is not required now.

JNI test

(base) 00:17 ~/workplace/k-NN/jni (stream-vectors)$ ./bin/jni_test 
Running main() from /Users/navneev/workplace/k-NN/jni/googletest-src/googletest/src/gtest_main.cc
[==========] Running 22 tests from 20 test suites.
[----------] Global test environment set-up.
[----------] 1 test from FaissCreateIndexTest
[ RUN      ] FaissCreateIndexTest.BasicAssertions
[       OK ] FaissCreateIndexTest.BasicAssertions (14 ms)
[----------] 1 test from FaissCreateIndexTest (14 ms total)

[----------] 1 test from FaissCreateIndexFromTemplateTest
[ RUN      ] FaissCreateIndexFromTemplateTest.BasicAssertions
[       OK ] FaissCreateIndexFromTemplateTest.BasicAssertions (5 ms)
[----------] 1 test from FaissCreateIndexFromTemplateTest (5 ms total)

[----------] 3 tests from FaissLoadIndexTest
[ RUN      ] FaissLoadIndexTest.BasicAssertions
[       OK ] FaissLoadIndexTest.BasicAssertions (4 ms)
[ RUN      ] FaissLoadIndexTest.HNSWPQDisableSdcTable
WARNING clustering 256 points to 16 centroids: please provide at least 624 training points
[       OK ] FaissLoadIndexTest.HNSWPQDisableSdcTable (372 ms)
[ RUN      ] FaissLoadIndexTest.IVFPQDisablePrecomputeTable
WARNING clustering 256 points to 16 centroids: please provide at least 624 training points
[       OK ] FaissLoadIndexTest.IVFPQDisablePrecomputeTable (359 ms)
[----------] 3 tests from FaissLoadIndexTest (736 ms total)

[----------] 1 test from FaissQueryIndexTest
[ RUN      ] FaissQueryIndexTest.BasicAssertions
[       OK ] FaissQueryIndexTest.BasicAssertions (5 ms)
[----------] 1 test from FaissQueryIndexTest (5 ms total)

[----------] 1 test from FaissQueryIndexWithFilterTest1435
[ RUN      ] FaissQueryIndexWithFilterTest1435.BasicAssertions
[       OK ] FaissQueryIndexWithFilterTest1435.BasicAssertions (10 ms)
[----------] 1 test from FaissQueryIndexWithFilterTest1435 (10 ms total)

[----------] 1 test from FaissQueryIndexWithParentFilterTest
[ RUN      ] FaissQueryIndexWithParentFilterTest.BasicAssertions
[       OK ] FaissQueryIndexWithParentFilterTest.BasicAssertions (5 ms)
[----------] 1 test from FaissQueryIndexWithParentFilterTest (5 ms total)

[----------] 1 test from FaissFreeTest
[ RUN      ] FaissFreeTest.BasicAssertions
[       OK ] FaissFreeTest.BasicAssertions (0 ms)
[----------] 1 test from FaissFreeTest (0 ms total)

[----------] 1 test from FaissInitLibraryTest
[ RUN      ] FaissInitLibraryTest.BasicAssertions
[       OK ] FaissInitLibraryTest.BasicAssertions (0 ms)
[----------] 1 test from FaissInitLibraryTest (0 ms total)

[----------] 1 test from FaissTrainIndexTest
[ RUN      ] FaissTrainIndexTest.BasicAssertions
[       OK ] FaissTrainIndexTest.BasicAssertions (1 ms)
[----------] 1 test from FaissTrainIndexTest (1 ms total)

[----------] 1 test from FaissCreateHnswSQfp16IndexTest
[ RUN      ] FaissCreateHnswSQfp16IndexTest.BasicAssertions
[       OK ] FaissCreateHnswSQfp16IndexTest.BasicAssertions (5 ms)
[----------] 1 test from FaissCreateHnswSQfp16IndexTest (5 ms total)

[----------] 1 test from FaissIsSharedIndexStateRequired
[ RUN      ] FaissIsSharedIndexStateRequired.BasicAssertions
[       OK ] FaissIsSharedIndexStateRequired.BasicAssertions (0 ms)
[----------] 1 test from FaissIsSharedIndexStateRequired (0 ms total)

[----------] 1 test from FaissInitAndSetSharedIndexState
[ RUN      ] FaissInitAndSetSharedIndexState.BasicAssertions
WARNING clustering 256 points to 16 centroids: please provide at least 624 training points
[       OK ] FaissInitAndSetSharedIndexState.BasicAssertions (360 ms)
[----------] 1 test from FaissInitAndSetSharedIndexState (360 ms total)

[----------] 1 test from IDGrouperBitMapTest
[ RUN      ] IDGrouperBitMapTest.BasicAssertions
[       OK ] IDGrouperBitMapTest.BasicAssertions (0 ms)
[----------] 1 test from IDGrouperBitMapTest (0 ms total)

[----------] 1 test from NmslibIndexWrapperSearchTest
[ RUN      ] NmslibIndexWrapperSearchTest.BasicAssertions
[       OK ] NmslibIndexWrapperSearchTest.BasicAssertions (0 ms)
[----------] 1 test from NmslibIndexWrapperSearchTest (0 ms total)

[----------] 1 test from NmslibCreateIndexTest
[ RUN      ] NmslibCreateIndexTest.BasicAssertions
[       OK ] NmslibCreateIndexTest.BasicAssertions (2 ms)
[----------] 1 test from NmslibCreateIndexTest (2 ms total)

[----------] 1 test from NmslibLoadIndexTest
[ RUN      ] NmslibLoadIndexTest.BasicAssertions
[       OK ] NmslibLoadIndexTest.BasicAssertions (1 ms)
[----------] 1 test from NmslibLoadIndexTest (1 ms total)

[----------] 1 test from NmslibQueryIndexTest
[ RUN      ] NmslibQueryIndexTest.BasicAssertions
[       OK ] NmslibQueryIndexTest.BasicAssertions (2 ms)
[----------] 1 test from NmslibQueryIndexTest (2 ms total)

[----------] 1 test from NmslibFreeTest
[ RUN      ] NmslibFreeTest.BasicAssertions
[       OK ] NmslibFreeTest.BasicAssertions (0 ms)
[----------] 1 test from NmslibFreeTest (0 ms total)

[----------] 1 test from NmslibInitLibraryTest
[ RUN      ] NmslibInitLibraryTest.BasicAssertions
[       OK ] NmslibInitLibraryTest.BasicAssertions (0 ms)
[----------] 1 test from NmslibInitLibraryTest (0 ms total)

[----------] 1 test from CommonsTests
[ RUN      ] CommonsTests.BasicAssertions
[       OK ] CommonsTests.BasicAssertions (0 ms)
[----------] 1 test from CommonsTests (0 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 20 test suites ran. (1154 ms total)
[  PASSED  ] 22 tests.

Old benchmarks

Benchmark	(dimension)	(vectorsPerTransfer)	Mode	Cnt	Score	Units	Heap Used (in Mb)	Number of trips to JNI Layer
TransferVectorsBenchmarks.transferVectorsInitialCapacity	128	100000	ss	3	1.206	s/op	48.82813	10
TransferVectorsBenchmarks.transferVectorsInitialCapacity	128	500000	ss	3	1.576	s/op	244.14063	2
TransferVectorsBenchmarks.transferVectorsInitialCapacity	128	1000000	ss	3	1.591	s/op	488.28125	1
TransferVectorsBenchmarks.transferVectorsInitialCapacity	256	100000	ss	3	2.926	s/op	97.65625	10
TransferVectorsBenchmarks.transferVectorsInitialCapacity	256	500000	ss	3	3.13	s/op	488.28125	2
TransferVectorsBenchmarks.transferVectorsInitialCapacity	256	1000000	ss	3	3.668	s/op	976.5625	1
TransferVectorsBenchmarks.transferVectorsInitialCapacity	384	100000	ss	3	4.651	s/op	146.48438	10
TransferVectorsBenchmarks.transferVectorsInitialCapacity	384	500000	ss	3	7.108	s/op	732.42188	2
TransferVectorsBenchmarks.transferVectorsInitialCapacity	384	1000000	ss	3	8.964	s/op	1464.84375	1
TransferVectorsBenchmarks.transferVectorsInitialCapacity	512	100000	ss	3	5.36	s/op	195.3125	10
TransferVectorsBenchmarks.transferVectorsInitialCapacity	512	500000	ss	3	9.59	s/op	976.5625	2
TransferVectorsBenchmarks.transferVectorsInitialCapacity	512	1000000	ss	3	9.959	s/op	1953.125	1

New Benchmarks

Benchmark	(dimension)	(vectorsPerTransfer)	Mode	Cnt	Score	Units	Heap Used (in Mb)	Number of trips to JNI Layer
TransferVectorsBenchmarks.transferVectors_withCapacity	128	100000	ss	3	0.873	s/op	48.82813	10
TransferVectorsBenchmarks.transferVectors_withCapacity	128	500000	ss	3	0.874	s/op	244.14063	2
TransferVectorsBenchmarks.transferVectors_withCapacity	128	1000000	ss	3	0.868	s/op	488.28125	1
TransferVectorsBenchmarks.transferVectors_withCapacity	256	100000	ss	3	1.366	s/op	97.65625	10
TransferVectorsBenchmarks.transferVectors_withCapacity	256	500000	ss	3	1.379	s/op	488.28125	2
TransferVectorsBenchmarks.transferVectors_withCapacity	256	1000000	ss	3	1.261	s/op	976.5625	1
TransferVectorsBenchmarks.transferVectors_withCapacity	384	100000	ss	3	1.773	s/op	146.48438	10
TransferVectorsBenchmarks.transferVectors_withCapacity	384	500000	ss	3	1.776	s/op	732.42188	2
TransferVectorsBenchmarks.transferVectors_withCapacity	384	1000000	ss	3	1.786	s/op	1464.84375	1
TransferVectorsBenchmarks.transferVectors_withCapacity	512	100000	ss	3	2.255	s/op	195.3125	10
TransferVectorsBenchmarks.transferVectors_withCapacity	512	500000	ss	3	2.289	s/op	976.5625	2
TransferVectorsBenchmarks.transferVectors_withCapacity	512	1000000	ss	3	2.283	s/op	1953.125	1

Issues Resolved

This is first of the many PR that will be raised for this issue: #1506

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed as per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2024-03-29T07:46:37Z

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 84.84%. Comparing base (771c4b5) to head (0a5b008).

Files	Patch %	Lines
...c/main/java/org/opensearch/knn/jni/JNICommons.java	0.00%	5 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##             feature/stream-vectors    #1586      +/-   ##
============================================================
- Coverage                     84.92%   84.84%   -0.08%     
+ Complexity                     1375     1374       -1     
============================================================
  Files                           172      173       +1     
  Lines                          5605     5609       +4     
  Branches                        553      553              
============================================================
- Hits                           4760     4759       -1     
- Misses                          612      616       +4     
- Partials                        233      234       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/main/java/org/opensearch/knn/jni/FaissService.java

src/main/java/org/opensearch/knn/jni/JNICommons.java

jni/src/commons.cpp

jmazanec15

Looks good to me. Minor comment, but not blocker

src/main/java/org/opensearch/knn/jni/JNICommons.java

jmazanec15 · 2024-04-02T15:33:42Z

One other thing @navneet1v - I think we might need to update https://github.com/opensearch-project/k-NN/blob/main/src/main/plugin-metadata/plugin-security.policy#L2-L4 with new jni lib.

martin-gaievski · 2024-04-02T16:35:30Z

One other thing @navneet1v - I think we might need to update https://github.com/opensearch-project/k-NN/blob/main/src/main/plugin-metadata/plugin-security.policy#L2-L4 with new jni lib.

if this change is needed how does it work today? Looks like Navneet done many benchmarks with current code.

navneet1v · 2024-04-02T17:20:55Z

One other thing @navneet1v - I think we might need to update https://github.com/opensearch-project/k-NN/blob/main/src/main/plugin-metadata/plugin-security.policy#L2-L4 with new jni lib.

if this change is needed how does it work today? Looks like Navneet done many benchmarks with current code.

the micro-benchmark code doesn't require that permission. I have not tested the code with the k-NN plugin. Which I would have done in the upcoming PRs.

…th initial capacity Signed-off-by: Navneet Verma <navneev@amazon.com>

navneet1v · 2024-04-02T17:23:59Z

One other thing @navneet1v - I think we might need to update https://github.com/opensearch-project/k-NN/blob/main/src/main/plugin-metadata/plugin-security.policy#L2-L4 with new jni lib.

updated the PR.

navneet1v · 2024-04-02T17:25:22Z

@jmazanec15 and @martin-gaievski updated the PR.

jmazanec15 · 2024-04-02T17:26:48Z

if this change is needed how does it work today? Looks like Navneet done many benchmarks with current code.

@martin-gaievski @navneet1v I have a suspicion that faiss is being called first which is then loading the common library in the C++ code - but not sure either how it was working.

navneet1v · 2024-04-02T17:29:04Z

if this change is needed how does it work today? Looks like Navneet done many benchmarks with current code.

@martin-gaievski @navneet1v I have a suspicion that faiss is being called first which is then loading the common library in the C++ code - but not sure either how it was working.

I was not loading Faiss for benchmarks. As the benchmark is just a simple java process it doesn't those permission(as per my understanding). But given that JNICommons will be used before we even call Faiss hence it make sense to add the permission. We would have caught this in next iteration of the PR when these interfaces will be actually added in the k-NN plugin create index call. But as of not as that integration was not added adding permission could have been missed.

martin-gaievski

Looks good to me, few things that left will be addressed in coming PRs

…layer to enable creation of larger segments for vector indices Changes include: 1. Add the interface for streaming the vectors from java to jni layer with initial capacity (opensearch-project#1586) 2. Integrating storeVectors interfaces with createIndex and createIndexTemplate functions. (opensearch-project#1588) 3. Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address(opensearch-project#1595) Signed-off-by: Navneet Verma <navneev@amazon.com>

…layer to enable creation of larger segments for vector indices Changes include: 1. Add the interface for streaming the vectors from java to jni layer with initial capacity (opensearch-project#1586) 2. Integrating storeVectors interfaces with createIndex and createIndexTemplate functions. (opensearch-project#1588) 3. Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address(opensearch-project#1595) 4. Move free vectorAddress from Java to JNI layer to reduce the memory footprint for Nmslib (opensearch-project#1602) Signed-off-by: Navneet Verma <navneev@amazon.com>

…layer to enable creation of larger segments for vector indices (#1604) Changes include: 1. Add the interface for streaming the vectors from java to jni layer with initial capacity (#1586) 2. Integrating storeVectors interfaces with createIndex and createIndexTemplate functions. (#1588) 3. Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address(#1595) 4. Move free vectorAddress from Java to JNI layer to reduce the memory footprint for Nmslib (#1602) Signed-off-by: Navneet Verma <navneev@amazon.com>

…layer to enable creation of larger segments for vector indices (opensearch-project#1604) Changes include: 1. Add the interface for streaming the vectors from java to jni layer with initial capacity (opensearch-project#1586) 2. Integrating storeVectors interfaces with createIndex and createIndexTemplate functions. (opensearch-project#1588) 3. Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address(opensearch-project#1595) 4. Move free vectorAddress from Java to JNI layer to reduce the memory footprint for Nmslib (opensearch-project#1602) Signed-off-by: Navneet Verma <navneev@amazon.com>

…layer to enable creation of larger segments for vector indices (#1604) (#1608) Changes include: 1. Add the interface for streaming the vectors from java to jni layer with initial capacity (#1586) 2. Integrating storeVectors interfaces with createIndex and createIndexTemplate functions. (#1588) 3. Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address(#1595) 4. Move free vectorAddress from Java to JNI layer to reduce the memory footprint for Nmslib (#1602) Signed-off-by: Navneet Verma <navneev@amazon.com>

…layer to enable creation of larger segments for vector indices (opensearch-project#1604) (opensearch-project#1608) Changes include: 1. Add the interface for streaming the vectors from java to jni layer with initial capacity (opensearch-project#1586) 2. Integrating storeVectors interfaces with createIndex and createIndexTemplate functions. (opensearch-project#1588) 3. Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address(opensearch-project#1595) 4. Move free vectorAddress from Java to JNI layer to reduce the memory footprint for Nmslib (opensearch-project#1602) Signed-off-by: Navneet Verma <navneev@amazon.com>

navneet1v added Enhancements Increases software capabilities beyond original client specifications skip-changelog v2.14.0 labels Mar 29, 2024

navneet1v requested review from heemin32, VijayanB, vamshin, jmazanec15, naveentatikonda, junqiu-lei, martin-gaievski and ryanbogan as code owners March 29, 2024 07:18

navneet1v force-pushed the stream-vectors branch from fd192ea to e980b6f Compare March 29, 2024 07:23

jmazanec15 reviewed Mar 29, 2024

View reviewed changes

src/main/java/org/opensearch/knn/jni/FaissService.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/knn/jni/JNICommons.java Outdated Show resolved Hide resolved

jni/src/commons.cpp Outdated Show resolved Hide resolved

jni/src/commons.cpp Outdated Show resolved Hide resolved

navneet1v force-pushed the stream-vectors branch 2 times, most recently from c4f4b63 to 32027fe Compare April 1, 2024 18:57

navneet1v requested a review from jmazanec15 April 1, 2024 18:58

jmazanec15 approved these changes Apr 1, 2024

View reviewed changes

src/main/java/org/opensearch/knn/jni/JNICommons.java Show resolved Hide resolved

navneet1v force-pushed the stream-vectors branch from 32027fe to 3368a67 Compare April 1, 2024 23:09

Add the interface for streaming the vectors from java to jni layer wi…

0a5b008

…th initial capacity Signed-off-by: Navneet Verma <navneev@amazon.com>

navneet1v force-pushed the stream-vectors branch from 3368a67 to 0a5b008 Compare April 2, 2024 17:23

navneet1v requested a review from jmazanec15 April 2, 2024 17:24

jmazanec15 approved these changes Apr 2, 2024

View reviewed changes

martin-gaievski approved these changes Apr 2, 2024

View reviewed changes

navneet1v merged commit fccc5a9 into opensearch-project:feature/stream-vectors Apr 2, 2024
45 of 51 checks passed

navneet1v mentioned this pull request Apr 5, 2024

Update KNN80BinaryDocValues reader count live docs and use live docs as initial capacity to initialize vector address #1595

Merged

5 tasks

navneet1v mentioned this pull request Apr 9, 2024

Implemented the Streaming Feature to stream vectors from Java to JNI layer to enable creation of larger segments for vector indices #1604

Merged

5 tasks

navneet1v mentioned this pull request Apr 10, 2024

[Backport 2.x] Implemented the Streaming Feature to stream vectors from Java to JNI layer to enable creation of larger segments for vector indices (#1604) #1608

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the interface for streaming the vectors from java to jni layer with initial capacity #1586

Add the interface for streaming the vectors from java to jni layer with initial capacity #1586

navneet1v commented Mar 29, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

jmazanec15 left a comment

jmazanec15 commented Apr 2, 2024

martin-gaievski commented Apr 2, 2024

navneet1v commented Apr 2, 2024

navneet1v commented Apr 2, 2024

navneet1v commented Apr 2, 2024

jmazanec15 commented Apr 2, 2024

navneet1v commented Apr 2, 2024

martin-gaievski left a comment

Add the interface for streaming the vectors from java to jni layer with initial capacity #1586

Add the interface for streaming the vectors from java to jni layer with initial capacity #1586

Conversation

navneet1v commented Mar 29, 2024 • edited Loading

Description

JNI test

Old benchmarks

New Benchmarks

Issues Resolved

Check List

codecov bot commented Mar 29, 2024 • edited Loading

Codecov Report

jmazanec15 left a comment

Choose a reason for hiding this comment

jmazanec15 commented Apr 2, 2024

martin-gaievski commented Apr 2, 2024

navneet1v commented Apr 2, 2024

navneet1v commented Apr 2, 2024

navneet1v commented Apr 2, 2024

jmazanec15 commented Apr 2, 2024

navneet1v commented Apr 2, 2024

martin-gaievski left a comment

Choose a reason for hiding this comment

navneet1v commented Mar 29, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading