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

gdx: Android: Build ELFs to support 16kB page size #7394

Closed
wants to merge 1 commit into from

Conversation

Kalesh-Singh
Copy link

Starting from V, android introduces support for 16kB page sizes. The NDK [1] and toolchain [2][3] have been updated to support the larger page size.

Update GDX build rules for android to use a 16kB max page size.

[1] https://github.com/android/ndk/wiki/Changelog-r27#announcements
[2] llvm/llvm-project#70251
[3] llvm/llvm-project#87413

Starting from V, android introduces support for 16kB page sizes.
The NDK [1] and toolchain [2][3] have been updated to support the
larger page size.

Update GDX build rules for android to use a 16kB max page size.

[1] https://github.com/android/ndk/wiki/Changelog-r27#announcements
[2] llvm/llvm-project#70251
[3] llvm/llvm-project#87413

Signed-off-by: Kalesh Singh <[email protected]>
@Berstanio
Copy link
Contributor

Thank you for bringing this up!

This is a good change, but I think libGDX is not the place that should implement this, but https://github.com/libgdx/gdx-jnigen directly.
You can either open a PR there or I can do it too, however you like.

Also, the docs mention to define __BIONIC_NO_PAGE_SIZE_MACRO, just in case.
They also mention to only set the page size when building for arm64-v8a or x86_64, but I wouldn't know why this is relevant/has any downside for the other targets?

@Frosty-J
Copy link
Contributor

Frosty-J commented Apr 30, 2024

They also mention to only set the page size when building for arm64-v8a or x86_64, but I wouldn't know why this is relevant/has any downside for the other targets?

I think it would just be a waste of space to 16K-align for 32-bit architectures, as there won't be any 32-bit devices running Android 15 (I'd like to end this sentence here but I'll hedge my bets) that have a page size larger than 4K. It shouldn't break them, not that I've tested. ARMv9 devices still use the arm64-v8a libs.

@Kalesh-Singh
Copy link
Author

Hi Berstanio,

Thanks for the pointers.

You can either open a PR there or I can do it too, however you like.

TBH I'm not very familiar with the build system here. I'd prefer if you can take a stab at it.

The docs mention to define __BIONIC_NO_PAGE_SIZE_MACRO

You are right we also need to to remove uses of #define PAGE_SIZE 4096

They also mention to only set the page size when building for arm64-v8a or x86_64, but I wouldn't know why this is relevant/has any downside for the other targets?

The reason for this is that only arm64 supports 16kB page size. On Android, x86_64 emulators for studio emulate the 16kB page size, so we need it there as well.

A larger ELF max-page-size would also work on other archs but is not needed since they don't support larger than 4kB page sizes.

The caveat is that the larger segment alignment in the ELFs cause the .so's to be slightly larger due to zero padding. On android this is mitigated by punching holes in the .so's padding when the app is installed. (i.e the disk space usage will not increase although the "apparent" file size will be a bit larger)

Thanks,
Kalesh

@Kalesh-Singh
Copy link
Author

Hi Berstanio, Frosty-J,

Do we have an ETA for when a build would be ready with these changes?

Thanks,
Kalesh

@Berstanio
Copy link
Contributor

Do we have an ETA for when a build would be ready with these changes?

If you mean a ready libGDX snapshot build, it's hard to tell.

  1. I would need to get around implementing the changes in jnigen (hopefully this weekend)
  2. They need to get tested, reviewed and merged into jnigen
  3. A release of jnigen needs to be done
  4. libGDX needs to get updated to the new jnigen version

All of this might take a bit of time.

@Kalesh-Singh
Copy link
Author

If you mean a ready libGDX snapshot build, it's hard to tell.

From what I understand, some apps use directly the prebuilts from the libgdx build server.

  1. I would need to get around implementing the changes in jnigen (hopefully this weekend)
  2. They need to get tested, reviewed and merged into jnigen
  3. A release of jnigen needs to be done
  4. libGDX needs to get updated to the new jnigen version

All of this might take a bit of time.

Understood. I'd appreciate if you can keep us posted once these happen.

Thanks,
Kalesh

@Kalesh-Singh
Copy link
Author

Do we have an ETA for when a build would be ready with these changes?

If you mean a ready libGDX snapshot build, it's hard to tell.

  1. I would need to get around implementing the changes in jnigen (hopefully this weekend)
  2. They need to get tested, reviewed and merged into jnigen
  3. A release of jnigen needs to be done
  4. libGDX needs to get updated to the new jnigen version

All of this might take a bit of time.

Hi @Berstanio, have yo had a chance to look into this?

@Berstanio
Copy link
Contributor

Hi @Kalesh-Singh ,
sorry for taking so long, I have opened the jnigen PR: libgdx/gdx-jnigen#73

@obigu obigu added the android label Nov 12, 2024
@obigu
Copy link
Contributor

obigu commented Nov 12, 2024

Closing. Follows in libgdx/gdx-jnigen#73

@obigu obigu closed this Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants