Skip to content

Commit

Permalink
[Java.Interop.Tools.JavaCallableWrappers] use less System.Linq for CAs (
Browse files Browse the repository at this point in the history
#1072)

Context: https://github.com/microsoft/dotnet-podcasts/tree/net8.0

When building the .NET Podcast sample for .NET 8, profiling an
incremental build with a `.xaml` change I noticed:

	80.42ms java.interop.tools.javacallablewrappers!Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.IsNonStaticInnerClass(...

There was a double-nested usage of System.Linq via a
`GetBaseConstructors()` method, so I "unrolled" this to a plain
`foreach` loop.

After this change:

	61.50ms java.interop.tools.javacallablewrappers!Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.IsNonStaticInnerClass(...

This made me review places using System.Linq `.Any()` calls:

	59.78ms System.Linq.il!System.Linq.Enumerable.Any(class System.Collections.Generic.IEnumerable`1<!!0>)
	15.87ms System.Linq.il!System.Linq.Enumerable.Any(class System.Collections.Generic.IEnumerable`1<!!0>,class System.Func`2<!!0,bool>)
	 1.98ms system.linq.il!System.Linq.Enumerable.Any(class System.Collections.Generic.IEnumerable`1<!!0>)

Which I was able to track down to calls to an extension method like:

	CustomAttributeProviderRocks.GetCustomAttributes().Any()

I created a new `CustomAttributeProviderRocks.AnyCustomAttributes()`
extension method, which is a bit better because:

  * We avoid a `yield return` & related compiler machinery.

  * We avoid allocating custom attribute objects in some cases, as
    System.Linq's `.Any()` will enumerate and create at least one.

Before:

	107.90ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.CustomAttributeProviderRocks+<GetCustomAttributes>d__1.MoveNext()
	  3.80ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.CustomAttributeProviderRocks.GetCustomAttributes(class Mono.Cecil.ICus...

After:

	 58.58ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.CustomAttributeProviderRocks.AnyCustomAttributes(class Mono.Cecil.ICustomAttributeProvider,class System.Type)
	 36.01ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.CustomAttributeProviderRocks+<GetCustomAttributes>d__3.MoveNext()
	  1.97ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.CustomAttributeProviderRocks.GetCustomAttributes(class Mono.Cecil.ICus...

These changes are about:

  * `IsNonStaticInnerClass`: ~19ms faster
  * `CustomAttributeProviderRocks (Any)`: ~15ms faster

Overall, saves about ~34ms for incremental builds of the
.NET podcast app.
  • Loading branch information
jonathanpeppers committed Jan 18, 2023
1 parent bde306d commit 1366d99
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 17 deletions.
Expand Up @@ -8,11 +8,23 @@ namespace Java.Interop.Tools.Cecil {

public static class CustomAttributeProviderRocks
{
public static bool AnyCustomAttributes (this ICustomAttributeProvider item, Type attribute) =>
item.AnyCustomAttributes (attribute.FullName);

public static IEnumerable<CustomAttribute> GetCustomAttributes (this ICustomAttributeProvider item, Type attribute)
{
return item.GetCustomAttributes (attribute.FullName);
}

public static bool AnyCustomAttributes (this ICustomAttributeProvider item, string attribute_fullname)
{
foreach (CustomAttribute custom_attribute in item.CustomAttributes) {
if (custom_attribute.Constructor.DeclaringType.FullName == attribute_fullname)
return true;
}
return false;
}

public static IEnumerable<CustomAttribute> GetCustomAttributes (this ICustomAttributeProvider item, string attribute_fullname)
{
foreach (CustomAttribute custom_attribute in item.CustomAttributes) {
Expand Down
Expand Up @@ -164,10 +164,10 @@ void AddNestedTypes (TypeDefinition type)
var baseRegisteredMethod = GetBaseRegisteredMethod (minfo);
if (baseRegisteredMethod != null)
AddMethod (baseRegisteredMethod, minfo);
else if (GetExportFieldAttributes (minfo).Any ()) {
else if (minfo.AnyCustomAttributes (typeof(ExportFieldAttribute))) {
AddMethod (null, minfo);
HasExport = true;
} else if (GetExportAttributes (minfo).Any ()) {
} else if (minfo.AnyCustomAttributes (typeof (ExportAttribute))) {
AddMethod (null, minfo);
HasExport = true;
}
Expand Down Expand Up @@ -205,8 +205,8 @@ void AddNestedTypes (TypeDefinition type)

var curCtors = new List<MethodDefinition> ();

foreach (MethodDefinition minfo in type.Methods.Where (m => m.IsConstructor)) {
if (GetExportAttributes (minfo).Any ()) {
foreach (MethodDefinition minfo in type.Methods) {
if (minfo.IsConstructor && minfo.AnyCustomAttributes (typeof (ExportAttribute))) {
if (minfo.IsStatic) {
// Diagnostic.Warning (log, "ExportAttribute does not work on static constructor");
}
Expand Down Expand Up @@ -278,8 +278,8 @@ static void ExtractJavaNames (string jniName, out string package, out string typ

void AddConstructors (TypeDefinition type, string? outerType, List<MethodDefinition>? baseCtors, List<MethodDefinition> curCtors, bool onlyRegisteredOrExportedCtors)
{
foreach (MethodDefinition ctor in type.Methods.Where (m => m.IsConstructor && !m.IsStatic))
if (!GetExportAttributes (ctor).Any ())
foreach (MethodDefinition ctor in type.Methods)
if (ctor.IsConstructor && !ctor.IsStatic && !ctor.AnyCustomAttributes (typeof (ExportAttribute)))
AddConstructor (ctor, type, outerType, baseCtors, curCtors, onlyRegisteredOrExportedCtors, false);
}

Expand Down Expand Up @@ -342,8 +342,7 @@ void AddConstructor (MethodDefinition ctor, TypeDefinition type, string? outerTy
while ((bmethod = method.GetBaseDefinition (cache)) != method) {
method = bmethod;

var attributes = method.GetCustomAttributes (typeof (RegisterAttribute));
if (attributes.Any ()) {
if (method.AnyCustomAttributes (typeof (RegisterAttribute))) {
return method;
}
}
Expand Down
Expand Up @@ -710,16 +710,24 @@ internal static bool IsNonStaticInnerClass (TypeDefinition? type, IMetadataResol
if (!type.DeclaringType.IsSubclassOf ("Java.Lang.Object", cache))
return false;

return GetBaseConstructors (type, cache)
.Any (ctor => ctor.Parameters.Any (p => p.Name == "__self"));
}
foreach (var baseType in type.GetBaseTypes (cache)) {
if (baseType == null)
continue;
if (!baseType.AnyCustomAttributes (typeof (RegisterAttribute)))
continue;

static IEnumerable<MethodDefinition> GetBaseConstructors (TypeDefinition type, IMetadataResolver cache)
{
var baseType = type.GetBaseTypes (cache).FirstOrDefault (t => t.GetCustomAttributes (typeof (RegisterAttribute)).Any ());
if (baseType != null)
return baseType.Methods.Where (m => m.IsConstructor && !m.IsStatic);
return Enumerable.Empty<MethodDefinition> ();
foreach (var method in baseType.Methods) {
if (!method.IsConstructor || method.IsStatic)
continue;
if (method.Parameters.Any (p => p.Name == "__self"))
return true;
}

// Stop at the first base type with [Register]
break;
}

return false;
}
#endif // HAVE_CECIL

Expand Down

0 comments on commit 1366d99

Please sign in to comment.