Skip to content

Commit b81cfbb

Browse files
jonathanpeppersjonathanpeppers
authored
[Java.Interop.Tools.Cecil] cache TypeReference.Resolve() calls (dotnet#570)
Context: dotnet/android#4268 I was profiling builds with the Mono profiler and noticed: Method call summary Total(ms) Self(ms) Calls Method name 70862 97 89713 Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference,Mono.Cecil.ReaderParameters) Almost 90K calls to `DirectoryAssemblyResolver.Resolve()`?! Where is that coming from??? 61422 calls from: Java.Interop.Tools.Cecil.TypeDefinitionRocks/<GetTypeAndBaseTypes>d__1:MoveNext () Java.Interop.Tools.Cecil.TypeDefinitionRocks:GetBaseType (Mono.Cecil.TypeDefinition) Mono.Cecil.TypeReference:Resolve () Mono.Cecil.ModuleDefinition:Resolve (Mono.Cecil.TypeReference) Mono.Cecil.MetadataResolver:Resolve (Mono.Cecil.TypeReference) Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference) OK, this jogged my memory. @StephaneDelcroix had mentioned one of the big wins for the `<XamlC/>` task within Xamarin.Forms was to cache any time `TypeReference.Resolve()` was called: https://github.com/xamarin/Xamarin.Forms/blob/1b9c22b4b9b1c1354a3a5c35ad445a2738c6f6c3/Xamarin.Forms.Build.Tasks/TypeReferenceExtensions.cs#L437-L443 `<XamlC/>` was able to use `static` here, because it's using a feature of MSBuild to run in a separate `AppDomain`: https://github.com/xamarin/Xamarin.Forms/blob/1b9c22b4b9b1c1354a3a5c35ad445a2738c6f6c3/Xamarin.Forms.Build.Tasks/XamlTask.cs#L20-L21 However, I think we can simply add a new `TypeDefinitionCache` class that would allow callers to control the caching strategy. Callers will need to control the scope of the `TypeDefinitionCache` so it matches any `DirectoryAssemblyResolver` being used. Right now most Xamarin.Android builds will open assemblies with Mono.Cecil twice: once for `<GenerateJavaStubs/>` and once for the linker. We can add caching in an API-compatible way: [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] public static TypeDefinition GetBaseType (this TypeDefinition type) => GetBaseType (type, cache: null); public static TypeDefinition GetBaseType (this TypeDefinition type, TypeDefinitionCache cache) { if (bt == null) return null; if (cache != null) return cache.Resolve (bt); return bt.Resolve (); } We just need to ensure `cache: null` is valid. I took this approach for any `public` APIs and made any `private` or `internal` APIs *require* a `TypeDefinitionCache`. I fixed every instance where `[Obsolete]` would cause a warning here in Java.Interop. We'll probably see a small improvement within `generator` and `jnimarshalmethod-gen`. Downstream in xamarin-android, in dotnet/android#4268 I fixed all the `[Obsolete]` warnings that were present in `<GenerateJavaStubs/>`. I can make more changes for the linker in a future PR. ~~ Results ~~ The reduced calls to `DirectoryAssemblyResolver.Resolve()`: * Before: Method call summary Total(ms) Self(ms) Calls Method name 70862 97 89713 Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference,Mono.Cecil.ReaderParameters) * After: Method call summary Total(ms) Self(ms) Calls Method name 68830 35 26315 Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference,Mono.Cecil.ReaderParameters) ~63,398 less calls. In a build of the Xamarin.Forms integration project on macOS / Mono: * Before: 1365 ms GenerateJavaStubs 1 calls * After: 862 ms GenerateJavaStubs 1 calls It is almost a 40% improvement, around ~500ms better.
1 parent 95fd014 commit b81cfbb

File tree

13 files changed

+289
-166
lines changed

13 files changed

+289
-166
lines changed

‎src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/MethodDefinitionRocks.cs‎

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,49 @@ namespace Java.Interop.Tools.Cecil {
99

1010
public static class MethodDefinitionRocks
1111
{
12-
public static MethodDefinition GetBaseDefinition (this MethodDefinition method)
12+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
13+
public static MethodDefinition GetBaseDefinition (this MethodDefinition method) =>
14+
GetBaseDefinition (method, cache: null);
15+
16+
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, TypeDefinitionCache cache)
1317
{
1418
if (method.IsStatic || method.IsNewSlot || !method.IsVirtual)
1519
return method;
1620

17-
foreach (var baseType in method.DeclaringType.GetBaseTypes ()) {
21+
foreach (var baseType in method.DeclaringType.GetBaseTypes (cache)) {
1822
foreach (var m in baseType.Methods) {
1923
if (!m.IsConstructor &&
2024
m.Name == method.Name &&
2125
(m.IsVirtual || m.IsAbstract) &&
22-
AreParametersCompatibleWith (m.Parameters, method.Parameters)) {
26+
AreParametersCompatibleWith (m.Parameters, method.Parameters, cache)) {
2327
return m;
2428
}
2529
}
2630
}
2731
return method;
2832
}
2933

30-
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit)
34+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
35+
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit) =>
36+
GetOverriddenMethods (method, inherit, cache: null);
37+
38+
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, TypeDefinitionCache cache)
3139
{
3240
yield return method;
3341
if (inherit) {
3442
MethodDefinition baseMethod = method;
35-
while ((baseMethod = method.GetBaseDefinition ()) != null && baseMethod != method) {
43+
while ((baseMethod = method.GetBaseDefinition (cache)) != null && baseMethod != method) {
3644
yield return method;
3745
method = baseMethod;
3846
}
3947
}
4048
}
4149

42-
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b)
50+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
51+
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b) =>
52+
AreParametersCompatibleWith (a, b, cache: null);
53+
54+
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, TypeDefinitionCache cache)
4355
{
4456
if (a.Count != b.Count)
4557
return false;
@@ -48,34 +60,34 @@ public static bool AreParametersCompatibleWith (this Collection<ParameterDefinit
4860
return true;
4961

5062
for (int i = 0; i < a.Count; i++)
51-
if (!IsParameterCompatibleWith (a [i].ParameterType, b [i].ParameterType))
63+
if (!IsParameterCompatibleWith (a [i].ParameterType, b [i].ParameterType, cache))
5264
return false;
5365

5466
return true;
5567
}
5668

57-
static bool IsParameterCompatibleWith (IModifierType a, IModifierType b)
69+
static bool IsParameterCompatibleWith (IModifierType a, IModifierType b, TypeDefinitionCache cache)
5870
{
59-
if (!IsParameterCompatibleWith (a.ModifierType, b.ModifierType))
71+
if (!IsParameterCompatibleWith (a.ModifierType, b.ModifierType, cache))
6072
return false;
6173

62-
return IsParameterCompatibleWith (a.ElementType, b.ElementType);
74+
return IsParameterCompatibleWith (a.ElementType, b.ElementType, cache);
6375
}
6476

65-
static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b)
77+
static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b, TypeDefinitionCache cache)
6678
{
6779
if (a is GenericInstanceType)
68-
return IsParameterCompatibleWith ((GenericInstanceType) a, (GenericInstanceType) b);
80+
return IsParameterCompatibleWith ((GenericInstanceType) a, (GenericInstanceType) b, cache);
6981

7082
if (a is IModifierType)
71-
return IsParameterCompatibleWith ((IModifierType) a, (IModifierType) b);
83+
return IsParameterCompatibleWith ((IModifierType) a, (IModifierType) b, cache);
7284

73-
return IsParameterCompatibleWith (a.ElementType, b.ElementType);
85+
return IsParameterCompatibleWith (a.ElementType, b.ElementType, cache);
7486
}
7587

