Use nativeloader instead of system loader to load JNI library for soloader compatibility. #29350
Conversation
Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/) [ghstack-poisoned]
Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/) ghstack-source-id: 93421897 Pull Request resolved: #29350
Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/) [ghstack-poisoned]
Pull Request resolved: #29350 ghstack-source-id: 93453413 Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/)
Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/) [ghstack-poisoned]
Pull Request resolved: #29350 ghstack-source-id: 93460392 Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/)
Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/) [ghstack-poisoned]
Pull Request resolved: #29350 ghstack-source-id: 93469427 Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/)
There was a problem hiding this comment.
Nit: The "T" in PyTorch is capitalized.
Title shouldn't have "[PyTorch]" in GitHub (ghimport should automatically add it internally).
Title should probably clearly state "Use NativeLoader instead of system loader to load JNI library", and should say why we want to do this (so it's compatible with SoLoader).
| libtorch | ||
| -Wl,--no-whole-archive |
There was a problem hiding this comment.
This is for the Android build. I don't think it needs to change.
| torch | ||
| -Wl,--no-whole-archive |
There was a problem hiding this comment.
Unfortunately, I suspect this breaks the Linux build, which links with gold. Any way we can get both Mac and Linux builds to work?
Also, this is not related to the subject of the diff. Please use separate diffs for unrelated changes.
| @@ -1,28 +1,22 @@ | |||
| package org.pytorch; | |||
|
|
|||
| import com.facebook.soloader.nativeloader.NativeLoader; | |||
There was a problem hiding this comment.
It looks like this file was reformatted. Can you do that as a separate diff first to make this change more self-contained?
| NativeLoader.init(new SystemDelegate()); | ||
| } | ||
| } | ||
| public static void setUpClass() {} |
| public void setUp() { | ||
| System.loadLibrary("pytorch"); | ||
| } | ||
| public void setUp() {} |
| 3.1415f, | ||
| -1, | ||
| 0, | ||
| 1, |
There was a problem hiding this comment.
Reformat should be a separate diff.
There was a problem hiding this comment.
when update the diff, it auto-format the file, i can revert it.
| @@ -418,7 +418,7 @@ class JIValue : public facebook::jni::JavaClass<JIValue> { | |||
| } else if (JIValue::kTypeCodeLong == typeCode) { | |||
| static const auto jMethodGetLong = | |||
| JIValue::javaClassStatic()->getMethod<jlong()>("toLong"); | |||
| return at::IValue{jMethodGetLong(jivalue)}; | |||
| return at::IValue{(int64_t)jMethodGetLong(jivalue)}; | |||
There was a problem hiding this comment.
This is part of the Mac PR, right?
There was a problem hiding this comment.
yes, It is for mac, I will create separate pr for it.
Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/) [ghstack-poisoned]
Pull Request resolved: #29350 ghstack-source-id: 93491099 Differential Revision: [D18352773](https://our.internmc.facebook.com/intern/diff/D18352773/)
xcheng16
changed the title
xcheng16
changed the title
|
This pull request has been merged in 8a33f11. |
Stack from ghstack:
Differential Revision: D18352773