[Mono] Optimize mono_dl_build_path for component and Android module loading. (#54971)

* 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.

Author: lateralusX

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);
@@ -150,59 +160,76 @@ 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);
+			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 (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;
 }
 
+
+
 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_free (component_base_lib);
-	if (!lib)
+		g_hash_table_insert (component_library_load_history, g_strdup (component_base_lib), (gpointer)component_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 (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:
+	g_free (component_base_lib);
 	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();

src/mono/mono/metadata/native-library.c

@@ -493,20 +493,18 @@ 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);
 			g_free (error_msg);
 		}
 		g_free (full_name);
 	}
-	g_free (full_name);
 
 	return module;
 }

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)
 {

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);