dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.93k stars 527 forks source link

assemblies.ARCH.blob should be per-ARCH .so files #8168

Closed jonpryor closed 7 months ago

jonpryor commented 1 year ago

Context: https://github.com/xamarin/xamarin-android/issues/8165#issuecomment-1620097409

assemblies.x86.blob and similar are always stored in the assemblies directory, meaning our per-arch assemblies*.blob files are present when downloaded for every architecture. This means that arch-specific .apk files are larger than is necessary.

We should move the per-arch assemblies.*.blob files into per-arch lib/ARCH/libassemblies.so files. This way when a per-arch .apk is produced, assemblies for other architectures can be properly skipped/removed from the .apk.

jonathanpeppers commented 1 year ago

Or should there only be 1 set of BCL assemblies instead?

In theory, we could make:

Benefits:

Drawbacks:

mosammo commented 1 year ago

please note same problem is also in "lib" folder inside the apk which has even larger size for unselected platforms as shown below image

jonathanpeppers commented 1 year ago

@mosammo we brought what you noticed here up with Google: https://github.com/google/bundletool/issues/190

There is not a way to split up custom files per-architecture for App Bundles. Our only option appears to rework .NET assemblies to be .so files as this issue mentions.

(Or make it so there are no architecture-specific files at all 😄)

jonpryor commented 1 year ago

@mosammo wrote:

please note same problem is also in "lib" folder inside the apk which has even larger size for unselected platforms as shown below

