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

Do not link onnxruntime jni output with JNI_LIBRARIES #2873

Merged
merged 1 commit into from
Jan 30, 2020

Conversation

mina-asham
Copy link
Contributor

Description: Remove unnecessary link with JNI_LIBRARIES

  • Linking onnxruntime with JNI_LIBRARIES includes some unnecessary links to native libraries (e.g. libawt) which are not actually used or required by the output onnx library. This causes unsatisfied link exceptions when trying to load the onnx library without including these libraries.

Motivation and Context

  • Why is this change required? What problem does it solve?

When I tried to build the library locally and tried to use it elsewhere I noticed that it failed due to an unsatisfied link to libawt (and others), upon investigations and looking at other JNI libraries I am using (I used ldd command to figure out links), none of them had links to JNI libraries as they are not required (at least in these simple JNI scenarios)

  • If it fixes an open issue, please link to the issue here.

N/A (Couldn't find an issue related to this, happy to open one if that's a requirement)

- Linking onnxruntime with JNI_LIBRARIES includes some unnecessary links to native libraries (e.g. libawt) which are not actually used or required by the output onnx library. This causes unsatisfied link exceptions when trying to load the onnx library without including these libraries.
@mina-asham mina-asham requested a review from a team as a code owner January 20, 2020 09:05
@Craigacp
Copy link
Contributor

This probably works because the JVM is already loaded at the time that the JNI binding is linked in, but it does call functions in libjvm.so, and so that should be linked against. Dynamic linking is a tricky thing in this case though. Have you checked that this works on macOS and Windows?

@mina-asham
Copy link
Contributor Author

mina-asham commented Jan 25, 2020

@Craigacp I suspect the JVM does some special loading for these libraries on its own as well. I looked at other native libraries I used (e.g. Junixsocket) and it didn't need that linking as well.

I did try it on MacOs and Linux but not on Windows, however that change will break the Java tests if it's not working right, so as long the auto builds work that means it's fine.

EDIT: I did try it on ALL platforms (Linux, MacOS, and Windows), all worked fine with this change.

@Craigacp
Copy link
Contributor

Craigacp commented Jan 28, 2020

It should have loaded libjvm.so or the equivalent by the time any JNI code runs, so it's not too much of an issue if it isn't linked to it. What platform is this causing an issue on? On my Linux box using Oracle JDK 8 using the rel-1.1.0 tag it's not linked against any JVM libraries.

apocock@hypnotoad:/local/ExternalRepositories/onnxruntime$ ldd build/Linux/Debug/libonnxruntime4j_jni.so 
	linux-vdso.so.1 (0x00007ffd005e0000)
	libgtk3-nocsd.so.0 => /usr/lib/x86_64-linux-gnu/libgtk3-nocsd.so.0 (0x00007fa51cabc000)
	libonnxruntime.so.1.1.0 => /local/ExternalRepositories/onnxruntime/build/Linux/Debug/libonnxruntime.so.1.1.0 (0x00007fa51bc4f000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa51ba5e000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fa51ba58000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa51ba35000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fa51b845000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa51b6f6000)
	libgomp.so.1 => /usr/lib/x86_64-linux-gnu/libgomp.so.1 (0x00007fa51b6bc000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fa51b6a2000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa51cd00000)

@mina-asham
Copy link
Contributor Author

mina-asham commented Jan 29, 2020

It should have loaded libjvm.so or the equivalent by the time any JNI code runs, so it's not too much of an issue if it isn't linked to it. What platform is this causing an issue on? On my Linux box using Oracle JDK 8 using the rel-1.1.0 tag it's not linked against any JVM libraries.

apocock@hypnotoad:/local/ExternalRepositories/onnxruntime$ ldd build/Linux/Debug/libonnxruntime4j_jni.so 
	linux-vdso.so.1 (0x00007ffd005e0000)
	libgtk3-nocsd.so.0 => /usr/lib/x86_64-linux-gnu/libgtk3-nocsd.so.0 (0x00007fa51cabc000)
	libonnxruntime.so.1.1.0 => /local/ExternalRepositories/onnxruntime/build/Linux/Debug/libonnxruntime.so.1.1.0 (0x00007fa51bc4f000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa51ba5e000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fa51ba58000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa51ba35000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fa51b845000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa51b6f6000)
	libgomp.so.1 => /usr/lib/x86_64-linux-gnu/libgomp.so.1 (0x00007fa51b6bc000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fa51b6a2000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa51cd00000)

