Make APK and shared library alignment configurable #9046
Conversation
Context: https://github.com/dotnet/android/wiki/Android-support-for-devices-with-16k-pages Context: #9020 Starting sometime next year Google will require all packages submitted to the Play Store to be aligned to 16 bytes, in order to support Android devices which use 16kb pages instead of the currently supported 4kb ones. The requirement also applies to native shared libaries (`.so`), which need to be linked so that their loaded sections are aligned to 16kb as well. This commit implements changes which make the alignment configurable, while still defaulting to 4kb alignment. The #9020 PR will enable (when NDK r27 is released) building of our runtime shared libraries with 16kb alignment. While strictly speaking NDK r27 is not necessary to enable 16kb alignment for the runtime (we could just modify our `CMakeLists.txt` script to pass the appropriate flag), I prefer to rely on the "official" support once NDK r27 is out. Linking of application shared libraries, however, doesn't use the NDK at all, and in this case we must pass the appropriate flag to the linker explicitly. This commit also prepares our runtime to verify alignment correctly when changing between 4k and 16k.
grendello
requested review from
dellis1972,
jonathanpeppers and
jonpryor
as code owners
| @@ -55,6 +55,9 @@ sealed class ApplicationConfig | |||
| public uint jni_remapping_replacement_type_count; | |||
| public uint jni_remapping_replacement_method_index_entry_count; | |||
|
|
|||
| // 3, for 4-byte alignment (4k memory pages); 15, for 16-byte alignment (16k memory pages) | |||
There was a problem hiding this comment.
why are the mask 3 and 15 respecitvely? noth 4 and 16? (silly question).
There was a problem hiding this comment.
With alignment to 4k, bits 0 and 1 of the address must be 0 and with 16k, bits 0-3 must be 0. We could use the actual alignment here and then subtract 1 to obtain a mask from it, but it would require an additional instruction on the run time :) As it is now, we can add 1 to the mask to get the alignment value when reporting an error - an extra instruction but only in error situations, so performance isn't an issue. Yes, I know, it's just one instruction :)
The relevant code is here
| uint zipAlignmentMask = ZipAlignmentPages switch { | ||
| 4 => 3, | ||
| 16 => 15, | ||
| _ => throw new InvalidOperationException ($"Internal error: unsupported zip page alignment value {ZipAlignmentPages}") |
There was a problem hiding this comment.
Should we log an error with an error code instead of throw?
There was a problem hiding this comment.
I treat this condition as an internal error, because the property that specifies the alignment is "private" and shouldn't be modified by our users (it's mostly for our convenience of testing) and so I want the invalid value to be signaled in as loud and annoying kind of way :)
| uint maxPageSize = ZipAlignmentPages switch { | ||
| 4 => 4096, | ||
| 16 => 16384, | ||
| _ => throw new InvalidOperationException ($"Internal error: unsupported zip page alignment value {ZipAlignmentPages}") |
There was a problem hiding this comment.
I guess we'd use LogError() here too, if we changed the other one.
There was a problem hiding this comment.
I probably should just move this switch to a method inside MonoAndroidHelper class.
| case 24: // mono_components_mask: uint32_t / .word | .long | ||
| case 24: // zip_alignment_mask: uint32_t / .word | .long | ||
| Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}"); | ||
| ret.zip_alignment_mask = ConvertFieldToUInt32 ("zip_alignment_mask", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]); | ||
| break; |
There was a problem hiding this comment.
Is there any benefit to putting new settings at the end? Then it could be slightly more compatible with existing files? (Does it even matter?)
There was a problem hiding this comment.
It doesn't matter. There's no guarantee of compatibility between any releases of XA with regards to this (by design).
* main: (26 commits) Make APK and shared library alignment configurable (#9046) [r8] update proguard rule to keep .NET runtime classes (#9044) Explicitly align to 4k (#9041) [trimming] preserve custom views and `$(AndroidHttpClientHandlerType)` (#8954) Ignore split configs when bundle config moves shared libraries to base.apk (#8987) Bump to dotnet/android-tools@1c09dcc (#9026) Bump to dotnet/java-interop@ccafbe6 (#9025) [Mono.Android-Tests] Fix repo URL in redirect tests (#9035) [ci] Update checkout path for nightly build (#9028) [ci] Fix android source path for MAUI test job (#9030) Link Code of Conduct (#9034) [ci] Update sdk-insertions trigger to manual only (#9029) Update java-interop and android-tools submodule mentions (#9023) LEGO: Merge pull request 9022 [Xamarin.Android.Build.Tasks] fastdev works with aab files (#8990) Use new binutils URL (#9019) Localized file check-in by OneLocBuild Task: Build definition ID 17928: Build ID 9686669 (#9011) LEGO: Merge pull request 9015 [api-merge] Update "constant" values to mirror latest API levels (#9004) [Mono.Android] Fix wrong value for `ApplicationExitInfoReason.Other` (#9003) ...
Context: https://github.com/dotnet/android/wiki/Android-support-for-devices-with-16k-pages
Context: #9020
Starting sometime next year Google will require all packages submitted
to the Play Store to be aligned to 16 bytes, in order to support Android
devices which use 16kb pages instead of the currently supported 4kb
ones. The requirement also applies to native shared libaries (
.so),which need to be linked so that their loaded sections are aligned to
16kb as well.
This commit implements changes which make the alignment configurable,
while still defaulting to 4kb alignment. The #9020 PR will enable (when
NDK r27 is released) building of our runtime shared libraries with 16kb
alignment. While strictly speaking NDK r27 is not necessary to enable
16kb alignment for the runtime (we could just modify our
CMakeLists.txtscript to pass the appropriate flag), I prefer to relyon the "official" support once NDK r27 is out.
Linking of application shared libraries, however, doesn't use the NDK at
all, and in this case we must pass the appropriate flag to the linker
explicitly.
This commit also prepares our runtime to verify alignment correctly when
changing between 4k and 16k.