From 1db799de521a2d6ab036b2749839e20fe613ae19 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 16 May 2018 16:10:00 -0500 Subject: [PATCH 1/2] [Xamarin.Android.Build.Tasks] fix timestamps in various intermediate folders When taking an audit of our build times, I was testing the following scenario: - `msbuild Droid.csproj /t:Install /bl:first.binlog` - `msbuild Droid.csproj /t:Install /bl:second.binlog` In theory, the "second" build should be very fast and basically not do anything. Unfortunately that was not the case. In my example, I saw build logs such as: Target _LinkAssembliesNoShrink Building target "_LinkAssembliesNoShrink" partially, because some output files are out of date with respect to their input files. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\FormsViewGroup.dll, Output=obj\Debug\android\assets\FormsViewGroup.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Core.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Core.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Platform.Android.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Platform.Android.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Platform.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Platform.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Xaml.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Xaml.dll] Input file is newer than output file. LinkAssemblies Looking at the `LinkAssemblies` MSBuild task, I didn't see anything that would be setting the timestamps on copied files. For timestamps to be correct, we should either: - Use `MonoAndroidHelper.SetLastAccessAndWriteTimeUtc` in C# - Use the `` MSBuild task after using the `` MSBuild task This lead me to write a `CheckTimestamps` unit test that does the following: - Store `DateTime.UtcNow` in a variable (and subtract one second for good measure) - Build an app - Make sure nothing in `bin` or `obj` are older than the start time This uncovered even more out of date files such as: - `mono.android.jar` - Assemblies in `obj/Debug/linksrc` or `$(MonoAndroidLinkerInputDir)` Fixes made in various places: - `` task needed to use `MonoAndroidHelper.SetLastAccessAndWriteTimeUtc` everywhere `MonoAndroidHelper.CopyIfChanged` is used - The `_CopyIntermediateAssemblies` target appeared to have a typo. It was running a `` on `ResolvedUserAssemblies` where it looked like it should be `ResolvedAssemblies` from the above `` - The `_AddStaticResources` target needed a `` for `mono.android.jar` This fix should improve our incremental build times dramatically, as these timestamps caused other targets to run that slow things down. --- .../Tasks/LinkAssemblies.cs | 18 ++++++++++--- .../Xamarin.Android.Build.Tests/BuildTest.cs | 25 +++++++++++++++++++ .../Xamarin.Android.Common.targets | 3 ++- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs index 5c093ef9f05..86d9015e703 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs @@ -167,14 +167,24 @@ bool Execute (DirectoryAssemblyResolver res) else if (!MonoAndroidHelper.IsForceRetainedAssembly (filename)) continue; - MonoAndroidHelper.CopyIfChanged (copysrc, Path.Combine (copydst, filename)); + var assemblyDestination = Path.Combine (copydst, filename); + if (MonoAndroidHelper.CopyIfChanged (copysrc, assemblyDestination)) { + MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (assemblyDestination, DateTime.UtcNow, Log); + } try { - MonoAndroidHelper.CopyIfChanged (assembly.ItemSpec + ".mdb", Path.Combine (copydst, filename + ".mdb")); + var mdbDestination = assemblyDestination + ".mdb"; + if (MonoAndroidHelper.CopyIfChanged (assembly.ItemSpec + ".mdb", mdbDestination)) { + MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (mdbDestination, DateTime.UtcNow, Log); + } } catch (Exception) { // skip it, mdb sometimes fails to read and it's optional } var pdb = Path.ChangeExtension (copysrc, "pdb"); - if (File.Exists (pdb) && Files.IsPortablePdb (pdb)) - MonoAndroidHelper.CopyIfChanged (pdb, Path.ChangeExtension (Path.Combine (copydst, filename), "pdb")); + if (File.Exists (pdb) && Files.IsPortablePdb (pdb)) { + var pdbDestination = Path.ChangeExtension (Path.Combine (copydst, filename), "pdb"); + if (MonoAndroidHelper.CopyIfChanged (pdb, pdbDestination)) { + MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (pdbDestination, DateTime.UtcNow, Log); + } + } } } catch (ResolutionException ex) { Diagnostic.Error (2006, ex, "Could not resolve reference to '{0}' (defined in assembly '{1}') with scope '{2}'. When the scope is different from the defining assembly, it usually means that the type is forwarded.", ex.Member, ex.Member.Module.Assembly, ex.Scope); 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 index 90848f243c9..ff5d1def835 100644 --- 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 @@ -120,6 +120,31 @@ public void FooMethod () { } } + [Test] + public void CheckTimestamps () + { + var start = DateTime.UtcNow.AddSeconds (-1); + var proj = new XamarinAndroidApplicationProject (); + using (var b = CreateApkBuilder ("temp/CheckTimestamps")) { + //To be sure we are at a clean state, delete bin/obj + var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath); + if (Directory.Exists (intermediate)) + Directory.Delete (intermediate, true); + var output = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath); + if (Directory.Exists (output)) + Directory.Delete (output, true); + Assert.IsTrue (b.Build (proj), "Build should have succeeded."); + + //Absolutely non of these files should be *older* than the starting time of this test! + var files = Directory.EnumerateFiles (intermediate, "*", SearchOption.AllDirectories).ToList (); + files.AddRange (Directory.EnumerateFiles (output, "*", SearchOption.AllDirectories)); + foreach (var file in files) { + var info = new FileInfo (file); + Assert.IsTrue (info.LastWriteTimeUtc > start, $"`{file}` is older than `{start}`, with a timestamp of `{info.LastWriteTimeUtc}`!"); + } + } + } + [Test] public void BuildApplicationAndClean ([Values (false, true)] bool isRelease) { diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index b9945ea7fdb..c66dc029365 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1704,6 +1704,7 @@ because xbuild doesn't support framework reference assemblies. SourceFiles="$(MonoPlatformJarPath)" DestinationFiles="$(IntermediateOutputPath)android\bin\mono.android.jar" SkipUnchangedFiles="true" /> + @@ -1730,7 +1731,7 @@ because xbuild doesn't support framework reference assemblies. DestinationFiles="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')" SkipUnchangedFiles="true" /> - + From 9d7655ed6135ebd5a6084afb41122fce7ca24a8b Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 18 May 2018 08:42:20 -0500 Subject: [PATCH 2/2] [tests] improve random failures of ResolveSdkTiming This test can be a bit flaky, and fail randomly: Xamarin.Android.Build.Tests.ResolveSdksTaskTests.ResolveSdkTiming Task should not take more than 1 second to run. Expected: less than or equal to 00:00:01 But was: 00:00:01.9140742 To improve this, I changed: - The timeout to 2 seconds - Added `[NonParallelizable]`, so the test should not have to compete for machine resources as much with other tests --- .../Tests/Xamarin.Android.Build.Tests/ResolveSdksTaskTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ResolveSdksTaskTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ResolveSdksTaskTests.cs index 903d3142aa1..8e77f4a385e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ResolveSdksTaskTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ResolveSdksTaskTests.cs @@ -190,7 +190,7 @@ public void UseLatestAndroidSdk (string buildtools, string jdk, ApiInfo[] apis, Directory.Delete (Path.Combine (Root, path), recursive: true); } - [Test] + [Test, NonParallelizable] public void ResolveSdkTiming () { var path = Path.Combine ("temp", TestName); @@ -226,7 +226,7 @@ public void ResolveSdkTiming () var start = DateTime.UtcNow; Assert.IsTrue (task.Execute ()); var executionTime = DateTime.UtcNow - start; - Assert.LessOrEqual (executionTime, TimeSpan.FromSeconds(1), "Task should not take more than 1 second to run."); + Assert.LessOrEqual (executionTime, TimeSpan.FromSeconds(2), "Task should not take more than 2 seconds to run."); Assert.AreEqual (task.AndroidApiLevel, "26", "AndroidApiLevel should be 26"); Assert.AreEqual (task.TargetFrameworkVersion, "v8.0", "TargetFrameworkVersion should be v8.0"); Assert.AreEqual (task.AndroidApiLevelName, "26", "AndroidApiLevelName should be 26");