onnxruntime icon indicating copy to clipboard operation
onnxruntime copied to clipboard

[Mobile] Will provide supported 16KB-page-size prebuilt Android .so in the future?

Open w11m opened this issue 1 year ago • 10 comments

Describe the issue

With Android A15 introducing a beta requirement for shared libraries to support a 16KB page size on ARM64 devices W

source: (https://source.android.com/docs/core/architecture/16kb-page-size/16kb)

We’re keen to ensure our project remains compatible with this and future Android versions, including A16. Our current prebuilt .so files follow the older page size specifications which might cause compatibility or performance concerns.

To reproduce

Use the existing prebuilt .so file on an Android A15 device or emulator. Check for failures in the VTS elf_alignment_test related to the 16KB page size.

Urgency

Maybe Urgent

As the Android A15 adoption grows, addressing this could become more pressing. We would appreciate guidance on whether there are plans to provide a prebuilt .so file that meets the 16KB page size requirement or any recommended steps we can take to ensure continued compatibility.

Platform

Android

OS Version

15

ONNX Runtime Installation

Released Package

Compiler Version (if 'Built from Source')

prebuild

Package Name (if 'Released Package')

onnxruntime-android

ONNX Runtime Version or Commit ID

version 1.16.1

ONNX Runtime API

C++/C

Architecture

ARM64

Execution Provider

Default CPU, NNAPI

Execution Provider Library Version

No response

w11m avatar Aug 23 '24 03:08 w11m

We'll look at adding the build flags in the next release. Based on this there are no actual devices with a 16KB page size yet.

https://android-developers.googleblog.com/2024/08/adding-16-kb-page-size-to-android.html

There are no production Android devices available today or expected for the Android 15 release that support a 16 KB page size.

skottmckay avatar Aug 26 '24 06:08 skottmckay

Thank you for the update! We appreciate that you're considering adding the build flags in the next release.

We'll look at adding the build flags in the next release. Based on this there are no actual devices with a 16KB page size yet.

https://android-developers.googleblog.com/2024/08/adding-16-kb-page-size-to-android.html

There are no production Android devices available today or expected for the Android 15 release that support a 16 KB page size.

w11m avatar Aug 26 '24 06:08 w11m

Hi, @skottmckay, could you please suggest which file would be best to add the flag '16KB page size' so I can quickly try build and check with the elf_alignment_test.

w11m avatar Aug 27 '24 06:08 w11m

If it needs to be set globally you could try adding an if (ANDROID) ... endif() section at the end of https://github.com/microsoft/onnxruntime/blob/main/cmake/adjust_global_compile_flags.cmake.

If it just needs to be set for libonnxruntime.so you could add something to https://github.com/microsoft/onnxruntime/blob/main/cmake/onnxruntime.cmake

Note that if (CMAKE_SYSTEM_NAME STREQUAL "Android") isn't necessary anymore as the minimum cmake version required supports if (ANDROID)

skottmckay avatar Aug 27 '24 06:08 skottmckay

Work great with the build comment and with modified https://github.com/microsoft/onnxruntime/blob/main/cmake/onnxruntime.cmake

build.bat --android --android_sdk_path <android_sdk_path> --android_ndk_path <android_ndk_path> --android_abi arm64-v8a --android_api 33 --cmake_generator Ninja --use_nnapi --build_shared_lib --skip_tests --config Release --cmake_extra_defines onnxruntime_BUILD_UNIT_TESTS=OFF

Also passed with my local VTS elf_alignment_test "16KB-page-size" check

Thanks for the help.

If it needs to be set globally you could try adding an if (ANDROID) ... endif() section at the end of https://github.com/microsoft/onnxruntime/blob/main/cmake/adjust_global_compile_flags.cmake.

If it just needs to be set for libonnxruntime.so you could add something to https://github.com/microsoft/onnxruntime/blob/main/cmake/onnxruntime.cmake

Note that if (CMAKE_SYSTEM_NAME STREQUAL "Android") isn't necessary anymore as the minimum cmake version required supports if (ANDROID)

w11m avatar Aug 27 '24 08:08 w11m

Excellent. Would you be able to submit a PR with the change?

skottmckay avatar Aug 27 '24 08:08 skottmckay

Submit a PR https://github.com/microsoft/onnxruntime/pull/21876 based on v1.16.1 changed

w11m avatar Aug 27 '24 10:08 w11m

Thanks for submitting the PR. You may need to merge your fork with the latest version of main though as it seems to be out of date leading to a massive number of changed files.

skottmckay avatar Aug 28 '24 05:08 skottmckay

If it's easier I can create a PR with your change.

Would be interested to know if this works instead so we limit where the linker flag is applied to just the onnxruntime shared library.

set_target_link_options(onnxruntime PRIVATE -Wl,-z,max-page-size=16384)

skottmckay avatar Aug 28 '24 06:08 skottmckay

On second thought we probably need to do this globally so the unit test projects have the same setting applied. adjust_global_compile_flags.cmake may be the best place to do this.

skottmckay avatar Aug 29 '24 08:08 skottmckay

@skottmckay You can create a PR with the changes. I'm currently without the necessary device to conduct the tests. However, based on what I know, the 16KB alignment check should cover all shared libraries on the device. It seems adjusting the .adjust_global_compile_flags.cmake might be appropriate.

w11m avatar Aug 30 '24 02:08 w11m

@skottmckay Just confirming, but it seems this issue was addressed by PR https://github.com/microsoft/onnxruntime/pull/22076. Should this be closed? Is there a reason it's still open?

kaseken avatar May 16 '25 00:05 kaseken

A quick note here (and also related to https://github.com/microsoft/onnxruntime/issues/24902); the way this is implemented will force 16KB alignment for ALL android ABIs. I don't think it should be done for armeabi-v7a (I am not 100% sure about this, but Google requires doing 16KB alignment ONLY for 64bit ABIs).

So, the change in cmake/adjust_global_compile_flags.cmake should use ANDROID_ABI and skip the change for "armeabi-v7a".

It also seems to me that the order of setting the value should be different, i.e. instead of

  set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-z,max-page-size=16384")

it should be

  set(CMAKE_SHARED_LINKER_FLAGS "-Wl,-z,max-page-size=16384 ${CMAKE_SHARED_LINKER_FLAGS}")

To allow proper override, if this option is set by the user. (I guess this may be open for debate if all the settings should be over-rideable).

ognjentodic avatar Aug 21 '25 21:08 ognjentodic