Enable ARM (aarch64) support for cuvs-java and cuvs-lucene - #2541
Conversation
The C++ core already builds and is tested on both amd64 and arm64; cuvs-java had a hardcoded amd64-only gate that cuvs-lucene inherited. Java bytecode is portable, but cuvs-java isn't pure Java — it uses the Panama FFM API to call into native libcuvs_c.so, and those bindings are generated by jextract, which bakes struct/function ABI layouts (computed by parsing the C headers with clang for the host's target triple) into the generated .class files at build time. Reusing an amd64-generated binding on aarch64 should work, since no arch-conditional logic in the jextract-visible header surface changes any type layout, struct offset, or function signature between x86_64 and aarch64 Linux. We therefore expect the plain (no-native-bundled) jar to be arch-portable in practice. To verify that, the aarch64 cuvs-lucene CI job installs the amd64-built jar and runs cuvs-lucene's full test suite against it on aarch64 hardware with an aarch64-native libcuvs_c.so. Closes NVIDIA#1236.
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test 762531b |
imotov
added
improvement
jameslamb
left a comment
jameslamb
left a comment
There was a problem hiding this comment.
Implementation looks good to me but I'm a bit confused about the goal, left some questions. Sorry in advance if they are naive / ignorant 😅
| script: "ci/build_java.sh" | ||
| artifact-name: "cuvs-java-cuda${{ matrix.CUDA_VER }}" | ||
| # amd64 is the official published artifact; keep its name unqualified and only suffix other arches so they don't collide with it. | ||
| artifact-name: "cuvs-java-cuda${{ matrix.CUDA_VER }}${{ matrix.ARCH != 'amd64' && format('-{0}', matrix.ARCH) || '' }}" |
There was a problem hiding this comment.
What do you mean here by "official published artifact" and what specifically would break if we appended the architecture unconditionally to it?
I think this is just the name of a GitHub Actions artifact used for passing files around between CI runs, not an identifier that makes it to Maven Central or anything.
There was a problem hiding this comment.
Actually, maybe here's my confusion. Seeing this in the PR description:
We therefore expect the plain (no-native-bundled) jar to be arch-portable in practice
Is that "plain jar" what's built by build_java.sh? And if so, shouldn't this job continue to just be run on amd64?
There was a problem hiding this comment.
@jameslamb Yep, that's a tricky one. So if you have any ideas on how to do it better, I could definitely use them. The problem is that part of cuvs-java is generated by Panama from C headers. Theoretically, that part is platform-dependent. In practice, though, in our case it's the same for both architectures. So on one hand, publishing two basically identical jars for different architectures seems silly; on the other hand, we should make sure that 1) it builds on arm, 2) it works on arm, and 3) we can use the amd64-built jar on arm - i.e. that our practical assumption that the jars are functionally the same actually holds.
There was a problem hiding this comment.
Let me rephrase... if "we can use the amd64-build jar on arm" is true, why does "it builds on arm" matter?
Do we have users who are building these jars from source in arm64 environments?
There was a problem hiding this comment.
I don't think any users build it from source on arm64 today, since there is a check preventing cuvs-java from running on arm64, which this PR removes. So until this commit is merged, nobody can use it on arm64 anyway. However, I think after this PR is merged it will useful to build on arm64 to catch breaking changes on that platform. Maybe the right answer is to build and test fully on arm64, and then separately test that arm64 works with the amd64 build?
There was a problem hiding this comment.
I have removed building cuvs-java and cuvs-lucene on arm64 and instead added a couple of scripts to just run the tests with jars downloaded from amd64 build.
Test jars built on amd64 to run tests instead.
|
/ok to test 292ed0f |
2 participants
The C++ core already builds and is tested on both amd64 and arm64; cuvs-java had a hardcoded amd64-only gate that cuvs-lucene inherited.
Java bytecode is portable, but cuvs-java isn't pure Java — it uses the Panama FFM API to call into native libcuvs_c.so, and those bindings are generated by jextract, which bakes struct/function ABI layouts (computed by parsing the C headers with clang for the host's target triple) into the generated .class files at build time. Reusing an amd64-generated binding on aarch64 should work, since no arch-conditional logic in the jextract-visible header surface changes any type layout, struct offset, or function signature between x86_64 and aarch64 Linux.
We therefore expect the plain (no-native-bundled) jar to be arch-portable in practice. To verify that, the aarch64 cuvs-lucene CI job installs the amd64-built jar and runs cuvs-lucene's full test suite against it on aarch64 hardware with an aarch64-native libcuvs_c.so.
Closes #1236.