brainflow icon indicating copy to clipboard operation
brainflow copied to clipboard

make dylibs go to temp folder / use JNI context classloader on mac

Open mgroth0 opened this issue 2 years ago • 5 comments

Resolves https://github.com/brainflow-dev/brainflow/issues/517 for mac. Should be generalized to all platforms.

This PR changes the way that the native libraries are built into the java package on mac. It also contains a commit adding a few lines to the .gitignore because these files were generated in my project while I was developing the PR and I figured they belonged in the .gitignore.

I have tested the jar in a simple program that connects to an OpenBCI Ganglion, and it works. No more dylibs are copied into the project folder.

I think that generalizing this to other platforms will be straightforward:

  1. Create more resource nodes in pom.xml for each platform. The correct platform name needs to be used. replace darwin-aarch64 with the result of this function for each platform.
  2. In Boardshim.java, copy_to_temp_dir should be generalized to other platforms by getting the correct temp dir from this logic. Currently it only has the correct temp dir for Mac.
  3. In the static block, unpack_from_jar should eventually be replaced by copy_to_temp_dir for all platforms.
  4. Each platform should be tested to make sure all native libraries are correctly loaded and no native libraries are copied into the project folder any more.

mgroth0 avatar Jul 06 '22 04:07 mgroth0

Thanks for the review. I agree that this needs work.

First of all it should be done for all 3 main classes, not only for BoardShim. There are also DataFilter and MLModule.

I've centralized these functions to a new class in the above commit, so all 3 classes will share the same logic.

mgroth0 avatar Jul 06 '22 22:07 mgroth0

Also, I think it should be done for all OSes at the same time and it should be consistent.

I am also not sure in folder name "darwin-aarch64", maybe it doesnt work for intel based macs. Libraries for brainflow are universal and support both architectures.

To solve the intel mac problem, we can just copy the native libraries to both architecture folders in the pom.xml. I don't know what the other architecture names are. I think the best way to find out would be to iterate through all platforms somehow and run com.sun.jna.Platform.getNativeLibraryResourcePrefix() and record the return value for each system. That function is also package private so reflection will be needed to access it.

mgroth0 avatar Jul 06 '22 22:07 mgroth0

According to this doc java-native-access.github.io/jna/4.2.1/com/sun/jna/NativeLibrary.html#library_search_paths "Context class loader classpath. Deployed native libraries may be installed on the classpath under{os-prefix} is the OS/Arch prefix returned by Platform.getNativeLibraryResourcePrefix(). If bundled in a jar file, the resource will be extracted to jna.tmpdir for loading, and later removed (but only if jna.nounpack is false or not set)." if I understand it correctly you should not extract it by yourself, jna should unpack it automatically, so more likely we dont even need copy_to_temp_dir method and JNA can(should) do it for all OSes

Yeah, I expected the same. I only wrote copy_to_temp_dir because I was getting errors originating from the cpp code.

I have no experience with c or c++ so I have very little idea of what I'm doing in that code. But from what I could tell, the error was coming from runtime_dll_loader. My very basic understanding/assumption is that cpp is trying to load certain libraries at runtime before JNA has tried to access them. JNA only copies the native libs when it needs them, so some of the native libs are still in the jar file when cpp asks for them. And the jar file is not within cpp's search path.

I'm not 100% sure that's what's happening. But copying all the libs to the temp dir manually with copy_to_temp_dir completely solved the issue for me.

mgroth0 avatar Jul 06 '22 22:07 mgroth0

Finally you need to use this formatter https://github.com/brainflow-dev/brainflow/blob/master/java_package/brainflow_formatter.xml to keep code style consistent

I have just tried reformatting. I do not have Eclipse, and I use IntelliJ. Luckily. IntelliJ has an "Adapter for Eclipse Code Formatter" plugin. I tried it and imported the file you linked, so I hope it worked. The plugin did have a lot of settings though, so I'm not sure.

mgroth0 avatar Jul 06 '22 22:07 mgroth0

Also, I think it should be done for all OSes at the same time and it should be consistent. I am also not sure in folder name "darwin-aarch64", maybe it doesnt work for intel based macs. Libraries for brainflow are universal and support both architectures.

To solve the intel mac problem, we can just copy the native libraries to both architecture folders in the pom.xml. I don't know what the other architecture names are. I think the best way to find out would be to iterate through all platforms somehow and run com.sun.jna.Platform.getNativeLibraryResourcePrefix() and record the return value for each system. That function is also package private so reflection will be needed to access it.

yes, I was also thinking about copying to a folder with different name. There is a project that does smth similar. You can download this jar and open it with 7zip https://mvnrepository.com/artifact/com.microsoft.onnxruntime/onnxruntime/1.11.0 it will tell you at least some of the folder names used by jna

Another options which is easier than reflection is to take a look at the source code https://github.com/java-native-access/jna/search?q=getNativeLibraryResourcePrefix and https://github.com/java-native-access/jna/blob/bf60e51eace6dffa18548019e2ba398ff84904ef/src/com/sun/jna/Platform.java#L307

Andrey1994 avatar Jul 07 '22 13:07 Andrey1994

fixed in another PR for all OSes

Andrey1994 avatar Jul 30 '23 22:07 Andrey1994