Skip to content

Commit cf80deb

Browse files
jonathanpeppersjonathanpeppers
authored
[Java.Interop.Tools.JavaCallableWrappers] use IMetadataResolver more (dotnet#1069)
Context: b81cfbb Reviewing code in `JavaCallableWrapperGenerator`, I found codepaths where we weren't caching `TypeReference.Resolve()` calls *at all*, e.g. `JavaNativeTypeManager.GetPrimitiveClass(TypeDefinition)`. We thus appear to be calling `TypeReference.Resolve()` potentially on the same types. For example, `dotnet trace` output of an incremental build of a `dotnet new maui` project: 41.65ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve() It appears that, in trying to make our maintenance lives easier by preserving backward API compatibility with callers that couldn't provide a `TypeDefinitionCache` or `IMetadataResolver` instance -- by providing method overloads which took e.g. `IMetadataResolver? resolver` parameters which could be `null` -- we made our optimization and performance lives *harder*, because not all codepaths which needed to use caching *were* using caching, as they were overlooked. As the `Java.Interop.Tools.*` assemblies are internal API, introduce an *API break* while preserving *ABI*: `IMetadataResolver` is now required. Thus, previous/current "compatibility methods" which allow *not* providing an `IMetadataResolver` instance such as: // old and busted partial class TypeDefinitionRocks { [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] public static TypeDefinition? GetBaseType (this TypeDefinition type) => GetBaseType (type, resolver: null); } will now become *errors* to use: // new hawtness partial class TypeDefinitionRocks { [Obsolete ("Use the TypeDefinitionCache overload for better performance.", error:true)] public static TypeDefinition? GetBaseType (this TypeDefinition type) => throw new NotSupportedException (); } This allows the C# compiler to help us audit our codebase, ensuring that *all* codepaths which call `TypeReference.Resolve()` instead use `IMetadataResolver.Resolve()`, including: * `JavaCallableWrapperGenerator.GetAnnotationsString()` * `JavaCallableWrapperGenerator.WriteAnnotations()` * `JavaNativeTypeManager.GetPrimitiveClass()` Requiring caching results in: 23.89ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve() Additionally, I updated two places to use plain `foreach` loops instead of using System.Linq. * Before: 1.03s xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask() * After: 944.48ms xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask() I think this likely saves about ~50ms off incremental builds of a `dotnet new maui` project.
1 parent 5c5dc08 commit cf80deb

File tree

8 files changed

+185
-207
lines changed

8 files changed

+185
-207
lines changed

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

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@ namespace Java.Interop.Tools.Cecil {
99

1010
public static class MethodDefinitionRocks
1111
{
12-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
13-
public static MethodDefinition GetBaseDefinition (this MethodDefinition method) =>
14-
GetBaseDefinition (method, resolver: null);
12+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
13+
public static MethodDefinition GetBaseDefinition (this MethodDefinition method) => throw new NotSupportedException ();
1514

16-
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, TypeDefinitionCache? cache) =>
17-
GetBaseDefinition (method, (IMetadataResolver?) cache);
15+
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, TypeDefinitionCache cache) =>
16+
GetBaseDefinition (method, (IMetadataResolver) cache);
1817

19-
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, IMetadataResolver? resolver)
18+
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, IMetadataResolver resolver)
2019
{
2120
if (method.IsStatic || method.IsNewSlot || !method.IsVirtual)
2221
return method;
@@ -34,14 +33,13 @@ public static MethodDefinition GetBaseDefinition (this MethodDefinition method,
3433
return method;
3534
}
3635

37-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
38-
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit) =>
39-
GetOverriddenMethods (method, inherit, resolver: null);
36+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
37+
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit) => throw new NotSupportedException ();
4038

41-
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, TypeDefinitionCache? cache) =>
42-
GetOverriddenMethods (method, inherit, (IMetadataResolver?) cache);
39+
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, TypeDefinitionCache cache) =>
40+
GetOverriddenMethods (method, inherit, (IMetadataResolver) cache);
4341

