Skip to content

Commit

Permalink
[Java.Interop.Tools.JavaCallableWrappers] use IMetadataResolver more (#…
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@jonathanpeppers
jonathanpeppers authored Jan 7, 2023
1 parent 5c5dc08 commit cf80deb
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 207 deletions.
41 changes: 19 additions & 22 deletions src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/MethodDefinitionRocks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@ namespace Java.Interop.Tools.Cecil {

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

public static MethodDefinition GetBaseDefinition (this MethodDefinition method, TypeDefinitionCache? cache) =>
GetBaseDefinition (method, (IMetadataResolver?) cache);
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, TypeDefinitionCache cache) =>
GetBaseDefinition (method, (IMetadataResolver) cache);

public static MethodDefinition GetBaseDefinition (this MethodDefinition method, IMetadataResolver? resolver)
public static MethodDefinition GetBaseDefinition (this MethodDefinition method, IMetadataResolver resolver)
{
if (method.IsStatic || method.IsNewSlot || !method.IsVirtual)
return method;
Expand All @@ -34,14 +33,13 @@ public static MethodDefinition GetBaseDefinition (this MethodDefinition method,
return method;
}

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

public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, TypeDefinitionCache? cache) =>
GetOverriddenMethods (method, inherit, (IMetadataResolver?) cache);
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, TypeDefinitionCache cache) =>
GetOverriddenMethods (method, inherit, (IMetadataResolver) cache);

public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, IMetadataResolver? resolver)
public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefinition method, bool inherit, IMetadataResolver resolver)
{
yield return method;
if (inherit) {
Expand All @@ -53,14 +51,13 @@ public static IEnumerable<MethodDefinition> GetOverriddenMethods (MethodDefiniti
}
}

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

public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, TypeDefinitionCache? cache) =>
AreParametersCompatibleWith (a, b, (IMetadataResolver?) cache);
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, TypeDefinitionCache cache) =>
AreParametersCompatibleWith (a, b, (IMetadataResolver) cache);

public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, IMetadataResolver? resolver)
public static bool AreParametersCompatibleWith (this Collection<ParameterDefinition> a, Collection<ParameterDefinition> b, IMetadataResolver resolver)
{
if (a.Count != b.Count)
return false;
Expand All @@ -75,15 +72,15 @@ public static bool AreParametersCompatibleWith (this Collection<ParameterDefinit
return true;
}

static bool IsParameterCompatibleWith (IModifierType a, IModifierType b, IMetadataResolver? cache)
static bool IsParameterCompatibleWith (IModifierType a, IModifierType b, IMetadataResolver cache)
{
if (!IsParameterCompatibleWith (a.ModifierType, b.ModifierType, cache))
return false;

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

static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b, IMetadataResolver? cache)
static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b, IMetadataResolver cache)
{
if (a is GenericInstanceType)
return IsParameterCompatibleWith ((GenericInstanceType) a, (GenericInstanceType) b, cache);
Expand All @@ -94,7 +91,7 @@ static bool IsParameterCompatibleWith (TypeSpecification a, TypeSpecification b,
return IsParameterCompatibleWith (a.ElementType, b.ElementType, cache);
}

static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceType b, IMetadataResolver? cache)
static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceType b, IMetadataResolver cache)
{
if (!IsParameterCompatibleWith (a.ElementType, b.ElementType, cache))
return false;
Expand All @@ -112,7 +109,7 @@ static bool IsParameterCompatibleWith (GenericInstanceType a, GenericInstanceTyp
return true;
}

static bool IsParameterCompatibleWith (TypeReference a, TypeReference b, IMetadataResolver? cache)
static bool IsParameterCompatibleWith (TypeReference a, TypeReference b, IMetadataResolver cache)
{
if (a is TypeSpecification || b is TypeSpecification) {
if (a.GetType () != b.GetType ())
Expand Down
109 changes: 49 additions & 60 deletions src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/TypeDefinitionRocks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,27 @@ namespace Java.Interop.Tools.Cecil {

public static class TypeDefinitionRocks {

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

public static TypeDefinition? GetBaseType (this TypeDefinition type, TypeDefinitionCache? cache) =>
GetBaseType (type, (IMetadataResolver?) cache);
public static TypeDefinition? GetBaseType (this TypeDefinition type, TypeDefinitionCache cache) =>
GetBaseType (type, (IMetadataResolver) cache);

public static TypeDefinition? GetBaseType (this TypeDefinition type, IMetadataResolver? resolver)
public static TypeDefinition? GetBaseType (this TypeDefinition type, IMetadataResolver resolver)
{
var bt = type.BaseType;
if (bt == null)
return null;
if (resolver != null)
return resolver.Resolve (bt);
return bt.Resolve ();
return resolver.Resolve (bt);
}

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

public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, TypeDefinitionCache? cache) =>
GetTypeAndBaseTypes (type, (IMetadataResolver?) cache);
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, TypeDefinitionCache cache) =>
GetTypeAndBaseTypes (type, (IMetadataResolver) cache);

public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, IMetadataResolver? resolver)
public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefinition type, IMetadataResolver resolver)
{
TypeDefinition? t = type;

Expand All @@ -41,14 +37,13 @@ public static IEnumerable<TypeDefinition> GetTypeAndBaseTypes (this TypeDefiniti
}
}

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

public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, TypeDefinitionCache? cache) =>
GetBaseTypes (type, (IMetadataResolver?) cache);
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, TypeDefinitionCache cache) =>
GetBaseTypes (type, (IMetadataResolver) cache);

