[Xamarin.Android.Build.Tasks] <GetImportedLibraries/> perf improvements (#2930)

jonathanpeppers · dellis1972 · commit 3f0152d8fc60 · 2019-04-08T20:30:30.000+01:00
* [Xamarin.Android.Build.Tasks] &lt;GetImportedLibraries/&gt; perf improvements

I noticed a target running on an incremental build with only a XAML
code change:

    116 ms  _BuildLibraryImportsCache                  1 calls

This was using VS 2019 GA, just a blank Xamarin.Forms app.

Reviewing `&lt;GetImportedLibraries/&gt;` it had some general perf
improvements that could be done:

* It called `Directory.EnumerateFiles` three times...
* It used a bunch of LINQ where a `foreach` would do.

Additionally, I saw some improvements I could do at the MSBuild target
level:

    &lt;Target Name="_BuildLibraryImportsCache"
            Inputs="$(MSBuildProjectFullPath);@(_ReferencePath);@(_ReferenceDependencyPaths);$(_AndroidBuildPropertiesCache);$(_AndroidLibraryProjectImportsCache)"
            Outputs="$(_AndroidLibraryImportsCache).stamp"&gt;

This target operates on the output of `_ResolveLibraryProjectImports`,
but it was re-running if *any* assembly changed?

I think the `Inputs` and `Outputs` would more appropriately be:

    Inputs="$(_AndroidLibraryProjectImportsCache)"
    Outputs="$(_AndroidStampDirectory)_BuildLibraryImportsCache.stamp"

This way, if `_ResolveLibraryProjectImports` runs and does work, it
writes to `$(_AndroidLibraryProjectImportsCache)`. We then only need
to run this task when that file changes.

I also updated this target so it follows our new convention for
placing stamp files in `$(_AndroidStampDirectory)`.

~~ Results ~~

An example of an initial build of the Xamarin.Forms project in this
repo:

    Before:
    299 ms  GetImportedLibraries                       1 calls
    After:
    172 ms  GetImportedLibraries                       1 calls

And then an incremental build, that is a XAML change:

    Before:
    109 ms  _BuildLibraryImportsCache                  1 calls
    After:
    n/a

The task is skipped now:

    _BuildLibraryImportsCache:
    Skipping target "_BuildLibraryImportsCache" because all output files are up-to-date with respect to the input files.

Overall, I would say there is a ~120ms improvement on initial build.
Incremental builds with XAML changes have a ~100ms improvement.

* Reorder if-else-if for less Path.GetFileName calls
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GetImportedLibraries.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GetImportedLibraries.cs
@@ -1,14 +1,10 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
-using System.Xml;
 using System.Xml.Linq;
-using Mono.Cecil;
 using Microsoft.Build.Utilities;
 using Microsoft.Build.Framework;
-using System.Text.RegularExpressions;
 
 namespace Xamarin.Android.Tasks
 {
@@ -36,28 +32,34 @@ public class GetImportedLibraries : Task
 
 		public override bool Execute ()
 		{
-			Log.LogDebugMessage ("GetImportedLibraries Task");
-			Log.LogDebugMessage ("  TargetDirectory: {0}", TargetDirectory);
 			if (!Directory.Exists (TargetDirectory)) {
 				Log.LogDebugMessage ("Target directory was not found");
 				return true;
 			}
-			// there could be ./AndroidManifest.xml and bin/AndroidManifest.xml, which will be the same. So, ignore "bin" ones.
+
 			var manifestDocuments = new List<ITaskItem> ();
-			foreach (var f in Directory.EnumerateFiles (TargetDirectory, "AndroidManifest.xml", SearchOption.AllDirectories)) {
-				var directory = Path.GetFileName (Path.GetDirectoryName (f));
-				if (IgnoredManifestDirectories.Contains (directory))
-					continue;
-				manifestDocuments.Add (new TaskItem (f));
+			var nativeLibraries   = new List<ITaskItem> ();
+			var jarFiles          = new List<ITaskItem> ();
+			foreach (var file in Directory.EnumerateFiles (TargetDirectory, "*", SearchOption.AllDirectories)) {
+				if (file.EndsWith (".so", StringComparison.OrdinalIgnoreCase)) {
+					if (MonoAndroidHelper.GetNativeLibraryAbi (file) != null)
+						nativeLibraries.Add (new TaskItem (file));
+				} else if (file.EndsWith (".jar", StringComparison.OrdinalIgnoreCase)) {
+					jarFiles.Add (new TaskItem (file));
+				} else if (file.EndsWith (".xml", StringComparison.OrdinalIgnoreCase)) {
+					if (Path.GetFileName (file) == "AndroidManifest.xml") {
+						// there could be ./AndroidManifest.xml and bin/AndroidManifest.xml, which will be the same. So, ignore "bin" ones.
+						var directory = Path.GetFileName (Path.GetDirectoryName (file));
+						if (IgnoredManifestDirectories.Contains (directory))
+							continue;
+						manifestDocuments.Add (new TaskItem (file));
+					}
+				}
 			}
+
 			ManifestDocuments = manifestDocuments.ToArray ();
-			NativeLibraries = Directory.GetFiles (TargetDirectory, "*.so", SearchOption.AllDirectories)
-				.Where (p => MonoAndroidHelper.GetNativeLibraryAbi (p) != null)
-				.Select (p => new TaskItem (p)).ToArray ();
-			Jars = Directory.GetFiles (TargetDirectory, "*.jar", SearchOption.AllDirectories).Select (p => new TaskItem (p)).ToArray ();
-			Log.LogDebugTaskItems ("  NativeLibraries: ", NativeLibraries);
-			Log.LogDebugTaskItems ("  Jars: ", Jars);
-			Log.LogDebugTaskItems ("  ManifestDocuments: ", ManifestDocuments);
+			NativeLibraries = nativeLibraries.ToArray ();
+			Jars = jarFiles.ToArray ();
 
 			if (!string.IsNullOrEmpty (CacheFile)) {
 				var document = new XDocument (
@@ -70,6 +72,10 @@ public override bool Execute ()
 				document.SaveIfChanged (CacheFile);
 			}
 
+			Log.LogDebugTaskItems ("  NativeLibraries: ", NativeLibraries);
+			Log.LogDebugTaskItems ("  Jars: ", Jars);
+			Log.LogDebugTaskItems ("  ManifestDocuments: ", ManifestDocuments);
+
 			return true;
 		}
 	}
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -1469,14 +1469,15 @@ because xbuild doesn't support framework reference assemblies.
 </PropertyGroup>
 
 <Target Name="_BuildLibraryImportsCache"
-		Inputs="$(MSBuildProjectFullPath);@(_ReferencePath);@(_ReferenceDependencyPaths);$(_AndroidBuildPropertiesCache);$(_AndroidLibraryProjectImportsCache)"
-		Outputs="$(_AndroidLibraryImportsCache).stamp">
-	<GetImportedLibraries TargetDirectory="$(_AndroidLibrayProjectIntermediatePath)"
+		Inputs="$(_AndroidLibraryProjectImportsCache)"
+		Outputs="$(_AndroidStampDirectory)_BuildLibraryImportsCache.stamp">
+	<GetImportedLibraries
+			TargetDirectory="$(_AndroidLibrayProjectIntermediatePath)"
 			CacheFile="$(_AndroidLibraryImportsCache)">
 	</GetImportedLibraries>
-	<Touch Files="$(_AndroidLibraryImportsCache).stamp" AlwaysCreate="True" />
+	<Touch Files="$(_AndroidStampDirectory)_BuildLibraryImportsCache.stamp" AlwaysCreate="True" />
 	<ItemGroup>
-		<FileWrites Include="$(_AndroidLibraryImportsCache).stamp" />
+		<FileWrites Include="$(_AndroidStampDirectory)_BuildLibraryImportsCache.stamp" />
 	</ItemGroup>
 </Target>
 
@@ -3227,7 +3228,6 @@ because xbuild doesn't support framework reference assemblies.
 	<Delete Files="$(_AndroidBuildPropertiesCache)" />
 	<Delete Files="$(_AdbPropertiesCache)" />
 	<Delete Files="$(_AndroidLibraryImportsCache)" />
-	<Delete Files="$(_AndroidLibraryImportsCache).stamp" />
 	<Delete Files="$(_AndroidStaticResourcesFlag)" />
 	<Delete Files="$(_AndroidLibraryProjectImportsCache)" />
 	<Delete Files="$(_AndroidLibrayProjectAssemblyMapFile)" />
