[monodroid] Use more standard C++ features, performance improvements (#…

…6159)

C++ features:

  * Use C++20 concepts to improve compile-time correctness
    verification
  * Use `std::vector` for dynamic heap buffers
  * Use `std::array` for static stack buffers
  * Use `std::unique_ptr` instead of the home-grown equivalent
    (also for C-allocated memory)
  * Use C++20 structure designators

Optimizations:

  * Add string routines (`ends_with()` and `find_last()`) optimized
    for the stack string types (derived from `string_base`)
  * Count assemblies at build time and pre-allocate enough memory (in
    the form of a `std::vector` instance) to store information about
    the bundled assemblies, thus avoiding 2 allocations per assembly
    (one `realloc()` and one `malloc()`) making the bundled assemblies
    loading algorithm closer to O(1) instead of the previous O(n*2).
  * Use character arrays instead of "plain" string literals to
    optimize some string operations.

These changes improve plain XA test app startup time by around 11%

Additionally, rename `string_segment::append(const char*)` to
`string_segment::append_c(const char*)`.  We [found that][0] the C++
compiler preferred the non-`template` `append(const char*)` overload
over the `template<size_t S> append(const char (&s)[S])` overload,
meaning `append("foo")` would hit the slower `strlen()`-based method.
Likewise create `assign_c(const char*)` and
`starts_with_c(const char*)` member functions.

Finally, [upon review][1] of the exported symbols, we found that many
symbols that we didn't want to export were being exported:

	% $HOME/android-toolchain/ndk/toolchains/llvm/prebuilt/darwin-x86_64/bin/x86_64-linux-android-nm -D -C bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android/lib/x86_64/libmono-android.debug.so
	…
	000000000003e480 T std::terminate()
	0000000000014f80 T operator delete[](void*)
	0000000000014f90 T operator delete[](void*, std::nothrow_t const&)
	0000000000014fa0 T operator delete[](void*, unsigned long)
	0000000000014f50 T operator delete(void*)
	0000000000014f60 T operator delete(void*, std::nothrow_t const&)
	0000000000014f70 T operator delete(void*, unsigned long)
	0000000000014ef0 T operator new[](unsigned long)
	0000000000014f30 T operator new[](unsigned long, std::nothrow_t const&)
	0000000000014e90 T operator new(unsigned long)
	0000000000014ed0 T operator new(unsigned long, std::nothrow_t const&)

Update our declarations of these functions so that they are instead
*weak* symbols, or otherwise no longer exported:

	0000000000011e10 W operator delete[](void*)
	0000000000011dfc W operator delete(void*)
	0000000000011dec W operator new[](unsigned long)
	0000000000011da8 W operator new(unsigned long)

[0]: #6159 (comment)
[1]: #6159 (comment)

src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs

@@ -28,6 +28,8 @@ public class GeneratePackageManagerJava : AndroidTask
 		[Required]
 		public ITaskItem[] ResolvedUserAssemblies { get; set; }
 
+		public ITaskItem[] SatelliteAssemblies { get; set; }
+
 		[Required]
 		public string OutputDirectory { get; set; }
 
@@ -264,6 +266,11 @@ void AddEnvironment ()
 				throw new InvalidOperationException ($"Unsupported BoundExceptionType value '{BoundExceptionType}'");
 			}
 
+			int assemblyCount = ResolvedAssemblies.Length;
+			if (SatelliteAssemblies != null) {
+				assemblyCount += SatelliteAssemblies.Length;
+			}
+
 			bool haveRuntimeConfigBlob = !String.IsNullOrEmpty (RuntimeConfigBinFilePath) && File.Exists (RuntimeConfigBinFilePath);
 			var appConfState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<ApplicationConfigTaskState> (ApplicationConfigTaskState.RegisterTaskObjectKey, RegisteredTaskObjectLifetime.Build);
 			foreach (string abi in SupportedAbis) {
@@ -284,6 +291,7 @@ void AddEnvironment ()
 					InstantRunEnabled = InstantRunEnabled,
 					JniAddNativeMethodRegistrationAttributePresent = appConfState != null ? appConfState.JniAddNativeMethodRegistrationAttributePresent : false,
 					HaveRuntimeConfigBlob = haveRuntimeConfigBlob,
+					NumberOfAssembliesInApk = assemblyCount,
 				};
 
 				using (var sw = MemoryStreamPool.Shared.CreateStreamWriter ()) {

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs

@@ -30,9 +30,10 @@ public sealed class ApplicationConfig
 			public uint   package_naming_policy;
 			public uint   environment_variable_count;
 			public uint   system_property_count;
+			public uint   number_of_assemblies_in_apk;
 			public string android_package_name;
 		};
