From 000cf5a8e5adfd45cac4014b81e8dafec1623c7c Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 19 Nov 2021 15:58:07 +0000 Subject: [PATCH] [One .NET] Don't blindly load Mono components (#6507) Whenever a .NET SDK for Android app starts, MonoVM will probe for a number of Mono components: I monodroid-assembly: Trying to load shared library '/data/app/~~5lwn5C40pkRva2-L4kilZA==/com.microsoft.net6.helloandroid-MrYUTOqX7yZPkmtLOPnuuw==/split_config.arm64_v8a.apk!/lib/arm64-v8a/libmono-component-debugger.so' I monodroid-assembly: Failed to load shared library '/data/app/~~5lwn5C40pkRva2-L4kilZA==/com.microsoft.net6.helloandroid-MrYUTOqX7yZPkmtLOPnuuw==/split_config.arm64_v8a.apk!/lib/arm64-v8a/libmono-component-debugger.so'. dlopen failed: library "/data/app/~~5lwn5C40pkRva2-L4kilZA==/com.microsoft.net6.helloandroid-MrYUTOqX7yZPkmtLOPnuuw==/split_config.arm64_v8a.apk!/lib/arm64-v8a/libmono-component-debugger.so" not found The above is repeated for, currently, 3 components. Together, the failed load attempts cost us around 1ms of startup time on Pixel 3 XL; the time will depend on the speed of device's storage and CPU. Since we know at the build time which components are included, we can optimize the startup process by recording a flag indicating which components can be loaded successfully. Add a `mono_components_mask` field to `ApplicationConfig` which is set to a bitmask indicating which components are packaged. On application startup, whenever `monodroid_dlopen` is called, we has the name of the library passed to us by Mono and see if it matches one of the known hashes for the various components. If yes, we consult the mask stored at build time and attempt to load the component only if its bit is set. The above check is performed **only** during application startup since that's when Mono probes for the components and it would be a waste of time later in the application life. --- .../Tasks/GeneratePackageManagerJava.cs | 16 +++++ .../Xamarin.Android.Build.Tests/BuildTest.cs | 60 +++++++++++++++++++ .../Utilities/EnvironmentHelper.cs | 10 +++- ...pplicationConfigNativeAssemblyGenerator.cs | 14 +++++ .../Xamarin.Android.Common.targets | 2 + src/monodroid/jni/application_dso_stub.cc | 1 + src/monodroid/jni/cpp-util.hh | 46 ++++++++++++++ src/monodroid/jni/embedded-assemblies-zip.cc | 1 + src/monodroid/jni/embedded-assemblies.cc | 2 +- src/monodroid/jni/generate-pinvoke-tables.cc | 4 +- src/monodroid/jni/monodroid-glue-internal.hh | 13 +++- src/monodroid/jni/monodroid-glue.cc | 35 +++++++++++ src/monodroid/jni/startup-aware-lock.hh | 4 +- src/monodroid/jni/xamarin-app.hh | 9 +++ src/monodroid/jni/xxhash.hh | 12 ++++ 15 files changed, 220 insertions(+), 9 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs index 4125581f24f..80c2bad6eeb 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs @@ -28,6 +28,8 @@ public class GeneratePackageManagerJava : AndroidTask [Required] public ITaskItem[] ResolvedUserAssemblies { get; set; } + public ITaskItem[] MonoComponents { get; set; } + public ITaskItem[] SatelliteAssemblies { get; set; } public bool UseAssemblyStore { get; set; } @@ -332,6 +334,19 @@ void AddEnvironment () assemblyNameWidth += abiNameLength + 2; // room for '/' and the terminating NUL } + MonoComponent monoComponents = MonoComponent.None; + if (MonoComponents != null && MonoComponents.Length > 0) { + foreach (ITaskItem item in MonoComponents) { + if (String.Compare ("diagnostics_tracing", item.ItemSpec, StringComparison.OrdinalIgnoreCase) == 0) { + monoComponents |= MonoComponent.Tracing; + } else if (String.Compare ("hot_reload", item.ItemSpec, StringComparison.OrdinalIgnoreCase) == 0) { + monoComponents |= MonoComponent.HotReload; + } else if (String.Compare ("debugger", item.ItemSpec, StringComparison.OrdinalIgnoreCase) == 0) { + monoComponents |= MonoComponent.Debugger; + } + } + } + bool haveRuntimeConfigBlob = !String.IsNullOrEmpty (RuntimeConfigBinFilePath) && File.Exists (RuntimeConfigBinFilePath); var appConfState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (ApplicationConfigTaskState.RegisterTaskObjectKey, RegisteredTaskObjectLifetime.Build); @@ -359,6 +374,7 @@ void AddEnvironment () // and up to 4 other for arch-specific assemblies. Only **one** arch-specific store is ever loaded on the app // runtime, thus the number 2 here. All architecture specific stores contain assemblies with the same names // and in the same order. + MonoComponents = monoComponents, HaveAssemblyStore = UseAssemblyStore, }; diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs index 1c262f38f0b..2dc5b5e103e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs @@ -23,6 +23,66 @@ namespace Xamarin.Android.Build.Tests [Parallelizable (ParallelScope.Children)] public partial class BuildTest : BaseTest { + static object [] MonoComponentMaskChecks () => new object [] { + new object[] { + true, // enableProfiler + true, // useInterpreter + true, // debugBuild + 0x07U, // expectedMask + }, + + new object[] { + true, // enableProfiler + false, // useInterpreter + true, // debugBuild + 0x05U, // expectedMask + }, + + new object[] { + false, // enableProfiler + false, // useInterpreter + true, // debugBuild + 0x01U, // expectedMask + }, + + new object[] { + true, // enableProfiler + false, // useInterpreter + false, // debugBuild + 0x04U, // expectedMask + }, + }; + + [Test] + [TestCaseSource (nameof (MonoComponentMaskChecks))] + public void CheckMonoComponentsMask (bool enableProfiler, bool useInterpreter, bool debugBuild, uint expectedMask) + { + if (!Builder.UseDotNet) { + Assert.Ignore ("Valid only for NET6+ builds"); + return; + } + + var proj = new XamarinAndroidApplicationProject () { + IsRelease = !debugBuild, + }; + + proj.SetProperty (proj.ActiveConfigurationProperties, "AndroidEnableProfiler", enableProfiler.ToString ()); + proj.SetProperty (proj.ActiveConfigurationProperties, "AndroidUseInterpreter", useInterpreter.ToString ()); + + var abis = new [] { "armeabi-v7a", "x86" }; + proj.SetAndroidSupportedAbis (abis); + + using (var b = CreateApkBuilder ()) { + Assert.IsTrue (b.Build (proj), "Build should have succeeded."); + string objPath = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath); + + List envFiles = EnvironmentHelper.GatherEnvironmentFiles (objPath, String.Join (";", abis), true); + EnvironmentHelper.ApplicationConfig app_config = EnvironmentHelper.ReadApplicationConfig (envFiles); + Assert.That (app_config, Is.Not.Null, "application_config must be present in the environment files"); + Assert.IsTrue (app_config.mono_components_mask == expectedMask, "Expected Mono Components mask 0x{expectedMask:x}, got 0x{app_config.mono_components_mask:x}"); + } + } + [Test] [Category ("SmokeTests")] public void SmokeTestBuildWithSpecialCharacters ([Values (false, true)] bool forms) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs index 5451c7d99e5..de155c445a9 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs @@ -34,9 +34,10 @@ public sealed class ApplicationConfig public uint number_of_assemblies_in_apk; public uint bundled_assembly_name_width; public uint number_of_assembly_blobs; + public uint mono_components_mask; public string android_package_name; }; - const uint ApplicationConfigFieldCount = 17; + const uint ApplicationConfigFieldCount = 18; static readonly object ndkInitLock = new object (); static readonly char[] readElfFieldSeparator = new [] { ' ', '\t' }; @@ -200,7 +201,12 @@ static ApplicationConfig ReadApplicationConfig (string envFile) ret.number_of_assembly_blobs = ConvertFieldToUInt32 ("number_of_assembly_blobs", envFile, i, field [1]); break; - case 16: // android_package_name: string / [pointer type] + case 16: // mono_components_mask: uint32_t / .word | .long + Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile}:{i}': {field [0]}"); + ret.mono_components_mask = ConvertFieldToUInt32 ("mono_components_mask", envFile, i, field [1]); + break; + + case 17: // android_package_name: string / [pointer type] Assert.IsTrue (expectedPointerTypes.Contains (field [0]), $"Unexpected pointer field type in '{envFile}:{i}': {field [0]}"); pointers.Add (field [1].Trim ()); break; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs index 4d3a2806c05..838ed0e9f1a 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs @@ -6,6 +6,16 @@ namespace Xamarin.Android.Tasks { + // Must match the MonoComponent enum in src/monodroid/jni/xamarin-app.hh + [Flags] + enum MonoComponent + { + None = 0x00, + Debugger = 0x01, + HotReload = 0x02, + Tracing = 0x04, + } + class ApplicationConfigNativeAssemblyGenerator : NativeAssemblyGenerator { SortedDictionary environmentVariables; @@ -28,6 +38,7 @@ class ApplicationConfigNativeAssemblyGenerator : NativeAssemblyGenerator public int NumberOfAssembliesInApk { get; set; } public int NumberOfAssemblyStoresInApks { get; set; } public int BundledAssemblyNameWidth { get; set; } // including the trailing NUL + public MonoComponent MonoComponents { get; set; } public PackageNamingPolicy PackageNamingPolicy { get; set; } @@ -103,6 +114,9 @@ protected override void WriteSymbols (StreamWriter output) WriteCommentLine (output, "number_of_assembly_store_files"); size += WriteData (output, NumberOfAssemblyStoresInApks); + WriteCommentLine (output, "mono_components_mask"); + size += WriteData (output, (uint)MonoComponents); + WriteCommentLine (output, "android_package_name"); size += WritePointer (output, MakeLocalLabel (stringLabel)); diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 3e77bb44a3e..b3237f9eea9 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1566,6 +1566,7 @@ because xbuild doesn't support framework reference assemblies. _PrepareAssemblies; _PrepareEnvironmentAssemblySources; _GenerateEnvironmentFiles; + _IncludeNativeSystemLibraries; @@ -1578,6 +1579,7 @@ because xbuild doesn't support framework reference assemblies. ResolvedAssemblies="@(_ResolvedAssemblies)" ResolvedUserAssemblies="@(_ResolvedUserAssemblies)" SatelliteAssemblies="@(_AndroidResolvedSatellitePaths)" + MonoComponents="@(_MonoComponent)" MainAssembly="$(TargetPath)" OutputDirectory="$(_AndroidIntermediateJavaSourceDirectory)mono" EnvironmentOutputDirectory="$(IntermediateOutputPath)android" diff --git a/src/monodroid/jni/application_dso_stub.cc b/src/monodroid/jni/application_dso_stub.cc index 51c6e1eecea..c8bbec84c16 100644 --- a/src/monodroid/jni/application_dso_stub.cc +++ b/src/monodroid/jni/application_dso_stub.cc @@ -55,6 +55,7 @@ ApplicationConfig application_config = { .number_of_assemblies_in_apk = 2, .bundled_assembly_name_width = 0, .number_of_assembly_store_files = 2, + .mono_components_mask = MonoComponent::None, .android_package_name = "com.xamarin.test", }; diff --git a/src/monodroid/jni/cpp-util.hh b/src/monodroid/jni/cpp-util.hh index 443c57c139c..388dd58d38c 100644 --- a/src/monodroid/jni/cpp-util.hh +++ b/src/monodroid/jni/cpp-util.hh @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -123,5 +124,50 @@ namespace xamarin::android return ret; }; + template , int> = 0> + constexpr TEnum operator & (TEnum l, TEnum r) noexcept + { + using etype = std::underlying_type_t; + return static_cast(static_cast(l) & static_cast(r)); + } + + template , int> = 0> + constexpr TEnum& operator &= (TEnum& l, TEnum r) noexcept + { + return l = (l & r); + } + + template , int> = 0> + constexpr TEnum operator | (TEnum l, TEnum r) noexcept + { + using etype = std::underlying_type_t; + return static_cast(static_cast(l) | static_cast(r)); + } + + template , int> = 0> + constexpr TEnum& operator |= (TEnum& l, TEnum r) noexcept + { + return l = (l | r); + } + + template , int> = 0> + constexpr TEnum operator ~ (TEnum r) noexcept + { + using etype = std::underlying_type_t; + return static_cast (~static_cast(r)); + } + + template , int> = 0> + constexpr TEnum operator ^ (TEnum l, TEnum r) noexcept + { + using etype = std::underlying_type_t; + return static_cast(static_cast(l) ^ static_cast(r)); + } + + template , int> = 0> + constexpr TEnum& operator ^= (TEnum& l, TEnum r) noexcept + { + return l = (l ^ r); + } } #endif // !def __CPP_UTIL_HH diff --git a/src/monodroid/jni/embedded-assemblies-zip.cc b/src/monodroid/jni/embedded-assemblies-zip.cc index b0f581b5b3d..f404910ac8d 100644 --- a/src/monodroid/jni/embedded-assemblies-zip.cc +++ b/src/monodroid/jni/embedded-assemblies-zip.cc @@ -297,6 +297,7 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, [[maybe_unus .prefix = get_assemblies_prefix (), .prefix_len = get_assemblies_prefix_length (), .buf_offset = 0, + .compression_method = 0, .local_header_offset = 0, .data_offset = 0, .file_size = 0, diff --git a/src/monodroid/jni/embedded-assemblies.cc b/src/monodroid/jni/embedded-assemblies.cc index c8099b2256a..e438b61eec8 100644 --- a/src/monodroid/jni/embedded-assemblies.cc +++ b/src/monodroid/jni/embedded-assemblies.cc @@ -189,7 +189,7 @@ force_inline void EmbeddedAssemblies::map_runtime_file (XamarinAndroidBundledAssembly& file) noexcept { md_mmap_info map_info = md_mmap_apk_file (file.apk_fd, file.data_offset, file.data_size, file.name); - if (monodroidRuntime.is_startup_in_progress ()) { + if (MonodroidRuntime::is_startup_in_progress ()) { file.data = static_cast(map_info.area); } else { uint8_t *expected_null = nullptr; diff --git a/src/monodroid/jni/generate-pinvoke-tables.cc b/src/monodroid/jni/generate-pinvoke-tables.cc index 6441195c575..e0340a56540 100644 --- a/src/monodroid/jni/generate-pinvoke-tables.cc +++ b/src/monodroid/jni/generate-pinvoke-tables.cc @@ -824,14 +824,14 @@ int main (int argc, char **argv) print (output, "64-bit internal p/invoke table", "internal_pinvokes", internal_pinvokes64); print (output, "64-bit DotNet p/invoke table", "dotnet_pinvokes", dotnet_pinvokes64); output << std::endl; - write_library_name_hashes (xxhash64::hash, output); + write_library_name_hashes (xxhash64::hash, output); output << "#else" << std::endl; print (output, "32-bit internal p/invoke table", "internal_pinvokes", internal_pinvokes32); print (output, "32-bit DotNet p/invoke table", "dotnet_pinvokes", dotnet_pinvokes32); output << std::endl; - write_library_name_hashes (xxhash32::hash, output); + write_library_name_hashes (xxhash32::hash, output); output << "#endif" << std::endl << std::endl; diff --git a/src/monodroid/jni/monodroid-glue-internal.hh b/src/monodroid/jni/monodroid-glue-internal.hh index 94cd0a5a45b..efcfb865505 100644 --- a/src/monodroid/jni/monodroid-glue-internal.hh +++ b/src/monodroid/jni/monodroid-glue-internal.hh @@ -136,6 +136,15 @@ namespace xamarin::android::internal true; #endif + static constexpr char mono_component_debugger_name[] = "libmono-component-debugger.so"; + static constexpr hash_t mono_component_debugger_hash = xxhash::hash (mono_component_debugger_name); + + static constexpr char mono_component_hot_reload_name[] = "libmono-component-hot_reload.so"; + static constexpr hash_t mono_component_hot_reload_hash = xxhash::hash (mono_component_hot_reload_name); + + static constexpr char mono_component_diagnostics_tracing_name[] = "libmono-component-diagnostics_tracing.so"; + static constexpr hash_t mono_component_diagnostics_tracing_hash = xxhash::hash (mono_component_diagnostics_tracing_name); + #if !defined (NET6) #define MAKE_API_DSO_NAME(_ext_) "libxa-internal-api." # _ext_ #if defined (WINDOWS) @@ -164,7 +173,7 @@ namespace xamarin::android::internal #endif jint Java_JNI_OnLoad (JavaVM *vm, void *reserved); - bool is_startup_in_progress () const noexcept + static bool is_startup_in_progress () noexcept { return startup_in_progress; } @@ -360,7 +369,7 @@ namespace xamarin::android::internal * able to switch our different contexts from different threads. */ int current_context_id = -1; - bool startup_in_progress = true; + static bool startup_in_progress; #if defined (NET6) MonoAssemblyLoadContextGCHandle default_alc = nullptr; diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index 2ddbe12f54b..dc38432521d 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -117,6 +117,8 @@ MonoCoreRuntimeProperties MonodroidRuntime::monovm_core_properties = { #endif // def NET6 +bool MonodroidRuntime::startup_in_progress = true; + #ifdef WINDOWS static const char* get_xamarin_android_msbuild_path (void); const char *BasicAndroidSystem::SYSTEM_LIB_PATH = get_xamarin_android_msbuild_path(); @@ -1302,6 +1304,39 @@ MonodroidRuntime::monodroid_dlopen_log_and_return (void *handle, char **err, con force_inline void* MonodroidRuntime::monodroid_dlopen (const char *name, int flags, char **err) { + if (startup_in_progress) { + hash_t name_hash = xxhash::hash (name, strlen (name)); + + auto ignore_component = [&](const char *label, MonoComponent component) -> bool { + if ((application_config.mono_components_mask & component) != component) { + log_info (LOG_ASSEMBLY, "Mono '%s' component requested but not packaged, ignoring", label); + return true; + } + + return false; + }; + + switch (name_hash) { + case mono_component_debugger_hash: + if (ignore_component ("Debugger", MonoComponent::Debugger)) { + return nullptr; + } + break; + + case mono_component_hot_reload_hash: + if (ignore_component ("Hot Reload", MonoComponent::HotReload)) { + return nullptr; + } + break; + + case mono_component_diagnostics_tracing_hash: + if (ignore_component ("Diagnostics Tracing", MonoComponent::Tracing)) { + return nullptr; + } + break; + } + } + unsigned int dl_flags = monodroidRuntime.convert_dl_flags (flags); void *h = androidSystem.load_dso_from_any_directories (name, dl_flags); if (h != nullptr) { diff --git a/src/monodroid/jni/startup-aware-lock.hh b/src/monodroid/jni/startup-aware-lock.hh index 1dbde47e210..9800370bcec 100644 --- a/src/monodroid/jni/startup-aware-lock.hh +++ b/src/monodroid/jni/startup-aware-lock.hh @@ -11,7 +11,7 @@ namespace xamarin::android::internal explicit StartupAwareLock (std::mutex &m) : lock (m) { - if (monodroidRuntime.is_startup_in_progress ()) { + if (MonodroidRuntime::is_startup_in_progress ()) { // During startup we run without threads, do nothing return; } @@ -21,7 +21,7 @@ namespace xamarin::android::internal ~StartupAwareLock () { - if (monodroidRuntime.is_startup_in_progress ()) { + if (MonodroidRuntime::is_startup_in_progress ()) { return; } diff --git a/src/monodroid/jni/xamarin-app.hh b/src/monodroid/jni/xamarin-app.hh index 41a5aee422d..8f68c551f59 100644 --- a/src/monodroid/jni/xamarin-app.hh +++ b/src/monodroid/jni/xamarin-app.hh @@ -189,6 +189,14 @@ struct AssemblyStoreSingleAssemblyRuntimeData final AssemblyStoreAssemblyDescriptor *descriptor; }; +enum class MonoComponent : uint32_t +{ + None = 0x00, + Debugger = 0x01, + HotReload = 0x02, + Tracing = 0x04, +}; + struct ApplicationConfig { bool uses_mono_llvm; @@ -207,6 +215,7 @@ struct ApplicationConfig uint32_t number_of_assemblies_in_apk; uint32_t bundled_assembly_name_width; uint32_t number_of_assembly_store_files; + MonoComponent mono_components_mask; const char *android_package_name; }; diff --git a/src/monodroid/jni/xxhash.hh b/src/monodroid/jni/xxhash.hh index aa01cfe4b45..4907e257ef4 100644 --- a/src/monodroid/jni/xxhash.hh +++ b/src/monodroid/jni/xxhash.hh @@ -58,6 +58,12 @@ namespace xamarin::android ); } + template + force_inline static constexpr uint32_t hash (const char (&input)[Size]) noexcept + { + return hash (input, Size - 1); + } + private: // 32-bit rotate left. template @@ -146,6 +152,12 @@ namespace xamarin::android return finalize ((len >= 32 ? h32bytes (p, len) : Seed + PRIME5) + len, p + (len & ~0x1FU), len & 0x1F); } + template + force_inline static constexpr uint64_t hash (const char (&input)[Size]) noexcept + { + return hash (input, Size - 1); + } + private: template force_inline static constexpr uint64_t rotl (uint64_t x) noexcept