Yes and no: Yes, you are absolutely correct that the .apk contains lib/* directories which increase the size of the .apk and may not be relevant for the target device.

However, "No" because .apk is no longer the preferred App Distribution method, Android App Bundle (.aab) files are. The .aab will be "huge", but the Google Play store will take the .aab file as input, and strip out entries that are not relevant for the target device.

For example, you upload a .aab containing all ABIs to the Google Play store. When you download the app onto a Pixel 6, the downloaded .apk will contain only arm64-v8a; it will not contain lib/x86, lib/x86_64, or lib/armeabi-v7a. Similarly, Android Resources will contain only resources applicable to the target device, e.g. no "tablet sized" resources. (Unfortunately, it will still contain all assemblies/assemblies.*.blob, and thus will be larger than desired, thus issue #8168.)

The Amazon Appstore also supports Android App Bundles.

Consequently, you (generally) shouldn't care about .apk files; those are mostly for local debugging.

Finally, it looks like the Archive Manager supports App Bundles; see: https://learn.microsoft.com/xamarin/android/deploy-test/release-prep/?tabs=windows#android-app-bundles

jonpryor commented 1 year ago

@jonathanpeppers asked:

Or should there only be 1 set of BCL assemblies instead?

Why not both?

The glorious thing about using .so files instead of .blob files is that we won't need to parse the .apk to find the assemblies/assemblies*.blob entries during startup, we could (maybe) just have direct symbol references. I'm sure @grendello will be thrilled! (HHOS)

There could thus be benefits to using .so files instead of assemblies*.blob files, even if we did have only 1 set of assemblies.

However, System.Private.CoreLib.dll is full of target-specific code, e.g.:

https://github.com/dotnet/runtime/blob/0224f86a886e0aaf72acb6a4f5573236ba929292/src/libraries/System.Private.CoreLib/src/System/Array.cs#L800-L802

Last time I heard, there was no intention to have an "architecture neutral" System.Private.CoreLib.dll a'la mono's mscorlib.dll, so we're always going to have an architecture-specific System.Private.CoreLib.dll. We could request dotnet/runtime to remove the ILLink Descriptors when building for Android, but (1) I'm not sure if they'd want to do so, (2) such a change might not land in time for .NET 8, and (3) such a change could be moot anyway, because anybody can provide those XML descriptors, reintroducing e.g. Issue #8155.

"1 set of assemblies" is only truly viable if there is a way to tell the linker to ignore all link descriptors (or all link descriptors which would introduce architecture-specific changes, which can't currently be done). This would also mean "no use of linker feature flags", which I suspect will go over like a lead balloon.

My "fear", then, is that "1 set of assemblies" is not viable, multi-arch assemblies are the present and future, and we need to ensure that we support them. (Note: We do support them! It's just that how we support them has various tradeoffs wrt .apk size.)

jonpryor commented 1 year ago

To expand upon "System.Private.CoreLib.dll is full of target-specific code":

% pwd
…/dotnet/runtime/src/libraries/System.Private.CoreLib

% git grep '#if ' | wc -l
     773

System.Private.CoreLib has 773 uses of #if, for:

% git grep -h '#if ' | awk '{ print $2 }' | sort | uniq -c | sort -n
   1 !((TARGET_BROWSER
   1 !(TARGET_BROWSER
   1 !NETSTANDARD2_1
   1 !TARGET_ARM64
   1 !TARGET_MACCATALYST
   1 !TARGET_OSX
   1 !TARGET_WASI
   1 (!TARGET_BROWSER
   1 EVENTSOURCE_GENERICS
   1 EVENT_SOURCE_LEGACY_NAMESPACE_SUPPORT
   1 NETSTANDARD2_0
   1 TARGET_ANDROID
   1 TARGET_FREEBSD
   1 TARGET_LINUX
   1 TARGET_MACCATALYST
   1 TARGET_WATCHOS
   1 TARGET_X86
   1 WINDOWS
   2 ENTITY_ENCODE_HIGH_ASCII_CHARS
   2 FEATURE_EVENTSOURCE_XPLAT
   2 MICROSOFT_INTEROP_SOURCEGENERATION
   2 REGISTRY_ASSEMBLY
   2 TARGET_AMD64
   2 TARGET_IOS
   2 TARGET_TVOS
   3 !DEBUG
   3 !RESOURCES_EXTENSIONS
   3 FEATURE_OBJCMARSHAL
   3 TARGET_ARM
   3 TARGET_ARM64
   4 !MONO
   4 !TARGET_BROWSER
   4 (!NETSTANDARD2_0
   4 FASTLOOP
   4 TARGET_UNIX
   6 !ES_BUILD_STANDALONE
   6 !FEATURE_WASM_PERFTRACING
   6 TARGET_WASI
   7 FEATURE_ADVANCED_MANAGED_ETW_CHANNELS
   7 HAS_CUSTOM_BLOCKS
   7 _LOGGING
   8 !CORECLR
  10 FEATURE_COMINTEROP
  10 RESOURCES_EXTENSIONS
  12 BIGENDIAN
  13 !FEATURE_WASM_THREADS
  15 NATIVEAOT
  19 !NATIVEAOT
  20 false
  23 FEATURE_MANAGED_ETW_CHANNELS
  23 TARGET_BROWSER
  25 RARE_ENUMS
  25 TARGET_WINDOWS
  27 FEATURE_MANAGED_ETW
  28 TARGET_32BIT
  28 TARGET_OSX
  32 CORECLR
  42 FEATURE_PERFTRACING
  47 MONO
  59 SYSTEM_PRIVATE_CORELIB
  87 DEBUG
 114 TARGET_64BIT

In particular note the 32 instances of TARGET_32BIT and 114 instances of TARGET_64BIT. An architecture-neutral System.Private.CoreLib.dll is not viable.

grendello commented 1 year ago

@jonathanpeppers asked:

Or should there only be 1 set of BCL assemblies instead?

Why not both?

The glorious thing about using .so files instead of .blob files is that we won't need to parse the .apk to find the assemblies/assemblies*.blob entries during startup, we could (maybe) just have direct symbol references. I'm sure @grendello will be thrilled! (HHOS)

One downside of the .so idea is its potential to double memory usage for the assemblies. The blobs we have now are mmapped and only data that's actually needed is read from them. Even if the assemblies are compressed, we won't waste much more memory over what would normally be used by the runtime loading the assembly from disk, since we decmpress the mmapped data to a pre-allocated (at build time) memory buffer, which we then pass as a pointer to the runtime. However, with an .so we will have the entire library loaded into memory (since data will be kept in a R/O data section, that's entirely loaded) and, if compression is involved, we'll repeat the same action as with blobs, but the original compressed data will remain in memory. We won't be able to unload the shared library since we won't know when all the assemblies have been decompressed. If compression isn't involved, we'll have to keep the .so with assemblies around forever, since the runtime will use its data but the memory usage won't be doubled. However, in either case we will pay the up-front cost of allocated memory whether or not all the assemblies will be used by the application.

mosammo commented 1 year ago

@mosammo wrote:

please note same problem is also in "lib" folder inside the apk which has even larger size for unselected platforms as shown below

Yes and no: Yes, you are absolutely correct that the .apk contains lib/* directories which increase the size of the .apk and may not be relevant for the target device.

However, "No" because .apk is no longer the preferred App Distribution method, Android App Bundle (.aab) files are. The .aab will be "huge", but the Google Play store will take the .aab file as input, and strip out entries that are not relevant for the target device.

For example, you upload a .aab containing all ABIs to the Google Play store. When you download the app onto a Pixel 6, the downloaded .apk will contain only arm64-v8a; it will not contain lib/x86, lib/x86_64, or lib/armeabi-v7a. Similarly, Android Resources will contain only resources applicable to the target device, e.g. no "tablet sized" resources. (Unfortunately, it will still contain all assemblies/assemblies.*.blob, and thus will be larger than desired, thus issue #8168.)

The Amazon Appstore also supports Android App Bundles.

Consequently, you (generally) shouldn't care about .apk files; those are mostly for local debugging.

Finally, it looks like the Archive Manager supports App Bundles; see: https://learn.microsoft.com/xamarin/android/deploy-test/release-prep/?tabs=windows#android-app-bundles

The problem is some projects do not use Google Play store for app distribution and instead using the .apk files which on Xamarin android didn't include such files of unselected architectures and were much smaller than the apk files currently generated by .net 7+ for android. Such unselected architectures files (like in "assemblies" and "lib" folders) should be excluded or removed from the generated apk.

dellis1972 commented 1 year ago

@mosammo

Are you using the RuntimeIdentifiers property group instead of AndroidSupportedAbis in your .net 7+ projects? see https://github.com/xamarin/xamarin-android/blob/main/Documentation/guides/OneDotNet.md#changes-to-msbuild-properties.

If you are only targeting say intel x86/64 you should be using the following

<PropertyGroup>
  <!-- Used going forward in .NET -->
  <RuntimeIdentifiers>android-x86;android-x64</RuntimeIdentifiers>
</PropertyGroup>

This should exclude the arm based abi's from the final apk. If it doesn't that is a bug.

mosammo commented 1 year ago

@mosammo

Are you using the RuntimeIdentifiers property group instead of AndroidSupportedAbis in your .net 7+ projects? see https://github.com/xamarin/xamarin-android/blob/main/Documentation/guides/OneDotNet.md#changes-to-msbuild-properties.

If you are only targeting say intel x86/64 you should be using the following

<PropertyGroup>
  <!-- Used going forward in .NET -->
  <RuntimeIdentifiers>android-x86;android-x64</RuntimeIdentifiers>
</PropertyGroup>

This should exclude the arm based abi's from the final apk. If it doesn't that is a bug.

Thank you!! it works!! turns out it's just a UI bug in visual studio, selecting "Platform target" in visual studio project settings should set the related settings in the project file, but it only sets "<PlatformTarget>ARM64</PlatformTarget>" and doesn't set "<RuntimeIdentifiers>android-arm;android-arm64;</RuntimeIdentifiers>" platform target option 64

After setting "<RuntimeIdentifiers>" manually in the android project file now it works perfectly and correctly excludes unselected platforms from the apk from both "assemblies" and "lib"! Thanks a lot 👍

dellis1972 commented 1 year ago

@mosammo Great :) On a side note you probably want to leave the PlatformTarget as AnyCPU.