Repository navigation
Support 16KB page size for Android - #2520
Conversation
WalkthroughAdds Android linker flag -Wl,-z,max-page-size=16384 in CMake for shared libs, inserts readelf -l checks on built .so files in the Android CI workflow per ABI, and updates short_name fields for several ASR models in the APK generation script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant CMake as CMake Build
participant Linker as Linker
participant CI as GitHub Actions
participant Art as Artifacts
Dev->>CMake: Configure Android build
Note right of CMake: Set shared linker flags<br/>-z,max-page-size=16384 (Android)
CMake->>Linker: Link shared libraries (.so)
Linker-->>CMake: Produce .so per ABI
CI->>CMake: Build per ABI (arm64-v8a, armv7-eabi, x86_64, x86)
CI->>CI: readelf -l build-android-.../install/lib/*.so
Note over CI: Verify program headers
CI->>Art: Package/copy artifacts
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
dosubot
Bot
added
the
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
CMakeLists.txt (1)
146-151: Pair max-page-size with common-page-size (keeps file size tight, guarantees 16KB LOAD p_align).Setting -Wl,-z,max-page-size=16384 is the right direction. To mirror the approach used by projects that solved the Android 16KB page-size issue, also pass -Wl,-z,common-page-size=4096. This keeps section/file layout at 4KB (avoids binary bloat) while ensuring PT_LOAD p_align is 16KB so the .so will map on 16KB devices.
Apply this diff:
if(ANDROID) # see https://github.com/microsoft/onnxruntime/pull/22076 # https://github.com/k2-fsa/sherpa-onnx/issues/2413 - set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-z,max-page-size=16384") + # Use 16KB max page size to support Android 15+ devices using 16KB pages, + # but keep common page size at 4KB to avoid inflating file size. + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-z,max-page-size=16384,-z,common-page-size=4096") endif()Optionally, if you want to minimize surface area, restrict this to AArch64 builds:
- replace if(ANDROID) with if(ANDROID AND ANDROID_ABI STREQUAL "arm64-v8a").
Current change is fine as-is, just a suggestion..github/workflows/android.yaml (4)
63-64: Make readelf check assertive for arm64-v8a (fail fast if Align != 0x4000).Right now we only print the program headers. Convert it into a verification so we actually guard against regressions on 16KB devices.
Apply this diff:
- readelf -l ./build-android-arm64-v8a/install/lib/*.so + for so in ./build-android-arm64-v8a/install/lib/*.so; do + echo "Verifying PT_LOAD alignment for $so" + if readelf -l "$so" | awk '/LOAD/ {in_load=1} in_load && /Align/ {print $2; in_load=0}' | grep -vq '0x4000'; then + echo "ERROR: $so has PT_LOAD Align != 0x4000" + readelf -l "$so" || true + exit 1 + fi + done(Optional) Add set -euo pipefail at the top of the step for robustness.
81-82: Consider keeping these as informative or tighten them per-ABI.For armeabi-v7a, devices expect 4KB; enforcing 0x4000 may be unnecessary. Keeping readelf as an informative print is fine. If you want parity, assert Align == 0x1000 here.
Example (if you want to enforce 4KB on v7a):
- readelf -l ./build-android-armv7-eabi/install/lib/*.so + for so in ./build-android-armv7-eabi/install/lib/*.so; do + echo "Verifying PT_LOAD alignment for $so" + if readelf -l "$so" | awk '/LOAD/ {in_load=1} in_load && /Align/ {print $2; in_load=0}' | grep -vq '0x1000'; then + echo "ERROR: $so has PT_LOAD Align != 0x1000" + readelf -l "$so" || true + exit 1 + fi + done
97-98: Same note for x86_64 as for armv7.x86_64 Android typically uses 4KB page size. Either keep readelf as-is (informational) or enforce 0x1000.
- readelf -l ./build-android-x86-64/install/lib/*.so + for so in ./build-android-x86-64/install/lib/*.so; do + echo "Verifying PT_LOAD alignment for $so" + if readelf -l "$so" | awk '/LOAD/ {in_load=1} in_load && /Align/ {print $2; in_load=0}' | grep -vq '0x1000'; then + echo "ERROR: $so has PT_LOAD Align != 0x1000" + readelf -l "$so" || true + exit 1 + fi + done
114-115: Same note for x86 as for armv7/x86_64.Keep as-is or enforce 0x1000. Consistency across ABIs helps debugging.
- readelf -l ./build-android-x86/install/lib/*.so + for so in ./build-android-x86/install/lib/*.so; do + echo "Verifying PT_LOAD alignment for $so" + if readelf -l "$so" | awk '/LOAD/ {in_load=1} in_load && /Align/ {print $2; in_load=0}' | grep -vq '0x1000'; then + echo "ERROR: $so has PT_LOAD Align != 0x1000" + readelf -l "$so" || true + exit 1 + fi + donescripts/apk/generate-asr-apk-script.py (6)
143-165: Changing short_name to a variant-specific token—confirm no Kotlin code relies on fixed values.short_name previously signaled a family like zipformer/small_zipformer. Moving to small_zipformer_14M_2023_02_23 is fine if the app treats it purely as display text. If any logic maps short_name to behavior/assets, this could break.
- Verify usages of short_name in android/* Kotlin.
- If logic depends on a stable key, keep short_name stable (e.g., small_zipformer) and introduce a display_name for the verbose variant.
Example refactor (outside this hunk):
@dataclass class Model: ... - short_name: str = "" + short_name: str = "" # stable key: e.g., small_zipformer + display_name: str = "" # human-friendly variant: e.g., small_zipformer_14M_2023_02_23Then set display_name here instead of overloading short_name.
167-185: Same concern for the English 20M model.Ensure the Android side doesn’t key off short_name. If it does, prefer stable short_name plus a separate display_name.
Would you like me to open a small follow-up to add display_name and update the template?
267-285: Same concern for zh int8 model (2025-04-01).Variant-specific short_name is fine for display-only. Please confirm it’s not used as a programmatic key.
287-305: Same concern for zh (fp32) model (2025-04-01).Check downstream usage and consider introducing display_name if needed.
443-456: Same concern for ru int8 model (2025-08-16).If the Kotlin code selects icons/resources by short_name, this change could cause mismatches.
458-471: Same concern for ru fp32 model (2025-08-16).Confirm usage or split display vs. key as suggested above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/android.yaml(4 hunks)CMakeLists.txt(1 hunks)scripts/apk/generate-asr-apk-script.py(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-06T04:23:50.237Z
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:23:50.237Z
Learning: The sherpa-onnx JNI library files are stored in Hugging Face repository at https://huggingface.co/csukuangfj/sherpa-onnx-libs under versioned directories like jni/1.12.7/, and the actual Windows JNI library filename is "sherpa-onnx-jni.dll" as defined in Core.java constants.
Applied to files:
.github/workflows/android.yaml
📚 Learning: 2025-08-06T04:18:47.981Z
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Applied to files:
.github/workflows/android.yaml
🔇 Additional comments (1)
CMakeLists.txt (1)
146-151: Ensure 16 KB PT_LOAD alignment in Android arm64-v8a artifactsThe existing check didn’t actually verify alignment because
readelfwasn’t available in the CI environment. We need to explicitly invoke the Android NDK’sreadelf(or fall back toobjdump -x) on each built.soto assert that everyPT_LOADsegment hasAlign 0x4000(16 KB). Failing this early in CI will catch regressions.• CMakeLists.txt: lines 146–151
if(ANDROID) # see https://github.com/microsoft/onnxruntime/pull/22076 # https://github.com/k2-fsa/sherpa-onnx/issues/2413 set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-z,max-page-size=16384") endif()• Add or update your CI script to use the NDK toolchain’s
readelf. For example:#!/usr/bin/env bash set -euo pipefail # Adjust this path to match your NDK install location or CI environment variable READELF="${ANDROID_NDK}/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android-readelf" for so in build-android-arm64-v8a/install/lib/*.so; do echo "Checking $so" "$READELF" -l "$so" \ | awk ' /LOAD/ { in_seg=1; next } in_seg && /Align/ { if ($2 != "0x4000") { print "ERROR: PT_LOAD alignment for", FILENAME, "is", $2 exit 1 } in_seg=0 }' FILENAME="$so" done echo "OK: all PT_LOAD alignments are 0x4000"• Verify flags propagate into ExternalProject_Add for onnxruntime. If you’re pulling ORT as an ExternalProject, ensure you forward
-DCMAKE_SHARED_LINKER_FLAGS=${CMAKE_SHARED_LINKER_FLAGS}via itsCMAKE_ARGS.This will cause CI to fail if any
.sois misaligned, preventing regressions.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Android's 16KB page size requirement by updating build configurations and improving model naming consistency. This addresses compatibility issues with newer Android versions that use larger page sizes.
- Configures Android builds to use 16KB page-aligned shared libraries through CMake linker flags
- Updates ASR model short names to include version/date information for better clarity
- Adds validation checks in CI workflow to verify proper library alignment across all Android architectures
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| CMakeLists.txt | Adds Android-specific linker flag for 16KB page alignment |
| scripts/apk/generate-asr-apk-script.py | Updates model short names to include version/date identifiers |
| .github/workflows/android.yaml | Adds readelf validation commands to verify library alignment |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Fixes #2417
Fixes #2413
Note that I have tested the 16KB page-aligned
.soalso works onAndroid < 15, so weuse the default 16KB in sherpa-onnx.
We have built 16KB page-aligned
libonnxruntime.sofor onnxruntime 1.17.1You only need to use the latest master of sherpa-onnx; everything should work as expected.
CC @A1aM0 @KiSaYuNa @tcoln @Skepller @anita-smith1 @shawshuai
Summary by CodeRabbit
New Features
Chores