[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> (#2624)

jonathanpeppers · jonpryor · commit afba30b0b79a · 2019-01-18T15:38:26.000-05:00
Just like in c22475d, we can use System.Reflection.Metadata in places instead of Mono.Cecil for performance benefits. One such case is the `<GetAdditionalResourcesFromAssemblies/>` MSBuild task, which only looks at custom attributes in assemblies: Before 1st: 98 ms GetAdditionalResourcesFromAssemblies 1 calls Incremental: 105 ms GetAdditionalResourcesFromAssemblies 1 calls After 1st: 54 ms GetAdditionalResourcesFromAssemblies 1 calls Incremental: 53 ms GetAdditionalResourcesFromAssemblies 1 calls This was a straightforward port to SRM, which seems to save ~50ms on every build.
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GetAdditionalResourcesFromAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GetAdditionalResourcesFromAssemblies.cs
@@ -1,23 +1,19 @@
+using Microsoft.Build.Framework;
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.IO;
+using System.Linq;
+using System.Net;
+using System.Reflection.Metadata;
+using System.Reflection.PortableExecutable;
+using System.Security.Cryptography;
 using System.Text;
 using System.Text.RegularExpressions;
-using Microsoft.Build.Framework;
-using Microsoft.Build.Utilities;
-using Mono.Cecil;
-using System.Xml;
 using System.Xml.Linq;
-using System.Security.Cryptography;
-using System.Net;
-using System.ComponentModel;
 using Xamarin.Android.Tools;
-using Xamarin.Tools.Zip;
-
-using Java.Interop.Tools.Cecil;
 
-namespace Xamarin.Android.Tasks {
+namespace Xamarin.Android.Tasks
+{
 
 	public class GetAdditionalResourcesFromAssemblies : AsyncTask
 	{
@@ -85,45 +81,45 @@ string SubstituteEnvVariables (string path)
 			return regex.Replace (path, replaceEnvVar);
 		}
 
-		internal static string ErrorMessage (CustomAttribute attr)
+		internal static string ErrorMessage (CustomAttributeValue<object> attributeValue)
 		{
-			if (!attr.HasProperties)
+			if (attributeValue.NamedArguments.Length == 0)
 				return "";
-			CustomAttributeNamedArgument arg = attr.Properties.FirstOrDefault (p => p.Name == "PackageName");
+			var arg = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "PackageName");
 			if (arg.Name != null) {
-				string packageName = arg.Argument.Value as string;
+				string packageName = arg.Value as string;
 				if (packageName != null)
 					return string.Format ("Please install package: '{0}' available in SDK installer", packageName);
 			}
-			arg = attr.Properties.FirstOrDefault (p => p.Name == "InstallInstructions");
+			arg = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "InstallInstructions");
 			if (arg.Name != null) {
-				string instInstructs = arg.Argument.Value as string;
+				string instInstructs = arg.Value as string;
 				if (instInstructs != null)
 					return "Installation instructions: " + instInstructs;
 			}
 			return null;
 		}
 
-		void AddAttributeValue (ICollection<string> items, CustomAttribute attr, string errorCode, string errorFmt,
-			bool isDirectory, string fullPath)
+		void AddAttributeValue (ICollection<string> items, CustomAttributeValue<object> attributeValue, string errorCode, string errorFmt,
+			bool isDirectory, string fullPath, string attributeFullName)
 		{
-			if (!attr.HasConstructorArguments || attr.ConstructorArguments.Count != 1) {
-				LogCodedWarning (errorCode, "Attribute {0} doesn't have expected one constructor agrument", attr.AttributeType.FullName);
+			if (attributeValue.NamedArguments.Length == 0 || attributeValue.FixedArguments.Length != 1) {
+				LogCodedWarning (errorCode, "Attribute {0} doesn't have expected one constructor agrument", attributeFullName);
 				return;
 			}
 
-			CustomAttributeArgument arg = attr.ConstructorArguments.First ();
+			var arg = attributeValue.FixedArguments.First ();
 			string path = arg.Value as string;
 			if (string.IsNullOrEmpty (path)) {
-				LogCodedWarning (errorCode, "Attribute {0} contructor argument is empty or not set to string", attr.AttributeType.FullName);
+				LogCodedWarning (errorCode, "Attribute {0} contructor argument is empty or not set to string", attributeFullName);
 				return;
 			}
 			path = SubstituteEnvVariables (path).TrimEnd (Path.DirectorySeparatorChar);
 			string baseDir = null;
-			CustomAttributeNamedArgument sourceUrl = attr.Properties.FirstOrDefault (p => p.Name == "SourceUrl");
-			CustomAttributeNamedArgument embeddedArchive = attr.Properties.FirstOrDefault (p => p.Name == "EmbeddedArchive");
-			CustomAttributeNamedArgument version = attr.Properties.FirstOrDefault (p => p.Name == "Version");
-			CustomAttributeNamedArgument sha1sum = attr.Properties.FirstOrDefault (p => p.Name == "Sha1sum");
+			var sourceUrl = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "SourceUrl");
+			var embeddedArchive = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "EmbeddedArchive");
+			var version = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "Version");
+			var sha1sum = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "Sha1sum");
 			var isXamarinAssembly = Path.GetFileName (fullPath).StartsWith (AssemblyNamePrefix, StringComparison.OrdinalIgnoreCase);
 			var assemblyDir = Path.Combine (CachePath, Path.GetFileNameWithoutExtension (fullPath));
 			// upgrade the paths to not strip off the Xamarin. prefix as it might cause assembly 
