[Xamarin.Android.Build.Tasks] improve aapt2 incremental builds (dotne…

…t#3108) Context: https://github.com/microsoft/SmartHotel360-Mobile In 84daf03, we had to do a workaround to support custom views when aapt2 is enabled. We had to run a second `aapt2 compile` command for any layouts with custom views. In testing our build performance, I noticed a slow MSBuild task when building SmartHotel360 after a XAML change: Task Aapt2Compile 19.959s It makes sense that `_GenerateJavaStubs` ran in this case. This project does not set `$(ProduceReferenceAssembly)`, which is likely the current norm for our users. I think we can do two things to improve this: 1. Run `aapt2 compile` on only the resource directories needed, it looks like we are running against all of them. 2. Setup a new target, `_ConvertCustomView`, that will be skipped in cases of small code changes. ~~ Fix No. 1 ~~ If I look at the `<Aapt2Compile/>` task: Executing compile -o obj\Debug\90\flata\2a00f65fb8a92bd112f342ce47ad717dee3acac0.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\1\jl\res Executing compile -o obj\Debug\90\flata\compiled.flata --dir obj\Debug\90\res Executing compile -o obj\Debug\90\flata\5232a1999272d3d9b72f98243ed486aaa1b4593e.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\38\jl\res Executing compile -o obj\Debug\90\flata\3f5267fa5538ba387cf6e603b96e00903df1914b.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\2\jl\res Executing compile -o obj\Debug\90\flata\0b987eee7d559f77cfd2e1e25371901bfdff8ec1.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\3\jl\res Executing compile -o obj\Debug\90\flata\5f8ee424582825b02052b46957c7e3c7670acde2.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\40\jl\res Executing compile -o obj\Debug\90\flata\7dc9d90cc8afd2ac8593b7fc3453cc0b12962428.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\8\jl\res Executing compile -o obj\Debug\90\flata\4ea0f4c451bba05912018e80c7f701bc1e71bdd8.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\4\jl\res Executing compile -o obj\Debug\90\flata\6ddbc5d6f28551b3610dcb62292152f3e5a4da40.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\10\jl\res Executing compile -o obj\Debug\90\flata\e5b8974158d857e7ad168a3e7eabd456a6379b0e.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\47\jl\res Executing compile -o obj\Debug\90\flata\15c91c5a3803931df9940672b0b87012bb56eed1.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\51\jl\res Executing compile -o obj\Debug\90\flata\1a25ed1b2a2de98e844508551fd3f18f3f60d534.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\13\jl\res Executing compile -o obj\Debug\90\flata\1063b71e02c710a47b032ae2fc8562f2b3bf94df.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\54\jl\res Executing compile -o obj\Debug\90\flata\7ba81f9192d677ba83e3f1f60ba27e3cedbbe600.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\15\jl\res Executing compile -o obj\Debug\90\flata\fc32e11566de7e5136f8c1280232e152f27589bf.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\17\jl\res Executing compile -o obj\Debug\90\flata\f0d523a4e46c0143b6e7a2a64304bd801b1c4d16.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\33\jl\res Executing compile -o obj\Debug\90\flata\782859d4e93cff6e7b533c2bcc1277f6c798ddd3.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\57\jl\res Executing compile -o obj\Debug\90\flata\eafdbe607beb895e86092f8162e013887090ed3f.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\66\jl\res Executing compile -o obj\Debug\90\flata\494e789af4faf31d77b998f73d376655215d5b4c.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\55\jl\res Executing compile -o obj\Debug\90\flata\b1948383c89f878bc1d049b3f6d00df65bed48fe.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\53\jl\res Executing compile -o obj\Debug\90\flata\0d013f17da77e518af4784c2b3fefafb6eb38d58.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\68\jl\res Executing compile -o obj\Debug\90\flata\8380133d1dcd4b4db10f23298668b8c7d691fe66.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\14\jl\res Executing compile -o obj\Debug\90\flata\8b09e95d85ed31f0b62ba610183151161499e461.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\61\jl\res Executing compile -o obj\Debug\90\flata\b7fec66873f5eb1356f2b20d94949cce3beb2e84.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\34\jl\res Executing compile -o obj\Debug\90\flata\24a8542616e4eae9d9a4c037207e18ee77b8ed11.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\16\jl\res Executing compile -o obj\Debug\90\flata\1393196416e23b44d0bb6e4884ea0121d8441723.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\63\jl\res Executing compile -o obj\Debug\90\flata\3660f9af31d03d46d80b5af133a9256879a2fb0e.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\65\jl\res We only need to do this for a subset. Looking at this item group: _ProcessedCustomViews D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\2\jl\res\layout\horizontal_viewpager.xml Hash = 3f5267fa5538ba387cf6e603b96e00903df1914b StampFile = D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\2.stamp D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\2\jl\res\layout\vertical_viewpager.xml Hash = 3f5267fa5538ba387cf6e603b96e00903df1914b StampFile = D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\2.stamp We should just run: Executing compile -o obj\Debug\90\flata\3f5267fa5538ba387cf6e603b96e00903df1914b.flata --dir D:\a\1\s\SmartHotel\Source\SmartHotel.Clients\SmartHotel.Clients.Android\obj\Debug\90\lp\2\jl\res To make this work: * I added `%(_ProcessedCustomViews.ResourceDirectory)` metadata, which is the resource directory containing the custom views. * We can run `aapt2 compile` against `%(_ProcessedCustomViews.ResourceDirectory)->Distinct()`: each directory containing an updated view, not all directories. ~~ Fix No 2. ~~ I moved the calls to `<ConvertCustomView/>` and `<Aapt2Compile/>` to a new `_ConvertCustomView` target. It can be skipped unless the `$(_CustomViewMapFile)` or `$(_AcwMapFile)` change in regards to a new `_ConvertCustomView.stamp` stamp file. This allows us to skip the task in a lot of cases for incremental builds. ~~ Other Changes ~~ I created a new `IncrementalBuildTest.ConvertCustomView()` test for these scenarios. I also made a few improvements to the XML formatting: use of spaces over tabs, fixing indentation, and putting the `Condition` attribute first. ~~ Results ~~ I tested the SmartHotel360 app, since this is where I saw the issue. Initial build: Before: 24492 ms _GenerateJavaStubs 1 calls After: 3933 ms _GenerateJavaStubs 1 calls 132 ms _ConvertCustomView 1 calls Incremental build with XAML change: Before: 24358 ms _GenerateJavaStubs 1 calls After: 3416 ms _GenerateJavaStubs 1 calls 29 ms _ConvertCustomView 1 calls They key here is going from 27 `aapt2 compile` calls to 1! NOTE: the before/after is not *exactly* accurate, since the before times were recorded on Azure DevOps. I would think this change could easily improve `aapt2` builds by 10 seconds or more.
grendello · May 22, 2019 · 373c6ed · 373c6ed
1 parent d5e002e
commit 373c6ed
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 23 deletions.
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ConvertCustomView.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ConvertCustomView.cs
@@ -90,8 +90,9 @@ public override bool Execute ()
 				var stampFile = !string.IsNullOrEmpty (stamp) ? stamp : $"{filename}.stamp";
 				Log.LogDebugMessage ($"{filename} {stampFile}");
 				output.Add (new TaskItem (file, new Dictionary<string, string> {
-					{ "StampFile" , $"{stampFile}" },
-					{ "Hash" , $"{filename}" },
+					{ "StampFile" , stampFile },
+					{ "Hash" , filename },
+					{ "ResourceDirectory", resdir.ItemSpec }
 				}));
 			}
 			Processed = output.ToArray ();

diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs
@@ -525,6 +525,88 @@ public void ProduceReferenceAssembly ()
 			}
 		}
 
+		[Test]
+		public void ConvertCustomView ([Values (true, false)] bool useAapt2)
+		{
+			var path = Path.Combine ("temp", TestName);
+			var app = new XamarinAndroidApplicationProject {
+				ProjectName = "MyApp",
+				Sources = {
+					new BuildItem.Source ("Foo.cs") {
+						TextContent = () => "public class Foo : Bar { }"
+					},
+					new BuildItem.Source ("CustomTextView.cs") {
+						TextContent = () => 
+							@"using Android.Widget;
+							using Android.Content;
+							using Android.Util;
+							namespace MyApp
+							{
+								public class CustomTextView : TextView
+								{
+									public CustomTextView(Context context, IAttributeSet attributes) : base(context, attributes)
+									{
+									}
+								}
+							}"
+					}
+				}
+			};
+			// Use a custom view
+			app.LayoutMain = app.LayoutMain.Replace ("</LinearLayout>", "<MyApp.CustomTextView android:id=\"@+id/myText\" /></LinearLayout>");
+			//NOTE: so _BuildApkEmbed runs in commercial tests
+			app.SetProperty ("EmbedAssembliesIntoApk", "True");
+			app.SetProperty ("AndroidUseSharedRuntime", "False");
+			app.SetProperty ("AndroidUseAapt2", useAapt2.ToString ());
+
+			int count = 0;
+			var lib = new DotNetStandard {
+				ProjectName = "MyLibrary",
+				Sdk = "Microsoft.NET.Sdk",
+				TargetFramework = "netstandard2.0",
+				Sources = {
+					new BuildItem.Source ("Bar.cs") {
+						TextContent = () => "public class Bar { public Bar () { System.Console.WriteLine (" + count++ + "); } }"
+					},
+				}
+			};
+			//NOTE: this test is checking when $(ProduceReferenceAssembly) is False
+			lib.SetProperty ("ProduceReferenceAssembly", "False");
+			app.References.Add (new BuildItem.ProjectReference ($"..\\{lib.ProjectName}\\{lib.ProjectName}.csproj", lib.ProjectName, lib.ProjectGuid));
+
+			using (var libBuilder = CreateDllBuilder (Path.Combine (path, lib.ProjectName), false))
+			using (var appBuilder = CreateApkBuilder (Path.Combine (path, app.ProjectName))) {
+				Assert.IsTrue (libBuilder.Build (lib), "first library build should have succeeded.");
+				Assert.IsTrue (appBuilder.Build (app), "first app build should have succeeded.");
+
+				lib.Touch ("Bar.cs");
+
+				Assert.IsTrue (libBuilder.Build (lib, doNotCleanupOnUpdate: true, saveProject: false), "second library build should have succeeded.");
+				Assert.IsTrue (appBuilder.Build (app, doNotCleanupOnUpdate: true, saveProject: false), "second app build should have succeeded.");
+
+				var targetsShouldSkip = new [] {
+					"_BuildLibraryImportsCache",
+					"_ResolveLibraryProjectImports",
+					"_ConvertCustomView",
+				};
+				foreach (var target in targetsShouldSkip) {
+					Assert.IsTrue (appBuilder.Output.IsTargetSkipped (target), $"`{target}` should be skipped!");
+				}
+
+				var targetsShouldRun = new [] {
+					//MyLibrary.dll changed and $(ProduceReferenceAssembly)=False
+					"CoreCompile",
+					"_GenerateJavaStubs",
+					"_BuildApkEmbed",
+					"_CopyPackage",
+					"_Sign",
+				};
+				foreach (var target in targetsShouldRun) {
+					Assert.IsFalse (appBuilder.Output.IsTargetSkipped (target), $"`{target}` should *not* be skipped!");
+				}
+			}
+		}
+
 		[Test]
 		public void ResolveLibraryProjectImports ()
 		{

diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -2309,34 +2309,41 @@ because xbuild doesn't support framework reference assemblies.
     <FileWrites Include="@(_TypeMapAssemblySource)" />
   </ItemGroup>
 
+  <Touch Files="$(_AndroidStampDirectory)_GenerateJavaStubs.stamp" AlwaysCreate="True" />
+</Target>
+
+<Target Name="_ConvertCustomView"
+    Condition="Exists('$(_CustomViewMapFile)')"
+    Inputs="$(_CustomViewMapFile);$(_AcwMapFile)"
+    Outputs="$(_AndroidStampDirectory)_ConvertCustomView.stamp">
   <ConvertCustomView
-	Condition="Exists('$(_CustomViewMapFile)')"
-	CustomViewMapFile="$(_CustomViewMapFile)"
-	AcwMapFile="$(_AcwMapFile)"
-	ResourceDirectories="$(MonoAndroidResDirIntermediate);@(_LibraryResourceHashDirectories)"
-	ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)"
-  >
+      CustomViewMapFile="$(_CustomViewMapFile)"
+      AcwMapFile="$(_AcwMapFile)"
+      ResourceDirectories="$(MonoAndroidResDirIntermediate);@(_LibraryResourceHashDirectories)"
+      ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)">
     <Output TaskParameter="Processed" ItemName="_ProcessedCustomViews" />	
   </ConvertCustomView>
