Patch files for nmslib and faiss creating enforcing compilation to happen during cmake

[navneet1v]

Problem

To enable features in Faiss and Nmslib there are 2 patch files which have been added in k-NN repo. The patches gets applied while running cmake or gradle task cmakeJniLib (nmslib, faiss).

Due to this the cmake command assumes that files have changed and it leads recompile the files and dependent files leading to increase in running of all the gradle tasks like integTest, test, run etc.

This really adds significant time during development process as you might have to run the tests/integTest task multiple times. This is not a problem for CIs and actual opensearch build process because it builds the k-NN plugin only once.

Solution

1. We should come up with a way to identify if patch is already applied on files and then skip adding the patches.
2. Another solution can be we should contribute back these features to respective libraries.

[jmazanec15]

@navneet1v yeah - I have noticed this. I have been building lib once locally then commenting out in gradle.

[navneet1v]

@navneet1v yeah - I have noticed this. I have been building lib once locally then commenting out in gradle.

+1 I am commenting the patch apply code in CMakeList.txt, as I got frustrated with build times. I cannot remove buildJni task as I needed changed in the C++ code.

I tried added --cached in the git apply patch command, seems to be working. But didn't fully understood its side effects as doc was not clear to me.

 --cached              apply a patch without touching the working tree

[jmazanec15]

@navneet1v tried this and it was failing to pick up source in the file:

1 warning generated.
[ 96%] Building CXX object CMakeFiles/opensearchknn_faiss.dir/src/faiss_wrapper.cpp.o
/Users/jmazane/workspace/Opensearch/DockerRunner/k-NN-1/jni/src/faiss_wrapper.cpp:276:17: error: no member named 'set_precomputed_table' in 'faiss::IndexIVFPQ'
    indexIVFPQ->set_precomputed_table(alignTable, usePrecomputedTable);
    ~~~~~~~~~~  ^
/Users/jmazane/workspace/Opensearch/DockerRunner/k-NN-1/jni/src/faiss_wrapper.cpp:330:28: error: no member named 'grp' in 'faiss::SearchParametersHNSW'
                hnswParams.grp = idGrouper.get();
                ~~~~~~~~~~ ^
/Users/jmazane/workspace/Opensearch/DockerRunner/k-NN-1/jni/src/faiss_wrapper.cpp:360:24: error: no member named 'grp' in 'faiss::SearchParametersHNSW'
            hnswParams.grp = idGrouper.get();
            ~~~~~~~~~~ ^
3 errors generated.
make[3]: *** [CMakeFiles/opensearchknn_faiss.dir/src/faiss_wrapper.cpp.o] Error 1
make[2]: *** [CMakeFiles/opensearchknn_faiss.dir/all] Error 2
make[1]: *** [CMakeFiles/opensearchknn_faiss.dir/rule] Error 2
make: *** [opensearchknn_faiss] Error 2

[jmazanec15]

I think we might be able to do:

git apply --reverse --check

ref: https://stackoverflow.com/questions/48167884/check-if-git-apply-was-already-applied

[jmazanec15]

This worked for me: https://gist.github.com/jmazanec15/a4cab0b6944e8e4b403c43a2113a5a4b. Ill raise a PR at some point. It is a little messy as of now.

[navneet1v]

will wait for the PR. I looked into that commands git apply --reverse --check but wasn't able to successfully make it. Good to see you are able to make it work. May be I won't be doing things in right way. Good to see that there is a path.