From aeeb51ae1e0ea8263947adc601c04fb925cd86bb Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 4 Sep 2024 08:40:53 -0500 Subject: [PATCH] [Xamarin.Android.Build.Tasks] reduce System.Linq usage in ManifestDocument (#9281) Profiling an incremental build of a `dotnet new maui` project with a XAML change, one thing I saw was time spent in `` MSBuild task, and the `ManifestDocument` class: 67.57ms (1.80%) xamarin.android.build.tasks!Xamarin.Android.Tasks.ManifestDocument.Merge(class Microsoft.Build.Utilities.TaskLoggingHel 19.99ms (0.53%) xamarin.android.build.tasks!Xamarin.Android.Tasks.ManifestDocument.CreateApplicationElement(class System.Xml.Linq.XEleme... Reviewing the code, there was a decent amount of LINQ usage that would create extra allocations for closures, such as: var properties = Assemblies.SelectMany (path => PropertyAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path), cache)) I unrolled the System.Linq usage, and just used plain `foreach` loops instead. I also found some places that did: if (!attrs.Any ()) yield break; We could just remove these as the `yield return` wouldn't return if `attrs` is empty *anyway*. After removing this code, I see a small improvement: 62.56ms (1.80%) xamarin.android.build.tasks!Xamarin.Android.Tasks.ManifestDocument.Merge(class Microsoft.Build.Utilities.TaskLoggingHe... 16.03ms (0.46%) xamarin.android.build.tasks!Xamarin.Android.Tasks.ManifestDocument.CreateApplicationElement(class System.Xml.Linq.XEleme... This probably saves ~5ms to incremental builds, but seems like a straightforward change. --- .../GrantUriPermissionAttribute.Partial.cs | 2 - .../IntentFilterAttribute.Partial.cs | 2 - .../Mono.Android/MetaDataAttribute.Partial.cs | 2 - .../Mono.Android/PropertyAttribute.Partial.cs | 2 - .../Utilities/ManifestDocument.cs | 117 ++++++++++-------- 5 files changed, 66 insertions(+), 59 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Mono.Android/GrantUriPermissionAttribute.Partial.cs b/src/Xamarin.Android.Build.Tasks/Mono.Android/GrantUriPermissionAttribute.Partial.cs index 0526ac2d8ed..86ba78e43c7 100644 --- a/src/Xamarin.Android.Build.Tasks/Mono.Android/GrantUriPermissionAttribute.Partial.cs +++ b/src/Xamarin.Android.Build.Tasks/Mono.Android/GrantUriPermissionAttribute.Partial.cs @@ -17,8 +17,6 @@ partial class GrantUriPermissionAttribute { public static IEnumerable FromTypeDefinition (TypeDefinition type, TypeDefinitionCache cache) { IEnumerable attrs = type.GetCustomAttributes ("Android.Content.GrantUriPermissionAttribute"); - if (!attrs.Any ()) - yield break; foreach (CustomAttribute attr in attrs) { var self = new GrantUriPermissionAttribute (); self.specified = mapping.Load (self, attr, cache); diff --git a/src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs b/src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs index fb9e76ad9d4..966edb0497c 100644 --- a/src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs +++ b/src/Xamarin.Android.Build.Tasks/Mono.Android/IntentFilterAttribute.Partial.cs @@ -69,8 +69,6 @@ static string[] ToStringArray (object value) public static IEnumerable FromTypeDefinition (TypeDefinition type, IMetadataResolver cache) { IEnumerable attrs = type.GetCustomAttributes ("Android.App.IntentFilterAttribute"); - if (!attrs.Any ()) - yield break; foreach (CustomAttribute attr in attrs) { var self = new IntentFilterAttribute (ToStringArray (attr.ConstructorArguments [0].Value)); foreach (var e in attr.Properties) { diff --git a/src/Xamarin.Android.Build.Tasks/Mono.Android/MetaDataAttribute.Partial.cs b/src/Xamarin.Android.Build.Tasks/Mono.Android/MetaDataAttribute.Partial.cs index 77989ebbf1c..8bdd9edf0eb 100644 --- a/src/Xamarin.Android.Build.Tasks/Mono.Android/MetaDataAttribute.Partial.cs +++ b/src/Xamarin.Android.Build.Tasks/Mono.Android/MetaDataAttribute.Partial.cs @@ -17,8 +17,6 @@ partial class MetaDataAttribute { public static IEnumerable FromCustomAttributeProvider (ICustomAttributeProvider type, TypeDefinitionCache cache) { IEnumerable attrs = type.GetCustomAttributes ("Android.App.MetaDataAttribute"); - if (!attrs.Any ()) - yield break; foreach (CustomAttribute attr in attrs) { var self = new MetaDataAttribute ((string) attr.ConstructorArguments [0].Value); self.specified = mapping.Load (self, attr, cache); diff --git a/src/Xamarin.Android.Build.Tasks/Mono.Android/PropertyAttribute.Partial.cs b/src/Xamarin.Android.Build.Tasks/Mono.Android/PropertyAttribute.Partial.cs index f6b59eda257..e4ea03f580f 100644 --- a/src/Xamarin.Android.Build.Tasks/Mono.Android/PropertyAttribute.Partial.cs +++ b/src/Xamarin.Android.Build.Tasks/Mono.Android/PropertyAttribute.Partial.cs @@ -17,8 +17,6 @@ partial class PropertyAttribute { public static IEnumerable FromCustomAttributeProvider (ICustomAttributeProvider type, TypeDefinitionCache cache) { IEnumerable attrs = type.GetCustomAttributes ("Android.App.PropertyAttribute"); - if (!attrs.Any ()) - yield break; foreach (CustomAttribute attr in attrs) { var self = new PropertyAttribute ((string) attr.ConstructorArguments [0].Value); self.specified = mapping.Load (self, attr, cache); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs index c9a40e43025..b954bb47e97 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs @@ -572,33 +572,45 @@ XElement CreateApplicationElement (XElement manifest, string applicationClass, L { var application = manifest.Descendants ("application").FirstOrDefault (); - List assemblyAttr = - Assemblies.Select (path => ApplicationAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path), cache)) - .Where (attr => attr != null) - .ToList (); - List metadata = - Assemblies.SelectMany (path => MetaDataAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path), cache)) - .Where (attr => attr != null) - .ToList (); - var properties = - Assemblies.SelectMany (path => PropertyAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path), cache)) - .Where (attr => attr != null) - .ToList (); - var usesLibraryAttr = - Assemblies.SelectMany (path => UsesLibraryAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path), cache)) - .Where (attr => attr != null); - var usesConfigurationAttr = - Assemblies.SelectMany (path => UsesConfigurationAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path), cache)) - .Where (attr => attr != null); + List assemblyAttr = []; + List metadata = []; + List properties = []; + List usesLibraryAttr = []; + List usesConfigurationAttr = []; + foreach (var assemblyPath in Assemblies) { + var assembly = Resolver.GetAssembly (assemblyPath); + if (ApplicationAttribute.FromCustomAttributeProvider (assembly, cache) is ApplicationAttribute a) { + assemblyAttr.Add (a); + } + foreach (var m in MetaDataAttribute.FromCustomAttributeProvider (assembly, cache)) { + if (m is null) + continue; + metadata.Add (m); + } + foreach (var p in PropertyAttribute.FromCustomAttributeProvider (assembly, cache)) { + if (p is null) + continue; + properties.Add (p); + } + foreach (var u in UsesLibraryAttribute.FromCustomAttributeProvider (assembly, cache)) { + if (u is null) + continue; + usesLibraryAttr.Add (u); + } + foreach (var uc in UsesConfigurationAttribute.FromCustomAttributeProvider (assembly, cache)) { + if (uc is null) + continue; + usesConfigurationAttr.Add (uc); + } + } + if (assemblyAttr.Count > 1) throw new InvalidOperationException ("There can be only one [assembly:Application] attribute defined."); - List typeAttr = new List (); - List typeUsesLibraryAttr = new List (); - List typeUsesConfigurationAttr = new List (); + List typeAttr = []; foreach (TypeDefinition t in subclasses) { ApplicationAttribute aa = ApplicationAttribute.FromCustomAttributeProvider (t, cache); - if (aa == null) + if (aa is null) continue; if (!t.IsSubclassOf ("Android.App.Application", cache)) @@ -608,7 +620,7 @@ XElement CreateApplicationElement (XElement manifest, string applicationClass, L metadata.AddRange (MetaDataAttribute.FromCustomAttributeProvider (t, cache)); properties.AddRange (PropertyAttribute.FromCustomAttributeProvider (t, cache)); - typeUsesLibraryAttr.AddRange (UsesLibraryAttribute.FromCustomAttributeProvider (t, cache)); + usesLibraryAttr.AddRange (UsesLibraryAttribute.FromCustomAttributeProvider (t, cache)); } if (typeAttr.Count > 1) @@ -619,12 +631,6 @@ XElement CreateApplicationElement (XElement manifest, string applicationClass, L throw new InvalidOperationException ("Application cannot have both a type with an [Application] attribute and an [assembly:Application] attribute."); ApplicationAttribute appAttr = assemblyAttr.SingleOrDefault () ?? typeAttr.SingleOrDefault (); - var ull1 = usesLibraryAttr ?? Array.Empty (); - var ull2 = typeUsesLibraryAttr.AsEnumerable () ?? Array.Empty (); - var usesLibraryAttrs = ull1.Concat (ull2); - var ucl1 = usesConfigurationAttr ?? Array.Empty(); - var ucl2 = typeUsesConfigurationAttr.AsEnumerable () ?? Array.Empty (); - var usesConfigurationattrs = ucl1.Concat (ucl2); bool needManifestAdd = true; if (appAttr != null) { @@ -643,14 +649,18 @@ XElement CreateApplicationElement (XElement manifest, string applicationClass, L application = new XElement ("application"); else needManifestAdd = false; - application.Add (metadata.Select (md => md.ToElement (PackageName, cache))); - application.Add (properties.Select (md => md.ToElement (PackageName, cache))); + foreach (var m in metadata) { + application.Add (m.ToElement (PackageName, cache)); + } + foreach (var p in properties) { + application.Add (p.ToElement (PackageName, cache)); + } if (needManifestAdd) manifest.Add (application); - AddUsesLibraries (application, usesLibraryAttrs, cache); - AddUsesConfigurations (application, usesConfigurationattrs, cache); + AddUsesLibraries (application, usesLibraryAttr, cache); + AddUsesConfigurations (application, usesConfigurationAttr, cache); if (applicationClass != null && application.Attribute (androidNs + "name") == null) application.Add (new XAttribute (androidNs + "name", applicationClass)); @@ -780,16 +790,18 @@ XElement ToElement (TypeDefinition type, string name, Func metadata = MetaDataAttribute.FromCustomAttributeProvider (type, cache); - IEnumerable intents = IntentFilterAttribute.FromTypeDefinition (type, cache); - var properties = PropertyAttribute.FromCustomAttributeProvider (type, cache); - XElement element = toElement (attr); if (element.Attribute (attName) == null) element.Add (new XAttribute (attName, name)); - element.Add (metadata.Select (md => md.ToElement (PackageName, cache))); - element.Add (intents.Select (intent => intent.ToElement (PackageName))); - element.Add (properties.Select (md => md.ToElement (PackageName, cache))); + foreach (var m in MetaDataAttribute.FromCustomAttributeProvider (type, cache)) { + element.Add (m.ToElement (PackageName, cache)); + } + foreach (var i in IntentFilterAttribute.FromTypeDefinition (type, cache)) { + element.Add (i.ToElement (PackageName)); + } + foreach (var p in PropertyAttribute.FromCustomAttributeProvider (type, cache)) { + element.Add (p.ToElement (PackageName, cache)); + } if (update != null) update (attr, element); return element; @@ -801,18 +813,21 @@ XElement ToProviderElement (TypeDefinition type, string name, TypeDefinitionCach if (attr == null) return null; - IEnumerable metadata = MetaDataAttribute.FromCustomAttributeProvider (type, cache); - IEnumerable grants = GrantUriPermissionAttribute.FromTypeDefinition (type, cache); - IEnumerable intents = IntentFilterAttribute.FromTypeDefinition (type, cache); - var properties = PropertyAttribute.FromCustomAttributeProvider (type, cache); - XElement element = attr.ToElement (PackageName, cache); if (element.Attribute (attName) == null) element.Add (new XAttribute (attName, name)); - element.Add (metadata.Select (md => md.ToElement (PackageName, cache))); - element.Add (grants.Select (intent => intent.ToElement (PackageName, cache))); - element.Add (intents.Select (intent => intent.ToElement (PackageName))); - element.Add (properties.Select (md => md.ToElement (PackageName, cache))); + foreach (var m in MetaDataAttribute.FromCustomAttributeProvider (type, cache)) { + element.Add (m.ToElement (PackageName, cache)); + } + foreach (var g in GrantUriPermissionAttribute.FromTypeDefinition (type, cache)) { + element.Add (g.ToElement (PackageName, cache)); + } + foreach (var i in IntentFilterAttribute.FromTypeDefinition (type, cache)) { + element.Add (i.ToElement (PackageName)); + } + foreach (var p in PropertyAttribute.FromCustomAttributeProvider (type, cache)) { + element.Add (p.ToElement (PackageName, cache)); + } return element; } @@ -886,13 +901,13 @@ void AddUsesPermissions (XElement application, TypeDefinitionCache cache) if (!application.Parent.Descendants ("uses-permission").Any (x => (string)x.Attribute (attName) == upa.Name)) application.AddBeforeSelf (upa.ToElement (PackageName, cache)); } - void AddUsesConfigurations (XElement application, IEnumerable configs, TypeDefinitionCache cache) + void AddUsesConfigurations (XElement application, List configs, TypeDefinitionCache cache) { foreach (var uca in configs) application.Add (uca.ToElement (PackageName, cache)); } - void AddUsesLibraries (XElement application, IEnumerable libraries, TypeDefinitionCache cache) + void AddUsesLibraries (XElement application, List libraries, TypeDefinitionCache cache) { // Add unique libraries to the manifest foreach (var ula in libraries)