Embed some data files in `libxamarin-app.so` #9367

grendello · 2024-10-03T19:55:20Z

Runtime blob config and assembly store are two candidates for embedding now.

If these two files are embedded in libxamarin-app.so, we can skip scanning the
APK for them and, potentially, speed up startup.

Changes implemented here give us a 2% speed up on the Displayed metrics, 2.7% on
the "Native to managed" metrics and 2% on the "Total runtime init" metrics.

Tests were performed on Pixel 6 Pro, using the dotnet new android template.

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs

grendello · 2024-10-24T20:31:13Z

/azp run

azure-pipelines · 2024-10-24T20:31:41Z

Azure Pipelines successfully started running 1 pipeline(s).

jonpryor · 2024-10-25T18:50:50Z

src/Xamarin.Android.Build.Tasks/Mono.Android/ApplicationAttribute.Partial.cs

@@ -57,6 +57,12 @@ static partial void AddManualMapping ()

 		public static ApplicationAttribute FromCustomAttributeProvider (ICustomAttributeProvider provider, TypeDefinitionCache cache)
 		{
+			// `provider` might be null in situations when application configuration is broken, and it surfaces in a number of


I'd like to know more about this scenario? How is provider null when the application config is broken?

This happened in the XA0119 test (which is, itself, a broken configuration scenario) where there were no assemblies to load and the code in ManifestDocument would iterate over the list of assemblies like so:

foreach (var assemblyPath in Assemblies) { var assembly = Resolver.GetAssembly (assemblyPath); if (ApplicationAttribute.FromCustomAttributeProvider (assembly, cache) is ApplicationAttribute a) { assemblyAttr.Add (a); } }

Each assembly is an instance of ICustomAttributeProvider, but Resolver.GetAssembly would return null since the assembly couldn't be found.

jonathanpeppers · 2024-10-30T16:10:03Z

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs

+				if (assembly == null) {
+					log.LogDebugMessage ($"Assembly '{assemblyPath}' not found.");
+					continue;
+				}
+


I think you can remove this with this one merged:

[Xamarin.Android.Build.Tasks] fix Debug mode and $(PublishTrimmed) #9452

I think I'd like to keep it, it's a good safeguard for situations when something goes really wrong - at least we have a clear message in the log, instead of a NREX

src/Xamarin.Android.Build.Tasks/Mono.Android/InstrumentationAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/ApplicationAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/PermissionAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/PermissionGroupAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/PermissionTreeAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/SupportsGLTextureAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/UsesFeatureAttribute.Partial.cs

src/Xamarin.Android.Build.Tasks/Mono.Android/UsesPermissionAttribute.Partial.cs

jonathanpeppers · 2024-11-04T15:44:45Z

src/native/runtime-base/android-system.cc

 	override_dirs [0] = override_dir;
+#if defined(DEBUG)
+	log_debug (LOG_DEFAULT, "Creating public update directory: `%s`", override_dir);
 	Util::create_public_directory (override_dir);


There are some cases we need to create the .__override__ directory in release mode that uses libmono-android.release.so:

Recording an AOT profile saves the custom.aprof file here

JIT times saves methods.txt here

These changes look like it would only create .__override__ in libmono-android.debug.so? Is that the case?

The other situation, for release build profiling, is handled here: https://github.com/dotnet/android/pull/9367/files/01677765e2d9f55b46b96aa43d24ffc50b430115#diff-5296758f320149c3439879b0262425a13756cddabe72b83f286a5ebfc392ec7fR1127

@grendello that link doesn't work? I think the commit was rebased

Oof, indeed, here's one which should work

android/src/native/monodroid/monodroid-glue.cc

Line 1127 in 8dfab2e

if (Util::create_directory (AndroidSystem::override_dirs[0], 0777, 000) < 0) {

Even though we might not have any content to embed, we need the symbols to be present, because they're part of the `libxamarin-app.so` ABI expected by `libmonodroid.so`

This reverts commit bc47870. It broke 481 tests

grendello · 2024-11-05T08:35:04Z

@jonathanpeppers reverting the _RemoveRegisterAttribute change broke 481 tests as the task is what actually creates the "shrunk" assemblies on disk, and those are required earlier in the process when the new CreateEmbedddedAssemblyStore target is used.

dellis1972 · 2024-11-05T09:26:25Z

src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs

@@ -61,6 +61,10 @@ public class GeneratePackageManagerJava : AndroidTask
 		[Required]
 		public bool EnablePreloadAssembliesDefault { get; set; }

+		// This property should be required but it will require modifying `monodroid` first
+		//[Required]
+		public string AndroidBinUtilsDirectory { get; set; }


I don't see a reference to GeneratePackageManagerJava in monodroid?

There are none, but I got errors in some tests when the property was required. My guess is that, for some reason, AndroidBinUtilsDirectory was empty in some tests with the commercial bits present.

Hmm interesting.

@grendello can we try adding the required back so I can see what the error was? It might be a build order issue which we are currently unaware of?

Sure, will push a commit in a moment

Looks like it worked this time, odd. Well, good :)

grendello · 2024-11-07T09:54:28Z

/azp run

azure-pipelines · 2024-11-07T09:54:59Z

Azure Pipelines successfully started running 1 pipeline(s).

grendello requested a review from jonpryor October 3, 2024 19:55

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch 5 times, most recently from 2730842 to 40a46b6 Compare October 10, 2024 20:28

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch 4 times, most recently from 6a18219 to c69913c Compare October 16, 2024 09:22

grendello marked this pull request as ready for review October 16, 2024 15:51

grendello requested review from dellis1972 and jonathanpeppers as code owners October 16, 2024 15:51

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch from b64f9e9 to f31d1a1 Compare October 17, 2024 18:45

grendello marked this pull request as draft October 18, 2024 11:50

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch 5 times, most recently from bafa9e1 to d76969a Compare October 23, 2024 11:19

dellis1972 reviewed Oct 23, 2024

View reviewed changes

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs Show resolved Hide resolved

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch from d76969a to c450cb6 Compare October 23, 2024 16:41

grendello marked this pull request as ready for review October 24, 2024 13:50

jonpryor reviewed Oct 25, 2024

View reviewed changes

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch 2 times, most recently from f2d1748 to 04b4ab8 Compare October 30, 2024 15:59

jonathanpeppers reviewed Oct 30, 2024

View reviewed changes

jonathanpeppers reviewed Nov 4, 2024

View reviewed changes

grendello added 22 commits November 4, 2024 19:37

Optionally produce "empty" embed.*.s files

872f662

Even though we might not have any content to embed, we need the symbols to be present, because they're part of the `libxamarin-app.so` ABI expected by `libmonodroid.so`

Don't embed assembly store when fastdev is used

26bf693

Let's see if designer tests pass now

ea9d5b2

Expect the unexpected, DTB doesn't specify any ABIs

6aebe30

[WIP] Builds but doesn't work

56f030d

Let's see how it works now

fb0e6e2

Always generate embedded store sources in CreateEmbeddedAssemblyStore

9e4b3fe

Let's see...

581991e

Don't assume assemblies exist

3a46110

better

81726be

Update apkdesc files

a84b663

Load embedded assembly store also in filesystem mode

1035cdd

Log it when an assembly is missing in ManifestDocument

d46e20f

Add some debug logging

d14811c

Don't throw

3648ab1

Always create the destination directory

4acc5a2

Post-rebase fixups

a7095d2

Let's see what's in the override dir listing

4031e9b

Let's see

836c4b4

Cleanup

f4f1502

Address feedback

8e41bdd

Let's see what breaks

bc47870

grendello force-pushed the dev/grendel/embed-binaries-in-xamarinapp branch from 0167776 to bc47870 Compare November 4, 2024 18:38

Revert "Let's see what breaks"

8dfab2e

This reverts commit bc47870. It broke 481 tests

dellis1972 reviewed Nov 5, 2024

View reviewed changes

grendello added 2 commits November 6, 2024 13:33

Restore [Require]

66179bc

Remove comment, no longer valid

c584dbd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embed some data files in `libxamarin-app.so` #9367

Embed some data files in `libxamarin-app.so` #9367

grendello commented Oct 3, 2024 •

edited

Loading

grendello commented Oct 24, 2024

azure-pipelines bot commented Oct 24, 2024

jonpryor Oct 25, 2024

grendello Oct 28, 2024

jonathanpeppers Oct 30, 2024

grendello Oct 31, 2024

jonathanpeppers Nov 4, 2024

grendello Nov 4, 2024

dellis1972 Nov 5, 2024

grendello Nov 5, 2024

grendello commented Nov 5, 2024

dellis1972 Nov 5, 2024

grendello Nov 5, 2024 •

edited

Loading

dellis1972 Nov 5, 2024

dellis1972 Nov 6, 2024

grendello Nov 6, 2024

grendello Nov 6, 2024

grendello commented Nov 7, 2024

azure-pipelines bot commented Nov 7, 2024

Embed some data files in libxamarin-app.so #9367

Are you sure you want to change the base?

Embed some data files in libxamarin-app.so #9367

Conversation

grendello commented Oct 3, 2024 • edited Loading

grendello commented Oct 24, 2024

azure-pipelines bot commented Oct 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grendello commented Nov 5, 2024

Choose a reason for hiding this comment

grendello Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grendello commented Nov 7, 2024

azure-pipelines bot commented Nov 7, 2024

Embed some data files in `libxamarin-app.so` #9367

Embed some data files in `libxamarin-app.so` #9367

grendello commented Oct 3, 2024 •

edited

Loading

grendello Nov 5, 2024 •

edited

Loading