From 48f0fe79c090abe6936914676306f0c1f163b34d Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 12 Nov 2019 10:31:11 -0600 Subject: [PATCH] [Xamarin.Android.Build.Tasks] DTBs and _ResolveLibraryProjectImports (#3891) I spent a little time looking at build logs of design-time builds inside VS Windows. To trigger these, I was editing an `.axml` file and saving it. I found some perf issues... The first thing I noticed was that `_ResolveLibraryProjectImports` runs on every DTB: Building target "_ResolveLibraryProjectImports" completely.I spent a little time looking at build logs of design-time builds inside VS Windows. To trigger these, I was editing an `.axml` file and saving it. I found some perf issues... The first thing I noticed was that the `_ResolveLibraryProjectImports` target runs on every DTB: Building target "_ResolveLibraryProjectImports" completely. Input file "obj\Debug\100\designtime\build.props" is newer than output file "obj\Debug\100\stamp\_ResolveLibraryProjectImports.stamp". Then I looked at how `build.props` was actually changing between different targets such as: * `UpdateGeneratedFiles`: the general DTB target * `SetupDependenciesForDesigner`: the Android designer runs this target Comparing the files, I saw this difference: - AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk + AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk\ - AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle + AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle\ - JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25 + JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25\ So in some cases the `_ResolveMonoAndroidSdks` target is running to normalize the paths and other times not! The `_CreatePropertiesCache` target should depend on the `_ResolveMonoAndroidSdks` target to fix this issue. After that I found the *next* issue: - DesignTimeBuild = True + DesignTimeBuild = true What??? It appears that the casing differs depending on which part of the IDE is triggering the build. I suspect the designer is using one casing while VS proper uses another. I think we should call `ToLowerInvariant()` on every line in the `build.props` file to work around MSBuild's case insensitivity. This change saves ~350ms on every design-time build. However, if `build.props` changes all the time it may do even better, since `build.props` triggers almost all MSBuild targets in Xamarin.Android. Input file "obj\Debug\100\designtime\build.props" is newer than output file "obj\Debug\100\stamp\_ResolveLibraryProjectImports.stamp". Then I looked at how `build.props` was actually changing between different targets such as: * `UpdateGeneratedFiles` - the general DTB target * `SetupDependenciesForDesigner` - the Android designer runs this target Comparing the files, I saw this difference: - AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk + AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk\ - AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle + AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle\ - JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25 + JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25\ So in some cases `_ResolveMonoAndroidSdks` is running to normalize the paths and other times not... `_CreatePropertiesCache` should depend on `_ResolveMonoAndroidSdks` to fix this issue. After that I found the *next* issue: - DesignTimeBuild = True + DesignTimeBuild = true What??? It appears that the casing differs depending on which part of the IDE is triggering the build. I suspect the designer is using one casing while VS proper uses another. I think we should call `ToLowerInvariant()` on every line in the `build.props` file to work around MSBuild's case insensitivity. This change saves ~350ms on every design-time build. However, if `build.props` changes all the time it may do even better, since `build.props` triggers almost all MSBuild targets in Xamarin.Android. --- .../IncrementalBuildTest.cs | 16 ++++++++++++++++ .../Xamarin.Android.Common.targets | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) 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 index da131d7cc25..1a93948deda 100644 --- 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 @@ -951,6 +951,22 @@ public void DeterministicBuilds ([Values (true, false)] bool deterministic) } } + [Test] + public void DesignTimeBuild () + { + var proj = new XamarinAndroidApplicationProject (); + using (var b = CreateApkBuilder (Path.Combine ("temp", $"{nameof (IncrementalBuildTest)}{TestName}"))) { + Assert.IsTrue (b.DesignTimeBuild (proj), "first dtb should have succeeded."); + // DesignTimeBuild=true lowercased + var parameters = new [] { "DesignTimeBuild=true" }; + Assert.IsTrue (b.RunTarget (proj, "Compile", doNotCleanupOnUpdate: true, parameters: parameters), "second dtb should have succeeded."); + var target = "_ResolveLibraryProjectImports"; + Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should have been skipped."); + Assert.IsTrue (b.RunTarget (proj, "UpdateGeneratedFiles", doNotCleanupOnUpdate: true, parameters: parameters), "UpdateGeneratedFiles should have succeeded."); + Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should have been skipped."); + } + } + [Test] public void ChangePackageNamingPolicy () { diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 04e7b9694fa..8403f0a8152 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1269,7 +1269,7 @@ because xbuild doesn't support framework reference assemblies. <_AndroidManagedResourceDesignerFile>$(_AndroidIntermediateDesignTimeBuildDirectory)$(_AndroidResourceDesigner) - + <_PropertyCacheItems Include="BundleAssemblies=$(BundleAssemblies)" /> @@ -1303,7 +1303,7 @@ because xbuild doesn't support framework reference assemblies.