@@ -144,10 +140,10 @@ void AddAttributeValue (ICollection<string> items, CustomAttribute attr, string
 				Directory.Delete (oldAssemblyDir, recursive: true);
 			}
 			if (sourceUrl.Name != null) {
-				if (new Uri (sourceUrl.Argument.Value as string).IsFile)
+				if (new Uri (sourceUrl.Value as string).IsFile)
 					assemblyDir = Path.GetDirectoryName (fullPath);
-				baseDir = MakeSureLibraryIsInPlace (assemblyDir, sourceUrl.Argument.Value as string,
-					version.Argument.Value as string, embeddedArchive.Argument.Value as string, sha1sum.Argument.Value as string);
+				baseDir = MakeSureLibraryIsInPlace (assemblyDir, sourceUrl.Value as string,
+					version.Value as string, embeddedArchive.Value as string, sha1sum.Value as string);
 			}
 			if (!string.IsNullOrEmpty (baseDir) && !Path.IsPathRooted (path))
 				path = Path.Combine (baseDir, path);
@@ -157,7 +153,7 @@ void AddAttributeValue (ICollection<string> items, CustomAttribute attr, string
 				return;
 			}
 			if (!DesignTimeBuild)
-				LogCodedError (errorCode, errorFmt, ErrorMessage (attr), path);
+				LogCodedError (errorCode, errorFmt, ErrorMessage (attributeValue), path);
 		}
 
 		bool ExtractArchive (string url, string file, string contentDir)
