From 4c9e454ef86ec88097fcedec69ecd4736952c352 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 10 Sep 2018 16:01:37 -0500 Subject: [PATCH] [Xamarin.Android.Build.Tasks] simplify LINQ in ResolveAssemblies Looking at `ResolveAssemblies`, I noticed a lot of LINQ usage that could be simplified. We were using `HashSet` that contains a list of all assemblies, and the following code: if (!topLevel && assemblies.All (a => new AssemblyNameDefinition (a, null).Name != assembly.Name.Name)) assemblies.Add (fullPath); This was called from a *recursive* function and needlessly calls `new AssemblyNameDefinition ().Name` for every other assembly... Roughly a O(N^2) operation. So I changed the `HashSet` to be a `Dictionary`, where the "key" is the `assembly.Name.Name` and the value the full path to the assembly. I also fixed up usage of `assembly.MainModule.FullyQualifiedName`, which was deprecated mentioning to use `FileName` instead. Lastly, I fixed up several LINQ expressions on the `Outputs` of `ResolveAssemblies`. I rewrote them to use a single `foreach` loop instead of the equivalent LINQ that would loop over the list multiple times. ~~ Results ~~ The build time improvements here were "not zero", but also not huge. Testing the `tests/Xamarin.Forms-Performance-Integration` project, a build with no changes... Before: 188 ms ResolveAssemblies 1 calls After: 181 ms ResolveAssemblies 1 calls I think this change is still worthwhile, since: - The code is a bit cleaner, less LINQ for others to profile/look at, and I plan to refactor this task more soon. - The time savings is part of the dev-loop, and even builds with no changes. - There should be overall less allocations. Other changes: - Removed extraneous log messages for inputs/outputs --- .../Tasks/ResolveAssemblies.cs | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs index e492b10ee62..0a5330c44b1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs @@ -68,18 +68,10 @@ public override bool Execute () void Execute (DirectoryAssemblyResolver resolver) { - LogDebugMessage ("ResolveAssemblies Task"); - LogDebugMessage (" ReferenceAssembliesDirectory: {0}", ReferenceAssembliesDirectory); - LogDebugMessage (" I18nAssemblies: {0}", I18nAssemblies); - LogDebugMessage (" LinkMode: {0}", LinkMode); - LogDebugTaskItems (" Assemblies:", Assemblies); - LogDebugMessage (" ProjectAssetFile: {0}", ProjectAssetFile); - LogDebugMessage (" TargetMoniker: {0}", TargetMoniker); - foreach (var dir in ReferenceAssembliesDirectory.Split (new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries)) resolver.SearchDirectories.Add (dir); - var assemblies = new HashSet (); + var assemblies = new Dictionary (); var topAssemblyReferences = new List (); var logger = new NuGetLogger((s) => { @@ -113,7 +105,7 @@ void Execute (DirectoryAssemblyResolver resolver) } } topAssemblyReferences.Add (assemblyDef); - assemblies.Add (Path.GetFullPath (assemblyDef.MainModule.FullyQualifiedName)); + assemblies [assemblyDef.Name.Name] = Path.GetFullPath (assemblyDef.MainModule.FileName); } } catch (Exception ex) { LogError ("Exception while loading assemblies: {0}", ex); @@ -130,21 +122,30 @@ void Execute (DirectoryAssemblyResolver resolver) // Add I18N assemblies if needed AddI18nAssemblies (resolver, assemblies); - ResolvedAssemblies = assemblies.Select (a => new TaskItem (a)).ToArray (); - ResolvedSymbols = assemblies.Select (a => a + ".mdb").Where (a => File.Exists (a)).Select (a => new TaskItem (a)).ToArray (); - ResolvedSymbols = ResolvedSymbols.Concat ( - assemblies.Select (a => Path.ChangeExtension (a, "pdb")) - .Where (a => File.Exists (a) && Files.IsPortablePdb (a)) - .Select (a => new TaskItem (a))) - .ToArray (); - ResolvedFrameworkAssemblies = ResolvedAssemblies.Where (p => MonoAndroidHelper.IsFrameworkAssembly (p.ItemSpec, true)).ToArray (); - ResolvedUserAssemblies = ResolvedAssemblies.Where (p => !MonoAndroidHelper.IsFrameworkAssembly (p.ItemSpec, true)).ToArray (); + var resolvedAssemblies = new List (assemblies.Count); + var resolvedSymbols = new List (assemblies.Count); + var resolvedFrameworkAssemblies = new List (assemblies.Count); + var resolvedUserAssemblies = new List (assemblies.Count); + foreach (var assembly in assemblies.Values) { + var mdb = assembly + ".mdb"; + var pdb = Path.ChangeExtension (assembly, "pdb"); + if (File.Exists (mdb)) + resolvedSymbols.Add (new TaskItem (mdb)); + if (File.Exists (pdb) && Files.IsPortablePdb (pdb)) + resolvedSymbols.Add (new TaskItem (pdb)); + var assemblyItem = new TaskItem (assembly); + resolvedAssemblies.Add (assemblyItem); + if (MonoAndroidHelper.IsFrameworkAssembly (assembly, checkSdkPath: true)) { + resolvedFrameworkAssemblies.Add (assemblyItem); + } else { + resolvedUserAssemblies.Add (assemblyItem); + } + } + ResolvedAssemblies = resolvedAssemblies.ToArray (); + ResolvedSymbols = resolvedSymbols.ToArray (); + ResolvedFrameworkAssemblies = resolvedFrameworkAssemblies.ToArray (); + ResolvedUserAssemblies = resolvedUserAssemblies.ToArray (); ResolvedDoNotPackageAttributes = do_not_package_atts.ToArray (); - - LogDebugTaskItems (" [Output] ResolvedAssemblies:", ResolvedAssemblies); - LogDebugTaskItems (" [Output] ResolvedUserAssemblies:", ResolvedUserAssemblies); - LogDebugTaskItems (" [Output] ResolvedFrameworkAssemblies:", ResolvedFrameworkAssemblies); - LogDebugTaskItems (" [Output] ResolvedDoNotPackageAttributes:", ResolvedDoNotPackageAttributes); } readonly List do_not_package_atts = new List (); @@ -185,14 +186,14 @@ AssemblyDefinition ResolveRuntimeAssemblyForReferenceAssembly (LockFile lockFile return null; } - void AddAssemblyReferences (DirectoryAssemblyResolver resolver, ICollection assemblies, AssemblyDefinition assembly, List resolutionPath) + void AddAssemblyReferences (DirectoryAssemblyResolver resolver, Dictionary assemblies, AssemblyDefinition assembly, List resolutionPath) { - var fqname = assembly.MainModule.FullyQualifiedName; - var fullPath = Path.GetFullPath (fqname); + var assemblyName = assembly.Name.Name; + var fullPath = Path.GetFullPath (assembly.MainModule.FileName); // Don't repeat assemblies we've already done bool topLevel = resolutionPath == null; - if (!topLevel && assemblies.Contains (fullPath)) + if (!topLevel && assemblies.ContainsKey (assemblyName)) return; if (resolutionPath == null) @@ -210,8 +211,8 @@ void AddAssemblyReferences (DirectoryAssemblyResolver resolver, ICollection new AssemblyNameDefinition (a, null).Name != assembly.Name.Name)) - assemblies.Add (fullPath); + if (!topLevel) + assemblies [assemblyName] = fullPath; // Recurse into each referenced assembly foreach (AssemblyNameReference reference in assembly.MainModule.AssemblyReferences) { @@ -256,7 +257,7 @@ static LinkModes ParseLinkMode (string linkmode) return mode; } - void AddI18nAssemblies (DirectoryAssemblyResolver resolver, ICollection assemblies) + void AddI18nAssemblies (DirectoryAssemblyResolver resolver, Dictionary assemblies) { var i18n = Linker.ParseI18nAssemblies (I18nAssemblies); var link = ParseLinkMode (LinkMode); @@ -265,28 +266,28 @@ void AddI18nAssemblies (DirectoryAssemblyResolver resolver, ICollection if (i18n == Mono.Linker.I18nAssemblies.None) return; - assemblies.Add (ResolveI18nAssembly (resolver, "I18N")); + ResolveI18nAssembly (resolver, "I18N", assemblies); if (i18n.HasFlag (Mono.Linker.I18nAssemblies.CJK)) - assemblies.Add (ResolveI18nAssembly (resolver, "I18N.CJK")); + ResolveI18nAssembly (resolver, "I18N.CJK", assemblies); if (i18n.HasFlag (Mono.Linker.I18nAssemblies.MidEast)) - assemblies.Add (ResolveI18nAssembly (resolver, "I18N.MidEast")); + ResolveI18nAssembly (resolver, "I18N.MidEast", assemblies); if (i18n.HasFlag (Mono.Linker.I18nAssemblies.Other)) - assemblies.Add (ResolveI18nAssembly (resolver, "I18N.Other")); + ResolveI18nAssembly (resolver, "I18N.Other", assemblies); if (i18n.HasFlag (Mono.Linker.I18nAssemblies.Rare)) - assemblies.Add (ResolveI18nAssembly (resolver, "I18N.Rare")); + ResolveI18nAssembly (resolver, "I18N.Rare", assemblies); if (i18n.HasFlag (Mono.Linker.I18nAssemblies.West)) - assemblies.Add (ResolveI18nAssembly (resolver, "I18N.West")); + ResolveI18nAssembly (resolver, "I18N.West", assemblies); } - string ResolveI18nAssembly (DirectoryAssemblyResolver resolver, string name) + void ResolveI18nAssembly (DirectoryAssemblyResolver resolver, string name, Dictionary assemblies) { var assembly = resolver.Resolve (AssemblyNameReference.Parse (name)); - return Path.GetFullPath (assembly.MainModule.FullyQualifiedName); + assemblies [name] = Path.GetFullPath (assembly.MainModule.FileName); } } }