Skip to content

Commit

Permalink
[monodroid] don't probe for .exe files (#3958)
Browse files Browse the repository at this point in the history 
I was noticing a lot of logging like this:

        I monodroid-assembly: open_from_update_dir: trying to open assembly: /data/user/0/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll
        I monodroid-assembly: open_from_update_dir: trying to open assembly: /storage/emulated/0/Android/data/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll
        I monodroid-assembly: open_from_update_dir: trying to open assembly: /data/user/0/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll.dll
        I monodroid-assembly: open_from_update_dir: trying to open assembly: /storage/emulated/0/Android/data/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll.dll
        I monodroid-assembly: open_from_update_dir: trying to open assembly: /data/user/0/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll.exe
        I monodroid-assembly: open_from_update_dir: trying to open assembly: /storage/emulated/0/Android/data/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll.exe
        I monodroid-assembly: open_from_bundles: looking for bundled name: 'mscorlib.dll'
        D Mono    : Image addref mscorlib[0x7f74876f3f00] (asmctx DEFAULT) -> mscorlib.dll[0x7f747fb32000]: 2

A couple things we could do here is:

 1. Don't probe for `.exe` files at all.  We don't even consider
    `.exe` files at build time anymore as of e390702.
 2. Check if the name ends with `.dll` to decide if `.dll` should be
    appended when probing.

This change is needed in both `open_from_update_dir()` and
`open_from_bundles()`.

Now the probing is cut down to:

        I monodroid-assembly: open_from_update_dir: trying to open assembly: /data/user/0/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll
        I monodroid-assembly: open_from_update_dir: trying to open assembly: /storage/emulated/0/Android/data/com.xamarin.android.helloworld/files/.__override__/mscorlib.dll
        I monodroid-assembly: open_from_bundles: looking for bundled name: 'mscorlib.dll'
        D Mono    : Image addref mscorlib[0x7f74876f2f80] (asmctx DEFAULT) -> mscorlib.dll[0x7f747fb33000]: 2

~~ Results ~~

Using the `samples\HelloWorld` app in this repo, on a Pixel 3 XL:

  * Before:

        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:165::704288
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:165::558089
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:165::936683
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:164::536735
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:166::357204

  * After:

        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:165::597673
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:164::106162
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:164::997516
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:164::931683
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:164::95381

My guess is this is a 1-2ms improvement on this device.

So then I tried an x86 emulator using HAXM:

  * Before:

        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:501::511300
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:545::571100
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:555::89900
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:541::920000
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:509::316700

  * After:

        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:472::543900
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:446::208700
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:469::216400
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:420::160500
        I monodroid-timing: Runtime.init: end, total time; elapsed: 0s:437::225200

Here the improvement might be more like 50-75ms.  I will probably
focus on timing the HAXM emulator from now on, since I am focusing on
improving the developer loop here.
  • Loading branch information
@jonathanpeppers @jonpryor
jonathanpeppers authored and jonpryor committed Nov 26, 2019
1 parent c36229c commit b882f66
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 42 deletions.
2 changes: 1 addition & 1 deletion src/monodroid/jni/embedded-assemblies-zip.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh
continue;
}

if (!(utils.ends_with (file_name, ".dll") || utils.ends_with (file_name, ".exe")))
if (!utils.ends_with (file_name, ".dll"))
continue;

if (entry_is_overridden)
Expand Down
32 changes: 16 additions & 16 deletions src/monodroid/jni/embedded-assemblies.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ EmbeddedAssemblies::open_from_bundles (MonoAssemblyName* aname, bool ref_only)
const char *asmname = mono_assembly_name_get_name (aname);

size_t name_len = culture == nullptr ? 0 : strlen (culture) + 1;
name_len += sizeof (".exe");
name_len += sizeof (".dll");
name_len += strlen (asmname);

size_t alloc_size = ADD_WITH_OVERFLOW_CHECK (size_t, name_len, 1);
Expand All @@ -71,25 +71,25 @@ EmbeddedAssemblies::open_from_bundles (MonoAssemblyName* aname, bool ref_only)
char *ename = name + strlen (name);

