Add Android 16KB page size support for ONNX Runtime Extensions#1007
Add Android 16KB page size support for ONNX Runtime Extensions#1007ssk18 wants to merge 23 commits intomicrosoft:mainfrom
Conversation
enable 16KB page size compatibility for Android 15+ devices. - Apply -Wl,-z,max-page-size=16384 linker flag to all Android targets - Modernize to target_link_options() for better maintainability - Add Clang/GNU compiler check before applying flags - Configure Gradle with -DANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES=ON - Support all ABIs: arm64-v8a, armeabi-v7a, x86_64, x86
|
@microsoft-github-policy-service agree |
|
/azp run extensions.ci |
|
No pipelines are associated with this pull request. |
- Add README section explaining ANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES option - Add GitHub workflow to verify 16KB alignment on arm64-v8a builds
| @@ -0,0 +1,127 @@ | |||
| name: Android 16KB Page Size Verification | |||
There was a problem hiding this comment.
@sayanshaw24 is it ok to add a new workflow for this check? or would it be preferable to add it to the existing CI build pipeline?
There was a problem hiding this comment.
it would be preferable to add it to an existing one - @ssk18 could you add this to AndroidBuilds?
…B support, project requires NDK r27d which fully supports the linker flag, so no need for optional configuration
…and updated readme
Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com>
|
@edgchen1 is it a pre existing issue in tokenizer that's causing the workflow failure? |
hm, it looks unrelated to the changes in this PR. are you able to reproduce it locally? looks like tokenizer_common.h is missing an |
|
@edgchen1 Locally, I am able to create a shared library without any issue and, i am using NDK 26.1 but in a workflow it is NDK r25c. Not sure, if that is causing the failure. If you agree, I can add "#include <unordered_map>" and see if it works. |
sure, I think it's good to add it anyway. the tokenizer_common.h header should be able to be included independently. |
…able-16KB-for-android
|
/azp run onnxruntime-extensions.CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
so, libonnxruntime_extensions4j_jni.so and libortextensions.so passes as expected but CI workflow is checking all other libraries as well. Removed checks for other shared libraries. |
|
/azp run onnxruntime-extensions.CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@edgchen1 these CI failures look unrelated to this change. |
they are not related to this change. we'll need to look into it. |
|
I urgently need to solve this problem when 16kb support will be available. |
|
/azp run onnxruntime-extensions.CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run onnxruntime-extensions.CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@edgchen1 Could you please get this change merged by fixing the build. It's needed for android 16KB compatibility by March. |
|
/azp run onnxruntime-extensions.CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This pull request adds Android 16KB page size support to ensure compatibility with Android 15+ devices while maintaining backward compatibility with 4KB page systems. The changes apply the -Wl,-z,max-page-size=16384 linker flag to Android builds and configure NDK ABI filters for multi-architecture support.
Changes:
- Added 16KB page size linker flag to CMakeLists.txt for Android builds
- Configured NDK ABI filters in Gradle build files for consistent multi-architecture support
- Added CI workflow to verify 16KB page alignment in built libraries
- Fixed indentation in test app build.gradle
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Added global linker flag -Wl,-z,max-page-size=16384 for Android builds to ensure 16KB page alignment |
| java/build-android.gradle | Added NDK ABI filters for multi-architecture support (armeabi-v7a, arm64-v8a, x86, x86_64) |
| java/src/test/android/app/build.gradle | Added NDK ABI filters and fixed indentation for consistency |
| operators/tokenizer/tokenizer_common.h | Added missing <unordered_map> include (unrelated to 16KB page size support) |
| .github/workflows/android-16kb-check.yml | New CI workflow to build Android libraries and verify 16KB page alignment using llvm-readelf |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #include <string> | ||
| #include <string_view> | ||
| #include <unordered_map> |
There was a problem hiding this comment.
The addition of the <unordered_map> include appears unrelated to the stated purpose of this PR, which is to add Android 16KB page size support. While this include is used on line 40 (using AddedTokenMap = std::unordered_map<std::u32string, AddedToken>;), this change should be in a separate pull request focused on fixing missing includes rather than mixed with Android build configuration changes. This makes it harder to track the purpose of changes and could cause confusion when reviewing or reverting changes.
| add_link_options(-Wl,-z,max-page-size=16384) | ||
| message(STATUS "Android build: 16KB page size support enabled") |
There was a problem hiding this comment.
The PR description mentions changes to cmake/ext_java.cmake to "Configure JNI library with 16KB page alignment," but this file is not included in the pull request diff. Please verify if:
- This change was intended but accidentally omitted from the PR
- The change is no longer necessary (in which case the PR description should be updated)
- The PR description is incorrect
Given that the linker flag in CMakeLists.txt uses add_link_options() which applies globally to all targets built under the Android configuration, it should affect the JNI library. However, if there are specific configurations needed in ext_java.cmake, they should be included in this PR.
| add_link_options(-Wl,-z,max-page-size=16384) | ||
| message(STATUS "Android build: 16KB page size support enabled") |
There was a problem hiding this comment.
The PR description states that tools/build.py should have changes to "Enable ANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES flag," but:
- This file is not included in the pull request diff
- A search of the file shows no references to
ANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES
The ANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES flag is a Gradle property that can be set in gradle.properties or as a CMake argument to signal Android build systems that the app supports flexible page sizes. Without this flag set, the changes in this PR may not provide the full intended compatibility. Please either:
- Add the missing changes to tools/build.py if they're needed
- Update the PR description if this change is no longer necessary
- Clarify where this flag should be set (e.g., in gradle configuration files)
| # Find android built libraries | ||
| LIBS=$(find build/Android/Release/lib -name "libortextensions.so" -o -name "libonnxruntime_extensions4j_jni.so") |
There was a problem hiding this comment.
The library search path assumes a specific directory structure build/Android/Release/lib, but depending on the build configuration and whether multiple ABIs are being built, the actual library paths might be different. Consider making the search more flexible by using a recursive search from build/Android or by using the actual build configuration directory. For example: find build/Android -name "*.so" -path "*/lib/*" would be more robust if the directory structure varies.
| # Find android built libraries | |
| LIBS=$(find build/Android/Release/lib -name "libortextensions.so" -o -name "libonnxruntime_extensions4j_jni.so") | |
| # Find android built libraries (search recursively under build/Android, limited to lib directories) | |
| LIBS=$(find build/Android -type f -path "*/lib/*" \( -name "libortextensions.so" -o -name "libonnxruntime_extensions4j_jni.so" \)) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
5 participants
Add linker flags and build configuration to support Android devices
using 16KB page sizes. This ensures compatibility with Android 15+
devices while maintaining backward compatibility with 4KB page systems.
Changes:
Android shared and executable targets
Technical details:
Testing: