Skip to content

Commit

Permalink
[Class] Sort our array of Class -> token references so that we can do…
Browse files Browse the repository at this point in the history
… binary instead of linear searches in it. (#5009)

Our type map has two sections: first come all the wrapper types, then all the
custom types. This means we need to sort these two sections separately, since
code elsewhere depends on this split in order to determine if a type is a
custom type or not.

We also need a minor modification in the array of skipped types, since it
contained indexes into the type map, which won't be valid after is has been
sorted. Instead store a type reference for the actual type in the array, and
use that to search the type map for the corresponding Class. This is a little
bit slower, but the results are cached in a dictionary, so it'll only happen
once for each type.

The performance is slightly slower when the type map has very few entries, but
that is repaid many times over when the number of entries go up.

Numbers
=======

Test case: rolfbjarne/TestApp@004283d

iPad Air 2
----------

| Configuration       | Before | After  | Improvement   |
| ------------------- | ------ | ------ | ------------: |
| Release (link all)  | 477 ms | 481 ms | -4 ms (-0,8%) |
| Release (dont link) | 738 ms | 656 ms |   82 ms (11%) |

iPhone X
--------

| Configuration       | Before | After  | Improvement |
| ------------------- | ------ | ------ | ----------: |
| Release (link all)  |  98 ms |  99 ms | -1 ms (-1%) |
| Release (dont link) | 197 ms | 153 ms | 44 ms (22%) |

When linking all assemblies, the type map has 24 entries, and when not linking
at all it has 2993 entries.

This is part 1 of multiple fixes for #4936.
  • Loading branch information
rolfbjarne authored Oct 19, 2018
1 parent 1ec90f0 commit 900356c
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 29 deletions.
16 changes: 16 additions & 0 deletions runtime/runtime.m
Original file line number Diff line number Diff line change
Expand Up @@ -1001,11 +1001,27 @@ -(void) xamarinSetGCHandle: (int) gc_handle;
* Registration map
*/

static int
compare_mtclassmap (const void *a, const void *b)
{
MTClassMap *mapa = (MTClassMap *) a;
MTClassMap *mapb = (MTClassMap *) b;
if (mapa->handle == mapb->handle)
return 0;
if ((intptr_t) mapa->handle < (intptr_t) mapb->handle)
return -1;
return 1;
}

void
xamarin_add_registration_map (struct MTRegistrationMap *map)
{
// COOP: no managed memory access: any mode
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);
}

/*
Expand Down
2 changes: 1 addition & 1 deletion runtime/xamarin/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ typedef struct __attribute__((packed)) {

typedef struct __attribute__((packed)) {
uint32_t /* MTTokenReference */ skipped_reference;
uint32_t /* index into MTRegistrationMap->map */ index;
uint32_t /* MTTokenReference */ actual_reference;
} MTManagedClassMap;

typedef struct __attribute__((packed)) {
Expand Down
52 changes: 33 additions & 19 deletions src/ObjCRuntime/Class.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,13 @@ unsafe static IntPtr FindClass (Type type, out bool is_custom_type)
if (!CompareTokenReference (asm_name, mod_token, type_token, token_reference))
continue;

// This is a skipped type, we now got the index into the normal table.
var idx = map->skipped_map [i].index;
is_custom_type = idx >= (map->map_count - map->custom_type_count);
return map->map [idx].handle;
// This is a skipped type, we now got the actual type reference of the type we're looking for,
// so go look for it in the type map.
var actual_reference = map->skipped_map [i].actual_reference;
for (int k = 0; k < map->map_count; k++) {
if (map->map [k].type_reference == actual_reference)
return map->map [k].handle;
}
}

return IntPtr.Zero;
Expand Down Expand Up @@ -293,10 +296,26 @@ unsafe static bool CompareTokenReference (string asm_name, int mod_token, int ty
return Runtime.StringEquals (assembly_name, asm_name);
}

static unsafe int FindMapIndex (Runtime.MTClassMap *array, int lo, int hi, IntPtr @class)
{
if (hi >= lo) {
int mid = lo + (hi - lo) / 2;

if (array [mid].handle == @class)
return mid;

if (array [mid].handle.ToInt64 () > @class.ToInt64 ())
return FindMapIndex (array, lo, mid - 1, @class);

return FindMapIndex (array, mid + 1, hi, @class);
}

return -1;
}

internal unsafe static Type FindType (IntPtr @class, out bool is_custom_type)
{
var map = Runtime.options->RegistrationMap;
Runtime.MTClassMap? entry = null;

is_custom_type = false;

Expand All @@ -308,34 +327,29 @@ internal unsafe static Type FindType (IntPtr @class, out bool is_custom_type)
}