MonoAssembly *a = nullptr;
for (size_t si = 0; si < SUFFIXES_SIZE && a == nullptr; ++si) {
MonoBundledAssembly **p;
MonoBundledAssembly **p;

*ename = '\0';
strcat (name, suffixes [si]);
*ename = '\0';
if (!utils.ends_with (name, ".dll")) {
strcat (name, ".dll");
}

log_info (LOG_ASSEMBLY, "open_from_bundles: looking for bundled name: '%s'", name);
log_info (LOG_ASSEMBLY, "open_from_bundles: looking for bundled name: '%s'", name);

for (p = bundled_assemblies; p != nullptr && *p; ++p) {
MonoImage *image = nullptr;
MonoImageOpenStatus status;
const MonoBundledAssembly *e = *p;
for (p = bundled_assemblies; p != nullptr && *p; ++p) {
MonoImage *image = nullptr;
MonoImageOpenStatus status;
const MonoBundledAssembly *e = *p;

if (strcmp (e->name, name) == 0 &&
(image = mono_image_open_from_data_with_name ((char*) e->data, e->size, 0, nullptr, ref_only, name)) != nullptr &&
(a = mono_assembly_load_from_full (image, name, &status, ref_only)) != nullptr) {
mono_config_for_assembly (image);
break;
}
if (strcmp (e->name, name) == 0 &&
(image = mono_image_open_from_data_with_name ((char*) e->data, e->size, 0, nullptr, ref_only, name)) != nullptr &&
(a = mono_assembly_load_from_full (image, name, &status, ref_only)) != nullptr) {
mono_config_for_assembly (image);
break;
}
}
delete[] name;
Expand Down
6 changes: 0 additions & 6 deletions src/monodroid/jni/embedded-assemblies.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ namespace xamarin::android::internal {
#if defined (DEBUG) || !defined (ANDROID)
static constexpr char override_typemap_entry_name[] = ".__override__";
#endif
static constexpr const char *suffixes[] = {
"",
".dll",
".exe",
};
static constexpr size_t SUFFIXES_SIZE = sizeof(suffixes) / sizeof (suffixes[0]);

public:
/* filename is e.g. System.dll, System.dll.mdb, System.pdb */
Expand Down
33 changes: 14 additions & 19 deletions src/monodroid/jni/monodroid-glue.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,28 +293,23 @@ MonodroidRuntime::open_from_update_dir (MonoAssemblyName *aname, char **assembli

simple_pointer_guard<char[]> pname (pname_raw_ptr);

constexpr const char *formats[] = {
"%s" MONODROID_PATH_SEPARATOR "%s",
"%s" MONODROID_PATH_SEPARATOR "%s.dll",
"%s" MONODROID_PATH_SEPARATOR "%s.exe",
};
constexpr size_t nformats = sizeof (formats)/sizeof (formats [0]);
constexpr const char *format_none = "%s" MONODROID_PATH_SEPARATOR "%s";
constexpr const char *format_dll = "%s" MONODROID_PATH_SEPARATOR "%s.dll";

for (size_t fi = 0; fi < nformats && result == nullptr; ++fi) {
for (uint32_t oi = 0; oi < AndroidSystem::MAX_OVERRIDES; ++oi) {
override_dir = androidSystem.get_override_dir (oi);
if (override_dir == nullptr || !utils.directory_exists (override_dir))
continue;
for (uint32_t oi = 0; oi < AndroidSystem::MAX_OVERRIDES; ++oi) {
override_dir = androidSystem.get_override_dir (oi);
if (override_dir == nullptr || !utils.directory_exists (override_dir))
continue;

simple_pointer_guard<char, false> fullpath (utils.monodroid_strdup_printf (formats [fi], override_dir, pname.get ()));
const char *format = utils.ends_with (name, ".dll") ? format_none : format_dll;
simple_pointer_guard<char, false> fullpath (utils.monodroid_strdup_printf (format, override_dir, pname.get ()));

log_info (LOG_ASSEMBLY, "open_from_update_dir: trying to open assembly: %s\n", static_cast<const char*>(fullpath));
if (utils.file_exists (fullpath))
result = mono_assembly_open_full (fullpath, nullptr, 0);
if (result) {
// TODO: register .mdb, .pdb file
break;
}
log_info (LOG_ASSEMBLY, "open_from_update_dir: trying to open assembly: %s\n", static_cast<const char*>(fullpath));
if (utils.file_exists (fullpath))
result = mono_assembly_open_full (fullpath, nullptr, 0);
if (result != nullptr) {
// TODO: register .mdb, .pdb file
break;
}
}

Expand Down

0 comments on commit b882f66

Please sign in to comment.