44-
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, IMetadataResolver? resolver)
42+
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, IMetadataResolver resolver)
4543
{
4644
yield return method;
4745
if (inherit) {
@@ -53,14 +51,13 @@ public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefiniti
5351
}
5452
}
5553

56-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
57-
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b) =>
58-
AreParametersCompatibleWith (a, b, resolver: null);
54+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
55+
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b) => throw new NotSupportedException ();
5956

60-
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, TypeDefinitionCache? cache) =>
61-
AreParametersCompatibleWith (a, b, (IMetadataResolver?) cache);
57+
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, TypeDefinitionCache cache) =>
58+
AreParametersCompatibleWith (a, b, (IMetadataResolver) cache);
6259

63-
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, IMetadataResolver? resolver)
60+
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, IMetadataResolver resolver)
6461
{
6562
if (a.Count != b.Count)
6663
return false;
@@ -75,15 +72,15 @@ public static bool AreParametersCompatibleWith (this Collection<ParameterDefinit
7572
return true;
7673
}
7774

78-
static bool IsParameterCompatibleWith (IModifierType a, IModifierType b, IMetadataResolver? cache)
75+
static bool IsParameterCompatibleWith (IModifierType a, IModifierType b, IMetadataResolver cache)
7976
{
8077
if (!IsParameterCompatibleWith (a.ModifierType, b.ModifierType, cache))
8178
return false;
8279

8380
return IsParameterCompatibleWith (a.ElementType, b.ElementType, cache);
8481
}
8582

86-
static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b, IMetadataResolver? cache)
83+
static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b, IMetadataResolver cache)
8784
{
8885
if (a is GenericInstanceType)
8986
return IsParameterCompatibleWith ((GenericInstanceType) a, (GenericInstanceType) b, cache);
@@ -94,7 +91,7 @@ static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b,
9491
return IsParameterCompatibleWith (a.ElementType, b.ElementType, cache);
9592
}
9693

97-
static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceType b, IMetadataResolver? cache)
94+
static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceType b, IMetadataResolver cache)
9895
{
9996
if (!IsParameterCompatibleWith (a.ElementType, b.ElementType, cache))
10097
return false;
@@ -112,7 +109,7 @@ static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceTyp
112109
return true;
113110
}
114111

115-
static bool IsParameterCompatibleWith (TypeReference a, TypeReference b, IMetadataResolver? cache)
112+
static bool IsParameterCompatibleWith (TypeReference a, TypeReference b, IMetadataResolver cache)
116113
{
117114
if (a is TypeSpecification || b is TypeSpecification) {
118115
if (a.GetType () != b.GetType ())

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

Lines changed: 49 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,27 @@ namespace Java.Interop.Tools.Cecil {
77

88
public static class TypeDefinitionRocks {
99

10-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
11-
public static TypeDefinition? GetBaseType (this TypeDefinition type) =>
12-
GetBaseType (type, resolver: null);
10+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
11+
public static TypeDefinition? GetBaseType (this TypeDefinition type) => throw new NotSupportedException ();
1312

14-
public static TypeDefinition? GetBaseType (this TypeDefinition type, TypeDefinitionCache? cache) =>
15-
GetBaseType (type, (IMetadataResolver?) cache);
13+
public static TypeDefinition? GetBaseType (this TypeDefinition type, TypeDefinitionCache cache) =>
14+
GetBaseType (type, (IMetadataResolver) cache);
1615

17-
public static TypeDefinition? GetBaseType (this TypeDefinition type, IMetadataResolver? resolver)
16+
public static TypeDefinition? GetBaseType (this TypeDefinition type, IMetadataResolver resolver)
1817
{
1918
var bt = type.BaseType;
2019
if (bt == null)
2120
return null;
22-
if (resolver != null)
23-
return resolver.Resolve (bt);
24-
return bt.Resolve ();
21+
return resolver.Resolve (bt);
2522
}
2623

27-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
28-
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type) =>
29-
GetTypeAndBaseTypes (type, resolver: null);
24+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
25+
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type) => throw new NotSupportedException ();
3026

31-
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, TypeDefinitionCache? cache) =>
32-
GetTypeAndBaseTypes (type, (IMetadataResolver?) cache);
27+
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, TypeDefinitionCache cache) =>
28+
GetTypeAndBaseTypes (type, (IMetadataResolver) cache);
3329

