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

Should we refactor jni layer to separate jni and pure c++ components completely? #1822

Open
jmazanec15 opened this issue Jul 12, 2024 · 0 comments
Labels
Refactoring Improve the design, structure, and implementation while preserving its functionality

Comments

@jmazanec15
Copy link
Member

Description

Comes as a spin off of #1776. With all of the mocking around the JNI components and running valgrind, I am having trouble to determine what memory issues are related to testing and what are actually functional.

Right now, our JNI functions make calls to library wrappers passing with them a JNIUtil class. The JNI util class provides a layer between the interfaces with pure c++ and the direct JNI calls so that testing and mocking are easier. In other words, the wrapper code will not directly call any jni functions.

However, as a lot of functionality has been added, the tests are getting more difficult to manage as we mix calls for java/c++ conversion with calls to the libraries. For example, see https://github.com/opensearch-project/k-NN/blob/main/jni/src/faiss_wrapper.cpp#L90-L156. This ends up requiring a complex and error prone testing and mocking structure.

I think we can simplify this quite a bit by making a clear separation between the pure c++ and java code. To do this, for our wrapper interfaces, instead of passing JNIUtil, we only pass pure C++ components. For performance reasons, if the component does need to be converted in the function that is being called, as opposed to before the function call (i.e. maybe to avoid a copy), we can pass suppliers and or callbacks that will take care of this instead. Then, we will have a thin jni->cpp layer that will bridge jni calls with pure c++ calls.

Then, the testing of c++ wrappers, which carries the most complexity, can be done in pure C++. For important JNI conversion functionality, we can implement our own mocking and unit tests and then also cover from the Java side tests.

@jmazanec15 jmazanec15 added the Refactoring Improve the design, structure, and implementation while preserving its functionality label Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactoring Improve the design, structure, and implementation while preserving its functionality
Projects
Status: Backlog
Development

No branches or pull requests

1 participant