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.

Sign up for GitHub

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

Jump to bottom

Simplify and fix split configs detection #9378

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Simplify and fix split configs detection #9378

wants to merge 2 commits into from

Conversation

grendello
Copy link
Contributor

@grendello grendello commented Oct 8, 2024 •
edited
Loading

We use ApplicationInfo.splitSourceDirs to detect whether the application uses split config files.
Those files are created by Android when an AAB is used to deploy application (e.g. via Google Play Store).

When they are present, the base APK file doesn't contain any lib/ directories and we can simply skip
scanning it for those entries, thus saving on startup time. We used to check whether there are more than
one entry in splitSourceDirs, which used to be the case, but it seems that the recent Android versions
(at least API 33 and newer) can have just a single entry in the array. Because of that, we were scanning
all the APK files on those Android versions, wasting time at startup.

Fix the check by probing whether array exists and contains at least a single entry. Additionally, remove
API 21 check, since this is our lowest supported API level.

@jonpryor
Copy link
Member

Unit test? (I assume this is testable?)

When they are present, the base APK file doesn't contain any lib/ directories and we can simply skip
scanning it for those entries, thus saving on startup time. We used to check whether there are more than
one entry in splitSourceDirs, which used to be the case, but it seems that the recent Android versions
(at least API 33 and newer) can have just a single entry in the array. Because of that, we were scanning
all the APK files on those Android versions, wasting time at startup.

I'm not sure I understand this.

Additionally, I now observe:

android/src/Xamarin.Android.Build.Tasks/Resources/MonoRuntimeProvider.Bundled.java

Line 25 in 4429c52

String[] splitApks = applicationInfo.splitPublicSourceDirs;

splitPublicSourceDirs is used to populate apks, which is registered with native code.

Is there any requirement that ApplicationInfo.splitPublicSourceDirs be consistent with ApplicationInfo.splitSourceDirs? It looks like "no"?

splitPublicSourceDirs: Full path to the publicly available parts of splitSourceDirs, including resources and manifest. This may be different from splitSourceDirs if an application is forward locked. May be null if no splits are installed.

Is it thus possible to have a scenario in which splitPublicSourceDirs contains only one entry, and thus apks has only one entry, and splitSourceDirs has one entry, and now haveSplitApks is true, whereas before it would be false.