34-
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, IMetadataResolver? resolver)
30+
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, IMetadataResolver resolver)
3531
{
3632
TypeDefinition? t = type;
3733

@@ -41,14 +37,13 @@ public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefiniti
4137
}
4238
}
4339

44-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
45-
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type) =>
46-
GetBaseTypes (type, resolver: null);
40+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
41+
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type) => throw new NotSupportedException();
4742

48-
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, TypeDefinitionCache? cache) =>
49-
GetBaseTypes (type, (IMetadataResolver?) cache);
43+
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, TypeDefinitionCache cache) =>
44+
GetBaseTypes (type, (IMetadataResolver) cache);
5045

51-
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, IMetadataResolver? resolver)
46+
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, IMetadataResolver resolver)
5247
{
5348
TypeDefinition? t = type;
5449

@@ -57,18 +52,17 @@ public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type
5752
}
5853
}
5954

60-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
61-
public static bool IsAssignableFrom (this TypeReference type, TypeReference c) =>
62-
IsAssignableFrom (type, c, resolver: null);
55+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
56+
public static bool IsAssignableFrom (this TypeReference type, TypeReference c) => throw new NotSupportedException ();
6357

64-
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, TypeDefinitionCache? cache) =>
65-
IsAssignableFrom (type, c, (IMetadataResolver?) cache);
58+
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, TypeDefinitionCache cache) =>
59+
IsAssignableFrom (type, c, (IMetadataResolver) cache);
6660

67-
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, IMetadataResolver? resolver)
61+
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, IMetadataResolver resolver)
6862
{
6963
if (type.FullName == c.FullName)
7064
return true;
71-
var d = (resolver?.Resolve (c)) ?? c.Resolve ();
65+
var d = resolver.Resolve (c);
7266
if (d == null)
7367
return false;
7468
foreach (var t in d.GetTypeAndBaseTypes (resolver)) {
@@ -83,13 +77,12 @@ public static bool IsAssignableFrom (this TypeReference type, TypeReference c, I
8377
return false;
8478
}
8579

86-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
87-
public static bool IsSubclassOf (this TypeDefinition type, string typeName) =>
88-
IsSubclassOf (type, typeName, resolver: null);
80+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
81+
public static bool IsSubclassOf (this TypeDefinition type, string typeName) => throw new NotSupportedException ();
8982

90-
public static bool IsSubclassOf (this TypeDefinition type, string typeName, TypeDefinitionCache? cache) =>
91-
IsSubclassOf (type, typeName, (IMetadataResolver?) cache);
92-
public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMetadataResolver? resolver)
83+
public static bool IsSubclassOf (this TypeDefinition type, string typeName, TypeDefinitionCache cache) =>
84+
IsSubclassOf (type, typeName, (IMetadataResolver) cache);
85+
public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMetadataResolver resolver)
9386
{
9487
foreach (var t in type.GetTypeAndBaseTypes (resolver)) {
9588
if (t.FullName == typeName) {
@@ -99,14 +92,13 @@ public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMet
9992
return false;
10093
}
10194

102-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
103-
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName) =>
104-
ImplementsInterface (type, interfaceName, resolver: null);
95+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
96+
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName) => throw new NotSupportedException ();
10597

106-
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, TypeDefinitionCache? cache) =>
107-
ImplementsInterface (type, interfaceName, (IMetadataResolver?) cache);
98+
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, TypeDefinitionCache cache) =>
99+
ImplementsInterface (type, interfaceName, (IMetadataResolver) cache);
108100

