[Xamarin.Android.Build.Tasks] Optionally skip ConvertResourcesCases (#2348)

jonathanpeppers · jonpryor · commit 02c07ed6d9e0 · 2018-10-30T21:34:34.000-04:00
Currently, we run the `&lt;ConvertResourcesCases/&gt;` MSBuild task for all
assemblies (on each assembly's `&lt;ResolveLibraryProjectImports/&gt;`
output).  `&lt;ConvertResourcesCases/&gt;` is one of our slower tasks.

Luckily, there are some "well-known" assemblies that we could skip,
so we have decided to "whitelist" certain assemblies such as the
Android support libraries, Google Play Services, Firebase, etc.

A new `@(_AndroidAssemblySkipCases)` item group has been added, e.g.:

	&lt;_AndroidAssemblySkipCases Include="Xamarin.Android.Support.v7.AppCompat" /&gt;
	&lt;_AndroidAssemblySkipCases Include="Xamarin.Android.Support.v7.CardView" /&gt;

The `@(_AndroidAssemblySkipCases)` item group is an indicator for
`&lt;ConvertResourcesCases/&gt;` to just completely skip these assemblies.

Additionally, we can flat out skip `.aar` files in the same manner.

To make this work:

  - Added support to put `ITaskItem` metadata in the cache file
    produced by `&lt;ResolveLibraryProjectImports/&gt;`
  - Added item metadata for `%(SkipAndroidResourceProcessing)` and
    `%(OriginalFile)`.
  - `&lt;ConvertResourcesCases/&gt;` now skips these directories and logs
    `%(OriginalFile)`.
  - `&lt;CollectNonEmptyDirectories/&gt;` needs to preserve item metadata
    for `$(AndroidUseAapt2)` to take advantage of the functionality.

The results appear to be well worth the effort!

Results with `$(AndroidUseAapt2)` enabled (note this is not currently
the default):

	Before:
	9912 ms  ConvertResourcesCases                      9 calls
	2219 ms  ResolveLibraryProjectImports               1 calls

	After:
	  49 ms  ConvertResourcesCases                      9 calls
	2185 ms  ResolveLibraryProjectImports               1 calls

Results with `$(AndroidUseAapt2)` disabled:

	Before:
	1564 ms  ConvertResourcesCases                      1 calls
	1826 ms  ResolveLibraryProjectImports               1 calls

	After:
	  25 ms  ConvertResourcesCases                      1 calls
	1685 ms  ResolveLibraryProjectImports               1 calls

This was the Xamarin.Forms-Integration project in this repo, an
initial clean build.  It is basically a "Hello World" Xamarin.Forms
project.  These updated numbers are from a `Release` build of
Xamarin.Android.

Overall this will save 1-2 seconds of `&lt;ConvertResourcesCases/&gt;` for
default projects.  This MSBuild task runs on an initial build or
incremental builds when Android resources have changed.  I have gotten
slightly different numbers on the difference, each time I've compared.

There does not appear to be any noticeable slowdown in
`&lt;ResolveLibraryProjectImports/&gt;` due to the changes.
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/CollectNonEmptyDirectories.cs b/src/Xamarin.Android.Build.Tasks/Tasks/CollectNonEmptyDirectories.cs
@@ -21,9 +21,11 @@ public override bool Execute ()
 			foreach (var directory in Directories) {
 				var firstFile = Directory.EnumerateFiles(directory.ItemSpec, "*.*", SearchOption.AllDirectories).FirstOrDefault ();
 				if (firstFile != null) {
-					output.Add (new TaskItem (directory.ItemSpec, new Dictionary<string, string> () {
+					var taskItem = new TaskItem (directory.ItemSpec, new Dictionary<string, string> () {
 						{"FileFound", firstFile}
-					}));
+					});
+					directory.CopyMetadataTo (taskItem);
+					output.Add (taskItem);
 				}
 			}
 			return !Log.HasLoggedErrors;
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs
@@ -47,11 +47,18 @@ public override bool Execute ()
 			return true;
 		}
 
-
 		void FixupResources (Dictionary<string, string> acwMap)
 		{
-			foreach (var dir in ResourceDirectories)
+			foreach (var dir in ResourceDirectories) {
+				var skipResourceProcessing = dir.GetMetadata (ResolveLibraryProjectImports.SkipAndroidResourceProcessing);
+				if (skipResourceProcessing != null && skipResourceProcessing.Equals ("true", StringComparison.OrdinalIgnoreCase)) {
+					var originalFile = dir.GetMetadata (ResolveLibraryProjectImports.OriginalFile);
+					Log.LogDebugMessage ($"Skipping: `{dir.ItemSpec}` via `{ResolveLibraryProjectImports.SkipAndroidResourceProcessing}`, original file: `{originalFile}`...");
+					continue;
+				}
+
 				FixupResources (dir, acwMap);
+			}
 		}
 
 		void FixupResources (ITaskItem item, Dictionary<string, string> acwMap)
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs
@@ -42,6 +42,8 @@ public class ResolveLibraryProjectImports : Task
 
 		public string CacheFile { get; set; }
 
