Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] timestamp fixes for incremental builds (d…
Browse files Browse the repository at this point in the history
…otnet#1930)

Context: https://github.com/xamarin/Xamarin.Forms/blob/42c07d1ae5aa56eb574b7d169499f1a9af7ec080/Xamarin.Forms.ControlGallery.Android/Xamarin.Forms.ControlGallery.Android.csproj

While working on build performance, I noticed two timestamp issues
that are preventing some targets in Xamarin.Android from building
incrementally. I spotted both of these while timing builds of the
`Xamarin.Forms.ControlGallery.Android` project. It is a good test
subject, because it builds PCLs, and seven (or so) Xamarin.Android
library projects.

~~ _LinkAssembliesNoShrink ~~

While timing builds, I noticed the following when
`_LinkAssembliesNoShrink` runs:

    Target Name=_LinkAssembliesNoShrink Project=PagesGallery.Droid.csproj
        Building target "_LinkAssembliesNoShrink" partially, because some output files are out of date with respect to their input files.
        [ResolvedUserAssemblies: Input=C:\Users\myuser\Desktop\Git\Xamarin.Forms\PagesGallery\PagesGallery.Droid\bin\Debug\PagesGallery.Droid.dll, Output=obj\\Debug\android\assets\PagesGallery.Droid.dll] Input file is newer than output file.
        ...
        Skip linking unchanged file: C:\Users\myuser\Desktop\Git\Xamarin.Forms\PagesGallery\PagesGallery.Droid\bin\Debug\PagesGallery.Droid.dll

It seems there is a case where MSBuild is thinking the target needs to
run (due to timestamp values), but the `LinkAssemblies` MSBuild task
does not copy the file. It actually doesn't need to do any work at all
here, since this was a build with no changes.

The fix here is to apply a timestamp for files that get skipped. This
prevents the target from running again when it doesn't need to.

I also removed a spot where a `copysrc` variable looks like it was
being set an extra time--did not seem to be needed.

~~ _GenerateJavaStubs ~~

After fixing `_LinkAssembliesNoShrink`, I noticed another timestamp
issue:

    Building target "_GenerateJavaStubs" completely.
    Input file "obj\\Debug\android\assets\PagesGallery.Droid.dll" is newer than output file "obj\\Debug\android\typemap.jm".

Looking at the `GenerateJavaStubs` MSBuild task, it was never setting
the timestamp on either `typemap.mj` or `typemap.jm`.

Fixing these two issues, I was able to get a build with no-changes in
this Xamarin.Forms project down from ~23 seconds to ~13 seconds. I
also updated the `CheckTimestamps` test to validate these changes.
  • Loading branch information
jonathanpeppers authored and dellis1972 committed Jul 13, 2018
1 parent 0ab8ec1 commit 3d999d3
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
3 changes: 2 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,8 @@ void UpdateWhenChanged (string path, Action<Stream> generator)
var np = path + ".new";
using (var o = File.OpenWrite (np))
generator (o);
Files.CopyIfChanged (np, path);
MonoAndroidHelper.CopyIfChanged (np, path);
MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (path, DateTime.UtcNow, Log);
File.Delete (np);
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,18 @@ bool Execute (DirectoryAssemblyResolver res)
foreach (var assembly in ResolvedAssemblies) {
var copysrc = assembly.ItemSpec;
var filename = Path.GetFileName (assembly.ItemSpec);
var assemblyDestination = Path.Combine (copydst, filename);

if (options.LinkNone) {
if (skiplist.Any (s => Path.GetFileNameWithoutExtension (filename) == s)) {
// For skipped assemblies, skip if there is existing file in the destination.
// We cannot just copy the linker output from *current* run output, because
// it always renew the assemblies, in *different* binary values, whereas
// the dll in the OptionalDestinationDirectory must retain old and unchanged.
if (File.Exists (Path.Combine (copydst, filename)))
if (File.Exists (assemblyDestination)) {
MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (assemblyDestination, DateTime.UtcNow, Log);
continue;
copysrc = assembly.ItemSpec;
}
} else {
// Prefer fixup assemblies if exists, otherwise just copy the original.
copysrc = Path.Combine (OutputDirectory, filename);
Expand All @@ -171,7 +173,6 @@ bool Execute (DirectoryAssemblyResolver res)
else if (!MonoAndroidHelper.IsForceRetainedAssembly (filename))
continue;

var assemblyDestination = Path.Combine (copydst, filename);
if (MonoAndroidHelper.CopyIfChanged (copysrc, assemblyDestination)) {
MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (assemblyDestination, DateTime.UtcNow, Log);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public void CheckTimestamps ()
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.");
Assert.IsTrue (b.Build (proj), "first 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 ();
Expand All @@ -167,6 +167,21 @@ public void CheckTimestamps ()
var info = new FileInfo (file);
Assert.IsTrue (info.LastWriteTimeUtc > start, $"`{file}` is older than `{start}`, with a timestamp of `{info.LastWriteTimeUtc}`!");
}

//Build again after a code change, checking a few files
proj.MainActivity = proj.DefaultMainActivity.Replace ("clicks", "CLICKS");
proj.Touch ("MainActivity.cs");
start = DateTime.UtcNow;
Assert.IsTrue (b.Build (proj), "second build should have succeeded.");

foreach (var file in new [] { "typemap.mj", "typemap.jm" }) {
var info = new FileInfo (Path.Combine (intermediate, "android", file));
Assert.IsTrue (info.LastWriteTimeUtc > start, $"`{file}` is older than `{start}`, with a timestamp of `{info.LastWriteTimeUtc}`!");
}

//One last build with no changes
Assert.IsTrue (b.Build (proj), "third build should have succeeded.");
Assert.IsTrue (b.Output.IsTargetSkipped ("_LinkAssembliesNoShrink"), "`_LinkAssembliesNoShrink` should be skipped!");
}
}

Expand Down

0 comments on commit 3d999d3

Please sign in to comment.