-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
[jvm-packages] JVM library loader extensions #6630
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, it doesn't seem to introduce any change to library release, ping @hcho3 for double check from CI, etc.
Once this is merged I'd like to figure out how to get the release jar for XGBoost4J to contain Windows x86_64, macOS x86_64 and Linux x86_64 as I mentioned in the linked discussion. Most of the time it's got both macOS and Linux x86_64 (though I note that the 1.3.2 in the S3 bucket is missing it as the macOS CI for Tribuo failed when I upgraded - https://github.com/oracle/tribuo/actions/runs/502639696), and the lack of Windows x86_64 means I have to patch around in the CI and some of the Tribuo tutorials don't run on Windows. I'm happy to do the work on the CI to make that happen, though it looks like it's mostly there as there is already a Windows JVM github action. This change allows me to put in an Apple Silicon binary and a Linux aarch64 binary into the same jar as the x86_64 binaries which makes life a lot simpler in my internal deployment. I can compile the former if there is interest in bundling it in, and when the Oracle ARM servers launch it might be possible to get CI builds for Linux aarch64 to bundle that in as well. |
@CodingCat @hcho3 Is it possible that we discuss a roadmap according to proposal from @Craigacp ? |
Is there any further discussion of including the Windows binary in the published Maven Central jars? Given it's already tested in Github Actions I'm not sure what else needs doing, but I'm happy to do the work if someone lets me know what it is. |
@Craigacp I just got back from a 1-month vacation. The next step is to modify the GitHub Actions pipeline to build |
Ah yes, sorry, you did mention you were away before. This action (https://github.com/dmlc/xgboost/blob/master/.github/workflows/main.yml#L144) is already building |
@Craigacp It's best that I take up the task, since it involves uploading an artifact to a private location and I am managing the CI/CD pipeline for this project. In the meanwhile, can you check whether adding |
That was sufficient for the XGBoost4J Java API to work on versions 0.7-1.0, as that's what we've done internally (we have a jar with Windows, macOS and Linux x86_64 in our internal artifactory). We only use the single node functionality though, I haven't used xgboost4j-spark. I will double check that it's sufficient for 1.3.3 at the weekend, but I can't see any reason for it not to be if the tests are passing. |
This PR modifies the JVM library loader, the
create_jni.py
script and the jar file structure so that the native libraries are stored in/lib/<os_name>/<arch_name>/<library_name>
inside the jar. This means you can pack all the native libraries into a single jar file and have it work on all the supported platforms.I need to double check that the Windows values reported by Python's
platform.machine()
are correct, I've tested Linux x86_64 and macOS arm64 and x86_64. I think it might be missing some values for Linux arm64 as it looks like that might report itself as armv8 or a variety of other things, but I don't have access to such a machine to check. The JVM will always report 64-bit ARM asaarch64
so it's only the mapping increate_jni.py
that will need updating. I poked in Solaris values too in case someone cares, I checked them on Solaris 11.4 x86_64 and sparc platforms.This is following on from this discussion - https://discuss.xgboost.ai/t/xgboost4j-support-on-windows/1829/8.