[Xamarin.Android.Build.Tasks] Asyncify Aapt Task#352
Conversation
| UncompressedFileExtensions="$(AndroidStoreUncompressedFileExtensions)" | ||
| AssetDirectory="$(MonoAndroidAssetsDirIntermediate)" | ||
| ToolPath="$(AaptToolPath)" | ||
| ToolExe="$(AaptToolExe)" |
There was a problem hiding this comment.
We don't want to remove this property, which means we'll need to add a ToolExe property and corresponding logic.
The rationale is that if (when?) Google changes the location of aapt in the future, we'd like developers to be able to specify the new location (file name?) without requiring a new release from us, Overriding $(AaptToolPath) and $(AaptToolExe) are the conventional ways to do this, which in turn means we need to continue supporting them.
(...and we should document those. doh!)
| return 0; | ||
| } | ||
|
|
||
| ExecuteForAbi (GenerateCommandLineCommands (manifestFile.ItemSpec, null)); |
There was a problem hiding this comment.
We could "merge" this and the following if:
var sabis = SupportedAbis.Split (new char [] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries);
var def = new string[]{null};
var abis = (CreatePackagePerAbi && sabis.Length > 1) ? def.Concat (abis) : def;
foreach (var abi in abis) {
ExecuteForAbi (GenerateCommandLineCommands (manifestFile.ItemSpec, abi));
}
This would reduce special-casing and help ensure that if we fix for one we fix for all.
| foreach (var dir in AdditionalAndroidResourcePaths) | ||
| if (!string.IsNullOrEmpty (dir.ItemSpec)) | ||
| upToDate = upToDate && ManifestIsUpToDate (string.Format ("{0}{1}{2}", dir, Path.DirectorySeparatorChar, "manifest", Path.DirectorySeparatorChar, "AndroidManifest.xml")); | ||
| ThreadingTasks.Parallel.ForEach (ManifestFiles, () => 0, (manifestFile, loopState, localSum) => { |
There was a problem hiding this comment.
Given that so much is being re-indented and moved around anyway, can we move this (manifestFile, loopState, localSum) lambda into a separate and distinct method? That would reduce the complexity of Execute().
| }; | ||
|
|
||
| var proc = new Process (); | ||
| proc.OutputDataReceived += (sender, e) => { LogEventsFromTextOutput (e.Data, MessageImportance.Normal); }; |
There was a problem hiding this comment.
Please format with multiple lines:
proc.OutputDataReceived += (sender, e) => {
LogEventsFromTextOutput (e.Data, MessageImportance.Normal);
};
|
|
||
| var proc = new Process (); | ||
| proc.OutputDataReceived += (sender, e) => { LogEventsFromTextOutput (e.Data, MessageImportance.Normal); }; | ||
| proc.ErrorDataReceived += (sender, e) => { LogEventsFromTextOutput (e.Data, MessageImportance.Normal); }; |
dellis1972
force-pushed
the
asyncaapt
branch
3 times, most recently
from
e5435e1 to
61d4641
Compare
| protected string GenerateFullPathToTool () | ||
| { | ||
| return Path.Combine (ToolPath, ToolExe); | ||
| return Path.Combine (ToolPath, ToolExe ?? ToolName); |
There was a problem hiding this comment.
This should use string.IsNullOrEmpty().
We currently use MSBuild Batching to call the aapt tool when processing the resources. This results in about 12 seperate calls to aapt when building a standard Xamarin.Forms app. The problem with Batching is that it is sequential. MSBuild and xbuild are unable to run those batches in parallel. So this commit reworks the Aapt task to be an AsyncTask. Which can take multiple AndroidManifest files at once. It will then spin up aapt in parallel rather than running them sequentially. The difference between the two systems is clear Using Batching: 6156.275 ms Aapt 10 calls Using Async: 3207.201 ms Aapt 3 calls This is a first build on a vanilla Xamarin.Form app with no changes.
3 participants
We currently use MSBuild Batching to call the aapt tool
when processing the resources. This results in about 12 seperate
calls to aapt when building a standard Xamarin.Forms app.
The problem with Batching is that it is sequential. MSBuild and
xbuild are unable to run those batches in parallel. So this
commit reworks the Aapt task to be an AsyncTask. Which can take
multiple AndroidManifest files at once. It will then spin up
aapt in parallel rather than running them sequentially.
The difference between the two systems is clear
Using Batching:
6156.275 ms Aapt 10 calls
Using Async:
3207.201 ms Aapt 3 calls
This is a first build on a vanilla Xamarin.Form app with no changes.