From 7280c9b6d487b876f80855a8ccbe498becb650f9 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 19 Oct 2018 09:19:23 +0200 Subject: [PATCH 1/2] Refactor type map to have a separate flag field for each type instead of magic location in the array. Refactor the type map to have a separate flag field for each type instead of a magic location in the array. This simplifies some code, but also allows us to introduce more flags in the future (which becomes increasingly complex if the location in the array is to determine yet another value). This has neglible performance impacts. --- runtime/runtime.m | 3 +-- runtime/xamarin/runtime.h | 7 ++++++- src/ObjCRuntime/Class.cs | 11 ++++------ src/ObjCRuntime/Runtime.cs | 9 +++++++- tools/common/StaticRegistrar.cs | 37 ++++++++++++--------------------- 5 files changed, 32 insertions(+), 35 deletions(-) diff --git a/runtime/runtime.m b/runtime/runtime.m index 2db8a577526c..c838994cc378 100644 --- a/runtime/runtime.m +++ b/runtime/runtime.m @@ -1020,8 +1020,7 @@ -(void) xamarinSetGCHandle: (int) gc_handle; options.RegistrationData = map; // Sort the type map according to Class - qsort (map->map, map->map_count - map->custom_type_count, sizeof (MTClassMap), compare_mtclassmap); - qsort (&map->map [map->map_count - map->custom_type_count], map->custom_type_count, sizeof (MTClassMap), compare_mtclassmap); + qsort (map->map, map->map_count, sizeof (MTClassMap), compare_mtclassmap); } /* diff --git a/runtime/xamarin/runtime.h b/runtime/xamarin/runtime.h index df5e85ca8579..277ff3fb884b 100644 --- a/runtime/xamarin/runtime.h +++ b/runtime/xamarin/runtime.h @@ -68,9 +68,15 @@ typedef struct __attribute__((packed)) { } MTTokenReference; static const uint32_t INVALID_TOKEN_REF = 0xFFFFFFFF; +enum MTTypeFlags { + MTTypeFlagsNone = 0, + MTTypeFlagsCustomType = 1, // not a platform type +}; + typedef struct __attribute__((packed)) { void *handle; uint32_t /* MTTokenReference */ type_reference; + uint32_t /* MTTypeFlags */ flags; } MTClassMap; typedef struct __attribute__((packed)) { @@ -109,7 +115,6 @@ struct MTRegistrationMap { const MTProtocolMap protocols; int assembly_count; int map_count; - int custom_type_count; int full_token_reference_count; int skipped_map_count; int protocol_wrapper_count; diff --git a/src/ObjCRuntime/Class.cs b/src/ObjCRuntime/Class.cs index d348b57f167e..62dec18e7e0d 100644 --- a/src/ObjCRuntime/Class.cs +++ b/src/ObjCRuntime/Class.cs @@ -241,7 +241,7 @@ unsafe static IntPtr FindClass (Type type, out bool is_custom_type) continue; var rv = map->map [i].handle; - is_custom_type = i >= (map->map_count - map->custom_type_count); + is_custom_type = (map->map [i].flags & Runtime.MTTypeFlags.CustomType) == Runtime.MTTypeFlags.CustomType; #if LOG_TYPELOAD Console.WriteLine ($"FindClass ({type.FullName}, {is_custom_type}): 0x{rv.ToString ("x")} = {class_getName (rv)}."); #endif @@ -331,12 +331,7 @@ internal unsafe static Type FindType (IntPtr @class, out bool is_custom_type) } // Find the ObjC class pointer in our map - var mapIndex = FindMapIndex (map->map, 0, map->map_count - map->custom_type_count - 1, @class); - if (mapIndex == -1) { - mapIndex = FindMapIndex (map->map, map->map_count - map->custom_type_count, map->map_count - 1, @class); - is_custom_type = true; - } - + var mapIndex = FindMapIndex (map->map, 0, map->map_count - 1, @class); if (mapIndex == -1) { #if LOG_TYPELOAD Console.WriteLine ($"FindType (0x{@class:X} = {Marshal.PtrToStringAuto (class_getName (@class))}) => found no type."); @@ -344,6 +339,8 @@ internal unsafe static Type FindType (IntPtr @class, out bool is_custom_type) return null; } + is_custom_type = (map->map [mapIndex].flags & Runtime.MTTypeFlags.CustomType) == Runtime.MTTypeFlags.CustomType; + Type type = class_to_type [mapIndex]; if (type != null) return type; diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 26a0d45fcadf..76901b0d5cd6 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -65,17 +65,24 @@ internal unsafe struct MTRegistrationMap { public MTProtocolMap protocol_map; public int assembly_count; public int map_count; - public int custom_type_count; public int full_token_reference_count; public int skipped_map_count; public int protocol_wrapper_count; public int protocol_count; } + [Flags] + internal enum MTTypeFlags : uint + { + None = 0, + CustomType = 1, + } + [StructLayout (LayoutKind.Sequential, Pack = 1)] internal struct MTClassMap { public IntPtr handle; public uint type_reference; + public MTTypeFlags flags; } [StructLayout (LayoutKind.Sequential, Pack = 1)] diff --git a/tools/common/StaticRegistrar.cs b/tools/common/StaticRegistrar.cs index 92f1bb605070..ec629c5dfdd9 100644 --- a/tools/common/StaticRegistrar.cs +++ b/tools/common/StaticRegistrar.cs @@ -2725,24 +2725,6 @@ void Specialize (AutoIndentStringBuilder sb) allTypes.Add (@class); } - // Move all the custom types to the end of the list, respecting - // existing order (so that a derived type always comes after - // its base type; the Types.Values has that property, and we - // need to keep it that way). - - var mappedEnd = allTypes.Count; - var counter = 0; - while (counter < mappedEnd) { - if (!IsPlatformType (allTypes [counter].Type)) { - var t = allTypes [counter]; - allTypes.RemoveAt (counter); - allTypes.Add (t); - mappedEnd--; - } else { - counter++; - } - } - if (string.IsNullOrEmpty (single_assembly)) { foreach (var assembly in GetAssemblies ()) registered_assemblies.Add (GetAssemblyName (assembly)); @@ -2750,23 +2732,24 @@ void Specialize (AutoIndentStringBuilder sb) registered_assemblies.Add (single_assembly); } - var customTypeCount = 0; foreach (var @class in allTypes) { var isPlatformType = IsPlatformType (@class.Type); + var flags = MTTypeFlags.None; skip.Clear (); uint token_ref = uint.MaxValue; if (!@class.IsProtocol && !@class.IsCategory) { if (!isPlatformType) - customTypeCount++; - + flags |= MTTypeFlags.CustomType; + CheckNamespace (@class, exceptions); token_ref = CreateTokenReference (@class.Type, TokenType.TypeDef); - map.AppendLine ("{{ NULL, 0x{1:X} /* #{3} '{0}' => '{2}' */ }},", + map.AppendLine ("{{ NULL, 0x{1:X} /* #{3} '{0}' => '{2}' */, (MTTypeFlags) ({4}) /* {5} */ }},", @class.ExportedName, CreateTokenReference (@class.Type, TokenType.TypeDef), - GetAssemblyQualifiedName (@class.Type), map_entries); + GetAssemblyQualifiedName (@class.Type), map_entries, + (int) flags, flags); map_dict [@class] = map_entries++; bool use_dynamic; @@ -3110,7 +3093,6 @@ void Specialize (AutoIndentStringBuilder sb) } map.AppendLine ("{0},", count); map.AppendLine ("{0},", i); - map.AppendLine ("{0},", customTypeCount); map.AppendLine ("{0},", full_token_reference_count); map.AppendLine ("{0},", skipped_types.Count); map.AppendLine ("{0},", protocol_wrapper_map.Count); @@ -5005,4 +4987,11 @@ class AdoptsAttribute : Attribute { public string ProtocolType { get; set; } } + + [Flags] + internal enum MTTypeFlags : uint + { + None = 0, + CustomType = 1, + } } From 267b0b244150e046fd0d3b530e5241f08bd40952 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 19 Oct 2018 09:34:55 +0200 Subject: [PATCH 2/2] Add a UserType flag for registered types, and use it to improve the performance for is_user_type. Reflection in the Objective-C runtime is apparently quite slow, so try to avoid it by computing the information we need for determining whether a particular Objective-C type represents a user type or not in the static registrar. We store this information in a flag for the type in question in the type map, and use a binary search to search the type map when needed. This provides a significant improvement, in particular in the dontlink scenario (probably because there are many more Objective-C types in the app, which made Objective-C reflection slow). In fact, it somehow made the dontlink scenario so fast that it's faster than the linkall scenario (which also improved, but not nearly as much). While quite inexplicable, it's a consistent result I've seen over multiple test runs. Numbers ======= Test case: https://github.com/rolfbjarne/TestApp/commit/004283d7b628a29fcf711d98d8842bfd4ef4393b Fix 1 refers to PR #5009. Fix 2 refers to PR #5013. Fix 3 refers to PR #5016. Fix 4 is this fix. iPad Air 2 ---------- | Configuration | Before | After fix 1 | After fix 2 | After fix 3 | After fix 4 | Improvement from fix 3 to fix 4 | Cumulative improvement | | ------------------- | ------ | ----------: | -----------: | -----------: | -----------: | ------------------------------: | ---------------------: | | Release (link all) | 477 ms | 481 ms | 224 ms | 172 ms | 148 ms | 24 ms (14%) | 329 ms (69%) | | Release (dont link) | 738 ms | 656 ms | 377 ms | 201 ms | 146 ms | 55 ms (27%) | 592 ms (80%) | iPhone X -------- | Configuration | Before | After fix 1 | After fix 2 | After fix 3 | After fix 4 | Improvement from fix 3 to fix 4 | Cumulative improvement | | ------------------- | ------ | ----------: | -----------: | -----------: | -----------: | ------------------------------: | ---------------------: | | Release (link all) | 98 ms | 99 ms | 42 ms | 31 ms | 29 ms | 2 ms ( 6%) | 69 ms (70%) | | Release (dont link) | 197 ms | 153 ms | 91 ms | 43 ms | 28 ms | 15 ms (35%) | 169 ms (86%) | When linking all assemblies, the type map has 24 entries, and when not linking at all it has 2993 entries. This is part 4 (the last) of multiple fixes for #4936. The total speed-up is 69-86% (3-7x faster). --- runtime/runtime.m | 39 ++++++++++++++++++++++++++++++--- runtime/xamarin/runtime.h | 3 ++- src/ObjCRuntime/Runtime.cs | 3 ++- tools/common/StaticRegistrar.cs | 6 ++++- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/runtime/runtime.m b/runtime/runtime.m index c838994cc378..8e829b39a987 100644 --- a/runtime/runtime.m +++ b/runtime/runtime.m @@ -131,7 +131,7 @@ }; enum InitializationFlags : int { - /* unused = 0x01,*/ + InitializationFlagsIsPartialStaticRegistrar = 0x01, /* unused = 0x02,*/ InitializationFlagsDynamicRegistrar = 0x04, /* unused = 0x08,*/ @@ -1014,10 +1014,12 @@ -(void) xamarinSetGCHandle: (int) gc_handle; } void -xamarin_add_registration_map (struct MTRegistrationMap *map) +xamarin_add_registration_map (struct MTRegistrationMap *map, bool partial) { // COOP: no managed memory access: any mode options.RegistrationData = map; + if (partial) + options.flags = (InitializationFlags) (options.flags | InitializationFlagsIsPartialStaticRegistrar); // Sort the type map according to Class qsort (map->map, map->map_count, sizeof (MTClassMap), compare_mtclassmap); @@ -1894,11 +1896,42 @@ -(void) xamarinSetGCHandle: (int) gc_handle; set_raw_gchandle (self, gchandle); } +static int +find_user_type_index (MTClassMap *map, int lo, int hi, Class cls) +{ + if (hi >= lo) { + int mid = lo + (hi - lo) / 2; + + if (map [mid].handle == cls) + return mid; + + if ((intptr_t) map [mid].handle > (intptr_t) cls) + return find_user_type_index (map, lo, mid - 1, cls); + + return find_user_type_index (map, mid + 1, hi, cls); + } + + return -1; +} + static inline bool is_user_type (id self) { + Class cls = object_getClass (self); + + if (options.RegistrationData != NULL && options.RegistrationData->map_count > 0) { + MTClassMap *map = options.RegistrationData->map; + int idx = find_user_type_index (map, 0, options.RegistrationData->map_count - 1, cls); + if (idx >= 0) + return (map [idx].flags & MTTypeFlagsUserType) == MTTypeFlagsUserType; + // If using the partial static registrar, we need to continue + // If full static registrar, we can return false + if ((options.flags & InitializationFlagsIsPartialStaticRegistrar) != InitializationFlagsIsPartialStaticRegistrar) + return false; + } + // COOP: no managed memory access: any mode - return class_getInstanceMethod (object_getClass (self), @selector (xamarinSetGCHandle:)) != NULL; + return class_getInstanceMethod (cls, @selector (xamarinSetGCHandle:)) != NULL; } #if defined(DEBUG_REF_COUNTING) diff --git a/runtime/xamarin/runtime.h b/runtime/xamarin/runtime.h index 277ff3fb884b..bfb443fd4d5b 100644 --- a/runtime/xamarin/runtime.h +++ b/runtime/xamarin/runtime.h @@ -71,6 +71,7 @@ static const uint32_t INVALID_TOKEN_REF = 0xFFFFFFFF; enum MTTypeFlags { MTTypeFlagsNone = 0, MTTypeFlagsCustomType = 1, // not a platform type + MTTypeFlagsUserType = 2, // not a wrapped type }; typedef struct __attribute__((packed)) { @@ -203,7 +204,7 @@ void xamarin_unhandled_exception_handler (MonoObject *exc, gpointer user_data) void xamarin_ftnptr_exception_handler (guint32 gchandle); void xamarin_create_classes (); const char * xamarin_skip_encoding_flags (const char *encoding); -void xamarin_add_registration_map (struct MTRegistrationMap *map); +void xamarin_add_registration_map (struct MTRegistrationMap *map, bool partial); uint32_t xamarin_find_protocol_wrapper_type (uint32_t token_ref); void xamarin_release_block_on_main_thread (void *obj); diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 76901b0d5cd6..ab4a161aa2bb 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -76,6 +76,7 @@ internal enum MTTypeFlags : uint { None = 0, CustomType = 1, + UserType = 2, } [StructLayout (LayoutKind.Sequential, Pack = 1)] @@ -131,7 +132,7 @@ internal struct Trampolines { [Flags] internal enum InitializationFlags : int { - /* unused = 0x01 */ + IsPartialStaticRegistrar= 0x01, /* unused = 0x02,*/ DynamicRegistrar = 0x04, /* unused = 0x08,*/ diff --git a/tools/common/StaticRegistrar.cs b/tools/common/StaticRegistrar.cs index ec629c5dfdd9..52a26776fe12 100644 --- a/tools/common/StaticRegistrar.cs +++ b/tools/common/StaticRegistrar.cs @@ -2743,6 +2743,9 @@ void Specialize (AutoIndentStringBuilder sb) if (!isPlatformType) flags |= MTTypeFlags.CustomType; + if (!@class.IsWrapper && !@class.IsModel) + flags |= MTTypeFlags.UserType; + CheckNamespace (@class, exceptions); token_ref = CreateTokenReference (@class.Type, TokenType.TypeDef); map.AppendLine ("{{ NULL, 0x{1:X} /* #{3} '{0}' => '{2}' */, (MTTypeFlags) ({4}) /* {5} */ }},", @@ -3100,7 +3103,7 @@ void Specialize (AutoIndentStringBuilder sb) map.AppendLine ("};"); - map_init.AppendLine ("xamarin_add_registration_map (&__xamarin_registration_map);"); + map_init.AppendLine ("xamarin_add_registration_map (&__xamarin_registration_map, {0});", string.IsNullOrEmpty (single_assembly) ? "false" : "true"); map_init.AppendLine ("}"); sb.WriteLine (map.ToString ()); @@ -4993,5 +4996,6 @@ internal enum MTTypeFlags : uint { None = 0, CustomType = 1, + UserType = 2, } }