From e0da1f152de84c4cc39983f2fda775ec98c08250 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 12 Feb 2019 18:34:48 +0100 Subject: [PATCH] [runtime] Optionally preload all the assemblies from the apk (#2724) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/Xamarin.Forms/issues/5164 Commit b90d3ab7 altered how assemblies were loaded during process startup in an effort to reduce time spent in `Runtime.init()`: assembly loading has unavoidable overhead. Reducing the number of assemblies loaded results in less time spent in `Runtime.init()`. This change also meant that the semantics of `System.AppDomain.GetAssemblies()` changed: prior to b90d3ab7, `AppDomain.CurrentDomain.GetAssemblies()` would return *all* assemblies present within the `.apk`. Starting with b90d3ab7, `AppDomain.CurrentDomain.GetAssemblies()` would *instead* return only the assemblies which had been loaded *up to that point in time*, which may not be "everything". We honestly didn't spend too much time thinking about this particular change. The b90d3ab7 behavior is what desktop .NET has done since the beginning -- which makes sense, as desktop .NET didn't have a concept of "application" for which one could ask for "all assemblies that make up the application" -- so why would this break anything? Anything broken in such an environment would fail on .NET. Turns Out™ that there are mobile frameworks which assume that `AppDomain.CurrentDomain.GetAssemblies()` will return all known assemblies, and thus *break* with b90d3ab7, including: * [Xamarin.Forms][0] * [MvvmCross][1] Which brings us to [Xamarin.Forms Issue #5164][2], in which a Xamarin.Forms app which uses CSS crashes with an NRE: UNHANDLED EXCEPTION: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.NullReferenceException: Object reference not set to an instance of an object. at Xamarin.Forms.Xaml.ApplyPropertiesVisitor.ProvideValue (System.Object& value, Xamarin.Forms.Xaml.ElementNode node, System.Object source, Xamarin.Forms.Xaml.XmlName propertyName) [0x000a7] in D:\a\1\s\Xamarin.Forms.Xaml\ApplyPropertiesVisitor.cs:243 at Xamarin.Forms.Xaml.ApplyPropertiesVisitor.Visit (Xamarin.Forms.Xaml.ElementNode node, Xamarin.Forms.Xaml.INode parentNode) [0x000c9] in D:\a\1\s\Xamarin.Forms.Xaml\ApplyPropertiesVisitor.cs:107 at Xamarin.Forms.Xaml.ElementNode.Accept (Xamarin.Forms.Xaml.IXamlNodeVisitor visitor, Xamarin.Forms.Xaml.INode parentNode) [0x000ac] in D:\a\1\s\Xamarin.Forms.Xaml\XamlNode.cs:149 at Xamarin.Forms.Xaml.RootNode.Accept (Xamarin.Forms.Xaml.IXamlNodeVisitor visitor, Xamarin.Forms.Xaml.INode parentNode) [0x00044] in D:\a\1\s\Xamarin.Forms.Xaml\XamlNode.cs:200 at Xamarin.Forms.Xaml.XamlLoader.Visit (Xamarin.Forms.Xaml.RootNode rootnode, Xamarin.Forms.Xaml.HydrationContext visitorContext, System.Boolean useDesignProperties) [0x0008b] in D:\a\1\s\Xamarin.Forms.Xaml\XamlLoader.cs:150 at Xamarin.Forms.Xaml.XamlLoader.Load (System.Object view, System.String xaml, System.Boolean useDesignProperties) [0x00058] in D:\a\1\s\Xamarin.Forms.Xaml\XamlLoader.cs:91 at Xamarin.Forms.Xaml.XamlLoader.Load (System.Object view, System.Type callingType) [0x00028] in D:\a\1\s\Xamarin.Forms.Xaml\XamlLoader.cs:69 at Xamarin.Forms.Xaml.Extensions.LoadFromXaml[TXaml] (TXaml view, System.Type callingType) [0x00000] in D:\a\1\s\Xamarin.Forms.Xaml\ViewExtensions.cs:36 at TheLittleThingsPlayground.Views.ThreeFivePage.InitializeComponent () [0x00001] in H:\github\TheLittleThingsPlayground\TheLittleThingsPlayground\obj\Debug g.cs:21 at TheLittleThingsPlayground.Views.ThreeFivePage..ctor () [0x00008] in H:\github\TheLittleThingsPlayground\TheLittleThingsPlayground\Views\ThreeFivePage.xaml.cs:12 at (wrapper managed-to-native) System.Reflection.MonoCMethod.InternalInvoke(System.Reflection.MonoCMethod,object,object[],System.Exception&) at System.Reflection.MonoCMethod.InternalInvoke (System.Object obj, System.Object[] parameters, System.Boolean wrapExceptions) [0x00005] in <84c6975c2cbc47b489a2a76477d7a312>:0 The `Xamarin.Forms.Xaml.dll` assembly isn't loaded when the `ThreeFivePage` type is constructed, which eventually means that a call to `DependencyService.Get()` returns `null`, because `Xamarin.Forms.Xaml.ResourcesLoader, Xamarin.Forms.Xaml` was not found or registered as an implementation for `IResourcesLoader`. There is a *workaround* for a post-b90d3ab7 environment: explicitly load the required assembly/assemblies earlier in the process: // In your MainActivity.cs protected override void OnCreate(Bundle bundle) { // Workaround Assembly.Load("Xamarin.Forms.Xaml"); base.OnCreate(bundle); global::Xamarin.Forms.Forms.Init(this, bundle); LoadApplication(new App()); } However, while there may be a workaround, this semantic change has unknown impact on the overall ecosystem and environment. Apps WILL break because of this change. We could revert the assembly loading portion of b90d3ab7, but that means *all* apps need to pay the cost of loading all assemblies, even if they don't need to pay that startup cost, a cost of ~30ms for the `tests/Xamarin.Forms-Performance-Integration` app. Instead, we'll enable the new functionality to be "opt-in", and make the previous behavior the default. Add a new `$(AndroidEnablePreloadAssemblies)` MSBuild property which, when True, restores the previous behavior of loading all assemblies within the `.apk` during process startup. The default value is True. If the `debug.mono.log` system property is set and contains `timing`, `adb logcat` will contain messages regarding how much time was spent loading each assembly, and the sum total for all assemblies: monodroid-timing: Assembly load: FormsViewGroup.dll preloaded; elapsed: 0s:1::267136 monodroid-timing: Assembly load: Newtonsoft.Json.dll preloaded; elapsed: 0s:0::999584 ... monodroid-timing: Finished loading assemblies: preloaded 29 assemblies; elapsed: 0s:26::783805 Finally, update `tests/Xamarin.Forms-Performance-Integration` so that it uses CSS, and thus hits the Xamarin.Forms codepath that triggers the above `NullReferenceException. This should validate that the fix is correct, and help prevent future regressions. [0]: https://github.com/xamarin/Xamarin.Forms/blob/cd1cf19fcad6ae049e90fdc5819c4f7c6adb57f6/Xamarin.Forms.Platform.Android/Forms.cs#L417 [1]: https://github.com/MvvmCross/MvvmCross/blob/43c1c7848d482adbc2bdc984746f36d74f64c190/MvvmCross/Core/MvxSetup.cs#L34 [2]: https://github.com/xamarin/Xamarin.Forms/issues/5164 --- Documentation/guides/BuildProcess.md | 24 +++++++ .../Xamarin.Android.Common.targets | 20 +++++- src/monodroid/jni/android-system.cc | 10 +++ src/monodroid/jni/android-system.h | 7 ++ src/monodroid/jni/monodroid-glue.cc | 66 +++++++++++++++++-- .../App.xaml.cs | 2 + .../Global.css | 5 ++ .../Views/MainPage.cs | 48 -------------- .../Views/MainPage.xaml | 7 ++ .../Views/MainPage.xaml.cs | 50 ++++++++++++++ 10 files changed, 185 insertions(+), 54 deletions(-) create mode 100644 tests/Xamarin.Forms-Performance-Integration/Global.css delete mode 100644 tests/Xamarin.Forms-Performance-Integration/Views/MainPage.cs create mode 100644 tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml create mode 100644 tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml.cs diff --git a/Documentation/guides/BuildProcess.md b/Documentation/guides/BuildProcess.md index f9c31f33cc2..5e21e91ca09 100644 --- a/Documentation/guides/BuildProcess.md +++ b/Documentation/guides/BuildProcess.md @@ -754,6 +754,30 @@ when packaing Release applications. Added in Xamarin.Android 8.3. +- **AndroidEnablePreloadAssemblies** – A boolean property which controls + whether or not all managed assemblies bundled within the application package + are loaded during process startup or not. + + When set to `True`, all assemblies bundled within the application package + will be loaded during process startup, before any application code is invoked. + This is consistent with what Xamarin.Android did in releases prior to + Xamarin.Andorid 9.2. + + When set to `False`, assemblies will only be loaded on an as-needed basis. + This allows applications to startup faster, and is also more consistent with + desktop .NET semantics. To see the time savings, set the `debug.mono.log` + System Property to include `timing`, and look for the + `Finished loading assemblies: preloaded` message within `adb logcat`. + + Applications or libraries which use dependency injection may *require* that + this property be `True` if they in turn require that + `AppDomain.CurrentDomain.GetAssemblies()` return all assemblies within the + application bundle, even if the assembly wouldn't otherwise have been needed. + + By default this value will be set to `True`. + + Added in Xamarin.Android 9.2. + ### Binding Project Build Properties The following MSBuild properties are used with diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 317d546f4c8..868617d5388 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -317,6 +317,9 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. <_AndroidDesignTimeBuildPropertiesCache>$(_AndroidIntermediateDesignTimeBuildDirectory)build.props False False + + + True @@ -2386,8 +2389,23 @@ because xbuild doesn't support framework reference assemblies. + + + + + + + + diff --git a/src/monodroid/jni/android-system.cc b/src/monodroid/jni/android-system.cc index 13e0689856f..112d727bec5 100644 --- a/src/monodroid/jni/android-system.cc +++ b/src/monodroid/jni/android-system.cc @@ -779,6 +779,16 @@ AndroidSystem::setup_environment (jstring_wrapper& name, jstring_wrapper& value) knownEnvVars.MonoLLVM = true; return; } + + if (strcmp (k, "mono.enable_assembly_preload") == 0) { + if (*v == '\0') + knownEnvVars.EnableAssemblyPreload = KnownEnvironmentVariables::AssemblyPreloadDefault; + else if (v[0] == '1') + knownEnvVars.EnableAssemblyPreload = true; + else + knownEnvVars.EnableAssemblyPreload = false; + return; + } } add_system_property (k, v); diff --git a/src/monodroid/jni/android-system.h b/src/monodroid/jni/android-system.h index d83607140b2..60292395cce 100644 --- a/src/monodroid/jni/android-system.h +++ b/src/monodroid/jni/android-system.h @@ -23,9 +23,12 @@ namespace xamarin { namespace android { namespace internal struct KnownEnvironmentVariables { + static constexpr bool AssemblyPreloadDefault = true; + bool DSOInApk = false; MonoAotMode MonoAOT = MonoAotMode::MONO_AOT_MODE_NONE; bool MonoLLVM = false; + bool EnableAssemblyPreload = AssemblyPreloadDefault; }; class AndroidSystem @@ -133,6 +136,10 @@ namespace xamarin { namespace android { namespace internal #if defined (WINDOWS) int setenv (const char *name, const char *value, int overwrite); #endif + bool is_assembly_preload_enabled () const + { + return knownEnvVars.EnableAssemblyPreload; + } bool is_mono_llvm_enabled () const { diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index 719c783de51..d3fd1e3bc39 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -957,7 +957,7 @@ mono_runtime_init (char *runtime_args) } static MonoDomain* -create_domain (JNIEnv *env, jclass runtimeClass, jstring_array_wrapper &runtimeApks, jstring assembly, jobject loader, bool is_root_domain) +create_domain (JNIEnv *env, jclass runtimeClass, jstring_array_wrapper &runtimeApks, jobject loader, bool is_root_domain) { MonoDomain *domain; int user_assemblies_count = 0;; @@ -1779,6 +1779,58 @@ _monodroid_counters_dump (const char *format, ...) monoFunctions.counters_dump (XA_LOG_COUNTERS, counters); } +static void +load_assembly (MonoDomain *domain, JNIEnv *env, jstring_wrapper &assembly) +{ + timing_period total_time; + if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) + total_time.mark_start (); + + const char *assm_name = assembly.get_cstr (); + MonoAssemblyName *aname; + + aname = monoFunctions.assembly_name_new (assm_name); + + if (domain != monoFunctions.domain_get ()) { + MonoDomain *current = monoFunctions.domain_get (); + monoFunctions.domain_set (domain, FALSE); + monoFunctions.assembly_load_full (aname, NULL, NULL, 0); + monoFunctions.domain_set (current, FALSE); + } else { + monoFunctions.assembly_load_full (aname, NULL, NULL, 0); + } + + monoFunctions.assembly_name_free (aname); + + if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { + total_time.mark_end (); + + timing_diff diff (total_time); + log_info (LOG_TIMING, "Assembly load: %s preloaded; elapsed: %lis:%lu::%lu", assm_name, diff.sec, diff.ms, diff.ns); + } +} + +static void +load_assemblies (MonoDomain *domain, JNIEnv *env, jstring_array_wrapper &assemblies) +{ + timing_period total_time; + if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) + total_time.mark_start (); + + /* skip element 0, as that's loaded in create_domain() */ + for (size_t i = 1; i < assemblies.get_length (); ++i) { + jstring_wrapper &assembly = assemblies [i]; + load_assembly (domain, env, assembly); + } + + if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { + total_time.mark_end (); + + timing_diff diff (total_time); + log_info (LOG_TIMING, "Finished loading assemblies: preloaded %u assemblies; wasted time: %lis:%lu::%lu", assemblies.get_length (), diff.sec, diff.ms, diff.ns); + } +} + static void monodroid_Mono_UnhandledException_internal (MonoException *ex) { @@ -1786,14 +1838,16 @@ monodroid_Mono_UnhandledException_internal (MonoException *ex) } static MonoDomain* -create_and_initialize_domain (JNIEnv* env, jclass runtimeClass, jstring_array_wrapper &runtimeApks, jobjectArray assemblies, jobject loader, bool is_root_domain) +create_and_initialize_domain (JNIEnv* env, jclass runtimeClass, jstring_array_wrapper &runtimeApks, jstring_array_wrapper &assemblies, jobject loader, bool is_root_domain) { - MonoDomain* domain = create_domain (env, runtimeClass, runtimeApks, reinterpret_cast (env->GetObjectArrayElement (assemblies, 0)), loader, is_root_domain); + MonoDomain* domain = create_domain (env, runtimeClass, runtimeApks, loader, is_root_domain); // When running on desktop, the root domain is only a dummy so don't initialize it if (is_running_on_desktop && is_root_domain) return domain; + if (androidSystem.is_assembly_preload_enabled ()) + load_assemblies (domain, env, assemblies); init_android_runtime (domain, env, runtimeClass, loader); osBridge.add_monodroid_domain (domain); @@ -1804,7 +1858,7 @@ create_and_initialize_domain (JNIEnv* env, jclass runtimeClass, jstring_array_wr JNIEXPORT void JNICALL Java_mono_android_Runtime_init (JNIEnv *env, jclass klass, jstring lang, jobjectArray runtimeApksJava, jstring runtimeNativeLibDir, jobjectArray appDirs, jobject loader, - jobjectArray externalStorageDirs, jobjectArray assemblies, jstring packageName, + jobjectArray externalStorageDirs, jobjectArray assembliesJava, jstring packageName, jint apiLevel, jobjectArray environmentVariables) { init_logging_categories (); @@ -1987,6 +2041,7 @@ Java_mono_android_Runtime_init (JNIEnv *env, jclass klass, jstring lang, jobject log_info_nocheck (LOG_TIMING, "Runtime.init: Mono runtime init; elapsed: %lis:%lu::%lu", diff.sec, diff.ms, diff.ns); } + jstring_array_wrapper assemblies (env, assembliesJava); /* the first assembly is used to initialize the AppDomain name */ create_and_initialize_domain (env, klass, runtimeApks, assemblies, loader, /*is_root_domain:*/ true); @@ -2070,7 +2125,7 @@ reinitialize_android_runtime_type_manager (JNIEnv *env) } JNIEXPORT jint -JNICALL Java_mono_android_Runtime_createNewContext (JNIEnv *env, jclass klass, jobjectArray runtimeApksJava, jobjectArray assemblies, jobject loader) +JNICALL Java_mono_android_Runtime_createNewContext (JNIEnv *env, jclass klass, jobjectArray runtimeApksJava, jobjectArray assembliesJava, jobject loader) { log_info (LOG_DEFAULT, "CREATING NEW CONTEXT"); reinitialize_android_runtime_type_manager (env); @@ -2078,6 +2133,7 @@ JNICALL Java_mono_android_Runtime_createNewContext (JNIEnv *env, jclass klass, j monoFunctions.jit_thread_attach (root_domain); jstring_array_wrapper runtimeApks (env, runtimeApksJava); + jstring_array_wrapper assemblies (env, assembliesJava); MonoDomain *domain = create_and_initialize_domain (env, klass, runtimeApks, assemblies, loader, /*is_root_domain:*/ false); monoFunctions.domain_set (domain, FALSE); int domain_id = monoFunctions.domain_get_id (domain); diff --git a/tests/Xamarin.Forms-Performance-Integration/App.xaml.cs b/tests/Xamarin.Forms-Performance-Integration/App.xaml.cs index 6a821006fef..09d63f63160 100644 --- a/tests/Xamarin.Forms-Performance-Integration/App.xaml.cs +++ b/tests/Xamarin.Forms-Performance-Integration/App.xaml.cs @@ -2,7 +2,9 @@ using Xamarin.Forms; using Xamarin.Forms.Xaml; +#if !DEBUG [assembly: XamlCompilation (XamlCompilationOptions.Compile)] +#endif namespace Xamarin.Forms.Performance.Integration { diff --git a/tests/Xamarin.Forms-Performance-Integration/Global.css b/tests/Xamarin.Forms-Performance-Integration/Global.css new file mode 100644 index 00000000000..e25f536375d --- /dev/null +++ b/tests/Xamarin.Forms-Performance-Integration/Global.css @@ -0,0 +1,5 @@ +.featureHeader { + font-size: 20; + font-style: bold; + margin: 0,20,0,10; +} \ No newline at end of file diff --git a/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.cs b/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.cs deleted file mode 100644 index 42242fc1e59..00000000000 --- a/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.cs +++ /dev/null @@ -1,48 +0,0 @@ -using System; - -using Xamarin.Forms; - -namespace Xamarin.Forms.Performance.Integration -{ - public class MainPage : TabbedPage - { - public MainPage () - { - Page itemsPage, aboutPage = null; - - switch (Device.RuntimePlatform) { - case Device.iOS: - itemsPage = new NavigationPage (new ItemsPage ()) { - Title = "Browse" - }; - - aboutPage = new NavigationPage (new AboutPage ()) { - Title = "About" - }; - itemsPage.Icon = "tab_feed.png"; - aboutPage.Icon = "tab_about.png"; - break; - default: - itemsPage = new ItemsPage () { - Title = "Browse" - }; - - aboutPage = new AboutPage () { - Title = "About" - }; - break; - } - - Children.Add (itemsPage); - Children.Add (aboutPage); - - Title = Children [0].Title; - } - - protected override void OnCurrentPageChanged () - { - base.OnCurrentPageChanged (); - Title = CurrentPage?.Title ?? string.Empty; - } - } -} diff --git a/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml b/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml new file mode 100644 index 00000000000..00ebf6b73ef --- /dev/null +++ b/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml @@ -0,0 +1,7 @@ + + + \ No newline at end of file diff --git a/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml.cs b/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml.cs new file mode 100644 index 00000000000..0c94b746b0f --- /dev/null +++ b/tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml.cs @@ -0,0 +1,50 @@ +using System; + +using Xamarin.Forms; + +namespace Xamarin.Forms.Performance.Integration +{ + public partial class MainPage : TabbedPage + { + public MainPage () + { + InitializeComponent (); + + Page itemsPage, aboutPage = null; + + switch (Device.RuntimePlatform) { + case Device.iOS: + itemsPage = new NavigationPage (new ItemsPage ()) { + Title = "Browse" + }; + + aboutPage = new NavigationPage (new AboutPage ()) { + Title = "About" + }; + itemsPage.Icon = "tab_feed.png"; + aboutPage.Icon = "tab_about.png"; + break; + default: + itemsPage = new ItemsPage () { + Title = "Browse" + }; + + aboutPage = new AboutPage () { + Title = "About" + }; + break; + } + + Children.Add (itemsPage); + Children.Add (aboutPage); + + Title = Children [0].Title; + } + + protected override void OnCurrentPageChanged () + { + base.OnCurrentPageChanged (); + Title = CurrentPage?.Title ?? string.Empty; + } + } +} \ No newline at end of file