+		public string [] AssembliesToSkipCases { get; set; }
+
 		[Required]
 		public bool DesignTimeBuild { get; set; }
 
@@ -60,7 +62,15 @@ public class ResolveLibraryProjectImports : Task
 		[Output]
 		public ITaskItem [] ResolvedResourceDirectoryStamps { get; set; }
 
+		internal const string OriginalFile = "OriginalFile";
+		internal const string SkipAndroidResourceProcessing = "SkipAndroidResourceProcessing";
+		static readonly string [] knownMetadata = new [] {
+			OriginalFile,
+			SkipAndroidResourceProcessing
+		};
+
 		AssemblyIdentityMap assemblyMap = new AssemblyIdentityMap();
+		HashSet<string> assembliesToSkip;
 
 		public ResolveLibraryProjectImports ()
 		{
@@ -70,20 +80,13 @@ public ResolveLibraryProjectImports ()
 		// Extracts library project contents under e.g. obj/Debug/[lp/*.jar | res/*/*]
 		public override bool Execute ()
 		{
-			Log.LogDebugMessage ("ResolveLibraryProjectImports Task");
-			Log.LogDebugMessage ("  ImportsDirectory: {0}", ImportsDirectory);
-			Log.LogDebugMessage ("  OutputDirectory: {0}", OutputDirectory);
-			Log.LogDebugMessage ("  OutputImportDirectory: {0}", OutputImportDirectory);
-			Log.LogDebugMessage ("  UseShortFileNames: {0}", UseShortFileNames);
-			Log.LogDebugTaskItems ("  Assemblies: ", Assemblies);
-			Log.LogDebugTaskItems ("  AarLibraries: ", AarLibraries);
-
 			var jars                          = new List<string> ();
-			var resolvedResourceDirectories   = new List<string> ();
+			var resolvedResourceDirectories   = new List<ITaskItem> ();
 			var resolvedAssetDirectories      = new List<string> ();
 			var resolvedEnvironmentFiles      = new List<string> ();
 
 			assemblyMap.Load (AssemblyIdentityMapFile);
+			assembliesToSkip = new HashSet<string> (AssembliesToSkipCases ?? new string [0], StringComparer.OrdinalIgnoreCase);
 
 			using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
 				try {
@@ -95,9 +98,7 @@ public override bool Execute ()
 			}
 
 			Jars                        = jars.ToArray ();
-			ResolvedResourceDirectories = resolvedResourceDirectories
-				.Select (s => new TaskItem (Path.GetFullPath (s)))
-				.ToArray ();
+			ResolvedResourceDirectories = resolvedResourceDirectories.ToArray ();
 			ResolvedAssetDirectories    = resolvedAssetDirectories.ToArray ();
 			ResolvedEnvironmentFiles    = resolvedEnvironmentFiles.ToArray ();
 
@@ -120,7 +121,15 @@ public override bool Execute ()
 						new XElement ("Jars",
 							Jars.Select(e => new XElement ("Jar", e))),
 						new XElement ("ResolvedResourceDirectories",
-							ResolvedResourceDirectories.Select(e => new XElement ("ResolvedResourceDirectory", e))),
+							ResolvedResourceDirectories.Select(dir => {
+								var e = new XElement ("ResolvedResourceDirectory", dir.ItemSpec);
+								foreach (var name in knownMetadata) {
+									var value = dir.GetMetadata (name);
+									if (!string.IsNullOrEmpty (value))
+										e.SetAttributeValue (name, value);
+								}
+								return e;
+							})),
 						new XElement ("ResolvedAssetDirectories", 
 							ResolvedAssetDirectories.Select(e => new XElement ("ResolvedAssetDirectory", e))),
 						new XElement ("ResolvedEnvironmentFiles", 
@@ -133,10 +142,10 @@ public override bool Execute ()
 
 			assemblyMap.Save (AssemblyIdentityMapFile);
 
-			Log.LogDebugTaskItems ("  Jars: ", Jars.Select (s => new TaskItem (s)).ToArray ());
-			Log.LogDebugTaskItems ("  ResolvedResourceDirectories: ", ResolvedResourceDirectories.Select (s => new TaskItem (s)).ToArray ());
-			Log.LogDebugTaskItems ("  ResolvedAssetDirectories: ", ResolvedAssetDirectories.Select (s => new TaskItem (s)).ToArray ());
-			Log.LogDebugTaskItems ("  ResolvedEnvironmentFiles: ", ResolvedEnvironmentFiles.Select (s => new TaskItem (s)).ToArray ());
+			Log.LogDebugTaskItems ("  Jars: ", Jars);
+			Log.LogDebugTaskItems ("  ResolvedResourceDirectories: ", ResolvedResourceDirectories);
+			Log.LogDebugTaskItems ("  ResolvedAssetDirectories: ", ResolvedAssetDirectories);
+			Log.LogDebugTaskItems ("  ResolvedEnvironmentFiles: ", ResolvedEnvironmentFiles);
 			Log.LogDebugTaskItems ("  ResolvedResourceDirectoryStamps: ", ResolvedResourceDirectoryStamps);
 
 			return !Log.HasLoggedErrors;
@@ -160,7 +169,7 @@ static string GetTargetAssembly (ITaskItem assemblyName)
 		void Extract (
 				DirectoryAssemblyResolver res,
 				ICollection<string> jars,
-				ICollection<string> resolvedResourceDirectories,
+				ICollection<ITaskItem> resolvedResourceDirectories,
 				ICollection<string> resolvedAssetDirectories,
 				ICollection<string> resolvedEnvironments)
 		{
@@ -183,9 +192,10 @@ void Extract (
 					.Select (a => GetTargetAssembly (a))
 					.Where (a => a != null)
 					.Distinct ()) {
-				string assemblyIdentName = Path.GetFileNameWithoutExtension (assemblyPath);
+				string assemblyFileName = Path.GetFileNameWithoutExtension (assemblyPath);
+				string assemblyIdentName = assemblyFileName;
 				if (UseShortFileNames) {
-					assemblyIdentName = assemblyMap.GetLibraryImportDirectoryNameForAssembly (assemblyIdentName);
+					assemblyIdentName = assemblyMap.GetLibraryImportDirectoryNameForAssembly (assemblyFileName);
 				}
 				string outDirForDll = Path.Combine (OutputImportDirectory, assemblyIdentName);
 				string importsDir = Path.Combine (outDirForDll, ImportsDirectory);
@@ -210,8 +220,14 @@ void Extract (
 					if (Directory.Exists (binAssemblyDir))
 						resolvedAssetDirectories.Add (binAssemblyDir);
 #endif
-					if (Directory.Exists (resDir))
-						resolvedResourceDirectories.Add (resDir);
+					if (Directory.Exists (resDir)) {
+						var taskItem = new TaskItem (resDir, new Dictionary<string, string> {
+							{ OriginalFile, assemblyPath },
+						});
+						if (assembliesToSkip.Contains (assemblyFileName))
+							taskItem.SetMetadata (SkipAndroidResourceProcessing, "True");
+						resolvedResourceDirectories.Add (taskItem);
+					}
 					if (Directory.Exists (assemblyDir))
 						resolvedAssetDirectories.Add (assemblyDir);
 					foreach (var env in Directory.EnumerateFiles (outDirForDll, "__AndroidEnvironment__*", SearchOption.TopDirectoryOnly)) {
@@ -308,8 +324,14 @@ void Extract (
 						if (Directory.Exists (binAssemblyDir))
 							resolvedAssetDirectories.Add (binAssemblyDir);
 #endif
-						if (Directory.Exists (resDir))
-							resolvedResourceDirectories.Add (resDir);
+						if (Directory.Exists (resDir)) {
+							var taskItem = new TaskItem (resDir, new Dictionary<string, string> {
+								{ OriginalFile, assemblyPath }
+							});
+							if (assembliesToSkip.Contains (assemblyFileName))
+								taskItem.SetMetadata (SkipAndroidResourceProcessing, "True");
+							resolvedResourceDirectories.Add (taskItem);
+						}
 						if (Directory.Exists (assemblyDir))
 							resolvedAssetDirectories.Add (assemblyDir);
 
@@ -362,7 +384,10 @@ void Extract (
 					}
 				}
 				if (Directory.Exists (resDir))
-					resolvedResourceDirectories.Add (resDir);
+					resolvedResourceDirectories.Add (new TaskItem (resDir, new Dictionary<string, string> {
+						{ OriginalFile, Path.GetFullPath (aarFile.ItemSpec) },
+						{ SkipAndroidResourceProcessing, "True" },
+					}));
 				if (Directory.Exists (assetsDir))
 					resolvedAssetDirectories.Add (assetsDir);
 			}
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs
@@ -56,6 +56,64 @@ public void BuildBasicApplicationReleaseFSharp ()
 			}
 		}
 
+		[Test]
+		public void SkipConvertResourcesCases ()
+		{
+			var target = "ConvertResourcesCases";
+			var proj = new XamarinFormsAndroidApplicationProject ();
+			proj.OtherBuildItems.Add (new BuildItem ("AndroidAarLibrary", "Jars\\material-menu-1.1.0.aar") {
+				WebContent = "https://repo.jfrog.org/artifactory/libs-release-bintray/com/balysv/material-menu/1.1.0/material-menu-1.1.0.aar"
+			});
+			using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
+				Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
+				Assert.IsFalse (b.Output.IsTargetSkipped (target), $"`{target}` should not be skipped.");
+
+				List<string> skipped = new List<string> (), processed = new List<string> ();
+				bool convertResourcesCases = false;
+				foreach (var line in b.LastBuildOutput) {
+					if (!convertResourcesCases) {
+						convertResourcesCases = line.StartsWith ($"Task \"{target}\"", StringComparison.OrdinalIgnoreCase);
+					} else if (line.StartsWith ($"Done executing task \"{target}\"", StringComparison.OrdinalIgnoreCase)) {
+						break; //end of target
+					} else if (line.IndexOf ("Processing:", StringComparison.OrdinalIgnoreCase) >= 0) {
+						//Processing: obj\Debug\res\layout\main.xml   10/29/2018 8:19:36 PM > 1/1/0001 12:00:00 AM
+						processed.Add (line);
+					} else if (line.IndexOf ("Skipping:", StringComparison.OrdinalIgnoreCase) >= 0) {
+						//Skipping: `obj\Debug\lp\5\jl\res` via `SkipAndroidResourceProcessing`, original file: `bin\TestDebug\temp\packages\Xamarin.Android.Support.Compat.27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Compat.dll`...
+						skipped.Add (line);
+					}
+				}
+
+				var resources = new [] {
+					Path.Combine ("layout", "main.xml"),
+					Path.Combine ("layout", "tabbar.xml"),
+					Path.Combine ("layout", "toolbar.xml"),
+					Path.Combine ("values", "colors.xml"),
+					Path.Combine ("values", "strings.xml"),
+					Path.Combine ("values", "styles.xml"),
+				};
+				foreach (var resource in resources) {
+					Assert.IsTrue (StringAssertEx.ContainsText (processed, resource), $"`{target}` should process `{resource}`.");
+				}
+
+				var files = new [] {
+					"Xamarin.Android.Support.Compat.dll",
+					"Xamarin.Android.Support.Design.dll",
+					"Xamarin.Android.Support.Media.Compat.dll",
+					"Xamarin.Android.Support.Transition.dll",
+					"Xamarin.Android.Support.v4.dll",
+					"Xamarin.Android.Support.v7.AppCompat.dll",
+					"Xamarin.Android.Support.v7.CardView.dll",
+					"Xamarin.Android.Support.v7.MediaRouter.dll",
+					"Xamarin.Android.Support.v7.RecyclerView.dll",
+					"material-menu-1.1.0.aar",
+				};
+				foreach (var file in skipped) {
+					Assert.IsTrue (StringAssertEx.ContainsText (skipped, file), $"`{target}` should skip `{file}`.");
+				}
+			}
+		}
+
 		[Test]
 		public void BuildInParallel ()
 		{
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/XDocumentExtensions.cs b/src/Xamarin.Android.Build.Tasks/Utilities/XDocumentExtensions.cs
@@ -10,16 +10,28 @@ namespace Xamarin.Android.Tasks
 {
 	public static class XDocumentExtensions
 	{
+		const string PathsElementName = "Paths";
+
 		public static ITaskItem[] GetPathsAsTaskItems (this XDocument doc, params string[] paths)
 		{
-			return doc.GetPaths (paths)
-				.Select(x => new TaskItem(x))
-				.ToArray ();
+			var e = doc.Elements (PathsElementName);
+			foreach (var p in paths)
+				e = e.Elements (p);
+			return e.Select (ToTaskItem).ToArray ();
+		}
+
+		static ITaskItem ToTaskItem (XElement element)
+		{
+			var taskItem = new TaskItem (element.Value);
+			foreach (var attribute in element.Attributes ()) {
+				taskItem.SetMetadata (attribute.Name.LocalName, attribute.Value);
+			}
+			return taskItem;
 		}
 
 		public static string[] GetPaths (this XDocument doc, params string[] paths)
 		{
-			var e = doc.Elements ("Paths");
+			var e = doc.Elements (PathsElementName);
 			foreach (var p in paths)
 				e = e.Elements (p);
 			return e.Select (p => p.Value).ToArray ();
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets
@@ -109,6 +109,9 @@
     <None Include="Xamarin.Android.VisualBasic.targets">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Include="Xamarin.Android.SkipCases.projitems">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
   </ItemGroup>
   <ItemGroup>
     <_SharedRuntimeAssemblies Include="@(MonoProfileAssembly->'$(_SharedRuntimeBuildPath)v1.0\%(Identity)')" />
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -368,6 +368,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
 -->
 <!-- As we split up/refactor this file, put new imports here -->
 <Import Project="$(MSBuildThisFileDirectory)Xamarin.Android.D8.targets" />
+<Import Project="$(MSBuildThisFileDirectory)Xamarin.Android.SkipCases.projitems" />
 
 <Target Name="_SeparateAppExtensionReferences">
 	<CreateItem Include="@(ProjectReference)" PreserveExistingMetadata="true" Condition="'%(Identity)' != '' AND '%(ProjectReference.IsAppExtension)' == 'true'">
@@ -1345,7 +1346,8 @@ because xbuild doesn't support framework reference assemblies.
 		UseShortFileNames="$(UseShortFileNames)"
 		OutputDirectory="$(IntermediateOutputPath)"
 		AssemblyIdentityMapFile="$(_AndroidLibrayProjectAssemblyMapFile)"
-		OutputImportDirectory="$(_AndroidLibrayProjectIntermediatePath)">
+		OutputImportDirectory="$(_AndroidLibrayProjectIntermediatePath)"
+		AssembliesToSkipCases="@(_AndroidAssemblySkipCases)">
 	</ResolveLibraryProjectImports>
 	<Touch Files="$(_AndroidStampDirectory)_ResolveLibraryProjectImports.stamp" AlwaysCreate="True" />
 </Target>
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.SkipCases.projitems b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.SkipCases.projitems

Original file line number	Diff line number	Diff line change
`@@ -21,9 +21,11 @@ public override bool Execute ()`
`21`	`21`	`foreach (var directory in Directories) {`
`22`	`22`	`var firstFile = Directory.EnumerateFiles(directory.ItemSpec, ".", SearchOption.AllDirectories).FirstOrDefault ();`
`23`	`23`	`if (firstFile != null) {`
`24`		`- output.Add (new TaskItem (directory.ItemSpec, new Dictionary<string, string> () {`
	`24`	`+ var taskItem = new TaskItem (directory.ItemSpec, new Dictionary<string, string> () {`
`25`	`25`	`{"FileFound", firstFile}`
`26`		`- }));`
	`26`	`+ });`
	`27`	`+ directory.CopyMetadataTo (taskItem);`
	`28`	`+ output.Add (taskItem);`
`27`	`29`	`}`
`28`	`30`	`}`
`29`	`31`	`return !Log.HasLoggedErrors;`
Original file line number	Diff line number	Diff line change
`@@ -47,11 +47,18 @@ public override bool Execute ()`
`47`	`47`	`return true;`
`48`	`48`	`}`
`49`	`49`
`50`		`-`
`51`	`50`	`void FixupResources (Dictionary<string, string> acwMap)`
`52`	`51`	`{`
`53`		`- foreach (var dir in ResourceDirectories)`
	`52`	`+ foreach (var dir in ResourceDirectories) {`
	`53`	`+ var skipResourceProcessing = dir.GetMetadata (ResolveLibraryProjectImports.SkipAndroidResourceProcessing);`
	`54`	`+ if (skipResourceProcessing != null && skipResourceProcessing.Equals ("true", StringComparison.OrdinalIgnoreCase)) {`
	`55`	`+ var originalFile = dir.GetMetadata (ResolveLibraryProjectImports.OriginalFile);`
	`56`	+ Log.LogDebugMessage ($"Skipping: `{dir.ItemSpec}` via `{ResolveLibraryProjectImports.SkipAndroidResourceProcessing}`, original file: `{originalFile}`...");
	`57`	`+ continue;`
	`58`	`+ }`
	`59`	`+`
`54`	`60`	`FixupResources (dir, acwMap);`
	`61`	`+ }`
`55`	`62`	`}`
`56`	`63`
`57`	`64`	`void FixupResources (ITaskItem item, Dictionary<string, string> acwMap)`