From 18730d6862f84edb31f6a63dc4832e9bafed12e5 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 24 Jan 2020 10:23:05 -0600 Subject: [PATCH] [WIP] remove NuGet & recursive lookup One of the remaining pain points in the Xamarin.Android build is the `` MSBuild task. 1. It isn't particularly fast and runs on *every* build. During the devloop it runs twice for `Build` and `Install`. It runs for DTBs, designer builds, too. 2. It breaks for certain NuGet packages as mentioned on a few Github issues: https://github.com/xamarin/xamarin-android/issues/3898 https://github.com/xamarin/xamarin-android/issues/4074 After some research and looking at build logs, we may be able to rework the behavior: 1. Remove NuGet-related code and recursive assembly searching. 2. Use `@(_ReferencePath)` and `@(_ReferenceDependencyPaths)`. 3. Include assemblies even if they have `%(ResolvedFrom)` == `ImplicitlyExpandDesignTimeFacades`. With these changes I am still able to build SmartHotel360, and the `` is an order of magnitude quicker: Before: 310 ms ResolveAssemblies 1 calls After: 44 ms ResolveAssemblies 1 calls So this would save ~532ms (x2 620ms -> 88ms) from the dev-loop. This is a big WIP, as I need to figure out what types of projects this might break. Let's see what happens on CI! --- .../Tasks/ResolveAssemblies.cs | 205 ++---------------- .../Xamarin.Android.Build.Tests.csproj | 1 + .../Utilities/MetadataResolver.cs | 6 +- .../Utilities/MonoAndroidHelper.cs | 19 +- .../Utilities/NuGetLogger.cs | 34 --- .../Xamarin.Android.Build.Tasks.csproj | 13 +- .../Xamarin.Android.Build.Tasks.targets | 10 - .../Xamarin.Android.Common.targets | 7 +- 8 files changed, 44 insertions(+), 251 deletions(-) delete mode 100644 src/Xamarin.Android.Build.Tasks/Utilities/NuGetLogger.cs diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs index 7d788b22d4f..25b9562a24f 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs @@ -4,16 +4,12 @@ using Microsoft.Build.Framework; using Microsoft.Build.Utilities; using MonoDroid.Tuner; -using NuGet.Frameworks; -using NuGet.ProjectModel; using System; using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection.Metadata; -using System.Text; using Xamarin.Android.Tools; -using Xamarin.Build; namespace Xamarin.Android.Tasks { @@ -39,7 +35,6 @@ public class ResolveAssemblies : AndroidAsyncTask public string TargetMoniker { get; set; } public string I18nAssemblies { get; set; } - public string LinkMode { get; set; } // The user's assemblies, and all referenced assemblies [Output] @@ -70,55 +65,37 @@ void Execute (MetadataResolver resolver) foreach (var dir in ReferenceAssembliesDirectory.Split (new char [] { ';' }, StringSplitOptions.RemoveEmptyEntries)) resolver.AddSearchDirectory (dir); - var assemblies = new Dictionary (Assemblies.Length); - var topAssemblyReferences = new List (Assemblies.Length); - var logger = new NuGetLogger((s) => { - LogDebugMessage ("{0}", s); - }); - - LockFile lockFile = null; - if (!string.IsNullOrEmpty (ProjectAssetFile) && File.Exists (ProjectAssetFile)) { - lockFile = LockFileUtilities.GetLockFile (ProjectAssetFile, logger); - } - + var assemblies = new Dictionary (Assemblies.Length, StringComparer.Ordinal); try { foreach (var assembly in Assemblies) { - // Add each user assembly and all referenced assemblies (recursive) - string resolved_assembly = resolver.Resolve (assembly.ItemSpec); - bool refAssembly = !string.IsNullOrEmpty (assembly.GetMetadata ("NuGetPackageId")) && resolved_assembly.Contains ($"{Path.DirectorySeparatorChar}ref{Path.DirectorySeparatorChar}"); - if (refAssembly || MonoAndroidHelper.IsReferenceAssembly (resolved_assembly)) { - // Resolve "runtime" library - if (lockFile != null) - resolved_assembly = ResolveRuntimeAssemblyForReferenceAssembly (lockFile, assembly.ItemSpec); - if (lockFile == null || resolved_assembly == null) { - var file = resolved_assembly ?? assembly.ItemSpec; - LogCodedWarning ("XA0107", file, 0, "Ignoring Reference Assembly `{0}`.", file); - continue; - } + string assemblyFileName = Path.GetFileNameWithoutExtension (assembly.ItemSpec); + if (assemblies.ContainsKey (assemblyFileName)) + continue; + + var reader = resolver.GetAssemblyReader (assembly.ItemSpec, out string assemblyPath); + if (!string.IsNullOrEmpty (assembly.GetMetadata ("NuGetPackageId")) && assemblyPath.Contains ($"{Path.DirectorySeparatorChar}ref{Path.DirectorySeparatorChar}")) { + LogDebugMessage ($"Skipping NuGet reference assembly: {assembly.ItemSpec}"); + continue; + } + var assemblyDefinition = reader.GetAssemblyDefinition (); + if (MonoAndroidHelper.IsReferenceAssembly (reader, assemblyDefinition)) { + LogDebugMessage ($"Skipping reference assembly: {assembly.ItemSpec}"); + continue; } - LogDebugMessage ($"Adding {resolved_assembly} to topAssemblyReferences"); - topAssemblyReferences.Add (resolved_assembly); - resolver.AddSearchDirectory (Path.GetDirectoryName (resolved_assembly)); + CheckAssemblyAttributes (assemblyDefinition, reader); var taskItem = new TaskItem (assembly) { - ItemSpec = Path.GetFullPath (resolved_assembly), + ItemSpec = Path.GetFullPath (assemblyPath), }; if (string.IsNullOrEmpty (taskItem.GetMetadata ("ReferenceAssembly"))) { taskItem.SetMetadata ("ReferenceAssembly", taskItem.ItemSpec); } - string assemblyName = Path.GetFileNameWithoutExtension (resolved_assembly); - assemblies [assemblyName] = taskItem; + assemblies [assemblyFileName] = taskItem; + resolver.AddSearchDirectory (Path.GetDirectoryName (assemblyPath)); } } catch (Exception ex) { LogError ("Exception while loading assemblies: {0}", ex); return; } - try { - foreach (var assembly in topAssemblyReferences) - AddAssemblyReferences (resolver, assemblies, assembly, null); - } catch (Exception ex) { - LogError ("Exception while loading assemblies: {0}", ex); - return; - } // Add I18N assemblies if needed AddI18nAssemblies (resolver, assemblies); @@ -158,124 +135,9 @@ void Execute (MetadataResolver resolver) readonly List do_not_package_atts = new List (); readonly Dictionary api_levels = new Dictionary (); - int indent = 2; - - string ResolveRuntimeAssemblyForReferenceAssembly (LockFile lockFile, string assemblyPath) - { - if (string.IsNullOrEmpty(TargetMoniker)) - return null; - - var framework = NuGetFramework.Parse (TargetMoniker); - if (framework == null) { - LogCodedWarning ("XA0118", $"Could not parse '{TargetMoniker}'"); - return null; - } - var target = lockFile.GetTarget (framework, string.Empty); - if (target == null) { - LogCodedWarning ("XA0118", $"Could not resolve target for '{TargetMoniker}'"); - return null; - } - foreach (var folder in lockFile.PackageFolders) { - var path = assemblyPath.Replace (folder.Path, string.Empty); - if (path.StartsWith ($"{Path.DirectorySeparatorChar}")) - path = path.Substring (1); - var libraryPath = lockFile.Libraries.FirstOrDefault (x => path.StartsWith (x.Path.Replace('/', Path.DirectorySeparatorChar), StringComparison.OrdinalIgnoreCase)); - if (libraryPath == null) - continue; - var library = target.Libraries.FirstOrDefault (x => String.Compare (x.Name, libraryPath.Name, StringComparison.OrdinalIgnoreCase) == 0); - if (libraryPath == null) - continue; - var runtime = library.RuntimeAssemblies.FirstOrDefault (); - if (runtime == null) - continue; - path = Path.Combine (folder.Path, libraryPath.Path, runtime.Path).Replace('/', Path.DirectorySeparatorChar); - if (!File.Exists (path)) - continue; - // _._ means its provided by the framework. However if we get here - // its NOT. So lets use what we got in the first place. - if (Path.GetFileName (path) == "_._") - return assemblyPath; - return path; - } - return null; - } - - void AddAssemblyReferences (MetadataResolver resolver, Dictionary assemblies, string assemblyPath, List resolutionPath) - { - var reader = resolver.GetAssemblyReader (assemblyPath); - var assembly = reader.GetAssemblyDefinition (); - var assemblyName = reader.GetString (assembly.Name); - - // Don't repeat assemblies we've already done - bool topLevel = resolutionPath == null; - if (!topLevel && assemblies.ContainsKey (assemblyName)) - return; - - if (resolutionPath == null) - resolutionPath = new List(); - - CheckAssemblyAttributes (assembly, reader, out string targetFrameworkIdentifier); - - LogMessage ("{0}Adding assembly reference for {1}, recursively...", new string (' ', indent), assemblyName); - resolutionPath.Add (assemblyName); - indent += 2; - // Add this assembly - ITaskItem assemblyItem = null; - if (topLevel) { - if (assemblies.TryGetValue (assemblyName, out assemblyItem)) { - if (!string.IsNullOrEmpty (targetFrameworkIdentifier) && string.IsNullOrEmpty (assemblyItem.GetMetadata ("TargetFrameworkIdentifier"))) { - assemblyItem.SetMetadata ("TargetFrameworkIdentifier", targetFrameworkIdentifier); - } - } - } else { - assemblies [assemblyName] = - assemblyItem = CreateAssemblyTaskItem (assemblyPath, targetFrameworkIdentifier); - } - - // Recurse into each referenced assembly - foreach (var handle in reader.AssemblyReferences) { - var reference = reader.GetAssemblyReference (handle); - string reference_assembly; - try { - var referenceName = reader.GetString (reference.Name); - if (assemblyItem != null && referenceName == "Mono.Android") { - assemblyItem.SetMetadata ("HasMonoAndroidReference", "True"); - } - reference_assembly = resolver.Resolve (referenceName); - } catch (FileNotFoundException ex) { - var references = new StringBuilder (); - for (int i = 0; i < resolutionPath.Count; i++) { - if (i != 0) - references.Append (" > "); - references.Append ('`'); - references.Append (resolutionPath [i]); - references.Append ('`'); - } - - string missingAssembly = ex.FileName; - if (missingAssembly.EndsWith (".dll", StringComparison.OrdinalIgnoreCase)) { - missingAssembly = Path.GetFileNameWithoutExtension (missingAssembly); - } - string message = $"Can not resolve reference: `{missingAssembly}`, referenced by {references}."; - if (MonoAndroidHelper.IsFrameworkAssembly (ex.FileName)) { - LogCodedError ("XA2002", $"{message} Perhaps it doesn't exist in the Mono for Android profile?"); - } else { - LogCodedError ("XA2002", $"{message} Please add a NuGet package or assembly reference for `{missingAssembly}`, or remove the reference to `{resolutionPath [0]}`."); - } - return; - } - AddAssemblyReferences (resolver, assemblies, reference_assembly, resolutionPath); - } - - indent -= 2; - resolutionPath.RemoveAt (resolutionPath.Count - 1); - } - - void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader, out string targetFrameworkIdentifier) + void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader) { - targetFrameworkIdentifier = null; - foreach (var handle in assembly.GetCustomAttributes ()) { var attribute = reader.GetCustomAttribute (handle); switch (reader.GetCustomAttributeFullName (attribute)) { @@ -297,7 +159,7 @@ void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader if (!string.IsNullOrEmpty (value)) { int commaIndex = value.IndexOf (",", StringComparison.Ordinal); if (commaIndex != -1) { - targetFrameworkIdentifier = value.Substring (0, commaIndex); + string targetFrameworkIdentifier = value.Substring (0, commaIndex); if (targetFrameworkIdentifier == "MonoAndroid") { const string match = "Version="; var versionIndex = value.IndexOf (match, commaIndex, StringComparison.Ordinal); @@ -323,22 +185,9 @@ void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader } } - static LinkModes ParseLinkMode (string linkmode) - { - if (string.IsNullOrWhiteSpace (linkmode)) - return LinkModes.SdkOnly; - - LinkModes mode = LinkModes.SdkOnly; - - Enum.TryParse (linkmode.Trim (), true, out mode); - - return mode; - } - void AddI18nAssemblies (MetadataResolver resolver, Dictionary assemblies) { var i18n = Linker.ParseI18nAssemblies (I18nAssemblies); - var link = ParseLinkMode (LinkMode); // Check if we should add any I18N assemblies if (i18n == Mono.Linker.I18nAssemblies.None) @@ -365,18 +214,10 @@ void AddI18nAssemblies (MetadataResolver resolver, Dictionary void ResolveI18nAssembly (MetadataResolver resolver, string name, Dictionary assemblies) { var assembly = resolver.Resolve (name); - assemblies [name] = CreateAssemblyTaskItem (assembly); - } - - static ITaskItem CreateAssemblyTaskItem (string assembly, string targetFrameworkIdentifier = null) - { - var assemblyFullPath = Path.GetFullPath (assembly); - var dictionary = new Dictionary (2) { - { "ReferenceAssembly", assemblyFullPath }, + var dictionary = new Dictionary (1) { + { "ReferenceAssembly", assembly }, }; - if (!string.IsNullOrEmpty (targetFrameworkIdentifier)) - dictionary.Add ("TargetFrameworkIdentifier", targetFrameworkIdentifier); - return new TaskItem (assemblyFullPath, dictionary); + assemblies [name] = new TaskItem (assembly, dictionary); } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj index c92d789e554..0d2e32dc011 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj @@ -11,6 +11,7 @@ + ..\..\..\..\bin\$(Configuration)\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Cecil.dll diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MetadataResolver.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MetadataResolver.cs index 82fea3650ba..a7b44d8a61c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MetadataResolver.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MetadataResolver.cs @@ -14,9 +14,11 @@ public class MetadataResolver : IDisposable readonly Dictionary cache = new Dictionary (); readonly List searchDirectories = new List (); - public MetadataReader GetAssemblyReader (string assemblyName) + public MetadataReader GetAssemblyReader (string assemblyName) => GetAssemblyReader (assemblyName, out _); + + public MetadataReader GetAssemblyReader (string assemblyName, out string assemblyPath) { - var assemblyPath = Resolve (assemblyName); + assemblyPath = Resolve (assemblyName); if (!cache.TryGetValue (assemblyPath, out PEReader reader)) { cache.Add (assemblyPath, reader = new PEReader (File.OpenRead (assemblyPath))); } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs index 64a06a7b9ed..1c5f09ab46c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs @@ -330,16 +330,21 @@ public static bool IsReferenceAssembly (string assembly) using (var pe = new PEReader (stream)) { var reader = pe.GetMetadataReader (); var assemblyDefinition = reader.GetAssemblyDefinition (); - foreach (var handle in assemblyDefinition.GetCustomAttributes ()) { - var attribute = reader.GetCustomAttribute (handle); - var attributeName = reader.GetCustomAttributeFullName (attribute); - if (attributeName == "System.Runtime.CompilerServices.ReferenceAssemblyAttribute") - return true; - } - return false; + return IsReferenceAssembly (reader, assemblyDefinition); } } + public static bool IsReferenceAssembly (MetadataReader reader, AssemblyDefinition assemblyDefinition) + { + foreach (var handle in assemblyDefinition.GetCustomAttributes ()) { + var attribute = reader.GetCustomAttribute (handle); + var attributeName = reader.GetCustomAttributeFullName (attribute); + if (attributeName == "System.Runtime.CompilerServices.ReferenceAssemblyAttribute") + return true; + } + return false; + } + public static bool ExistsInFrameworkPath (string assembly) { return TargetFrameworkDirectories diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/NuGetLogger.cs b/src/Xamarin.Android.Build.Tasks/Utilities/NuGetLogger.cs deleted file mode 100644 index 873d6811c35..00000000000 --- a/src/Xamarin.Android.Build.Tasks/Utilities/NuGetLogger.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System; -using NuGet.Common; -using Microsoft.Build.Utilities; -using TPL = System.Threading.Tasks; - -namespace Xamarin.Android.Tasks { - - class NuGetLogger : LoggerBase { - Action log; - - public NuGetLogger (Action log) - { - this.log = log; - } - - public override void Log (ILogMessage message) { - log (message.Message); - } - - public override void Log (LogLevel level, string data) { - log (data); - } - - public override TPL.Task LogAsync (ILogMessage message) { - Log (message); - return TPL.Task.FromResult(0); - } - - public override TPL.Task LogAsync (LogLevel level, string data) { - Log (level, data); - return TPL.Task.FromResult(0); - } - } -} diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj index fcf5358a0ec..2e3e1b6444d 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj @@ -61,22 +61,12 @@ ..\..\bin\$(Configuration)\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Cecil.Mdb.dll - + - - - - - - - - - - @@ -525,7 +515,6 @@ - diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets index 646b18fe9b5..0294b988d14 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets @@ -293,16 +293,6 @@ - - - - - - - - - - diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index faed5012d3e..e5f5c048445 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1693,16 +1693,15 @@ because xbuild doesn't support framework reference assemblies. - + + Condition="Exists ('%(_ReferencePath.Identity)')"/>