That's interesting, doing almost the same (only difference is building Release instead of Debug) I get a different result (I am using Corretto 8 on Amazon Linux, but I don't think that should cause difference here):

git clone https://github.com/microsoft/onnxruntime.git
cd onnxruntime
git checkout tags/v1.1.0

./build.sh --config Release --build_java --parallel --skip_tests

ldd build/Linux/Release/libonnxruntime4j_jni.so

	linux-vdso.so.1 =>  (0x00007fffb0777000)
	libjawt.so => /usr/lib/jvm/jre/lib/amd64/libjawt.so (0x00007f4041992000)
	libjvm.so => /usr/lib/jvm/jre/lib/amd64/server/libjvm.so (0x00007f4040a0e000)
	libonnxruntime.so.1.1.0 => /tmp/onnxruntime/build/Linux/Release/libonnxruntime.so.1.1.0 (0x00007f403ffa8000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f403fc14000)
	libawt.so => /usr/lib/jvm/jre/lib/amd64/libawt.so (0x00007f403f943000)
	libawt_xawt.so => /usr/lib/jvm/jre/lib/amd64/libawt_xawt.so (0x00007f403f6e2000)
	libjava.so => /usr/lib/jvm/jre/lib/amd64/libjava.so (0x00007f403f4b6000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f403f232000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007f403f02e000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f403ee11000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f4041d9d000)
	librt.so.1 => /lib64/librt.so.1 (0x00007f403ec09000)
	libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x00007f403e884000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f403e66e000)
	libXext.so.6 => /usr/lib64/libXext.so.6 (0x00007f403e45c000)
	libX11.so.6 => /usr/lib64/libX11.so.6 (0x00007f403e11f000)
	libXrender.so.1 => /usr/lib64/libXrender.so.1 (0x00007f403df16000)
	libXtst.so.6 => /usr/lib64/libXtst.so.6 (0x00007f403dd11000)
	libXi.so.6 => /usr/lib64/libXi.so.6 (0x00007f403db02000)
	libverify.so => /usr/lib/jvm/jre/lib/amd64/libverify.so (0x00007f403d8ef000)
	libxcb.so.1 => /usr/lib64/libxcb.so.1 (0x00007f403d6ca000)
	libXau.so.6 => /usr/lib64/libXau.so.6 (0x00007f403d4c7000)

@Craigacp
Copy link
Contributor

Craigacp commented Jan 29, 2020

That's very odd. I looked at the CMake FindJNI file and it's a ball of platform specific hacks and regexes (https://github.com/Kitware/CMake/blob/master/Modules/FindJNI.cmake) so I'm assuming it's hitting something slightly different on the two machines. I'll try building for Release and see if that makes a difference.

I did wonder if it was an arm64 thing, as it doesn't seem to be special cased in the jni finder, but you're on x86_64.

@Craigacp
Copy link
Contributor

I built on release and I still don't have any links. I suspect this is because the FindJNI script is doing something a little wrong on my Ubuntu 19.10 box and it's carrying on regardless. So it's probably fine to remove the link. It worked ok without the link on windows when I tested the fix for the native loader earlier.

@mina-asham
Copy link
Contributor Author

@Craigacp thanks for helping out here, should we merge this or do you want to incorporate it in the bigger refactor #2866?

@snnn
Copy link
Member

snnn commented Jan 29, 2020

/azp run Linux CPU CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,MacOS CI Pipeline,Win CPU x86 CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,Win CPU x64 NoContribops CI Pipeline,MacOS NoContribops CI Pipeline,Linux CPU x64 NoContribops CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 10 pipeline(s).

@yuzawa-san
Copy link
Contributor

yuzawa-san commented Jan 29, 2020

Assuming we are all in agreement this works, I can take this tiny change into #2866. I'll do that once this passes CI

@snnn
Copy link
Member

snnn commented Jan 29, 2020

We can keep it separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants