Skip to content

Commit

Permalink
[runtime] Optionally preload all the assemblies from the apk (#2724)
Browse files Browse the repository at this point in the history
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/789540

Context: xamarin/Xamarin.Forms#5164

Commit b90d3ab 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 b90d3ab,
`AppDomain.CurrentDomain.GetAssemblies()` would return *all*
assemblies present within the `.apk`.  Starting with b90d3ab,
`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 b90d3ab 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 b90d3ab, 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<IResourcesLoader>()` 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 b90d3ab, 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]: xamarin/Xamarin.Forms#5164
  • Loading branch information
grendello authored and jonpryor committed Feb 12, 2019
1 parent 4503651 commit a6202e6
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 54 deletions.
24 changes: 24 additions & 0 deletions Documentation/guides/BuildProcess.md
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,30 @@ when packaing Release applications.

Added in Xamarin.Android 8.3.

- **AndroidEnablePreloadAssemblies** &ndash; 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
Expand Down
20 changes: 19 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<_AndroidDesignTimeBuildPropertiesCache>$(_AndroidIntermediateDesignTimeBuildDirectory)build.props</_AndroidDesignTimeBuildPropertiesCache>
<AndroidGenerateJniMarshalMethods Condition=" '$(AndroidGenerateJniMarshalMethods)' == '' ">False</AndroidGenerateJniMarshalMethods>
<AndroidMakeBundleKeepTemporaryFiles Condition=" '$(AndroidMakeBundleKeepTemporaryFiles)' == '' ">False</AndroidMakeBundleKeepTemporaryFiles>

<!-- If true it will cause all the assemblies in the apk to be preloaded on startup time -->
<AndroidEnablePreloadAssemblies Condition=" '$(AndroidEnablePreloadAssemblies)' == '' ">True</AndroidEnablePreloadAssemblies>
</PropertyGroup>

<Choose>
Expand Down Expand Up @@ -2344,8 +2347,23 @@ because xbuild doesn't support framework reference assemblies.
</GetAddOnPlatformLibraries>
</Target>

<Target Name="_SetupAssemblyPreload"
Condition=" '$(AndroidEnablePreloadAssemblies)' != 'True' "
Inputs="$(IntermediateOutputPath)javastubs.cache"
Outputs="$(IntermediateOutputPath)preloadAssembliesEnvironment.txt">
<WriteLinesToFile
File="$(IntermediateOutputPath)preloadAssembliesEnvironment.txt"
Lines="mono.enable_assembly_preload=0"
Overwrite="True"
/>
<ItemGroup>
<AndroidEnvironment Include="$(IntermediateOutputPath)preloadAssembliesEnvironment.txt" />
<FileWrites Include="$(IntermediateOutputPath)preloadAssembliesEnvironment.txt" />
</ItemGroup>
</Target>

<Target Name="_GeneratePackageManagerJava"
DependsOnTargets="_GetAddOnPlatformLibraries;_AddStaticResources;$(_AfterAddStaticResources);_PrepareAssemblies"
DependsOnTargets="_GetAddOnPlatformLibraries;_AddStaticResources;$(_AfterAddStaticResources);_PrepareAssemblies;_SetupAssemblyPreload"
Inputs="$(MSBuildAllProjects);$(_ResolvedUserAssembliesHashFile);$(MSBuildProjectFile);$(_AndroidBuildPropertiesCache)"
Outputs="$(_AndroidStampDirectory)_GeneratePackageManagerJava.stamp">
<!-- Create java needed for Mono runtime -->
Expand Down
10 changes: 10 additions & 0 deletions src/monodroid/jni/android-system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,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);
Expand Down
7 changes: 7 additions & 0 deletions src/monodroid/jni/android-system.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,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
Expand Down Expand Up @@ -131,6 +134,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
{
Expand Down
66 changes: 61 additions & 5 deletions src/monodroid/jni/monodroid-glue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,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;;
Expand Down Expand Up @@ -1732,21 +1732,75 @@ _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)
{
// Do nothing with it here, we let the exception naturally propagate on the managed side
}

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 <jstring> (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);
Expand All @@ -1757,7 +1811,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 ();
Expand Down Expand Up @@ -1939,6 +1993,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);

Expand Down Expand Up @@ -2023,14 +2078,15 @@ 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);
MonoDomain *root_domain = monoFunctions.get_root_domain ();
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);
Expand Down
2 changes: 2 additions & 0 deletions tests/Xamarin.Forms-Performance-Integration/App.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
using Xamarin.Forms;
using Xamarin.Forms.Xaml;

#if !DEBUG
[assembly: XamlCompilation (XamlCompilationOptions.Compile)]
#endif

namespace Xamarin.Forms.Performance.Integration
{
Expand Down
5 changes: 5 additions & 0 deletions tests/Xamarin.Forms-Performance-Integration/Global.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.featureHeader {
font-size: 20;
font-style: bold;
margin: 0,20,0,10;
}
48 changes: 0 additions & 48 deletions tests/Xamarin.Forms-Performance-Integration/Views/MainPage.cs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8" ?>
<TabbedPage
xmlns="http://xamarin.com/schemas/2014/forms"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Xamarin.Forms.Performance.Integration.MainPage"
Resources="{StyleSheet Source=../Global.css}">
</TabbedPage>
50 changes: 50 additions & 0 deletions tests/Xamarin.Forms-Performance-Integration/Views/MainPage.xaml.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
}

0 comments on commit a6202e6

Please sign in to comment.