@@ -382,12 +378,6 @@ public override bool Execute ()
 
 		void DoExecute ()
 		{
-			LogDebugMessage ("GetAdditionalResourcesFromAssemblies Task");
-			LogDebugMessage ("  AndroidSdkDirectory: {0}", AndroidSdkDirectory);
-			LogDebugMessage ("  AndroidNdkDirectory: {0}", AndroidNdkDirectory);
-			LogDebugMessage ("  CacheFile: {0}", CacheFile);
-			LogDebugTaskItems ("  Assemblies: ", Assemblies);
-
 			if (Environment.GetEnvironmentVariable ("XA_DL_IGNORE_CERT_ERRROS") == "yesyesyes") {
 				ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
 				LogDebugMessage ("    Disabling download certificate validation callback.");
@@ -411,35 +401,38 @@ void DoExecute ()
 				? CachePath
 				: Path.Combine (Environment.GetFolderPath (Environment.SpecialFolder.LocalApplicationData), CacheBaseDir);
 
-				using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
-					foreach (var assemblyItem in Assemblies) {
-						string fullPath = Path.GetFullPath (assemblyItem.ItemSpec);
-						if (DesignTimeBuild && !File.Exists (fullPath)) {
-							LogWarning ("Failed to load '{0}'. Check the file exists or the project has been built.", fullPath);
-							continue;
-						}
-						if (assemblies.Contains (fullPath)) {
-							LogDebugMessage ("  Skip assembly: {0}, it was already processed", fullPath);
-							continue;
-						}
-						// don't try to even load mscorlib it will fail.
-						if (string.Compare (Path.GetFileNameWithoutExtension (fullPath), "mscorlib", StringComparison.OrdinalIgnoreCase) == 0)
-							continue;
-						assemblies.Add (fullPath);
-						resolver.Load (fullPath);
+				foreach (var assemblyItem in Assemblies) {
+					string fullPath = Path.GetFullPath (assemblyItem.ItemSpec);
+					if (DesignTimeBuild && !File.Exists (fullPath)) {
+						LogWarning ("Failed to load '{0}'. Check the file exists or the project has been built.", fullPath);
+						continue;
+					}
+					if (assemblies.Contains (fullPath)) {
+						LogDebugMessage ("  Skip assembly: {0}, it was already processed", fullPath);
+						continue;
+					}
+					// don't try to even load mscorlib it will fail.
+					if (string.Compare (Path.GetFileNameWithoutExtension (fullPath), "mscorlib", StringComparison.OrdinalIgnoreCase) == 0)
+						continue;
+					assemblies.Add (fullPath);
+					using (var pe = new PEReader (File.OpenRead (fullPath))) {
+						var reader = pe.GetMetadataReader ();
+						var assembly = reader.GetAssemblyDefinition ();
 						// Append source file name (without the Xamarin. prefix or extension) to the base folder
 						// This would help avoid potential collisions.
-						foreach (var ca in resolver.GetAssembly (assemblyItem.ItemSpec).CustomAttributes) {
-							switch (ca.AttributeType.FullName) {
-							case "Android.IncludeAndroidResourcesFromAttribute":
-								AddAttributeValue (androidResources, ca, "XA5206", "{0}. Android resource directory {1} doesn't exist.", true, fullPath);
-								break;
-							case "Java.Interop.JavaLibraryReferenceAttribute":
-								AddAttributeValue (javaLibraries, ca, "XA5207", "{0}. Java library file {1} doesn't exist.", false, fullPath);
-								break;
-							case "Android.NativeLibraryReferenceAttribute":
-								AddAttributeValue (nativeLibraries, ca, "XA5210", "{0}. Native library file {1} doesn't exist.", false, fullPath);
-								break;
+						foreach (var handle in assembly.GetCustomAttributes ()) {
+							var attribute = reader.GetCustomAttribute (handle);
+							var fullName = reader.GetCustomAttributeFullName (attribute);
+							switch (fullName) {
+								case "Android.IncludeAndroidResourcesFromAttribute":
+									AddAttributeValue (androidResources, attribute.GetCustomAttributeArguments (), "XA5206", "{0}. Android resource directory {1} doesn't exist.", true, fullPath, fullName);
+									break;
+								case "Java.Interop.JavaLibraryReferenceAttribute":
+									AddAttributeValue (javaLibraries, attribute.GetCustomAttributeArguments (), "XA5207", "{0}. Java library file {1} doesn't exist.", false, fullPath, fullName);
+									break;
+								case "Android.NativeLibraryReferenceAttribute":
+									AddAttributeValue (nativeLibraries, attribute.GetCustomAttributeArguments (), "XA5210", "{0}. Native library file {1} doesn't exist.", false, fullPath, fullName);
+									break;
 							}
 						}
 					}
