[CoreCLR] Remove robin-map from p/invoke override

.gitignore

@@ -30,8 +30,6 @@ Novell
 *.keystore
 /omnisharp.json
 src/native/CMakePresets.json
-src/native/clr/pinvoke-override/generate-pinvoke-tables
-src/native/clr/pinvoke-override/pinvoke-tables.include.generated
 src/native/mono/pinvoke-override/generate-pinvoke-tables
 src/native/mono/pinvoke-override/pinvoke-tables.include.generated
 dotnet-install.ps1

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc

@@ -5,34 +5,34 @@
       "Size": 3036
     },
     "classes.dex": {
-      "Size": 402352
+      "Size": 402852
     },
     "lib/arm64-v8a/libassembly-store.so": {
-      "Size": 2717688
+      "Size": 2724872
     },
     "lib/arm64-v8a/libclrjit.so": {
-      "Size": 2835128
+      "Size": 2757816
     },
     "lib/arm64-v8a/libcoreclr.so": {
-      "Size": 4891056
+      "Size": 4837240
     },
     "lib/arm64-v8a/libmonodroid.so": {
-      "Size": 1324320
+      "Size": 1246136
     },
     "lib/arm64-v8a/libSystem.Globalization.Native.so": {
       "Size": 72112
     },
     "lib/arm64-v8a/libSystem.IO.Compression.Native.so": {
-      "Size": 1262888
+      "Size": 1258776
     },
     "lib/arm64-v8a/libSystem.Native.so": {
-      "Size": 101888
+      "Size": 99664
     },
     "lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": {
-      "Size": 168080
+      "Size": 163936
     },
     "lib/arm64-v8a/libxamarin-app.so": {
-      "Size": 20968
+      "Size": 21448
     },
     "res/drawable-hdpi-v4/icon.png": {
       "Size": 2178
@@ -59,5 +59,5 @@
       "Size": 1904
     }
   },
-  "PackageSize": 7271867
+  "PackageSize": 7239099
 }

src/native/CMakeLists.txt

@@ -225,7 +225,10 @@ file(REAL_PATH "../../" REPO_ROOT_DIR)
 set(EXTERNAL_DIR "${REPO_ROOT_DIR}/external")
 set(JAVA_INTEROP_SRC_PATH "${EXTERNAL_DIR}/Java.Interop/src/java-interop")
 set(LIBUNWIND_SOURCE_DIR "${EXTERNAL_DIR}/libunwind")
-set(ROBIN_MAP_DIR "${EXTERNAL_DIR}/robin-map")
+
+if(IS_MONO_RUNTIME)
+  set(ROBIN_MAP_DIR "${EXTERNAL_DIR}/robin-map")
+endif()
 
 #
 # Include directories

src/native/clr/host/CMakeLists.txt

@@ -119,7 +119,6 @@ macro(lib_target_options TARGET_NAME)
     PRIVATE
     ${CMAKE_CURRENT_BINARY_DIR}/include
     ${EXTERNAL_DIR}
