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

mgroth0 · 2022-07-06T04:48:19Z

Resolves #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:

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.
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.
In the static block, unpack_from_jar should eventually be replaced by copy_to_temp_dir for all platforms.
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.

Andrey1994

Hi and thanks for PR.

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

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.

According to this doc https://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}/LIBRARY_FILENAME, where ${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

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

mgroth0 · 2022-07-06T22:16:32Z

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 · 2022-07-06T22:23:48Z

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 · 2022-07-06T22:29:28Z

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 · 2022-07-06T22:33:40Z

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.

Andrey1994 · 2022-07-07T13:49:08Z

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 · 2022-07-07T13:51:18Z

java_package/brainflow/src/main/java/brainflow/BoardShim.java

@@ -152,31 +152,14 @@ int get_current_board_data (int num_samples, double[] data_buf, int[] returned_s
        } else
        {
            // need to extract libraries from jar
-            unpack_from_jar (lib_name);
+            /* unpack_from_jar (lib_name); */


why is it commented out?

Andrey1994 · 2022-07-07T13:54:10Z

java_package/brainflow/src/main/java/brainflow/NativeLoader.java

+    static void copy_to_temp_dir (String lib_name)
+    {
+        // https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/MacOSXDirectories/MacOSXDirectories.html
+        File jnatmp = new File (System.getProperty ("user.home"), "Library/Caches/JNA/temp/" + lib_name);


I dont like hardcode for jna temp dir, there is a method for it https://github.com/java-native-access/jna/blob/bf60e51eace6dffa18548019e2ba398ff84904ef/src/com/sun/jna/Native.java#L1321 and its OS agnostic

moreover jna temp dir can be overriden and this method takes it into account

…d for recording ganglion data, and just cause unneccesary warnings

Andrey1994 · 2023-07-30T22:19:26Z

fixed in another PR for all OSes

mgroth0 added 2 commits July 5, 2022 21:32

add to gitignore

3e99fb8

make dylibs go to temp folder on mac

9406d67

mgroth0 mentioned this pull request Jul 6, 2022

[Java] Can there be a solution that is better than BoardShim.unpack_from_jar? #517

Closed

Andrey1994 self-requested a review July 6, 2022 05:33

Andrey1994 requested changes Jul 6, 2022

View reviewed changes

centralize native lib loading logic

2710c13

mgroth0 marked this pull request as draft July 6, 2022 22:12

reformat code

7cc2f19

Andrey1994 reviewed Jul 7, 2022

View reviewed changes

mgroth0 added 4 commits September 19, 2022 14:14

merge

eaaf3a9

Merge branch 'master' of https://github.com/brainflow-dev/brainflow

ed03d69

disable mac native labs that are not built for some reason, not neede…

8cc1562

…d for recording ganglion data, and just cause unneccesary warnings

did merge

0158282

Andrey1994 mentioned this pull request Jul 30, 2023

move unpack_from_jar to its own class #656

Merged

Andrey1994 closed this Jul 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mgroth0 commented Jul 6, 2022 •

edited

Loading

Andrey1994 left a comment

mgroth0 commented Jul 6, 2022

mgroth0 commented Jul 6, 2022

mgroth0 commented Jul 6, 2022

mgroth0 commented Jul 6, 2022

Andrey1994 commented Jul 7, 2022

Andrey1994 Jul 7, 2022

Andrey1994 Jul 7, 2022

Andrey1994 Jul 7, 2022

Andrey1994 commented Jul 30, 2023

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

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

Conversation

mgroth0 commented Jul 6, 2022 • edited Loading

Andrey1994 left a comment

Choose a reason for hiding this comment

mgroth0 commented Jul 6, 2022

mgroth0 commented Jul 6, 2022

mgroth0 commented Jul 6, 2022

mgroth0 commented Jul 6, 2022

Andrey1994 commented Jul 7, 2022

Andrey1994 Jul 7, 2022

Choose a reason for hiding this comment

Andrey1994 Jul 7, 2022

Choose a reason for hiding this comment

Andrey1994 Jul 7, 2022

Choose a reason for hiding this comment

Andrey1994 commented Jul 30, 2023

mgroth0 commented Jul 6, 2022 •

edited

Loading