-		const uint ApplicationConfigFieldCount = 13;
+		const uint ApplicationConfigFieldCount = 14;
 
 		static readonly object ndkInitLock = new object ();
 		static readonly char[] readElfFieldSeparator = new [] { ' ', '\t' };
@@ -176,7 +177,12 @@ static ApplicationConfig ReadApplicationConfig (string envFile)
 						ret.system_property_count = ConvertFieldToUInt32 ("system_property_count", envFile, i, field [1]);
 						break;
 
-					case 12: // android_package_name: string / [pointer type]
+					case 12: // number_of_assemblies_in_apk: uint32_t / .word | .long
+						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile}:{i}': {field [0]}");
+						ret.number_of_assemblies_in_apk = ConvertFieldToUInt32 ("number_of_assemblies_in_apk", envFile, i, field [1]);
+						break;
+
+					case 13: // 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;

src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs

@@ -23,6 +23,7 @@ class ApplicationConfigNativeAssemblyGenerator : NativeAssemblyGenerator
 		public bool InstantRunEnabled { get; set; }
 		public bool JniAddNativeMethodRegistrationAttributePresent { get; set; }
 		public bool HaveRuntimeConfigBlob { get; set; }
+		public int NumberOfAssembliesInApk { get; set; }
 
 		public PackageNamingPolicy PackageNamingPolicy { get; set; }
 
@@ -86,6 +87,9 @@ protected override void WriteSymbols (StreamWriter output)
 				WriteCommentLine (output, "system_property_count");
 				size += WriteData (output, systemProperties == null ? 0 : systemProperties.Count * 2);
 
+				WriteCommentLine (output, "number_of_assemblies_in_apk");
+				size += WriteData (output, NumberOfAssembliesInApk);
+
 				WriteCommentLine (output, "android_package_name");
 				size += WritePointer (output, MakeLocalLabel (stringLabel));
 

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

@@ -1574,6 +1574,7 @@ because xbuild doesn't support framework reference assemblies.
   <GeneratePackageManagerJava
     ResolvedAssemblies="@(_ResolvedAssemblies)"
     ResolvedUserAssemblies="@(_ResolvedUserAssemblies)"
+    SatelliteAssemblies="@(_AndroidResolvedSatellitePaths)"
     MainAssembly="$(TargetPath)"
     OutputDirectory="$(_AndroidIntermediateJavaSourceDirectory)mono"
     EnvironmentOutputDirectory="$(IntermediateOutputPath)android"

src/monodroid/CMakeLists.txt

@@ -492,6 +492,7 @@ if(ANDROID)
       ${BIONIC_SOURCES_DIR}/cxa_guard.cc
       ${SOURCES_DIR}/cxx-abi/string.cc
       ${SOURCES_DIR}/cxx-abi/terminate.cc
+      ${SOURCES_DIR}/cxx-abi/vector.cc
       )
   endif()
 else()

src/monodroid/jni/android-system.cc