-  <Delete Files="@(_ProcessedCustomViews->'$(_AndroidLibraryFlatArchivesDirectory)%(Hash).stamp')"
-    Condition=" '$(AndroidUseAapt2)' == 'True' And '@(_ProcessedCustomViews)' != '' "
+  <Delete
+      Condition=" '$(AndroidUseAapt2)' == 'True' And '@(_ProcessedCustomViews)' != '' "
+      Files="@(_ProcessedCustomViews->'$(_AndroidLibraryFlatArchivesDirectory)%(Hash).stamp')"
   />
   <Aapt2Compile
-    Condition=" '$(AndroidUseAapt2)' == 'True' And '@(_ProcessedCustomViews)' != '' "
-    ContinueOnError="$(DesignTimeBuild)"
-    ResourceDirectories="@(_LibraryResourceHashDirectories);$(MonoAndroidResDirIntermediate)"
-    ExplicitCrunch="$(AndroidExplicitCrunch)"
-    ExtraArgs="$(AndroidAapt2CompileExtraArgs)"
-    FlatArchivesDirectory="$(_AndroidLibraryFlatArchivesDirectory)"
-    ToolPath="$(Aapt2ToolPath)"
-    ToolExe="$(Aapt2ToolExe)">
+      Condition=" '$(AndroidUseAapt2)' == 'True' And '@(_ProcessedCustomViews)' != '' "
+      ContinueOnError="$(DesignTimeBuild)"
+      ResourceDirectories="@(_ProcessedCustomViews->'%(ResourceDirectory)'->Distinct())"
+      ExplicitCrunch="$(AndroidExplicitCrunch)"
+      ExtraArgs="$(AndroidAapt2CompileExtraArgs)"
+      FlatArchivesDirectory="$(_AndroidLibraryFlatArchivesDirectory)"
+      ToolPath="$(Aapt2ToolPath)"
+      ToolExe="$(Aapt2ToolExe)">
     <Output TaskParameter="CompiledResourceFlatArchives" ItemName="_UpdatedFlatArchives" />	
   </Aapt2Compile>