-    ${ROBIN_MAP_DIR}/include
   )
 
   target_include_directories(

src/native/clr/include/host/pinvoke-override-impl.hh

@@ -4,154 +4,50 @@
 #error The PINVOKE_OVERRIDE_INLINE macro must be defined before including this header file
 #endif
 
-#include <mutex>
-#include <string>
-
 #include "pinvoke-override.hh"
 #include "../runtime-base/logger.hh"
 #include "../runtime-base/monodroid-dl.hh"
-#include "../runtime-base/startup-aware-lock.hh"
 
 namespace xamarin::android {
 	PINVOKE_OVERRIDE_INLINE
-	auto PinvokeOverride::load_library_symbol (std::string_view const& library_name, std::string_view const& symbol_name, void **dso_handle) noexcept -> void*
+	auto PinvokeOverride::load_library_symbol (std::string_view const& library_name, std::string_view const& symbol_name) noexcept -> void*
 	{
-		void *lib_handle = dso_handle == nullptr ? nullptr : *dso_handle;
-
-		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 PREFER_AOT_CACHE = false;
+		void *lib_handle = nullptr;
 
-			// Handle p/invokes of the form [DllImport ("liblog")] or [DllImport ("log")]
-			// TODO: try modifying the name to contain both the `log` prefix and the `.so` suffix
-			dynamic_local_path_string short_library_name;
-			if (!Util::path_has_directory_components (library_name)) {
-				if (!library_name.starts_with (Constants::DSO_PREFIX)) {
-					short_library_name.append (Constants::DSO_PREFIX);
-				}
-				short_library_name.append (library_name);
-				if (!short_library_name.ends_with (Constants::dso_suffix)) {
-					short_library_name.append (Constants::dso_suffix);
-				}
+		// We're being called as part of the p/invoke mechanism, so skip AOT DSO lookup.
+		constexpr bool PREFER_AOT_CACHE = false;
 
-				log_debug (LOG_ASSEMBLY, "Modified p/invoke library name to '{}'", short_library_name.get ());
-				lib_handle = MonodroidDl::monodroid_dlopen<PREFER_AOT_CACHE> (short_library_name.get (), microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY);
+		// Handle p/invokes of the form [DllImport ("liblog")] or [DllImport ("log")]
+		// TODO: try modifying the name to contain both the `log` prefix and the `.so` suffix
+		dynamic_local_path_string short_library_name;
+		if (!Util::path_has_directory_components (library_name)) {
+			if (!library_name.starts_with (Constants::DSO_PREFIX)) {
+				short_library_name.append (Constants::DSO_PREFIX);
 			}
-
-			if (lib_handle == nullptr) {
-				lib_handle = MonodroidDl::monodroid_dlopen<PREFER_AOT_CACHE> (library_name, microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY);
+			short_library_name.append (library_name);
+			if (!short_library_name.ends_with (Constants::dso_suffix)) {
+				short_library_name.append (Constants::dso_suffix);
 			}
 
-			if (lib_handle == nullptr) {
-				log_warn (LOG_ASSEMBLY, "Shared library '{}' not loaded, p/invoke '{}' may fail", library_name, symbol_name);
-				return nullptr;
-			}
-
-			if (dso_handle != nullptr) {
-				void *expected_null = nullptr;
-				if (!__atomic_compare_exchange (dso_handle, &expected_null, &lib_handle, false /* weak */, __ATOMIC_ACQUIRE /* success_memorder */, __ATOMIC_RELAXED /* xxxfailure_memorder */)) {
-					log_debug (LOG_ASSEMBLY, "Library '{}' handle already cached by another thread", library_name);
-				}
-			}
+			log_debug (LOG_ASSEMBLY, "Modified p/invoke library name to '{}'", short_library_name.get ());
+			lib_handle = MonodroidDl::monodroid_dlopen<PREFER_AOT_CACHE> (short_library_name.get (), microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY);
 		}
 
-		void *entry_handle = MonodroidDl::monodroid_dlsym (lib_handle, symbol_name);
-		if (entry_handle == nullptr) {
-			log_warn (LOG_ASSEMBLY, "Symbol '{}' not found in shared library '{}', p/invoke may fail", symbol_name, library_name);
-			return nullptr;
+		if (lib_handle == nullptr) {
+			lib_handle = MonodroidDl::monodroid_dlopen<PREFER_AOT_CACHE> (library_name, microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY);
 		}
 
-		return entry_handle;
-	}
-
-	PINVOKE_OVERRIDE_INLINE
-	auto PinvokeOverride::load_library_entry (std::string const& library_name, std::string const& entrypoint_name, pinvoke_api_map_ptr api_map) noexcept -> void*
-	{
-		// Make sure some other thread hasn't just added the entry
-		auto iter = api_map->find (entrypoint_name);
-		if (iter != api_map->end () && iter->second != nullptr) {
-			return iter->second;
+		if (lib_handle == nullptr) {
+			log_warn (LOG_ASSEMBLY, "Shared library '{}' not loaded, p/invoke '{}' may fail", library_name, symbol_name);
+			return nullptr;
 		}
 
-		void *entry_handle = load_library_symbol (library_name.c_str (), entrypoint_name.c_str ());
+		void *entry_handle = MonodroidDl::monodroid_dlsym (lib_handle, symbol_name);
 		if (entry_handle == nullptr) {
-			// error already logged
+			log_warn (LOG_ASSEMBLY, "Symbol '{}' not found in shared library '{}', p/invoke may fail", symbol_name, library_name);
 			return nullptr;
 		}
 
-		log_debug (LOG_ASSEMBLY, "Caching p/invoke entry {} @ {}", library_name, entrypoint_name);
-		(*api_map)[entrypoint_name] = entry_handle;
 		return entry_handle;
 	}
-
-	PINVOKE_OVERRIDE_INLINE
-	auto PinvokeOverride::fetch_or_create_pinvoke_map_entry (std::string const& library_name, std::string const& entrypoint_name, hash_t entrypoint_name_hash, pinvoke_api_map_ptr api_map, bool need_lock) noexcept -> void*
-	{
-		auto iter = api_map->find (entrypoint_name, entrypoint_name_hash);
-		if (iter != api_map->end () && iter->second != nullptr) {
-			return iter->second;
-		}
-
-		if (!need_lock) {
-			return load_library_entry (library_name, entrypoint_name, api_map);
-		}
-
-		StartupAwareLock lock (pinvoke_map_write_lock);
-		return load_library_entry (library_name, entrypoint_name, api_map);
-	}
-
-	PINVOKE_OVERRIDE_INLINE
-	auto PinvokeOverride::find_pinvoke_address (hash_t hash, const PinvokeEntry *entries, size_t entry_count) noexcept -> PinvokeEntry*
-	{
-		while (entry_count > 0uz) {
-			const size_t mid = entry_count / 2uz;
-			const PinvokeEntry *const ret = entries + mid;
-			const std::strong_ordering result = hash <=> ret->hash;
-
-			if (result < 0) {
-				entry_count = mid;
-			} else if (result > 0) {
-				entries = ret + 1;
-				entry_count -= mid + 1uz;
-			} else {
-				return const_cast<PinvokeEntry*>(ret);
-			}
-		}
-
-		return nullptr;
-	}
-
-	PINVOKE_OVERRIDE_INLINE
-	auto PinvokeOverride::handle_other_pinvoke_request (std::string_view const& library_name, hash_t library_name_hash, std::string_view const& entrypoint_name, hash_t entrypoint_name_hash) noexcept -> void*
-	{
-		std::string lib_name {library_name};
-		std::string entry_name {entrypoint_name};
-
-		auto iter = other_pinvoke_map.find (lib_name, library_name_hash);
-		void *handle = nullptr;
-		if (iter == other_pinvoke_map.end ()) {
-			StartupAwareLock lock (pinvoke_map_write_lock);
-
-			pinvoke_api_map_ptr lib_map;
-			// Make sure some other thread hasn't just added the map
-			iter = other_pinvoke_map.find (lib_name, library_name_hash);
-			if (iter == other_pinvoke_map.end () || iter->second == nullptr) {
-				lib_map = new pinvoke_api_map (1);
-				other_pinvoke_map[lib_name] = lib_map;
-			} else {
-				lib_map = iter->second;
-			}
-
-			handle = fetch_or_create_pinvoke_map_entry (lib_name, entry_name, entrypoint_name_hash, lib_map, /* need_lock */ false);
-		} else {
-			if (iter->second == nullptr) [[unlikely]] {
-				log_warn (LOG_ASSEMBLY, "Internal error: null entry in p/invoke map for key '{}'", library_name);
-				return nullptr; // fall back to `monodroid_dlopen`
-			}
-
-			handle = fetch_or_create_pinvoke_map_entry (lib_name, entry_name, entrypoint_name_hash, iter->second, /* need_lock */ true);
-		}
-
-		return handle;
-	}
 }

src/native/clr/include/host/pinvoke-override.hh

@@ -1,89 +1,16 @@
 #pragma once
 
-#include <mutex>
-#include <string>
 #include <string_view>
 
 #include <jni.h>
 
-// NDEBUG causes robin_map.h not to include <iostream> which, in turn, prevents indirect inclusion of <mutex>. <mutex>
-// conflicts with our std::mutex definition in cppcompat.hh
-#if !defined (NDEBUG)
-#define NDEBUG
-#define NDEBUG_UNDEFINE
-#endif
-
-// hush some compiler warnings
-#if defined (__clang__)
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wunused-parameter"
-#endif // __clang__
-
-#include <tsl/robin_map.h>
-
-#if defined (__clang__)
-#pragma clang diagnostic pop
-#endif // __clang__
-
-#if defined (NDEBUG_UNDEFINE)
-#undef NDEBUG
-#undef NDEBUG_UNDEFINE
-#endif
-
 #include "../runtime-base/monodroid-dl.hh"
-#include <shared/xxhash.hh>
 
 namespace xamarin::android {
-	struct PinvokeEntry
-	{
-		hash_t      hash;
-		const char *name;
-		void       *func;
-	};
-
-	struct string_hash
-	{
-		[[gnu::always_inline]]
-		xamarin::android::hash_t operator() (std::string const& s) const noexcept
-		{
-			return xamarin::android::xxhash::hash (s.c_str (), s.length ());
-		}
-	};
-
 	class PinvokeOverride
 	{
-		using pinvoke_api_map = tsl::robin_map<
-			std::string,
-			void*,
-			string_hash,
-			std::equal_to<std::string>,
-			std::allocator<std::pair<std::string, void*>>,
-			true
-		>;
-
-		using pinvoke_api_map_ptr = pinvoke_api_map*;
-		using pinvoke_library_map = tsl::robin_map<
-			std::string,
-			pinvoke_api_map_ptr,
-			string_hash,
-			std::equal_to<std::string>,
-			std::allocator<std::pair<std::string, pinvoke_api_map_ptr>>,
-			true
-		>;
-
-		static inline constexpr pinvoke_library_map::size_type LIBRARY_MAP_INITIAL_BUCKET_COUNT = 1uz;
-
 	public:
-		static auto load_library_symbol (std::string_view const& library_name, std::string_view const& symbol_name, void **dso_handle = nullptr) noexcept -> void*;
-		static auto load_library_entry (std::string const& library_name, std::string const& entrypoint_name, pinvoke_api_map_ptr api_map) noexcept -> void*;
-		static auto fetch_or_create_pinvoke_map_entry (std::string const& library_name, std::string const& entrypoint_name, hash_t entrypoint_name_hash, pinvoke_api_map_ptr api_map, bool need_lock) noexcept -> void*;
-		static auto find_pinvoke_address (hash_t hash, const PinvokeEntry *entries, size_t entry_count) noexcept -> PinvokeEntry*;
-		static auto handle_other_pinvoke_request (std::string_view const& library_name, hash_t library_name_hash, std::string_view const& entrypoint_name, hash_t entrypoint_name_hash) noexcept -> void*;
-		static void handle_jni_on_load (JavaVM *vm, void *reserved) noexcept;
+		static auto load_library_symbol (std::string_view const& library_name, std::string_view const& symbol_name) noexcept -> void*;
 		static auto monodroid_pinvoke_override (const char *library_name, const char *entrypoint_name) noexcept -> void*;
-
-	private:
-		static inline std::mutex          pinvoke_map_write_lock{};
-		static inline pinvoke_library_map other_pinvoke_map { PinvokeOverride::LIBRARY_MAP_INITIAL_BUCKET_COUNT };
 	};
 }

src/native/clr/pinvoke-override/CMakeLists.txt

@@ -32,25 +32,13 @@ macro(create_library _libname _alias _sources)
 
   set_static_library_suffix(${_libname})
 
-  target_compile_definitions(
-    ${_libname}
-    PRIVATE
-    TSL_NO_EXCEPTIONS
-  )
-
   target_compile_options(
     ${_libname}
     PRIVATE
     ${XA_COMMON_CXX_ARGS}
     ${PINVOKE_OVERRIDE_CXX_ARGS}
   )
 
-  target_include_directories(
-    ${_libname}
-    PRIVATE
-    ${ROBIN_MAP_DIR}/include
-  )
-
   target_include_directories(
     ${_libname}
     PUBLIC