@@ -307,9 +307,9 @@ AndroidSystem::monodroid_get_system_property_from_overrides ([[maybe_unused]] co
 #if defined (DEBUG) || !defined (ANDROID)
 	for (size_t oi = 0; oi < MAX_OVERRIDES; ++oi) {
 		if (override_dirs [oi]) {
-			simple_pointer_guard<char[]> override_file (utils.path_combine (override_dirs [oi], name));
+			std::unique_ptr<char[]> override_file {utils.path_combine (override_dirs [oi], name)};
 			log_info (LOG_DEFAULT, "Trying to get property from %s", override_file.get ());
-			size_t result = _monodroid_get_system_property_from_file (override_file, value);
+			size_t result = _monodroid_get_system_property_from_file (override_file.get (), value);
 			if (result == 0 || value == nullptr || (*value) == nullptr || **value == '\0') {
 				continue;
 			}
@@ -351,9 +351,9 @@ AndroidSystem::get_full_dso_path (const char *base_dir, const char *dso_path, dy
 	if (base_dir == nullptr || utils.is_path_rooted (dso_path))
 		return const_cast<char*>(dso_path); // Absolute path or no base path, can't do much with it
 
-	path.assign (base_dir)
+	path.assign_c (base_dir)
 		.append (MONODROID_PATH_SEPARATOR)
-		.append (dso_path);
+		.append_c (dso_path);
 
 	return true;
 }
@@ -567,7 +567,7 @@ AndroidSystem::setup_environment_from_override_file (const char *path)
 	auto     file_size = static_cast<size_t>(sbuf.st_size);
 	size_t   nread = 0;
 	ssize_t  r;
-	simple_pointer_guard<char[]> buf (new char [file_size]);
+	auto     buf = std::make_unique<char[]> (file_size);
 
 	do {
 		auto read_count = static_cast<read_count_type>(file_size - nread);
@@ -603,14 +603,14 @@ AndroidSystem::setup_environment_from_override_file (const char *path)
 	}
 
 	char *endptr;
-	unsigned long name_width = strtoul (buf, &endptr, 16);
-	if ((name_width == std::numeric_limits<unsigned long>::max () && errno == ERANGE) || (*buf != '\0' && *endptr != '\0')) {
+	unsigned long name_width = strtoul (buf.get (), &endptr, 16);
+	if ((name_width == std::numeric_limits<unsigned long>::max () && errno == ERANGE) || (buf[0] != '\0' && *endptr != '\0')) {
 		log_warn (LOG_DEFAULT, "Malformed header of the environment override file %s: name width has invalid format", path);
 		return;
 	}
 
 	unsigned long value_width = strtoul (buf.get () + 11, &endptr, 16);
-	if ((value_width == std::numeric_limits<unsigned long>::max () && errno == ERANGE) || (*buf != '\0' && *endptr != '\0')) {
+	if ((value_width == std::numeric_limits<unsigned long>::max () && errno == ERANGE) || (buf[0] != '\0' && *endptr != '\0')) {
 		log_warn (LOG_DEFAULT, "Malformed header of the environment override file %s: value width has invalid format", path);
 		return;
 	}
@@ -625,7 +625,7 @@ AndroidSystem::setup_environment_from_override_file (const char *path)
 	char *name = buf.get () + OVERRIDE_ENVIRONMENT_FILE_HEADER_SIZE;
 	while (data_size > 0 && data_size >= data_width) {
 		if (*name == '\0') {
-			log_warn (LOG_DEFAULT, "Malformed environment override file %s: name at offset %lu is empty", path, name - buf);
+			log_warn (LOG_DEFAULT, "Malformed environment override file %s: name at offset %lu is empty", path, name - buf.get ());
 			return;
 		}
 
@@ -707,9 +707,9 @@ AndroidSystem::setup_environment ()
 #if defined (DEBUG) || !defined (ANDROID)
 	// TODO: for debug read from file in the override directory named `environment`
 	for (size_t oi = 0; oi < MAX_OVERRIDES; oi++) {
-		simple_pointer_guard<char[]> env_override_file (utils.path_combine (override_dirs [oi], OVERRIDE_ENVIRONMENT_FILE_NAME));
-		if (utils.file_exists (env_override_file)) {
-			setup_environment_from_override_file (env_override_file);
+		std::unique_ptr<char[]> env_override_file {utils.path_combine (override_dirs [oi], OVERRIDE_ENVIRONMENT_FILE_NAME)};
+		if (utils.file_exists (env_override_file.get ())) {
+			setup_environment_from_override_file (env_override_file.get ());
 		}
 	}
 #endif

src/monodroid/jni/application_dso_stub.cc

@@ -35,19 +35,20 @@ CompressedAssemblies compressed_assemblies = {
 };
 
 ApplicationConfig application_config = {
-	/*.uses_mono_llvm =*/ false,
-	/*.uses_mono_aot =*/ false,
-	/*.uses_assembly_preload =*/ false,
-	/*.is_a_bundled_app =*/ false,
-	/*.broken_exception_transitions =*/ false,
-	/*.instant_run_enabled =*/ false,
-	/*.jni_add_native_method_registration_attribute_present =*/ false,
-	/*.have_runtime_config_blob =*/ false,
-	/*.bound_exception_type =*/ 0, // System
-	/*.package_naming_policy =*/ 0,
-	/*.environment_variable_count =*/ 0,
-	/*.system_property_count =*/ 0,
-	/*.android_package_name =*/ "com.xamarin.test",
+	.uses_mono_llvm = false,
+	.uses_mono_aot = false,
+	.uses_assembly_preload = false,
+	.is_a_bundled_app = false,
+	.broken_exception_transitions = false,
+	.instant_run_enabled = false,
+	.jni_add_native_method_registration_attribute_present = false,
+	.have_runtime_config_blob = false,
+	.bound_exception_type = 0, // System
+	.package_naming_policy = 0,
+	.environment_variable_count = 0,
+	.system_property_count = 0,
+	.number_of_assemblies_in_apk = 0,
+	.android_package_name = "com.xamarin.test",
 };
 
 const char* mono_aot_mode_name = "";

src/monodroid/jni/basic-android-system.cc

@@ -16,9 +16,9 @@ void
 BasicAndroidSystem::detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcept
 {
 	// appDirs[2] points to the native library directory
-	simple_pointer_guard<char[]> libmonodroid_path = utils.path_combine (appDirs[2].get_cstr (), "libmonodroid.so");
+	std::unique_ptr<char> libmonodroid_path {utils.path_combine (appDirs[2].get_cstr (), "libmonodroid.so")};
 	log_debug (LOG_ASSEMBLY, "Checking if libmonodroid was unpacked to %s", libmonodroid_path.get ());
-	if (!utils.file_exists (libmonodroid_path)) {
+	if (!utils.file_exists (libmonodroid_path.get ())) {
 		log_debug (LOG_ASSEMBLY, "%s not found, assuming application/android:extractNativeLibs == false", libmonodroid_path.get ());
 		set_embedded_dso_mode_enabled (true);
 	} else {

src/monodroid/jni/basic-utilities.cc

@@ -65,14 +65,14 @@ BasicUtilities::create_directory (const char *pathname, mode_t mode)
 	mode_t oldumask;
 #endif
 	oldumask = umask (022);
-	simple_pointer_guard<char[]> path (strdup_new (pathname));
+	std::unique_ptr<char> path {strdup_new (pathname)};
 	int rv, ret = 0;
-	for (char *d = path; d != nullptr && *d; ++d) {
+	for (char *d = path.get (); d != nullptr && *d; ++d) {
 		if (*d != '/')
 			continue;
 		*d = 0;
 		if (*path) {
-			rv = make_directory (path, mode);
+			rv = make_directory (path.get (), mode);
 			if  (rv == -1 && errno != EEXIST)  {
 				ret = -1;
 				break;

src/monodroid/jni/basic-utilities.hh

@@ -58,17 +58,17 @@ namespace xamarin::android
 			abort_unless (path1 != nullptr || path2 != nullptr, "At least one path must be a valid pointer");
 
 			if (path1 == nullptr) {
-				buf.append (path2);
+				buf.append_c (path2);
 				return;
 			}
 
 			if (path2 == nullptr) {
-				buf.append (path1);
+				buf.append_c (path1);
 				return;
 			}
 
 			buf.append (path1, path1_len);
-			buf.append (MONODROID_PATH_SEPARATOR, MONODROID_PATH_SEPARATOR_LENGTH);
+			buf.append (MONODROID_PATH_SEPARATOR);
 			buf.append (path2, path2_len);
 		}
 
@@ -109,6 +109,35 @@ namespace xamarin::android
 			return p != nullptr && p [N - 1] == '\0';
 		}
 
+		template<size_t N, size_t MaxStackSize, typename TStorage, typename TChar = char>
+		bool ends_with (internal::string_base<MaxStackSize, TStorage, TChar> const& str, const char (&end)[N]) const noexcept
+		{
+			constexpr size_t end_length = N - 1;
+
+			size_t len = str.length ();
+			if (XA_UNLIKELY (len < end_length)) {
+				return false;
+			}
+
+			return memcmp (str.get () + len - end_length, end, end_length) == 0;
+		}
+
+		template<size_t MaxStackSize, typename TStorage, typename TChar = char>
+		const TChar* find_last (internal::string_base<MaxStackSize, TStorage, TChar> const& str, const char ch) const noexcept
+		{
+			if (str.empty ()) {
+				return nullptr;
+			}
+
+			for (size_t i = str.length () - 1; i >= 0; i--) {
+				if (str[i] == ch) {
+					return str.get () + i;
+				}
+			}
+
+			return nullptr;
+		}
+
 		void *xmalloc (size_t size)
 		{
 			return ::xmalloc (size);