public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, IMetadataResolver? resolver)
public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type, IMetadataResolver resolver)
{
TypeDefinition? t = type;

Expand All @@ -57,18 +52,17 @@ public static IEnumerable<TypeDefinition> GetBaseTypes (this TypeDefinition type
}
}

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

public static bool IsAssignableFrom (this TypeReference type, TypeReference c, TypeDefinitionCache? cache) =>
IsAssignableFrom (type, c, (IMetadataResolver?) cache);
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, TypeDefinitionCache cache) =>
IsAssignableFrom (type, c, (IMetadataResolver) cache);

public static bool IsAssignableFrom (this TypeReference type, TypeReference c, IMetadataResolver? resolver)
public static bool IsAssignableFrom (this TypeReference type, TypeReference c, IMetadataResolver resolver)
{
if (type.FullName == c.FullName)
return true;
var d = (resolver?.Resolve (c)) ?? c.Resolve ();
var d = resolver.Resolve (c);
if (d == null)
return false;
foreach (var t in d.GetTypeAndBaseTypes (resolver)) {
Expand All @@ -83,13 +77,12 @@ public static bool IsAssignableFrom (this TypeReference type, TypeReference c, I
return false;
}

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

public static bool IsSubclassOf (this TypeDefinition type, string typeName, TypeDefinitionCache? cache) =>
IsSubclassOf (type, typeName, (IMetadataResolver?) cache);
public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMetadataResolver? resolver)
public static bool IsSubclassOf (this TypeDefinition type, string typeName, TypeDefinitionCache cache) =>
IsSubclassOf (type, typeName, (IMetadataResolver) cache);
public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMetadataResolver resolver)
{
foreach (var t in type.GetTypeAndBaseTypes (resolver)) {
if (t.FullName == typeName) {
Expand All @@ -99,14 +92,13 @@ public static bool IsSubclassOf (this TypeDefinition type, string typeName, IMet
return false;
}

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

public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, TypeDefinitionCache? cache) =>
ImplementsInterface (type, interfaceName, (IMetadataResolver?) cache);
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, TypeDefinitionCache cache) =>
ImplementsInterface (type, interfaceName, (IMetadataResolver) cache);

public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, IMetadataResolver? resolver)
public static bool ImplementsInterface (this TypeDefinition type, string interfaceName, IMetadataResolver resolver)
{
foreach (var t in type.GetTypeAndBaseTypes (resolver)) {
foreach (var i in t.Interfaces) {
Expand All @@ -118,27 +110,25 @@ public static bool ImplementsInterface (this TypeDefinition type, string interfa
return false;
}

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

public static string GetPartialAssemblyName (this TypeReference type, TypeDefinitionCache? cache) =>
GetPartialAssemblyName (type, (IMetadataResolver?) cache);
public static string GetPartialAssemblyName (this TypeReference type, TypeDefinitionCache cache) =>
GetPartialAssemblyName (type, (IMetadataResolver) cache);

public static string GetPartialAssemblyName (this TypeReference type, IMetadataResolver? resolver)
public static string GetPartialAssemblyName (this TypeReference type, IMetadataResolver resolver)
{
TypeDefinition? def = (resolver?.Resolve (type)) ?? type.Resolve ();
TypeDefinition? def = resolver.Resolve (type);
return (def ?? type).Module.Assembly.Name.Name;
}

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

public static string GetPartialAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache? cache) =>
GetPartialAssemblyQualifiedName (type, (IMetadataResolver?) cache);
public static string GetPartialAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache cache) =>
GetPartialAssemblyQualifiedName (type, (IMetadataResolver) cache);

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

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

public static string GetAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache? cache) =>
GetAssemblyQualifiedName (type, (IMetadataResolver?) cache);
public static string GetAssemblyQualifiedName (this TypeReference type, TypeDefinitionCache cache) =>
GetAssemblyQualifiedName (type, (IMetadataResolver) cache);

public static string GetAssemblyQualifiedName (this TypeReference type, IMetadataResolver? resolver)
public static string GetAssemblyQualifiedName (this TypeReference type, IMetadataResolver resolver)
{
TypeDefinition? def = (resolver?.Resolve (type)) ?? type.Resolve ();
TypeDefinition? def = resolver.Resolve (type);
return string.Format ("{0}, {1}",
// Cecil likes to use '/' as the nested type separator, while
// Reflection uses '+' as the nested type separator. Use Reflection.
Expand Down
Loading

0 comments on commit cf80deb

Please sign in to comment.