-  <Touch Files="@(_UpdatedFlatArchives->'$(_AndroidLibraryFlatArchivesDirectory)\%(Filename).stamp')"
-    Condition=" '$(AndroidUseAapt2)' == 'True' And '@(_UpdatedFlatArchives)' != '' "
-    AlwaysCreate="True"
+  <Touch
+      Condition=" '@(_UpdatedFlatArchives)' != '' "
+      Files="@(_UpdatedFlatArchives->'$(_AndroidLibraryFlatArchivesDirectory)\%(Filename).stamp')"
+      AlwaysCreate="True"
   />
-  <Touch Files="$(_AndroidStampDirectory)_GenerateJavaStubs.stamp" AlwaysCreate="True" />
+  <Touch Files="$(_AndroidStampDirectory)_ConvertCustomView.stamp" AlwaysCreate="True" />
 </Target>
 
 <Target Name="_ReadAndroidManifest"
@@ -2400,6 +2407,7 @@ because xbuild doesn't support framework reference assemblies.
 <PropertyGroup>
   <_GeneratePackageManagerJavaDependsOn>
     _GenerateJavaStubs;
+    _ConvertCustomView;
     _GenerateEnvironmentFiles;
     _AddStaticResources;
     $(_AfterAddStaticResources);
@@ -2453,6 +2461,7 @@ because xbuild doesn't support framework reference assemblies.
 <PropertyGroup>
 	<_CreateBaseApkDependsOnTargets>
 		_GenerateJavaStubs;
+		_ConvertCustomView;
 		_GenerateEnvironmentFiles;
 		_GetLibraryImports;
 		_CheckDuplicateJavaLibraries;
@@ -2578,7 +2587,7 @@ because xbuild doesn't support framework reference assemblies.
   </ItemGroup>
 </Target>
 
-<Target Name="_FindJavaStubFiles" DependsOnTargets="_GenerateJavaStubs;_GenerateEnvironmentFiles;">
+<Target Name="_FindJavaStubFiles" DependsOnTargets="_GenerateJavaStubs;_ConvertCustomView;_GenerateEnvironmentFiles;">
   <CreateItem
     Include="$(IntermediateOutputPath)android\src\\**\*.java">
     <Output TaskParameter="Include" ItemName="_JavaStubFiles" />
@@ -2944,6 +2953,7 @@ because xbuild doesn't support framework reference assemblies.
 		_CopyMdbFiles;
 		_LinkAssemblies;
 		_GenerateJavaStubs;
+		_ConvertCustomView;
 		_GenerateEnvironmentFiles;
 		_CompileJava;
 		_CompileDex;