[Xamarin.Android.Build.Tasks] Allow BuildApk to run incrementally. #3999

dellis1972 · 2019-12-06T17:27:48Z

The BuildApk task currently does not work incrementally.
When it is called it throws away the previously built apk
in favour of using the base on that aapt/aapt2 just built.
With incremental builds it is highly likely that only a
few files have actually changed so we are wasting a ton of time.

During my tests on a simple app BuildApk generally takes
about 4 seconds to run. This is the same for both clean
and incremental builds.
With these changes, this time could now go down to 600 ms
for an incremental build. Which is a decent improvement.

The new system does not throw away all the work done in
the previous build. Instead it uses the previous output
apk as a base on which to produce the new one.
The output from aapt/aapt2 is instead merged into the
current apk. We do this by checking the CRC values of
each entry and only merging in files which have actually
changed. We need to use the CRC here because aapt/aapt2
is NEVER setting the ModifiedDate on the zip items :(.
It seems to always be 01/01/1980.

Once those changes are out of the way , we can now move on
to our custom content. This time we can use the ModifiedDate
of both the ZipEntry AND the source file. This allows us
to, again, only update the items which have accutally changed.

One final part is we need to keep track of what files are in the
apk in the first place and which ones get added to it. This is
what the existingEntries List is used for. This allows us to
detect when files are REMOVED from the system. Otherwise we end up
with stale files in the apk from previous builds.

On my tests when changing a single AndroidResource file I got
the following results

3664 ms BuildApk                  1 calls (Before)
669 ms BuildApk                  1 calls (After)

Which is a decent improvement.

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs

dellis1972 · 2019-12-07T16:48:14Z

/azp run

azure-pipelines · 2019-12-07T16:48:32Z

Azure Pipelines successfully started running 1 pipeline(s).

jonathanpeppers

Since Dean is out for the holidays, I'll look into these.

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs

jonathanpeppers · 2019-12-09T20:36:29Z

@dellis1972 if you happen to stop by, I'm getting something weird on Windows:

In this unit test, the zip entry has a ModificationTime older than the file on disk?

Could the code producing these timestamps have a problem?

dellis1972 · 2019-12-10T15:03:25Z

@dellis1972 if you happen to stop by, I'm getting something weird on Windows:

In this unit test, the zip entry has a ModificationTime older than the file on disk?

Could the code producing these timestamps have a problem?

The timestamps in the zip are from the File afaik, and libzip handles all of that.

jonathanpeppers · 2019-12-11T21:12:59Z

In this unit test, the zip entry has a ModificationTime older than the file on disk?

The issue was that the zip file doesn't include milliseconds, but on Windows you get millisecond precision from file timestamps.

dellis1972 · 2020-01-14T15:52:33Z

Getting weird test results with the SkipExistingFile_Exist and SkipExistingFile_TimeStamp tests.

The A.txt file is written to disk before the zip is created...

BUT when we examine the date times later.. the file on disk has a NEWER time???? By almost a second. Nothing touches this file between it being created, added to the zip and then being read to get the TimeStamp.

14/01/2020 15:49:59 (disk 01/14/2020 15:49:59:688) <= 14/01/2020 15:49:58 (zip 01/14/2020 15:49:58:000)

dellis1972 · 2020-01-17T16:23:02Z

We have a weird problem where the Modified Date of the entry in a zip is somehow OLDER than the source file it is created from. Its consistently 1 second older, but its not 100% of the time. It seems like some sort of rounding error 🤷‍♂

It turns out this is a limitation of the Zip file format. If we read the Structure document [1] we come across this sentence.

The FAT filesystem of DOS has a timestamp resolution of only two seconds; ZIP file records mimic this. As a result, the built-in timestamp resolution of files in a ZIP archive is only two seconds, though extra fields can be used to store more precise timestamps.
So a modified date in a zip file can be one of TWO seconds. We can confirm this issue by looking at the time encoding functions used within lib zip itself. The Encoding code makes used of bit shifting to encode the unix timestamp into the required Zip format. The Decoding code reverses this operation. So lets run through a few examples.

So given a time of 12:46 and 29 seconds if we run through the code we get a value of 26062 to store in the zip file local header. If we use 12:46 and 30 seconds we get 26063. So far so go. Now let's plug these encoded values into our decoding code (x << 1) & 62.

(26062 << 1) & 62 = 28
(26063 << 1) & 62 = 30

As you can see while the 30 second timestamp decoded correctly, the 29 second one did not. So when we convert these values back to DateTime values we can end up with a timestamp that is 1 second in the past as it always rounds down. This is not great when we want to try to compare timestamps with files created on disk. Say a file was created at 29 seconds and we create the zip at the same time (within a second), according to the zip it would have been created at 28 seconds. So if we use the timestamps to compare it will thing the file is NEWER. Because this only happens if the zip and the file are created within the same 1 second timespan, this is not always a problem.

The work around in this case is to detect when we get these collisions and bump UP to the next second when we are creating the zip file.

But to be honest, I'm not sure there will ever be a good workaround because of the format of the zip file.

[1] https://en.wikipedia.org/wiki/Zip_(file_format)#Structure

dellis1972 · 2020-01-20T09:38:49Z

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ZipArchiveExTests.cs

-			string root = Path.GetDirectoryName (GetType ().Assembly.Location);
-			temp = Path.Combine (root, "temp", TestContext.CurrentContext.Test.Name);
+			root = Path.GetDirectoryName (GetType ().Assembly.Location);
+			string temp = Path.Combine (root, "temp", TestContext.CurrentContext.Test.Name);


Tests were running in Parallel, and because some used the same root folder they tripped up over each other. So I had to rework this a bit so it would work in a parallel environment.

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ZipArchiveExTests.cs

dellis1972 · 2020-01-21T16:41:46Z

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/XamarinProject.cs

@@ -272,7 +272,7 @@ public virtual void UpdateProjectFiles (string directory, IEnumerable<ProjectRes
 					continue;
 				}
 				string filedir = directory;
-				if (path.Contains (Path.DirectorySeparatorChar)) {
+				if (path.Contains ($"{Path.DirectorySeparatorChar}")) {


@jonathanpeppers do we prefer this or .ToString ?

I think both look fine, so I looked at the IL produced:

// if (text.Contains($"{Path.DirectorySeparatorChar}")) IL_00bb: ldloc.2 IL_00bc: ldstr "{0}" IL_00c1: ldsfld char [netstandard]System.IO.Path::DirectorySeparatorChar IL_00c6: box [netstandard]System.Char IL_00cb: call string [netstandard]System.String::Format(string, object) IL_00d0: callvirt instance bool [netstandard]System.String::Contains(string)

So I think it only matters if perf is a concern, ToString() would be better and avoid a string.Format("{0}", Path.DirectorySeparatorChar). It also appears to box!

This PR is green and it's a test, so I wouldn't change it.

Its good to know that ToString is more performant :D

dellis1972 · 2020-01-27T10:20:09Z

/azp run

azure-pipelines · 2020-01-27T10:20:34Z

Azure Pipelines successfully started running 1 pipeline(s).

dellis1972 · 2020-01-28T19:15:42Z

/azp run

azure-pipelines · 2020-01-28T19:16:15Z

Azure Pipelines successfully started running 1 pipeline(s).

The `BuildApk` task currently does not work incrementally. When it is called it throws away the previously built apk in favour of using the `base` on that `aapt/aapt2` just built. With incremental builds it is highly likely that only a few files have actually changed so we are wasting a ton of time. During my tests on a simple app `BuildApk` generally takes about 4 seconds to run. This is the same for both clean and incremental builds. With these changes, this time could now go down to 600 ms for an incremental build. Which is a decent improvement. The new system does not throw away all the work done in the previous build. Instead it uses the previous output apk as a base on which to produce the new one. The output from `aapt/aapt2` is instead `merged` into the current apk. We do this by checking the `CRC` values of each entry and only merging in files which have actually changed. We need to use the `CRC` here because `aapt/aapt2` is NEVER setting the `ModifiedDate` on the zip items :(. It seems to always be 01/01/1980. Once those changes are out of the way , we can now move on to our custom content. This time we can use the `ModifiedDate` of both the `ZipEntry` AND the source file. This allows us to, again, only update the items which have accutally changed. One final part is we need to keep track of what files are in the `apk` in the first place and which ones get added to it. This is what the `existingEntries` List is used for. This allows us to detect when files are REMOVED from the system. Otherwise we end up with stale files in the apk from previous builds. On my tests when changing a single AndroidResource file I got the following results 3664 ms BuildApk 1 calls (Before) 669 ms BuildApk 1 calls (After) Which is a decent improvement.

dellis1972 · 2020-01-29T17:13:44Z

/azp run

azure-pipelines · 2020-01-29T17:14:07Z

Azure Pipelines successfully started running 1 pipeline(s).

dellis1972 commented Dec 6, 2019

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs Outdated Show resolved Hide resolved

dellis1972 force-pushed the buildapkinc branch from a284a49 to 045fa23 Compare December 6, 2019 19:25

dellis1972 marked this pull request as ready for review December 6, 2019 20:13

dellis1972 requested a review from jonathanpeppers as a code owner December 6, 2019 20:13

dellis1972 requested a review from jonpryor December 9, 2019 12:47

jonathanpeppers requested changes Dec 9, 2019

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs Show resolved Hide resolved

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs Outdated Show resolved Hide resolved

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs Outdated Show resolved Hide resolved

dellis1972 force-pushed the buildapkinc branch 3 times, most recently from 1f2f47d to c960fd5 Compare January 10, 2020 17:36

dellis1972 requested a review from jonathanpeppers January 20, 2020 09:32

dellis1972 force-pushed the buildapkinc branch from b08a2d7 to 6a4065b Compare January 20, 2020 09:37

dellis1972 commented Jan 20, 2020

View reviewed changes

jonathanpeppers reviewed Jan 21, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs Show resolved Hide resolved

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs Show resolved Hide resolved

dellis1972 requested a review from jonathanpeppers January 21, 2020 11:14

jonathanpeppers approved these changes Jan 21, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ZipArchiveExTests.cs Outdated Show resolved Hide resolved

dellis1972 commented Jan 21, 2020

View reviewed changes

jonathanpeppers approved these changes Jan 21, 2020

View reviewed changes

dellis1972 force-pushed the buildapkinc branch from 9ab0b90 to 471851b Compare January 24, 2020 15:57

dellis1972 force-pushed the buildapkinc branch from 471851b to b69a3e5 Compare January 27, 2020 14:41

dellis1972 added 2 commits January 29, 2020 12:44

Update test Timings

c6d4e7a

dellis1972 force-pushed the buildapkinc branch from e48b2bf to c6d4e7a Compare January 29, 2020 12:44

dellis1972 merged commit 1c4727c into dotnet:master Jan 30, 2020

dellis1972 deleted the buildapkinc branch January 30, 2020 09:35

brendanzagaeski added this to the In Preview: 10.3 (d16-6) milestone Mar 17, 2020

github-actions bot locked and limited conversation to collaborators Jan 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xamarin.Android.Build.Tasks] Allow BuildApk to run incrementally. #3999

[Xamarin.Android.Build.Tasks] Allow BuildApk to run incrementally. #3999

dellis1972 commented Dec 6, 2019

dellis1972 commented Dec 7, 2019

azure-pipelines bot commented Dec 7, 2019

jonathanpeppers left a comment

jonathanpeppers commented Dec 9, 2019

dellis1972 commented Dec 10, 2019

jonathanpeppers commented Dec 11, 2019

dellis1972 commented Jan 14, 2020

dellis1972 commented Jan 17, 2020

dellis1972 Jan 20, 2020

dellis1972 Jan 21, 2020

jonathanpeppers Jan 21, 2020 •

edited

Loading

dellis1972 Jan 22, 2020

dellis1972 commented Jan 27, 2020

azure-pipelines bot commented Jan 27, 2020

dellis1972 commented Jan 28, 2020

azure-pipelines bot commented Jan 28, 2020

dellis1972 commented Jan 29, 2020

azure-pipelines bot commented Jan 29, 2020

[Xamarin.Android.Build.Tasks] Allow BuildApk to run incrementally. #3999

[Xamarin.Android.Build.Tasks] Allow BuildApk to run incrementally. #3999

Conversation

dellis1972 commented Dec 6, 2019

dellis1972 commented Dec 7, 2019

azure-pipelines bot commented Dec 7, 2019

jonathanpeppers left a comment

Choose a reason for hiding this comment

jonathanpeppers commented Dec 9, 2019

dellis1972 commented Dec 10, 2019

jonathanpeppers commented Dec 11, 2019

dellis1972 commented Jan 14, 2020

dellis1972 commented Jan 17, 2020

dellis1972 Jan 20, 2020

Choose a reason for hiding this comment

dellis1972 Jan 21, 2020

Choose a reason for hiding this comment

jonathanpeppers Jan 21, 2020 • edited Loading

Choose a reason for hiding this comment

dellis1972 Jan 22, 2020

Choose a reason for hiding this comment

dellis1972 commented Jan 27, 2020

azure-pipelines bot commented Jan 27, 2020

dellis1972 commented Jan 28, 2020

azure-pipelines bot commented Jan 28, 2020

dellis1972 commented Jan 29, 2020

azure-pipelines bot commented Jan 29, 2020

jonathanpeppers Jan 21, 2020 •

edited

Loading