-
Notifications
You must be signed in to change notification settings - Fork 591
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
Add property for directly loading libraries #565
Conversation
An alternative that saves the addition of a new property is to check for the presence of the library to load in the directory of runtime libraries before searching in the resources and other paths, and if it is present, |
Could you explain the purpose of this? Is it just to save a couple of ms at launch time? Do you have numbers to back your proposal? |
It saves the search of resources we know they do not exist , indeed, but it's not the main reason of this PR: |
When the cache is disabled for libraries, no links are created, so I still don't understand what we get from disabling the search. |
Since my english seems deficient, here are some command lines: $ cd /home/java/javafx-sample/build/image/bin
$ ./java -Dorg.bytedeco.javacpp.cacheLibraries=false -m org.bytedeco.javafxsample/org.bytedeco.javafxsample.HelloApplication &
[1] 3704
grep opencv /proc/3704/maps
...
7fe6f57f7000-7fe6f58bd000 r--p 00354000 103:02 2747494 /usr/lib64/libopencv_core.so
... $ ./java Dorg.bytedeco.javacpp.librariesAreProvided -m org.bytedeco.javafxsample/org.bytedeco.javafxsample.HelloApplication &
[1] 3800
grep opencv /proc/3800/maps
...
7f526b9c2000-7f526ba19000 r--p 00000000 103:04 3605252 /home/java/javafx-sample/build/image/lib/libopencv_core.so.405 In other words, and if I understand
This PR suggests to change the precedence of 3) so that it becomes prioritary.
This is probably more satisfactory. It doesn't require to set a property, and can work for mixed cases where some libraries are in the runtime, and some are not. But needs some more lines of codes, and the use of |
That's what the "org.bytedeco.javacpp.pathsFirst" system property is for, so I guess we should add "sun.boot.library.path" before "java.library.path"? Let me try to do that |
… for jlink (pull #565) * Add `__int8`, `__int16`, `__int32`, and `__int64` to `InfoMap` as "basic/types" to support combinations allowed by Visual Studio
I've added that in commit 213d621 and it seems to work fine for me. Please give it a try and let me know what you think! |
What I understand to your commit is that, when So now without
and with
Right ? This behaviour is logical for a property called |
What are the "system paths" that you are referring to? Those get added to "java.library.path" by default, so if we search "sun.boot.library.path" before "java.library.path", then the "system paths" get searched after "sun.boot.library.path" because they are part of "java.library.path", which comes after, regardless of the value of |
The content of |
BTW, I'm currently preparing a MacOSX app for distribution and I'm realizing that, when an app is signed (which is more or less mandatory now), the executable cannot dynamically link with a library unless it is signed by the same team or by Apple. It conforts me in the idea that extracting the libs in the runtime (before signature) is the way to go (and that disabling loading from resources or non-standard path could be disabled like proposed first in this PR, but I guess there are situations where we must link with library installed by the user that we cannot bundle, and thus we need |
There's the preload path, yes, but that's looked up after bundled resources, because it's also used at build time to bundle libraries. That makes it possible for users to leave out bundled resources and have everything loaded from the system, yes. However, for applications like yours, with |
Are you sure about this ? javacpp/src/main/java/org/bytedeco/javacpp/Loader.java Lines 1542 to 1546 in 213d621
is that linkpath and preloadpath are searched before sub.boot.library.path and java.library.path
|
Hum, you're right. I think that should be changed/fixed because those paths are pretty much hard coded from an end-user perspective, so there's no point in always forcing them. |
After looking into this some more, we can't move the library paths before the preload ones by default, because at build time, it would make it too easy to pick incorrectly libraries from the system instead of the ones in the build directories. I think your idea of a flag to disable searching altogether is probably more desirable than another setting to tamper further with the search order. I've pushed a couple of adjustments however. Could you give that a try and let me know what you think? |
Thank you. I'll will try it out. However, people may want to extract the libs in a directory which is not the runtime library directory (to avoid to mess with it) and point the JVM to this directory using |
I don't think that's a good idea. On most non-Windows platforms, libraries have versions, and even when a library has the same name and version as another already loaded library, it is still possible to load both in the same process, as long as the symbols are not loaded globally, which is what |
After this fix, the new property works as expected. |
Add a property (
librariesAreProvided
, feel free to suggest a better name) that allows to skip entirely library search in both resources and paths, and directlySystem.load
them.It's meant to be used when launching applications shipped with a Java runtime providing the native libraries and not including the native jars/modules.
Instead of a boolean property, we could arrange to allow specifying which libraries are provided, for instance with
-DprovidedLibraries=opencv_core,opencv_imgproc
but It probably doesn't worth the effort.With the addition of this property, I'm not sur the newly added property
cacheLibraries
still has a use case. We could remove it.This PR also include a small fix to shorten the codepath when defining
styles
andstyles2
.