From 10401c15d048216c60d3d254bc11bc54dc37caeb Mon Sep 17 00:00:00 2001 From: lateralusX Date: Wed, 30 Jun 2021 19:05:04 +0200 Subject: [PATCH 1/3] Optimize mono_dl_build_path for component and Android module loading. mono_dl_build_path probes a lot of variations of paths and in some situations where modules uses platform specific naming, it could try at least two paths before getting to the correct one. This have been a problem for Android a long time since IO is slow on devices, so always failing two probes per module load adds to the startup time on Android. For dynamic component loading the existing schema is particular bad since the scenario of disabling a component means that the component won't exist, and using the existing probing it could do 3 or more load attempts before realize that the component is not present and use the stub implmenentation. Commit splits current mono_dl_build_path probing logic so it can be customized. For components a new function has been added that will always use platform prefix and platform suffixes, and if not found it will stop looking. For most platform that means only one attempt to load a component instead of at least 3, optimize both the scenario when the component is awailable as well as when it should be disabled. Commit also change the default behavior of mono_dl_build_path on Android reversing current schema making it more likely to find modules using platform specific naming, like libMyLibrary.so, in first attempt. It will still fallback using no prefix and default suffix and then the verbatim name to make sure dllimports will still work as before, but favorizing modules using platform specific naming. On all other platforms mono_dl_build_path will continue to keep its current behaviour. --- src/mono/mono/metadata/components.c | 7 +- src/mono/mono/metadata/native-library.c | 8 +- src/mono/mono/utils/mono-dl.c | 168 ++++++++++++++++++------ src/mono/mono/utils/mono-dl.h | 1 + 4 files changed, 134 insertions(+), 50 deletions(-) diff --git a/src/mono/mono/metadata/components.c b/src/mono/mono/metadata/components.c index d0a2c1257f23db..dae7b336470e1f 100644 --- a/src/mono/mono/metadata/components.c +++ b/src/mono/mono/metadata/components.c @@ -150,20 +150,17 @@ try_load (const char* dir, const MonoComponentEntry *component, const char* comp char *path = NULL; void *iter = NULL; - while ((path = mono_dl_build_path (dir, component_base_lib, &iter))) { + while (lib == NULL && (path = mono_dl_build_platform_path (dir, component_base_lib, &iter))) { char *error_msg = NULL; lib = mono_dl_open (path, MONO_DL_EAGER | MONO_DL_LOCAL, &error_msg); - if (lib) - break; if (!lib) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s not found: %s", component->name, error_msg); + g_free (error_msg); } - g_free (error_msg); g_free (path); } if (lib) mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s found at %s", component->name, path); - g_free (path); return lib; } diff --git a/src/mono/mono/metadata/native-library.c b/src/mono/mono/metadata/native-library.c index 10808d2144b134..6867a07379cd88 100644 --- a/src/mono/mono/metadata/native-library.c +++ b/src/mono/mono/metadata/native-library.c @@ -493,12 +493,11 @@ static MonoDl * netcore_probe_for_module_variations (const char *mdirname, const char *file_name, int raw_flags) { void *iter = NULL; - char *full_name; + char *full_name = NULL; MonoDl *module = NULL; - // FIXME: this appears to search *.dylib twice for some reason - while ((full_name = mono_dl_build_path (mdirname, file_name, &iter)) && module == NULL) { - char *error_msg; + while (module == NULL && (full_name = mono_dl_build_path (mdirname, file_name, &iter))) { + char *error_msg = NULL; module = mono_dl_open_full (full_name, MONO_DL_LAZY, raw_flags, &error_msg); if (!module) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "DllImport error loading library '%s': '%s'.", full_name, error_msg); @@ -506,7 +505,6 @@ netcore_probe_for_module_variations (const char *mdirname, const char *file_name } g_free (full_name); } - g_free (full_name); return module; } diff --git a/src/mono/mono/utils/mono-dl.c b/src/mono/mono/utils/mono-dl.c index eec013399de3af..166cd5d602a8fb 100644 --- a/src/mono/mono/utils/mono-dl.c +++ b/src/mono/mono/utils/mono-dl.c @@ -390,35 +390,12 @@ mono_dl_close (MonoDl *module) g_free (module); } -/** - * mono_dl_build_path: - * \param directory optional directory - * \param name base name of the library - * \param iter iterator token - * Given a directory name and the base name of a library, iterate - * over the possible file names of the library, taking into account - * the possible different suffixes and prefixes on the host platform. - * - * The returned file name must be freed by the caller. - * \p iter must point to a NULL pointer the first time the function is called - * and then passed unchanged to the following calls. - * \returns the filename or NULL at the end of the iteration - */ -char* -mono_dl_build_path (const char *directory, const char *name, void **iter) -{ - int idx; - const char *prefix; - const char *suffix; - gboolean need_prefix = TRUE, need_suffix = TRUE; - int prlen; - int suffixlen; - char *res; - int iteration; - - if (!iter) - return NULL; +typedef gboolean (*dl_library_name_formatting_func)(int idx, gboolean *need_prefix, gboolean *need_suffix, const char **suffix); +static +gboolean +dl_default_library_name_formatting (int idx, gboolean *need_prefix, gboolean *need_suffix, const char **suffix) +{ /* The first time we are called, idx = 0 (as *iter is initialized to NULL). This is our "bootstrap" phase in which we check the passed name verbatim and only if we fail to find @@ -428,26 +405,79 @@ mono_dl_build_path (const char *directory, const char *name, void **iter) libsomething.so.1.1 or libsomething.so - testing it algorithmically would be an overkill here. */ - iteration = GPOINTER_TO_UINT (*iter); - idx = iteration; if (idx == 0) { /* Name */ - need_prefix = FALSE; - need_suffix = FALSE; - suffix = ""; + *need_prefix = FALSE; + *need_suffix = FALSE; + *suffix = ""; } else if (idx == 1) { /* netcore system libs have a suffix but no prefix */ - need_prefix = FALSE; - need_suffix = TRUE; - suffix = mono_dl_get_so_suffixes () [0]; - suffixlen = strlen (suffix); + *need_prefix = FALSE; + *need_suffix = TRUE; + *suffix = mono_dl_get_so_suffixes () [0]; } else { /* Prefix.Name.suffix */ - suffix = mono_dl_get_so_suffixes () [idx - 2]; - if (suffix [0] == '\0') - return NULL; + *need_prefix = TRUE; + *need_suffix = TRUE; + *suffix = mono_dl_get_so_suffixes () [idx - 2]; + if ((*suffix) [0] == '\0') + return FALSE; + } + + return TRUE; +} + +static +gboolean +dl_reverse_library_name_formatting (int idx, gboolean *need_prefix, gboolean *need_suffix, const char **suffix) +{ + const char ** suffixes = mono_dl_get_so_suffixes (); + int suffix_count = 0; + + while (suffixes [suffix_count][0] != '\0') + suffix_count++; + + if (idx < suffix_count) { + /* Prefix.Name.suffix */ + *need_prefix = TRUE; + *need_suffix = TRUE; + *suffix = suffixes [idx]; + } else if (idx == suffix_count) { + /* netcore system libs have a suffix but no prefix */ + *need_prefix = FALSE; + *need_suffix = TRUE; + *suffix = suffixes [0]; + } else if (idx == suffix_count + 1) { + /* Name */ + *need_prefix = FALSE; + *need_suffix = FALSE; + *suffix = ""; + } else { + return FALSE; } + return TRUE; +} + +static char* +dl_build_path (const char *directory, const char *name, void **iter, dl_library_name_formatting_func func) +{ + const char *prefix; + const char *suffix; + gboolean need_prefix = TRUE, need_suffix = TRUE; + int prlen; + int suffixlen; + char *res; + int iteration; + + if (!iter) + return NULL; + + + iteration = GPOINTER_TO_UINT (*iter); + if (!func (iteration, &need_prefix, &need_suffix, &suffix)) + return NULL; + if (need_prefix) { prlen = strlen (mono_dl_get_so_prefix ()); if (prlen && strncmp (name, mono_dl_get_so_prefix (), prlen) != 0) @@ -471,6 +501,64 @@ mono_dl_build_path (const char *directory, const char *name, void **iter) return res; } +/** + * mono_dl_build_path: + * \param directory optional directory + * \param name base name of the library + * \param iter iterator token + * Given a directory name and the base name of a library, iterate + * over the possible file names of the library, taking into account + * the possible different suffixes and prefixes on the host platform. + * + * The returned file name must be freed by the caller. + * \p iter must point to a NULL pointer the first time the function is called + * and then passed unchanged to the following calls. + * \returns the filename or NULL at the end of the iteration + */ +char* +mono_dl_build_path (const char *directory, const char *name, void **iter) +{ +#ifdef HOST_ANDROID + return dl_build_path (directory, name, iter, dl_reverse_library_name_formatting); +#else + return dl_build_path (directory, name, iter, dl_default_library_name_formatting); +#endif +} + +static +gboolean +dl_prefix_suffix_library_name_formatting (int idx, gboolean *need_prefix, gboolean *need_suffix, const char **suffix) +{ + /* Prefix.Name.suffix */ + *need_prefix = TRUE; + *need_suffix = TRUE; + *suffix = mono_dl_get_so_suffixes () [idx]; + if ((*suffix) [0] == '\0') + return FALSE; + + return TRUE; +} + +/** + * mono_dl_build_platform_path: + * \param directory optional directory + * \param name base name of the library + * \param iter iterator token + * Given a directory name and the base name of a library, iterate + * over platform prefix and suffixes generating a library name + * suitable for the platform. Function won't try additional fallbacks. + * + * The returned file name must be freed by the caller. + * \p iter must point to a NULL pointer the first time the function is called + * and then passed unchanged to the following calls. + * \returns the filename or NULL at the end of the iteration + */ +char* +mono_dl_build_platform_path (const char *directory, const char *name, void **iter) +{ + return dl_build_path (directory, name, iter, dl_prefix_suffix_library_name_formatting); +} + MonoDlFallbackHandler * mono_dl_fallback_register (MonoDlFallbackLoad load_func, MonoDlFallbackSymbol symbol_func, MonoDlFallbackClose close_func, void *user_data) { diff --git a/src/mono/mono/utils/mono-dl.h b/src/mono/mono/utils/mono-dl.h index 44e7e9e34e1864..5a810f8c295dac 100644 --- a/src/mono/mono/utils/mono-dl.h +++ b/src/mono/mono/utils/mono-dl.h @@ -36,6 +36,7 @@ MONO_EXTERN_C void mono_dl_close (MonoDl *module); char* mono_dl_build_path (const char *directory, const char *name, void **iter); +char* mono_dl_build_platform_path (const char *directory, const char *name, void **iter); MonoDl* mono_dl_open_runtime_lib (const char *lib_name, int flags, char **error_msg); From 0f597e74db08b06820adb0a354df7216d065313e Mon Sep 17 00:00:00 2001 From: lateralusX Date: Thu, 1 Jul 2021 12:52:50 +0200 Subject: [PATCH 2/3] Reduce try_load attempts when using dynamic components. Cache try load history reduce try_load calls when library host multiple components (like diagnostics_tracing). Change mobile to not try components_dir, just loading using name directly. Above changes result in mono_dl_get_so_suffixes number of dl_open attempts per component library and on Android that will be one attempt independt of components begin enabled or disabled. --- src/mono/mono/metadata/components.c | 65 ++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/src/mono/mono/metadata/components.c b/src/mono/mono/metadata/components.c index dae7b336470e1f..4b5d2d40656679 100644 --- a/src/mono/mono/metadata/components.c +++ b/src/mono/mono/metadata/components.c @@ -16,17 +16,23 @@ #include "mono/utils/mono-logger-internals.h" #include "mono/utils/mono-path.h" #include "mono/utils/mono-time.h" +#include "mono/utils/refcount.h" static gint64 event_pipe_100ns_ticks; typedef MonoComponent * (*MonoComponentInitFn) (void); +typedef struct _MonoComponentLibrary { + MonoRefCount ref; + MonoDl *lib; +} MonoComponentLibrary; + typedef struct _MonoComponentEntry { const char *lib_name; const char *name; MonoComponentInitFn init; MonoComponent **component; - MonoDl *lib; + MonoComponentLibrary *lib; } MonoComponentEntry; #define COMPONENT_INIT_FUNC(name) (MonoComponentInitFn) mono_component_ ## name ## _init @@ -58,8 +64,10 @@ MonoComponentEntry components[] = { }; #ifndef STATIC_COMPONENTS +static GHashTable *component_library_load_history = NULL; + static MonoComponent* -get_component (const MonoComponentEntry *component, MonoDl **component_lib); +get_component (const MonoComponentEntry *component, MonoComponentLibrary **component_lib); #endif void @@ -82,12 +90,14 @@ mono_components_init (void) for (int i = 0; i < G_N_ELEMENTS (components); ++i) *components [i].component = components [i].init (); #else + component_library_load_history = g_hash_table_new (g_str_hash, g_str_equal); + /* call get_component for each component and init it or its stubs and add it to loaded_components */ - MonoDl *lib = NULL; + MonoComponentLibrary *component_lib = NULL; for (int i = 0; i < G_N_ELEMENTS (components); ++i) { - *components [i].component = get_component (&components [i], &lib); - components [i].lib = lib; + *components [i].component = get_component (&components [i], &component_lib); + components [i].lib = component_lib; if (!*components [i].component) *components [i].component = components [i].init (); } @@ -106,11 +116,11 @@ component_init_name (const MonoComponentEntry *component) } static gpointer -load_component_entrypoint (MonoDl *lib, const MonoComponentEntry *component) +load_component_entrypoint (MonoComponentLibrary *component_lib, const MonoComponentEntry *component) { char *component_init = component_init_name (component); gpointer sym = NULL; - char *error_msg = mono_dl_symbol (lib, component_init, &sym); + char *error_msg = mono_dl_symbol (component_lib->lib, component_init, &sym); if (error_msg) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s library does not have symbol %s: %s", component->name, component_init, error_msg); g_free (error_msg); @@ -164,42 +174,57 @@ try_load (const char* dir, const MonoComponentEntry *component, const char* comp return lib; } + + static MonoComponentInitFn -load_component (const MonoComponentEntry *component, MonoDl **lib_out) +load_component (const MonoComponentEntry *component, MonoComponentLibrary **component_lib_out) { // If init method has been static linked not using stub library, use that instead of dynamic component. if (component->init() && component->init()->available ()) { - *lib_out = NULL; + *component_lib_out = NULL; return component->init; } char *component_base_lib = component_library_base_name (component); MonoComponentInitFn result = NULL; + MonoComponentLibrary *component_lib = NULL; - /* FIXME: just copy what mono_profiler_load does, assuming it works */ + // Check history of component library loads to reduce try_load attempts (optimization for libraries hosting multiple components). + if (!g_hash_table_lookup_extended (component_library_load_history, component_base_lib, NULL, (gpointer *)&component_lib)) { + MonoDl *lib = NULL; +#if !defined(HOST_IOS) && !defined(HOST_TVOS) && !defined(HOST_WATCHOS) && !defined(HOST_MACCAT) && !defined(HOST_ANDROID) + lib = try_load (components_dir (), component, component_base_lib); +#endif + if (!lib) + lib = try_load (NULL, component, component_base_lib); - /* FIXME: do I need to provide a path? */ - MonoDl *lib = NULL; - lib = try_load (components_dir (), component, component_base_lib); - if (!lib) - lib = try_load (NULL, component, component_base_lib); + component_lib = g_new0 (MonoComponentLibrary, 1); + if (component_lib) { + mono_refcount_init (component_lib, NULL); + component_lib->lib = lib; + } + + g_hash_table_insert (component_library_load_history, g_strdup (component_base_lib), (gpointer)component_lib); + } g_free (component_base_lib); - if (!lib) + if (!component_lib || !component_lib->lib) goto done; - gpointer sym = load_component_entrypoint (lib, component); + gpointer sym = load_component_entrypoint (component_lib, component); result = (MonoComponentInitFn)sym; - *lib_out = lib; + + mono_refcount_inc (component_lib); + *component_lib_out = component_lib; done: return result; } MonoComponent* -get_component (const MonoComponentEntry *component, MonoDl **lib_out) +get_component (const MonoComponentEntry *component, MonoComponentLibrary **component_lib_out) { - MonoComponentInitFn initfn = load_component (component, lib_out); + MonoComponentInitFn initfn = load_component (component, component_lib_out); if (!initfn) return NULL; return initfn(); From 032341fb8d091bd6d3b9fff1d2604424c2ae74da Mon Sep 17 00:00:00 2001 From: lateralusX Date: Sun, 4 Jul 2021 13:29:57 +0200 Subject: [PATCH 3/3] Adjust component logging. --- src/mono/mono/metadata/components.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/metadata/components.c b/src/mono/mono/metadata/components.c index 4b5d2d40656679..242580434fd37d 100644 --- a/src/mono/mono/metadata/components.c +++ b/src/mono/mono/metadata/components.c @@ -164,13 +164,14 @@ try_load (const char* dir, const MonoComponentEntry *component, const char* comp char *error_msg = NULL; lib = mono_dl_open (path, MONO_DL_EAGER | MONO_DL_LOCAL, &error_msg); if (!lib) { - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s not found: %s", component->name, error_msg); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component library %s not found at %s: %s", component_base_lib, path, error_msg); g_free (error_msg); + } else { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component library %s found at %s", component_base_lib, path); } g_free (path); } - if (lib) - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s found at %s", component->name, path); + return lib; } @@ -207,9 +208,12 @@ load_component (const MonoComponentEntry *component, MonoComponentLibrary **comp g_hash_table_insert (component_library_load_history, g_strdup (component_base_lib), (gpointer)component_lib); } - g_free (component_base_lib); - if (!component_lib || !component_lib->lib) + if (!component_lib || !component_lib->lib) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s not found", component->name); goto done; + } + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_DLLIMPORT, "Component %s found in %s", component->name, component_base_lib); gpointer sym = load_component_entrypoint (component_lib, component); @@ -218,6 +222,7 @@ load_component (const MonoComponentEntry *component, MonoComponentLibrary **comp mono_refcount_inc (component_lib); *component_lib_out = component_lib; done: + g_free (component_base_lib); return result; }