From 66140b58f8199d722c59c0dae3c323207dcf4109 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 26 Aug 2024 21:01:30 +0200 Subject: [PATCH] Address feedback --- .../pinvoke-override-api-impl.hh | 4 +- src/native/runtime-base/monodroid-dl.hh | 63 ++++++++++--------- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/native/pinvoke-override/pinvoke-override-api-impl.hh b/src/native/pinvoke-override/pinvoke-override-api-impl.hh index e796a7f6ef8..20f57058f17 100644 --- a/src/native/pinvoke-override/pinvoke-override-api-impl.hh +++ b/src/native/pinvoke-override/pinvoke-override-api-impl.hh @@ -20,8 +20,8 @@ namespace xamarin::android { if (lib_handle == nullptr) { // We're being called as part of the p/invoke mechanism, we don't need to look in the AOT cache - constexpr bool USE_AOT_CACHE = false; - lib_handle = internal::MonodroidDl::monodroid_dlopen (library_name, MONO_DL_LOCAL, nullptr, USE_AOT_CACHE); + constexpr bool PREFER_AOT_CACHE = false; + lib_handle = internal::MonodroidDl::monodroid_dlopen (library_name, MONO_DL_LOCAL, nullptr, PREFER_AOT_CACHE); if (lib_handle == nullptr) { log_warn (LOG_ASSEMBLY, "Shared library '%s' not loaded, p/invoke '%s' may fail", library_name, symbol_name); return nullptr; diff --git a/src/native/runtime-base/monodroid-dl.hh b/src/native/runtime-base/monodroid-dl.hh index cc88f926596..00ab2b69138 100644 --- a/src/native/runtime-base/monodroid-dl.hh +++ b/src/native/runtime-base/monodroid-dl.hh @@ -19,6 +19,15 @@ namespace xamarin::android::internal { class MonodroidDl { + enum class CacheKind + { + // Access AOT cache + AOT, + + // Access DSO cache + DSO, + }; + static inline xamarin::android::mutex dso_handle_write_lock; static unsigned int convert_dl_flags (int flags) noexcept @@ -29,18 +38,20 @@ namespace xamarin::android::internal return lflags; } - template + template [[gnu::always_inline, gnu::flatten]] static DSOCacheEntry* find_dso_cache_entry_common (hash_t hash) noexcept { + static_assert (WhichCache == CacheKind::AOT || WhichCache == CacheKind::DSO, "Unknown cache type specified"); + DSOCacheEntry *arr; size_t arr_size; - if constexpr (AotCache) { + if constexpr (WhichCache == CacheKind::AOT) { log_debug (LOG_ASSEMBLY, "Looking for hash 0x%x in AOT cache", hash); arr = aot_dso_cache; arr_size = application_config.number_of_aot_cache_entries; - } else { + } else if constexpr (WhichCache == CacheKind::DSO) { log_debug (LOG_ASSEMBLY, "Looking for hash 0x%x in DSO cache", hash); arr = dso_cache; arr_size = application_config.number_of_dso_cache_entries; @@ -60,32 +71,13 @@ namespace xamarin::android::internal [[gnu::always_inline, gnu::flatten]] static DSOCacheEntry* find_only_aot_cache_entry (hash_t hash) noexcept { - constexpr bool IsAotCache = true; - return find_dso_cache_entry_common (hash); + return find_dso_cache_entry_common (hash); } [[gnu::always_inline, gnu::flatten]] static DSOCacheEntry* find_only_dso_cache_entry (hash_t hash) noexcept { - constexpr bool IsAotCache = false; - return find_dso_cache_entry_common (hash); - } - - [[gnu::always_inline, gnu::flatten]] - static DSOCacheEntry* find_any_dso_cache_entry (hash_t hash) noexcept - { - // If we're asked to look in the AOT DSO cache, do it first. This is because we're likely called from the - // MonoVM's dlopen fallback handler and it will not be a request to resolved a p/invoke, but most likely to - // find and load an AOT image for a managed assembly. Since there might be naming/hash conflicts in this - // scenario, we look at the AOT cache first. - // - // See: https://github.com/dotnet/android/issues/9081 - DSOCacheEntry *ret = find_only_aot_cache_entry (hash); - if (ret != nullptr) { - return ret; - } - - return find_only_dso_cache_entry (hash); + return find_dso_cache_entry_common (hash); } static void* monodroid_dlopen_log_and_return (void *handle, char **err, const char *full_name, bool free_memory) @@ -150,7 +142,7 @@ namespace xamarin::android::internal public: [[gnu::flatten]] - static void* monodroid_dlopen (const char *name, int flags, char **err, bool use_aot_cache) noexcept + static void* monodroid_dlopen (const char *name, int flags, char **err, bool prefer_aot_cache) noexcept { if (name == nullptr) { log_warn (LOG_ASSEMBLY, "monodroid_dlopen got a null name. This is not supported in NET+"); @@ -159,7 +151,22 @@ namespace xamarin::android::internal hash_t name_hash = xxhash::hash (name, strlen (name)); log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash for name '%s' is 0x%zx", name, name_hash); - DSOCacheEntry *dso = use_aot_cache ? find_any_dso_cache_entry (name_hash) : find_only_dso_cache_entry (name_hash); + + DSOCacheEntry *dso = nullptr; + if (prefer_aot_cache) { + // If we're asked to look in the AOT DSO cache, do it first. This is because we're likely called from the + // MonoVM's dlopen fallback handler and it will not be a request to resolved a p/invoke, but most likely to + // find and load an AOT image for a managed assembly. Since there might be naming/hash conflicts in this + // scenario, we look at the AOT cache first. + // + // See: https://github.com/dotnet/android/issues/9081 + dso = find_only_aot_cache_entry (name_hash); + } + + if (dso == nullptr) { + dso = find_only_dso_cache_entry (name_hash); + } + log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash match %sfound, DSO name is '%s'", dso == nullptr ? "not " : "", dso == nullptr ? "" : dso->name); if (dso == nullptr) { @@ -213,8 +220,8 @@ namespace xamarin::android::internal { // We're called by MonoVM via a callback, we might need to return an AOT DSO. // See: https://github.com/dotnet/android/issues/9081 - constexpr bool USE_AOT_CACHE = true; - return monodroid_dlopen (name, flags, err, USE_AOT_CACHE); + constexpr bool PREFER_AOT_CACHE = true; + return monodroid_dlopen (name, flags, err, PREFER_AOT_CACHE); } [[gnu::flatten]]