76-
static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceType b)
88+
static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceType b, TypeDefinitionCache cache)
7789
{
78-
if (!IsParameterCompatibleWith (a.ElementType, b.ElementType))
90+
if (!IsParameterCompatibleWith (a.ElementType, b.ElementType, cache))
7991
return false;
8092

8193
if (a.GenericArguments.Count != b.GenericArguments.Count)
@@ -85,32 +97,27 @@ static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceTyp
8597
return true;
8698

8799
for (int i = 0; i < a.GenericArguments.Count; i++)
88-
if (!IsParameterCompatibleWith (a.GenericArguments [i], b.GenericArguments [i]))
100+
if (!IsParameterCompatibleWith (a.GenericArguments [i], b.GenericArguments [i], cache))
89101
return false;
90102

91103
return true;
92104
}
93105

94-
static bool IsParameterCompatibleWith (GenericParameter a, GenericParameter b)
95-
{
96-
return a.Position == b.Position;
97-
}
98-
99-
static bool IsParameterCompatibleWith (TypeReference a, TypeReference b)
106+
static bool IsParameterCompatibleWith (TypeReference a, TypeReference b, TypeDefinitionCache cache)
100107
{
101108
if (a is TypeSpecification || b is TypeSpecification) {
102109
if (a.GetType () != b.GetType ())
103110
return false;
104111

105-
return IsParameterCompatibleWith ((TypeSpecification) a, (TypeSpecification) b);
112+
return IsParameterCompatibleWith ((TypeSpecification) a, (TypeSpecification) b, cache);
106113
}
107114

108115
if (a.IsGenericParameter) {
109116
if (b.IsGenericParameter && a.Name == b.Name)
110117
return true;
111118
var gpa = (GenericParameter) a;
112119
foreach (var c in gpa.Constraints) {
113-
if (!c.ConstraintType.IsAssignableFrom (b))
120+
if (!c.ConstraintType.IsAssignableFrom (b, cache))
114121
return false;
115122
}
116123
return true;

‎src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/TypeDefinitionCache.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using System.Collections.Generic;
2+
using Mono.Cecil;
3+
4+
namespace Java.Interop.Tools.Cecil
5+
{
6+
/// <summary>
7+
/// A class for caching lookups from TypeReference -> TypeDefinition.
8+
/// Generally its lifetime should match an AssemblyResolver instance.
9+
/// </summary>
10+
public class TypeDefinitionCache
11+
{
12+
readonly Dictionary<TypeReference, TypeDefinition> cache = new Dictionary<TypeReference, TypeDefinition> ();
13+
14+
public virtual TypeDefinition Resolve (TypeReference typeReference)
15+
{
16+
if (cache.TryGetValue (typeReference, out var typeDefinition))
17+
return typeDefinition;
18+
return cache [typeReference] = typeReference.Resolve ();
19+
}
20+
}
21+
}

‎src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/TypeDefinitionRocks.cs‎

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,54 +8,82 @@ namespace Java.Interop.Tools.Cecil {
88

99
public static class TypeDefinitionRocks {
1010

11-
public static TypeDefinition GetBaseType (this TypeDefinition type)
11+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
12+
public static TypeDefinition GetBaseType (this TypeDefinition type) =>
13+
GetBaseType (type, cache: null);
14+
15+
public static TypeDefinition GetBaseType (this TypeDefinition type, TypeDefinitionCache cache)
1216
{
1317
var bt = type.BaseType;
14-
return bt == null ? null : bt.Resolve ();
18+
if (bt == null)
19+
return null;
20+
if (cache != null)
21+
return cache.Resolve (bt);
22+
return bt.Resolve ();
1523
}
1624

17-
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type)
25+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
26+
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type) =>
27+
GetTypeAndBaseTypes (type, cache: null);
28+
29+
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, TypeDefinitionCache cache)
1830
{
1931
while (type != null) {
2032
yield return type;
21-
type = type.GetBaseType ();
33+
type = type.GetBaseType (cache);
2234
}
2335
}
2436

25-
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type)
37+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
38+
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type) =>
39+
GetBaseTypes (type, cache: null);
40+
41+
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, TypeDefinitionCache cache)
2642
{
27-
while ((type = type.GetBaseType ()) != null) {
43+
while ((type = type.GetBaseType (cache)) != null) {
2844
yield return type;
2945
}
3046
}
3147

32-
public static bool IsAssignableFrom (this TypeReference type, TypeReference c)
48+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
49+
public static bool IsAssignableFrom (this TypeReference type, TypeReference c) =>
50+
IsAssignableFrom (type, c, cache: null);
51+
52+
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, TypeDefinitionCache cache)
3353
{
3454
if (type.FullName == c.FullName)
3555
return true;
3656
var d = c.Resolve ();
3757
if (d == null)
3858
return false;
39-
foreach (var t in d.GetTypeAndBaseTypes ()) {
59+
foreach (var t in d.GetTypeAndBaseTypes (cache)) {
4060
if (type.FullName == t.FullName)
4161
return true;
4262
foreach (var ifaceImpl in t.Interfaces) {
4363
var i = ifaceImpl.InterfaceType;
44-
if (IsAssignableFrom (type, i))
64+
if (IsAssignableFrom (type, i, cache))
4565
return true;
4666
}
4767
}
4868
return false;
4969
}
5070

51-
public static bool IsSubclassOf (this TypeDefinition type, string typeName)
71+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
72+
public static bool IsSubclassOf (this TypeDefinition type, string typeName) =>
73+
IsSubclassOf (type, typeName, cache: null);
74+
75+
public static bool IsSubclassOf (this TypeDefinition type, string typeName, TypeDefinitionCache cache)
5276
{
53-
return type.GetTypeAndBaseTypes ().Any (t => t.FullName == typeName);
77+
return type.GetTypeAndBaseTypes (cache).Any (t => t.FullName == typeName);
5478
}
5579

56-
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName)
80+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
81+
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName) =>
82+
ImplementsInterface (type, interfaceName, cache: null);
83+
84+
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, TypeDefinitionCache cache)
5785
{
58-
foreach (var t in type.GetTypeAndBaseTypes ()) {
86+
foreach (var t in type.GetTypeAndBaseTypes (cache)) {
5987
foreach (var i in t.Interfaces) {
6088
if (i.InterfaceType.FullName == interfaceName) {
6189
return true;
@@ -65,24 +93,36 @@ public static bool ImplementsInterface (this TypeDefinition type, string interfa
6593
return false;
6694
}
6795

68-
public static string GetPartialAssemblyName (this TypeReference type)
96+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
97+
public static string GetPartialAssemblyName (this TypeReference type) =>
98+
GetPartialAssemblyName (type, cache: null);
99+
100+
public static string GetPartialAssemblyName (this TypeReference type, TypeDefinitionCache cache)
69101
{
70-
TypeDefinition def = type.Resolve ();
102+
TypeDefinition def = cache != null ? cache.Resolve (type) : type.Resolve ();
71103
return (def ?? type).Module.Assembly.Name.Name;
72104
}
73105

74-
public static string GetPartialAssemblyQualifiedName (this TypeReference type)
106+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
107+
public static string GetPartialAssemblyQualifiedName (this TypeReference type) =>
108+
GetPartialAssemblyQualifiedName (type, cache: null);
109+
110+
public static string GetPartialAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache cache)
75111
{
76112
return string.Format ("{0}, {1}",
77113
// Cecil likes to use '/' as the nested type separator, while
78114
// Reflection uses '+' as the nested type separator. Use Reflection.
79115
type.FullName.Replace ('/', '+'),
80-
type.GetPartialAssemblyName ());
116+
type.GetPartialAssemblyName (cache));
81117
}
82118

83-
public static string GetAssemblyQualifiedName (this TypeReference type)
119+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
120+
public static string GetAssemblyQualifiedName (this TypeReference type) =>
121+
GetAssemblyQualifiedName (type, cache: null);
122+
123+
public static string GetAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache cache)
84124
{
85-
TypeDefinition def = type.Resolve ();
125+
TypeDefinition def = cache != null ? cache.Resolve (type) : type.Resolve ();
86126
return string.Format ("{0}, {1}",
87127
// Cecil likes to use '/' as the nested type separator, while
88128
// Reflection uses '+' as the nested type separator. Use Reflection.

0 commit comments

Comments
 (0)