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.

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

All app assemblies are loaded on startup #1443

Closed
kzu opened this issue Mar 19, 2018 · 9 comments
Closed

All app assemblies are loaded on startup #1443

kzu opened this issue Mar 19, 2018 · 9 comments
Labels
Area: App Runtime Issues in `libmonodroid.so`. Area: Performance Issues with performance.

Comments

@kzu
Copy link
Contributor

kzu commented Mar 19, 2018

Steps to Reproduce

Given code like the following:

// Some static dictionary somewhere:
public static Dictionary<string, Func<object>> Services { get; } = new Dictionary<string, Func<object>>();

// From MainActivity.cs;
Services.Add(nameof(BarServices.Bar), CreateBar);
Services.Add(nameof(FooServices.Foo), CreateFoo);

And given that the two methods are defined to explicitly not be inlined or otherwise loaded until invoked:

[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
static object CreateBar() => new BarServices.Bar();

[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
static object CreateFoo() => new FooServices.Foo();

And given FooServices.dll and BarServices.dll are two completely independent assemblies from the main app's, it would be highly desirable for the assemblies to only be loaded when the dictionary entry is accessed and the delegate is invoked, instead of happening on app start, which slows down the process "unnecessarily".

The ~same code in a .NET app properly lazy loads the assemblies as needed:

        public static Dictionary<string, Func<object>> Services { get; } = new Dictionary<string, Func<object>>(StringComparer.OrdinalIgnoreCase);

        static void Main(string[] args)
        {
            // Both this approach as well as the inline lambda properly delay-load the assemblies
            //Services.Add(nameof(BarServices.Bar), CreateBar);
            //Services.Add(nameof(FooServices.Foo), CreateFoo);

            Services.Add(nameof(BarServices.Bar), () => new BarServices.Bar());
            Services.Add(nameof(FooServices.Foo), () => new FooServices.Foo());

            var line = "";
            while ((line = Console.ReadLine()).Length > 0)
            {
                Console.WriteLine(Services[line.Trim()].Invoke().ToString());
            }
        }

        [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
        static object CreateBar() => new BarServices.Bar();

        [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
        static object CreateFoo() => new FooServices.Foo();

This is important for apps that use assembly-partitioning to split logic across modules, which should only be loaded whenever needed (i.e. when the UI that consumes them is navigated to, say).

Expected Behavior

Assemblies are lazy-loaded.

Actual Behavior

Both assemblies are loaded up-front:

03-19 15:36:33.243    nexus_5x    Debug    13769    Mono    Assembly FooServices[0x9de4ed60] added to domain RootDomain, ref_count=1

Version Information

Currently verified against 15.7 pre2

@kzu kzu added the Area: App Runtime Issues in `libmonodroid.so`. label Mar 19, 2018
@kzu
Copy link
Contributor Author

kzu commented Mar 19, 2018

@jonpryor
Copy link
Member

Related: https://github.com/xamarin/xamarin-android/blob/8c210a0b5d4b3bd33c6bd2cfb010260c65fed1b3/src/monodroid/jni/monodroid-glue.c#L3684-L3700

From the original commit introducing the register_packages() function:

During app startup, check assemblies to see if they contain the
Java.Interop.__TypeRegistrations type. If present, invoke
__TypeRegistrations.RegisterPackages().

This allows better type mappings to be done from
Java.Interop.TypeManager.CreateInstance(): instead of always returning
a wrapper type from Mono.Android.dll, we can use a "best fit" wrapper
type from any assembly containing Java.Interop.__TypeRegistrations,
including Mono.Android.GoogleMaps.dll.

For example, previously activity.FindViewById(id) would return a
ViewGroupInvoker for a MapView. With this change, it will return a
MapView instance. See also: f0dad0a3.

This isn't ideal, as we check every assembly during startup to see if
it contains a Java.Interop.__TypeRegistrations type.

The mentioned f0dad0a3 commit has this to say:

[Mono.Android] Fix View.FindViewById<T>() for 3rd party libraries.

View.FindViewById<T>() needs to use JavaCast<T>(), just as
Activity.FindViewById<T>() does (see commit ec965bd0).

Why? So that it actually works properly with 3rd party libs:

    http://lists.ximian.com/pipermail/monodroid/2012-February/009030.html

The odd thing is that the user is reporting that null is being
returned instead of throwing an InvalidCastException, but the problem
is the same: we shouldn't be using a runtime cast, we need to use
JavaCast<T>().

@jonpryor
Copy link
Member

Summarizing the above, here's the scenario:

  1. You have a Java library which exports a com.example.MyView type which inherits android.view.View.

  2. The type from (1) is bound into an Example.MyView type in Example.dll

  3. The type from (1) is used in Resources\layout\Main.axml within the App, a'la

    <LinearLayout ...>
      <com.example.MyView android:id="@+id/myView" .../>
  4. The App has code which calls:

    var boundView = FindViewById (Resource.Id.myView);

    Note: non-generic Activity.FindViewById() invocation.

The expectation is that Activity.FindViewById() will return the Example.MyView binding type. In order for that to happen, Xamarin.Android needs to know that the Example.MyView type exists, possibly before it is instantiated from C# code (which may never happen; all use could be via Activity.FindViewById()!).

The chosen implementation strategy was to load all assemblies during process startup, look for the Java.Interop.__TypeRegistrations type within the assembly, and -- if found -- invoke Java.Interop. __TypeRegistrations.RegisterPackages(). This allows Mono.Android.dll to know that Example.MyView exists and use it for type mappings.

@jonpryor
Copy link
Member

jonpryor commented Jul 2, 2018

I believe that the previous description is now -- and has been! -- moot, since the introduction of "type maps" in 2015-Apr-22 and subsequent fixes.

The typemap.jm and typemap.mj files should address the scenario described, without requiring any startup assembly loading.

Additionally, allow assemblies to be loaded on-demand -- instead of all at once, during process startup -- should help with our startup times.

@jonpryor jonpryor added the Area: Performance Issues with performance. label Jul 2, 2018
@jonpryor
Copy link
Member

jonpryor commented Jul 2, 2018

Related: #1829

@jonpryor
Copy link
Member

jonpryor commented Jul 2, 2018

The fix for "all assemblies are loaded on startup" is to remove register_packages(), and make sure that nothing breaks. :-)

@kzu
Copy link
Contributor Author

kzu commented Jul 11, 2018

Awesome news @jonpryor 💯 . I think this could have quite an impact in terms of app startup!

jonpryor pushed a commit that referenced this issue Jan 17, 2019
Fixes: #1443
Context: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android/plot/Tests%20times/

A frequent complaint with Xamarin.Android is that application startup
times can be slower than desired.  For example, see the
[Xamarin.Forms app startup data][xf-startup], in particular the
**init-Release** column (times in milliseconds), which is the time
at which the `Runtime.init: end native-to-managed transition` message
is printed when `timing` messages are enabled.

As of Jenkins build 1359, the **init-Release** value is 518ms.

(Note that the times in that table are not necessarily useful, nor
are they future proof: they come from the emulator that the Jenkins
build machine uses, and that emulator *has* changed and *will* change
in the future.  Additionally, because it's an x86-accelerated
emulator, times are not meaningful to Android hardware devices.  The
values also fluctuate a lot.  Better than nothing? ¯\_(ツ)_/¯)

Optimize some startup operations to reduce the **init-Release** value:

  * JNI Handling Improvements:
      * Reduce Java class lookup.
      * Don't lookup `android.os.Build.VERSION.SDK_INT` via JNI.
      * `jstring` handling improvements.
  * Review logging messages.
  * Improve package name hash generation.
  * Improve environment variable processing.
  * Mono Handling Improvements
      * Stop preloading all assemblies.
      * Avoid using "system properties" to control Mono features.
  * Desktop version is now a compile-time build option, not runtime.
  * Initialize `xamarin_getifaddrs()` on-demand, not at startup.

[xf-startup]: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android/plot/Tests%20times/#plot-8694435842335223270.csv

~~ JNI Handling Improvements ~~

Previously, JNI was used to lookup the
`android.os.Build.VERSION.SDK_INT` value.  We now pass
`Build.VERSION.SDK_INT` into `Runtime.init()`, avoiding the JNI calls.

Previously, `JNIEnv::FindClass()` was used during process startup in
order to lookup multiple types which are required for operation.
These types are now fields on the `mono.android.Runtime` type, and
`JNIEnv::GetStaticObjectField()` is used to obtain the `jclass`.

Additionally, a handful of class field/method lookups were moved out
of the init code so that the code that doesn't use them doesn't have
to pay the tax.

`jstring` and `jobjectArray`-of-`jstring` values are now handled by
the new `jstring_wrapper` and `jstring_array_wrapper` types, which
take care of efficiently caching the retrieved strings as well as of
correctly deleting local references to the obtained objects.
Both classes are optimized so that they compile into the equivalent
of the current, hand-written, code.  They also take care to make the
minimum necessary number of calls in order to access the strings,
both standalone and from arrays, as well as to release the resources.
The string and array wrapper instances are passed around as
references, thus using the minimum amount of memory.


~~ Log Messages ~~

Previously whenever any of the `log_{info,debug}()` functions were
called we'd spend time preparing all the parameters to pass to the
function, sometimes involving memory allocation, function calls, etc.,
only to discard all of that work **inside** the `log_*` call because
the logging category used in that call was disabled.

Now we check whether the category is enabled before we set out to
construct the parameters.


~~ Environment Processing ~~

Since [2012-Aug-16][env-support], `@(AndroidEnvironment)` has worked
by creating a file named `environment` within the `.apk` -- which is
stored uncompressed within the `.apk` -- and the file is then
processed, calling **setenv**(3) to store the recorded values.

There's a fair bit of potentially hairy string manipulation here,
from ***C***, which is not entirely ideal or performant.

To speed the process up, this commit replaces the `environment` file
with a Java class generated during application build which contains
an array of `"name", "value"` pairs.  The class is passed to
`Java_mono_android_Runtime_init()` and its elements are used to
create the requested environment variables.

Some of these variables are special-cased; instead of using them for
**setenv**(3), they control flags in the `AndroidSystem` class

  * `mono.aot`: The `$(AndroidAotMode)` value; which *kind* of AOT
    mono should support at runtime.
  * `mono.llvm`: The `$(EnableLLVM)` value; whether LLVM-generated
    AOT files should be used.
  * `__XA_DSO_IN_APK`: Support in-place reading of `.so` files;
    See commit 95ca102.

[env-support]: xamarin/monodroid@dbd73ec


~~ Mono Handling Improvements ~~

During process startup startup, *every* assembly within the `.apk`
would be loaded so that a `Java.Interop.__TypeRegistrations` type
could be probed, and if it existed, the
`__TypeRegistrations.RegisterPackages()` method would be invoked.

This was done in order to [better support "type mappings"][typemaps]
between Java names and C# names (and vice versa).

However, this support hasn't been required since the introduction
of the [`typemap.jm` and `typemap.mj` files][typemap-files]; we just
hadn't gotten around to removing the
`__TypeRegistrations.RegisterPackages()` invocation.

*Not* loading every assembly on startup allows assemblies to be
loaded on-demand, and improves startup times.

[typemaps]: #1443 (comment)
[typemap-files]: xamarin/monodroid@e69b76e


~~ Startup Time Summary ~~

Startup times for `tests/Xamarin.Forms-Performance-Integration`,
average of 50 iterations of (uninstall app, install app, launch app):

  * Debug configuration:

        Old: 1s 440ms
        New: 1s 100ms

  * Release configuration:

        Old: 650ms
        New: 270ms
@claudioredi
Copy link

@jonpryor Was this already released on stable channel?

@jonpryor
Copy link
Member

jonpryor commented Mar 9, 2022

@claudioredi: yes, this was released as part of Visual Studio 16.1 / Xamarin.Android 9.2. See also:

Note that $(AndroidEnablePreloadAssemblies) is True by default in "Classic" Xamarin.Android, and False by default in .NET 6.

@ghost ghost locked as resolved and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App Runtime Issues in `libmonodroid.so`. Area: Performance Issues with performance.
Projects
None yet
Development

No branches or pull requests

3 participants