From de7ed667202dc7c745e0685849ec6ad3c1157f76 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 11 Jan 2023 11:06:28 -0600 Subject: [PATCH 1/2] [Java.Interop.Tools.JavaCallableWrappers] use less System.Linq for CAs 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) 15.87ms System.Linq.il!System.Linq.Enumerable.Any(class System.Collections.Generic.IEnumerable`1,class System.Func`2) 1.98ms system.linq.il!System.Linq.Enumerable.Any(class System.Collections.Generic.IEnumerable`1) 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+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+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, saving about ~34ms for incremental builds of the .NET podcast app. --- .../CustomAttributeProviderRocks.cs | 12 +++++++++ .../JavaCallableWrapperGenerator.cs | 15 +++++------ .../JavaNativeTypeManager.cs | 26 ++++++++++++------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/CustomAttributeProviderRocks.cs b/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/CustomAttributeProviderRocks.cs index 0eaa830ed..11b498593 100644 --- a/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/CustomAttributeProviderRocks.cs +++ b/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/CustomAttributeProviderRocks.cs @@ -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 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 GetCustomAttributes (this ICustomAttributeProvider item, string attribute_fullname) { foreach (CustomAttribute custom_attribute in item.CustomAttributes) { diff --git a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs index cd489d00d..76f2bfc04 100644 --- a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs +++ b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs @@ -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; } @@ -205,8 +205,8 @@ void AddNestedTypes (TypeDefinition type) var curCtors = new List (); - 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"); } @@ -278,8 +278,8 @@ static void ExtractJavaNames (string jniName, out string package, out string typ void AddConstructors (TypeDefinition type, string? outerType, List? baseCtors, List 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); } @@ -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; } } diff --git a/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs b/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs index a8bba4531..f49008398 100644 --- a/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs +++ b/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs @@ -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 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 (); + 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 From de6104a2824541d7ac595d80f87663973338786d Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 11 Jan 2023 15:00:37 -0600 Subject: [PATCH 2/2] Whoops! `__self` typo --- .../JavaNativeTypeManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs b/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs index f49008398..176e478a4 100644 --- a/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs +++ b/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs @@ -719,7 +719,7 @@ internal static bool IsNonStaticInnerClass (TypeDefinition? type, IMetadataResol foreach (var method in baseType.Methods) { if (!method.IsConstructor || method.IsStatic) continue; - if (method.Parameters.Any (p => p.Name == "_self")) + if (method.Parameters.Any (p => p.Name == "__self")) return true; }