// Find the ObjC class pointer in our map
// Potential improvement: order the type handles after loading them, which means we could do a binary search here.
// A binary search will likely be faster than a dictionary for any real-world scenario (and if slower, not much slower),
// but it would need a lot less memory (very little when sorting, could probably use stack memory, and then nothing at all afterwards).
for (int i = 0; i < map->map_count; i++) {
if (map->map [i].handle != @class)
continue;

entry = map->map [i];
is_custom_type = i >= (map->map_count - map->custom_type_count);
break;
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;
}

if (!entry.HasValue) {
if (mapIndex == -1) {
#if LOG_TYPELOAD
Console.WriteLine ($"FindType (0x{@class:X} = {Marshal.PtrToStringAuto (class_getName (@class))}) => found no type.");
#endif
return null;
}

// Resolve the map entry we found to a managed type
var member = ResolveTokenReference (entry.Value.type_reference, 0x02000000);
var type_reference = map->map [mapIndex].type_reference;
var member = ResolveTokenReference (type_reference, 0x02000000);
var type = member as Type;

if (type == null && member != null)
throw ErrorHelper.CreateError (8022, $"Expected the token reference 0x{entry.Value.type_reference:X} to be a type, but it's a {member.GetType ().Name}. Please file a bug report at https://github.com/xamarin/xamarin-macios/issues/new.");
throw ErrorHelper.CreateError (8022, $"Expected the token reference 0x{type_reference:X} to be a type, but it's a {member.GetType ().Name}. Please file a bug report at https://github.com/xamarin/xamarin-macios/issues/new.");

#if LOG_TYPELOAD
Console.WriteLine ($"FindType (0x{@class:X} = {Marshal.PtrToStringAuto (class_getName (@class))}) => {type.FullName}; is custom: {is_custom_type} (token reference: 0x{entry.Value.type_reference:X}).");
Console.WriteLine ($"FindType (0x{@class:X} = {Marshal.PtrToStringAuto (class_getName (@class))}) => {type.FullName}; is custom: {is_custom_type} (token reference: 0x{type_reference:X}).");
#endif

return type;
Expand Down
2 changes: 1 addition & 1 deletion src/ObjCRuntime/Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ internal struct MTClassMap {
internal struct MTManagedClassMap
{
public uint skipped_reference; // implied token type: TypeDef
public uint index; // index into MTRegistrationMap.map
public uint actual_reference; // implied token type: TypeDef
}

[StructLayout (LayoutKind.Sequential, Pack = 1)]
Expand Down
14 changes: 6 additions & 8 deletions tools/common/StaticRegistrar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2636,6 +2636,7 @@ class SkippedType
public TypeReference Skipped;
public ObjCType Actual;
public uint SkippedTokenReference;
public uint ActualTokenReference;
}
List<SkippedType> skipped_types = new List<SkippedType> ();
protected override void OnSkipType (TypeReference type, ObjCType registered_type)
Expand Down Expand Up @@ -3031,16 +3032,13 @@ void Specialize (AutoIndentStringBuilder sb)

if (skipped_types.Count > 0) {
map.AppendLine ("static const MTManagedClassMap __xamarin_skipped_map [] = {");
foreach (var skipped in skipped_types)
foreach (var skipped in skipped_types) {
skipped.SkippedTokenReference = CreateTokenReference (skipped.Skipped, TokenType.TypeDef);

foreach (var skipped in skipped_types.OrderBy ((v) => v.SkippedTokenReference)) {
if (map_dict.TryGetValue (skipped.Actual, out var index)) {
map.AppendLine ("{{ 0x{0:X}, {1} /* '{2}' => '{3}' */ }},", skipped.SkippedTokenReference, map_dict [skipped.Actual], skipped.Skipped.FullName, skipped.Actual.Type.FullName);
} else {
throw ErrorHelper.CreateError (99, $"Internal error: could not find the native type for {skipped.Skipped.FullName} (failed to find {skipped.Actual.Type.FullName}). Please file a bug report with a test case (https://github.com/xamarin/xamarin-macios/issues/new).");
}
skipped.ActualTokenReference = CreateTokenReference (skipped.Actual.Type, TokenType.TypeDef);
}

foreach (var skipped in skipped_types.OrderBy ((v) => v.SkippedTokenReference))
map.AppendLine ("{{ 0x{0:X}, 0x{1:X} /* '{2}' => '{3}' */ }},", skipped.SkippedTokenReference, skipped.ActualTokenReference, skipped.Skipped.FullName, skipped.Actual.Type.FullName);
map.AppendLine ("};");
map.AppendLine ();
}
Expand Down

1 comment on commit 900356c

@xamarin-release-manager
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jenkins job (on internal Jenkins) succeeded

Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
Generator Diff (no change)
Test run succeeded

Please sign in to comment.