109-
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, IMetadataResolver? resolver)
101+
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, IMetadataResolver resolver)
110102
{
111103
foreach (var t in type.GetTypeAndBaseTypes (resolver)) {
112104
foreach (var i in t.Interfaces) {
@@ -118,27 +110,25 @@ public static bool ImplementsInterface (this TypeDefinition type, string interfa
118110
return false;
119111
}
120112

121-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
122-
public static string GetPartialAssemblyName (this TypeReference type) =>
123-
GetPartialAssemblyName (type, resolver: null);
113+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
114+
public static string GetPartialAssemblyName (this TypeReference type) => throw new NotSupportedException ();
124115

125-
public static string GetPartialAssemblyName (this TypeReference type, TypeDefinitionCache? cache) =>
126-
GetPartialAssemblyName (type, (IMetadataResolver?) cache);
116+
public static string GetPartialAssemblyName (this TypeReference type, TypeDefinitionCache cache) =>
117+
GetPartialAssemblyName (type, (IMetadataResolver) cache);
127118

128-
public static string GetPartialAssemblyName (this TypeReference type, IMetadataResolver? resolver)
119+
public static string GetPartialAssemblyName (this TypeReference type, IMetadataResolver resolver)
129120
{
130-
TypeDefinition? def = (resolver?.Resolve (type)) ?? type.Resolve ();
121+
TypeDefinition? def = resolver.Resolve (type);
131122
return (def ?? type).Module.Assembly.Name.Name;
132123
}
133124

134-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
135-
public static string GetPartialAssemblyQualifiedName (this TypeReference type) =>
136-
GetPartialAssemblyQualifiedName (type, resolver: null);
125+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
126+
public static string GetPartialAssemblyQualifiedName (this TypeReference type) => throw new NotSupportedException ();
137127

138-
public static string GetPartialAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache? cache) =>
139-
GetPartialAssemblyQualifiedName (type, (IMetadataResolver?) cache);
128+
public static string GetPartialAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache cache) =>
129+
GetPartialAssemblyQualifiedName (type, (IMetadataResolver) cache);
140130

141-
public static string GetPartialAssemblyQualifiedName (this TypeReference type, IMetadataResolver? resolver)
131+
public static string GetPartialAssemblyQualifiedName (this TypeReference type, IMetadataResolver resolver)
142132
{
143133
return string.Format ("{0}, {1}",
144134
// Cecil likes to use '/' as the nested type separator, while
@@ -147,16 +137,15 @@ public static string GetPartialAssemblyQualifiedName (this TypeReference type, I
147137
type.GetPartialAssemblyName (resolver));
148138
}
149139

150-
[Obsolete ("Use the TypeDefinitionCache overload for better performance.")]
151-
public static string GetAssemblyQualifiedName (this TypeReference type) =>
152-
GetAssemblyQualifiedName (type, resolver: null);
140+
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
141+
public static string GetAssemblyQualifiedName (this TypeReference type) => throw new NotSupportedException ();
153142

154-
public static string GetAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache? cache) =>
155-
GetAssemblyQualifiedName (type, (IMetadataResolver?) cache);
143+
public static string GetAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache cache) =>
144+
GetAssemblyQualifiedName (type, (IMetadataResolver) cache);
156145

157-
public static string GetAssemblyQualifiedName (this TypeReference type, IMetadataResolver? resolver)
146+
public static string GetAssemblyQualifiedName (this TypeReference type, IMetadataResolver resolver)
158147
{
159-
TypeDefinition? def = (resolver?.Resolve (type)) ?? type.Resolve ();
148+
TypeDefinition? def = resolver.Resolve (type);
160149
return string.Format ("{0}, {1}",
161150
// Cecil likes to use '/' as the nested type separator, while
162151
// Reflection uses '+' as the nested type separator. Use Reflection.

0 commit comments

Comments
 (0)