What would this do to app startup? Would it work properly? (I can't trivially follow the "split apk" logic in monodroid-glue.cc.)

@grendello
Copy link
Contributor Author

Unit test? (I assume this is testable?)

When they are present, the base APK file doesn't contain any lib/ directories and we can simply skip
scanning it for those entries, thus saving on startup time. We used to check whether there are more than
one entry in splitSourceDirs, which used to be the case, but it seems that the recent Android versions
(at least API 33 and newer) can have just a single entry in the array. Because of that, we were scanning
all the APK files on those Android versions, wasting time at startup.

I'm not sure I understand this.

Whenever the app is built in the AAB format, Android splits things up into a single base apk and a set of split APKs.
When I originally implemented this code, there were always two split APKs but it no longer appears to be the case as for
e.g. our "Hello world" app, I see just a single split apk. Here's the listing of the two APKs created from the "Hello world"'s
AAB:

$ zipinfo base.apk
Archive:  base.apk
Zip file size: 67510 bytes, number of entries: 25
-rw-rw-rw-  0.0 unx     3508 b- defN 81-Jan-01 01:01 AndroidManifest.xml
-rw-rw-rw-  0.0 unx    22408 b- stor 81-Jan-01 01:01 classes.dex
-rw-rw-rw-  0.0 unx      632 b- defN 81-Jan-01 01:01 res/layout/activity_main.xml
-rw-rw-rw-  0.0 unx      448 b- defN 81-Jan-01 01:01 res/mipmap-anydpi-v26/appicon.xml
-rw-rw-rw-  0.0 unx      448 b- defN 81-Jan-01 01:01 res/mipmap-anydpi-v26/appicon_round.xml
-rw-rw-rw-  0.0 unx     2178 b- stor 81-Jan-01 01:01 res/mipmap-hdpi-v4/appicon.png
-rw-rw-rw-  0.0 unx       97 b- stor 81-Jan-01 01:01 res/mipmap-hdpi-v4/appicon_background.png
-rw-rw-rw-  0.0 unx     1276 b- stor 81-Jan-01 01:01 res/mipmap-hdpi-v4/appicon_foreground.png
-rw-rw-rw-  0.0 unx     1490 b- stor 81-Jan-01 01:01 res/mipmap-mdpi-v4/appicon.png
-rw-rw-rw-  0.0 unx       92 b- stor 81-Jan-01 01:01 res/mipmap-mdpi-v4/appicon_background.png
-rw-rw-rw-  0.0 unx     1273 b- stor 81-Jan-01 01:01 res/mipmap-mdpi-v4/appicon_foreground.png
-rw-rw-rw-  0.0 unx     3098 b- stor 81-Jan-01 01:01 res/mipmap-xhdpi-v4/appicon.png
-rw-rw-rw-  0.0 unx      100 b- stor 81-Jan-01 01:01 res/mipmap-xhdpi-v4/appicon_background.png
-rw-rw-rw-  0.0 unx     1805 b- stor 81-Jan-01 01:01 res/mipmap-xhdpi-v4/appicon_foreground.png
-rw-rw-rw-  0.0 unx     4674 b- stor 81-Jan-01 01:01 res/mipmap-xxhdpi-v4/appicon.png
-rw-rw-rw-  0.0 unx      108 b- stor 81-Jan-01 01:01 res/mipmap-xxhdpi-v4/appicon_background.png
-rw-rw-rw-  0.0 unx     1926 b- stor 81-Jan-01 01:01 res/mipmap-xxhdpi-v4/appicon_foreground.png
-rw-rw-rw-  0.0 unx     6832 b- stor 81-Jan-01 01:01 res/mipmap-xxxhdpi-v4/appicon.png
-rw-rw-rw-  0.0 unx      117 b- stor 81-Jan-01 01:01 res/mipmap-xxxhdpi-v4/appicon_background.png
-rw-rw-rw-  0.0 unx     2801 b- stor 81-Jan-01 01:01 res/mipmap-xxxhdpi-v4/appicon_foreground.png
-rw-rw-rw-  0.0 unx      120 b- defN 81-Jan-01 01:01 res/xml/splits0.xml
-rw-rw-rw-  0.0 unx     2952 b- stor 81-Jan-01 01:01 resources.arsc
-rw----     2.0 fat     2477 b- defN 81-Jan-01 01:01 META-INF/BNDLTOOL.SF
-rw----     2.0 fat     1211 b- defN 81-Jan-01 01:01 META-INF/BNDLTOOL.RSA
-rw----     2.0 fat     2350 b- defN 81-Jan-01 01:01 META-INF/MANIFEST.MF
25 files, 64421 bytes uncompressed, 58325 bytes compressed:  9.5%

As you can see, this is resources and the Java code (it would also contain our assemblies and assembly blobs
before we started packaging them as ELF shared libraries.

The only split APK here, split_config.arm64_v8a.apk, contains the following:

$ zipinfo split_config.arm64_v8a.apk
Archive:  split_config.arm64_v8a.apk
Zip file size: 8976016 bytes, number of entries: 20
-rw-rw-rw-  0.0 unx      896 b- defN 81-Jan-01 01:01 AndroidManifest.xml
-rw-rw-rw-  0.0 unx    67248 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libSystem.Globalization.Native.so
-rw-rw-rw-  0.0 unx   775136 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libSystem.IO.Compression.Native.so
-rw-rw-rw-  0.0 unx    95632 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libSystem.Native.so
-rw-rw-rw-  0.0 unx   160232 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so
-rw-rw-rw-  0.0 unx   329296 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libaot-Java.Interop.dll.so
-rw-rw-rw-  0.0 unx     8672 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libaot-Mono.Android.Runtime.dll.so
-rw-rw-rw-  0.0 unx   290568 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libaot-Mono.Android.dll.so
-rw-rw-rw-  0.0 unx    62232 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libaot-System.Linq.dll.so
-rw-rw-rw-  0.0 unx  2366112 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libaot-System.Private.CoreLib.dll.so
-rw-rw-rw-  0.0 unx     9304 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libaot-XAPerfTest.dll.so
-rw-rw-rw-  0.0 unx    18776 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libarc.bin.so
-rw-rw-rw-  0.0 unx   850080 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libassemblies.arm64-v8a.blob.so
-rw-rw-rw-  0.0 unx    87432 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libmono-component-marshal-ilgen.so
-rw-rw-rw-  0.0 unx   485088 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libmonodroid.so
-rw-rw-rw-  0.0 unx  3196512 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libmonosgen-2.0.so
-rw-rw-rw-  0.0 unx    21904 b- stor 81-Jan-01 01:01 lib/arm64-v8a/libxamarin-app.so
-rw----     2.0 fat     1711 b- defN 81-Jan-01 01:01 META-INF/BNDLTOOL.SF
-rw----     2.0 fat     1203 b- defN 81-Jan-01 01:01 META-INF/BNDLTOOL.RSA
-rw----     2.0 fat     1603 b- defN 81-Jan-01 01:01 META-INF/MANIFEST.MF
20 files, 8829637 bytes uncompressed, 8827115 bytes compressed:  0.0%

So we're only interested in scanning the split APK(s) for assembly blobs, assemblies and the runtime config,
we can happily ignore the base APK.

@grendello
Copy link
Contributor Author

Additionally, I now observe:

android/src/Xamarin.Android.Build.Tasks/Resources/MonoRuntimeProvider.Bundled.java

Line 25 in 4429c52

String[] splitApks = applicationInfo.splitPublicSourceDirs;

splitPublicSourceDirs is used to populate apks, which is registered with native code.

Is there any requirement that ApplicationInfo.splitPublicSourceDirs be consistent with ApplicationInfo.splitSourceDirs? It looks like "no"?

splitPublicSourceDirs: Full path to the publicly available parts of splitSourceDirs, including resources and manifest. This may be different from splitSourceDirs if an application is forward locked. May be null if no splits are installed.

Is it thus possible to have a scenario in which splitPublicSourceDirs contains only one entry, and thus apks has only one entry, and splitSourceDirs has one entry, and now haveSplitApks is true, whereas before it would be false.

What would this do to app startup? Would it work properly? (I can't trivially follow the "split apk" logic in monodroid-glue.cc.)

I added some log entries to see what's in the three ApplicationInfo properties (we also use sourceDir):

10-14 14:05:25.532 10370 10370 I monodroid: apks passed from MonoRuntimeProvider:
10-14 14:05:25.532 10370 10370 I monodroid:   /data/app/~~JVG3UpPVkklCTm8OFf0sDQ==/com.xamarin.XAPerfTest.net9-e2yvKulo9T4bZltNhR06Cw==/base.apk
10-14 14:05:25.532 10370 10370 I monodroid:   /data/app/~~JVG3UpPVkklCTm8OFf0sDQ==/com.xamarin.XAPerfTest.net9-e2yvKulo9T4bZltNhR06Cw==/split_config.arm64_v8a.apk
10-14 14:05:25.532 10370 10370 I monodroid: splitSourceDirs:
10-14 14:05:25.532 10370 10370 I monodroid:   /data/app/~~JVG3UpPVkklCTm8OFf0sDQ==/com.xamarin.XAPerfTest.net9-e2yvKulo9T4bZltNhR06Cw==/split_config.arm64_v8a.apk
10-14 14:05:25.532 10370 10370 I monodroid: splitPublicSourceDirs:
10-14 14:05:25.532 10370 10370 I monodroid:   /data/app/~~JVG3UpPVkklCTm8OFf0sDQ==/com.xamarin.XAPerfTest.net9-e2yvKulo9T4bZltNhR06Cw==/split_config.arm64_v8a.apk

The array passed from MonoRuntimeProvider has its first entry set to sourceDir and the rest comes from splitPublicSourceDirs which is where we'll find the APKs we are interested in. I have never observed splitSourceDirs and splitPublicSourceDirs differ, but perhaps we should use the former in MonoRuntimeProvider as it may, in theory, contain more entries.

@grendello grendello force-pushed the dev/grendel/fix-split-configs-detection branch from 7cc380c to f986da3 Compare October 14, 2024 12:24
jonathanpeppers
jonathanpeppers reviewed Oct 14, 2024
View reviewed changes
src/java-runtime/java/mono/android/MonoPackageManager.java
haveSplitApks = runtimePackage.splitSourceDirs.length > 1;
}
}
boolean haveSplitApks = runtimePackage.splitSourceDirs != null && runtimePackage.splitSourceDirs.length > 0;
Copy link
Member

Choose a reason for hiding this comment

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

The previous code was checking > 1? Does it actually need to contain more than one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It used to contain 2, for all the apps I tested (long time ago), but now the apps I test with create just a single entry, thus the change. I think the change might have been made in Android 14, since I remember seeing 2 split apks in Android 13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanpeppers jonathanpeppers jonathanpeppers left review comments

@dellis1972 dellis1972 dellis1972 approved these changes

@jonpryor jonpryor Awaiting requested review from jonpryor jonpryor is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@grendello @jonpryor @dellis1972 @jonathanpeppers