Skip to content
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

[ BUILD ] Add 16K shared lib package option for Android #2699

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

jijoongmoon
Copy link
Collaborator

Android encourage to use 16KB package for the shared library. This PR add the 16KB package option and also recommand to use ndk which is higher or equal version of r27.

Resolves:

Self evaluation:

  1. Build test: [X]Passed [ ]Failed [ ]Skipped
  2. Run test: [X]Passed [ ]Failed [ ]Skipped

Android encourage to use 16KB package for the shared library. This PR
add the 16KB package option and also recommand to use ndk which is
higher or equal version of r27.

Resolves:

**Self evaluation:**
1. Build test:	 [X]Passed [ ]Failed [ ]Skipped
2. Run test:	 [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: jijoong.moon <[email protected]>
@taos-ci
Copy link
Collaborator

taos-ci commented Aug 7, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2699. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

Copy link
Collaborator

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jijoongmoon, 💯 All CI checkers are successfully verified. Thanks.

Copy link
Member

@DonghakPark DonghakPark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@djeong20 djeong20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also update Application.mk to support 16 KB alignment with Android NDK version r27 and higher. (link)

DonghakPark added a commit to DonghakPark/nntrainer that referenced this pull request Aug 13, 2024
After nnstreamer#2699 : add option's for all android.mk file

**Self evaluation:**
1. Build test:	 [X]Passed [ ]Failed [ ]Skipped
2. Run test:	 [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: Donghak PARK <[email protected]>
@DonghakPark
Copy link
Member

we should also update Application.mk to support 16 KB alignment with Android NDK version r27 and higher. (link)

added Flags for all Android.mk file. Please Take a look #2705

@djeong20
Copy link
Contributor

added Flags for all Android.mk file. Please Take a look #2705

updating Android.mk is to compile with NDK version r26 or lower. The problem is whether we support NDK version r26 or lower or restrict to use of version r27 or higher.

Regardless, it is recommended to move to Android NDK r27 or higher to update to the 16 KB ELF-aligned version of the C++ standard library. Otherwise, your app will fail to install on 16 KB devices today.

if we restrict it to use Android NDK version r27 or higher, we do not need to change Android.mk and only update Application.mk.

@DonghakPark
Copy link
Member

DonghakPark commented Aug 13, 2024

added Flags for all Android.mk file. Please Take a look #2705

updating Android.mk is to compile with NDK version r26 or lower. The problem is whether we support NDK version r26 or lower or restrict to use of version r27 or higher.

Regardless, it is recommended to move to Android NDK r27 or higher to update to the 16 KB ELF-aligned version of the C++ standard library. Otherwise, your app will fail to install on 16 KB devices today.

if we restrict it to use Android NDK version r27 or higher, we do not need to change Android.mk and only update Application.mk.

Would it not be necessary to set up in Android.mk as well for when the ndk version is below 27?

According to Android Developer site they say

To support compiling 16 KB-aligned shared libraries with Android NDK version r27 and higher, you need to update your ndk-build, build.gradle, build.gradle.kts, or linker flags as follows:

In your Application.mk:

APP_SUPPORT_FLEXIBLE_PAGE_SIZES := true

Specify the following linker flags:

-Wl,-z,max-page-size=16384

@djeong20
Copy link
Contributor

Would it not be necessary to set up in Android.mk as well for when the ndk version is below 27?

you're right! If we plan to support the ndk version below 27, we should specify these flags. though, I'm not sure what would be the right choice. maybe we can allow ndk version 26 <= for now, then check the ndk version and print a proper message to upgrade to r27.

@jijoongmoon jijoongmoon merged commit 9ae7d2d into nnstreamer:main Aug 29, 2024
39 checks passed
@jijoongmoon jijoongmoon deleted the 16kb branch August 29, 2024 04:08
jijoongmoon pushed a commit that referenced this pull request Aug 29, 2024
After #2699 : add option's for all android.mk file

**Self evaluation:**
1. Build test:	 [X]Passed [ ]Failed [ ]Skipped
2. Run test:	 [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: Donghak PARK <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants