Skip to content

[Xamarin.Android.Build.Tasks] Add support for new InstantRun - #609

Merged
jonpryor merged 2 commits into
dotnet:masterfrom
dellis1972:instantrun
Sep 25, 2018
Merged

[Xamarin.Android.Build.Tasks] Add support for new InstantRun#609
jonpryor merged 2 commits into
dotnet:masterfrom
dellis1972:instantrun

Conversation

@dellis1972

@dellis1972 dellis1972 commented May 26, 2017

Copy link
Copy Markdown
Contributor

This commit does a couple of things.

  1. It reworks the monodroid-glue to copy any .so file it finds
    in the .override directory over to the local app directory.
    This should allow us to fast deploy native libraries. Which means
    we no longer need to re-build the apk if a native library changes.

  2. Removes and replaces the StubApplication with a new MultiDexLoader
    ContentProvider. This provider is responsible for doing some the things
    the previous StubApplication used to do. But because it is not
    derived from an Application it means we should be able to support
    custom Application types for our debug builds. The MultiDexLoader will
    run before EVERYTHING else. This is so we can use dex files from the
    override directory.

  3. Move the Resource Patching code from StubApplication over to a
    dedicated MonkeyPatcher class so it is all in one place.

  4. Introduce a ResourceLoader ContentProvider which will deal with
    patching the resource in a FastDev environment. This is to support Custom
    Application classes
    .
    The problem we had with this is the resource patching requires that the application
    be created. But because the code was in the MultiDexLoader or StubApplication
    this could not happen since it would require the app to exist. So the solution is to move
    the patching to a dedicated ContentProvider which is loaded AFTER the user app has
    been created.

@jonpryor

Copy link
Copy Markdown
Contributor

This is failing .apk execution on the emulator:

java.lang.RuntimeException: Unable to get provider mono.android.MultiDexLoader: java.lang.ClassNotFoundException: Didn't find class "mono.android.MultiDexLoader" on path: DexPathList[[zip file "/data/app/Xamarin.Android.Locale_Tests-1/base.apk", zip file "/data/app/Xamarin.Android.Locale_Tests-1/base.apk"],nativeLibraryDirectories=[/data/app/Xamarin.Android.Locale_Tests-1/lib/x86, /data/app/Xamarin.Android.Locale_Tests-1/lib/x86, /vendor/lib, /system/lib]]
	at android.app.ActivityThread.installProvider(ActivityThread.java:4967)
	at android.app.ActivityThread.installContentProviders(ActivityThread.java:4559)
	at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4499)
	at android.app.ActivityThread.access$1500(ActivityThread.java:144)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1339)
	at android.os.Handler.dispatchMessage(Handler.java:102)
	at android.os.Looper.loop(Looper.java:135)
	at android.app.ActivityThread.main(ActivityThread.java:5221)
	at java.lang.reflect.Method.invoke(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:372)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)
Caused by: java.lang.ClassNotFoundException: Didn't find class "mono.android.MultiDexLoader" on path: DexPathList[[zip file "/data/app/Xamarin.Android.Locale_Tests-1/base.apk", zip file "/data/app/Xamarin.Android.Locale_Tests-1/base.apk"],nativeLibraryDirectories=[/data/app/Xamarin.Android.Locale_Tests-1/lib/x86, /data/app/Xamarin.Android.Locale_Tests-1/lib/x86, /vendor/lib, /system/lib]]
	at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:56)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:511)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:469)
	at android.app.ActivityThread.installProvider(ActivityThread.java:4952)
	... 11 more
	Suppressed: java.lang.ClassNotFoundException: mono.android.MultiDexLoader
		at java.lang.Class.classForName(Native Method)
		at java.lang.BootClassLoader.findClass(ClassLoader.java:781)
		at java.lang.BootClassLoader.loadClass(ClassLoader.java:841)
		at java.lang.ClassLoader.loadClass(ClassLoader.java:504)
		... 13 more
	Caused by: java.lang.NoClassDefFoundError: Class not found using the boot class loader; no stack available

@dellis1972
dellis1972 force-pushed the instantrun branch 2 times, most recently from 202adfc to d40bc5d Compare May 26, 2017 15:53
Comment thread src/monodroid/jni/monodroid-glue.c Outdated
char *to_libmonoso = path_combine (to, "libmonosgen-2.0.so");
unlink (to_libmonoso);
char *to_file = path_combine (to, file);
unlink (to_file);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I realize that the original code didn't do this...but we should probably cleanup anyway. ;-)

This should probably be:

int r = unlink (to_file);
if (r < 0 && errno != ENOENT) {
    log_warn (LOG_DEFAULT, "Unable to delete file `%s`", to_file);
}

Though this raises entire questions about ensuring that memory is freed...

Which means I now get to do what I keep suggesting on ##csharp! "How do we sanely cleanup resources without using goto? By abusing do{}while!"

static void
copy_file_to_internal_location (const char *to_dir, const char *from_dir, const char *file)
{
	char *from_file = path_combine (from_dir, file);
	char *to_file   = NULL;
	
	do {
		if (!from_file || !file_exists (from_file))
			break;

		log_warn (LOG_DEFAULT, "Copying file `%s` from external location `%s` to internal location `%s`",
				file, from_dir, to_dir);
		
		to_file = path_combine (to_dir, file);
		if (!to_file)
			break;
		
		int r = unlink (to_file);
		if (r < 0 && errno != ENOENT) {
			log_warn (LOG_DEFAULT, "Unable to delete file `%s`: %s", to_file, strerror (errno));
			break;
		}
		
		if (file_copy (to_file, from_file) < 0) {
			log_warn (LOG_DEFAULT, "Copy failed from `%s` to `%s`: %s", from_file, to_file, strerror (errno));
			break;
		}
		
		set_user_executable (to_file);
	} while (0);
	free (from_file);
	free (to_file);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jonpryor do you still want me to implement this while loop?

Comment thread src/monodroid/jni/monodroid-glue.c
if ((dir = monodroid_opendir (dir_path)) == NULL)
continue;

while (readdir_r (dir, &b, &e) == 0 && e) {

@jonpryor jonpryor May 27, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suspect that this will break on the Windows build; if we add the full-mono-integration-build Labe and rebuildl, we'll find out. ;-)

Assuming this is the case, we'll need to add a monodroid_readdir() function to util.c and use it here.

Comment thread src/Xamarin.Android.Build.Tasks/Resources/MultiDexLoader.java

public class MultiDexLoader extends ContentProvider {

boolean created;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see why we need this field. It doesn't appear to be used anywhere, other than onCreate().

@dellis1972 dellis1972 added the full-mono-integration-build For PRs; run a full build (~6-10h for mono bumps), not the faster PR subset (~2h for mono bumps) label May 27, 2017
@dellis1972

Copy link
Copy Markdown
Contributor Author

build

@dellis1972

Copy link
Copy Markdown
Contributor Author

@jonpryor I updated this and added the full build label. not sure if that did anything though? How can you tell?

@jonpryor

Copy link
Copy Markdown
Contributor

How can you tell?

Normal PR build will only build one Mono.Android.dll, and ~3 ABIs for mono (if no bundle.zip exists): armv7, x86, "host".

full-mono-integration-build does everything that a non-PR build does, so all API levels, all the ABIs, both Debug and Release.

As such:

  1. The full-mono-integration build takes longer.
  2. If Mono was bumped, AOT-related tests more reliably pass, because the AOT compilers will exist. (In theory these should pass anyway, but historically they don't.)
  3. If you download the oss*.zip build artifact, you'll see all the Mono.Android.dll assemblies. Ditto if you read the the log file.

@jonpryor

Copy link
Copy Markdown
Contributor

I think this PR should also have a new test, "somewhere/somehow", which sets $(AndroidEnableMultiDex)=True.

I'm less certain on how this should look; perhaps we could make e.g. src/Mono.Android/Test/*.csproj into a shared project+.csproj "combo", and add a new tests/Runtime-MultiDex directory which references the shared project?

Or perhaps just a new "Hello, World"-esque test would be sufficient to test MultiDex?

Either way, this new test should also contain an Android.App.Application subclass, to ensure that this works. See also https://bugzilla.xamarin.com/show_bug.cgi?id=55050.

Comment thread src/monodroid/jni/monodroid-glue.c Outdated
char *to_libmonoso = path_combine (to, "libmonosgen-2.0.so");
unlink (to_libmonoso);
if (dir_path == NULL || !directory_exists (dir_path)) {
if (dir_path != NULL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need to check for null. free(3) will accept the NULL pointer and ignore it:

The free() function.... If ptr is NULL, no operation is performed.

@dellis1972
dellis1972 force-pushed the instantrun branch 2 times, most recently from 5ac892a to 619b55c Compare June 7, 2017 14:09
@dellis1972

Copy link
Copy Markdown
Contributor Author

build

@dellis1972
dellis1972 force-pushed the instantrun branch 4 times, most recently from ca83568 to 552bace Compare July 25, 2018 10:06
@dellis1972
dellis1972 force-pushed the instantrun branch 5 times, most recently from 0c88697 to e8463e5 Compare August 30, 2018 12:50
@dellis1972

Copy link
Copy Markdown
Contributor Author

build

@dellis1972

Copy link
Copy Markdown
Contributor Author

build

@dellis1972
dellis1972 force-pushed the instantrun branch 2 times, most recently from 833152b to c0d01a5 Compare September 14, 2018 08:57
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we have this file? It doesn't appear to be used from anywhere, and it's confusing being alongside Main.axml.

@@ -0,0 +1,49 @@
<?xml version="1.0" encoding="utf-8"?>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we have this file? It doesn't appear to be used from anywhere, and it's confusing being alongside NameFixups.axml.

@jonpryor

Copy link
Copy Markdown
Contributor

@dellis1972

dellis1972 commented Sep 17, 2018 via email

Copy link
Copy Markdown
Contributor Author

@dellis1972
dellis1972 force-pushed the instantrun branch 3 times, most recently from 395d5ee to 4460c76 Compare September 18, 2018 12:06
// Older platforms
Map activities = (HashMap) collection;
c = activities.values();
} else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will probably fail when building on e.g. API-10, as Build.VERSION_CODES.KITKAT won't exist.

@dellis1972

Copy link
Copy Markdown
Contributor Author

build

1 similar comment
@dellis1972

Copy link
Copy Markdown
Contributor Author

build

This commit does a couple of things.

1) It reworks the monodroid-glue to copy any .so file it finds
in the .__override__ directory over to the local app directory.
This should allow us to fast deploy native libraries. Which means
we no longer need to re-build the apk if a native library changes.

2) Replaces the StubApplication with a new MultiDexLoader
ContentProvider. This provider is reposible for doing all the things
the previous StubApplication used to do. But because it is not
derived from an Application it means we can now support custom
Application types for our debug builds.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Area: Performance Issues with performance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants