Skip to content

Commit 3b1b17d

Browse files
[XamlG] builds incrementally
Context: xamarin#2230 The main performance problem with the collection of MSBuild targets in `Xamarin.Forms.targets` is they don't build incrementally. I addressed this with `XamlC` using a "stamp" file; however, it is not quite so easy to setup the same thing with `XamlG`. They way "incremental" builds are setup in MSBuild, is by specifying the `Inputs` and `Outputs` of a `<Target />`. MSBuild will partially build a target when some outputs are not up to date, and skip it entirely if they are all up to date. The best docs I can find on MSBuild incremental builds: https://msdn.microsoft.com/en-us/library/ms171483.aspx Unfortunately a few things had to happen to make this work for `XamlG`: - Define a new target `_FindXamlGFiles` that is invoked before `XamlG` - `_FindXamlGFiles` defines the `_XamlGInputs` and `_XamlGOutputs` `<ItemGroup />`'s - `_FindXamlGFiles` must also define `<Compile />` and `<FileWrites />`, in case the `XamlG` target is skipped - `XamlGTask` now needs to get passed in a list of `OutputFiles`, since we have computed these paths ahead of time - `XamlGTask` should validate the lengths of `XamlFiles` and `OutputFiles` match, used error message from MSBuild proper: https://github.com/Microsoft/msbuild/blob/a691a44f0e515e9a03ede8df0bff22185681c8b9/src/Tasks/Copy.cs#L505 `XamlG` now builds incrementally! To give some context on how much improvement we can see with build times, consider the following command: msbuild Xamarin.Forms.ControlGallery.Android/Xamarin.Forms.ControlGallery.Android.csproj If you run it once, it will take a while--this change will not improve the first build. On the second build with the exact same command, it *should* be much faster. Before this commit, the second build on my machine takes: 40.563s After the change: 23.692s `XamlG` has cascading impact on build times when it isn't built incrementally: - The C# assembly always changes - Hence, `XamlC` will always run - Hence, `GenerateJavaStubs` will always run - Hence, `javac.exe` and `dx.jar` will always run I am making other improvements like this in Xamarin.Android itself, that will further improve these times, such as: dotnet/android#1693 Other changes: - I reordered my past `<Output />` element in XamlC to match others in this file - `FilesWrite` is actually supposed to be `FileWrites`, see canonical source of how `Clean` works and what `FileWrites` is here: dotnet/msbuild#2408 (comment) Unfortunately, there is more to do to get this PR "feature complete": - TESTS? This refactoring is complex and has a bit of nuance to it. I can't imagine we can get this improvement right (at all), without some new testing infrastructure in place that invokes MSBuild. - There were some checks for `%(TargetPath)` being blank in the C# code of `XamlGTask`. I presume this is from the designer and/or design-time builds. We probably need to figure out the proper way to do this instead of using `%(TargetPath)` at all. After testing the current implementation I had all kinds of crazy temporary files in my `$(IntermediateOutputPath)`. This probably isn't the right way to handle this issue. - CssG needs the exact same setup, as it was patterned after `XamlG`
1 parent 882bf07 commit 3b1b17d

File tree

3 files changed

+33
-35
lines changed

3 files changed

+33
-35
lines changed

.nuspec/Xamarin.Forms.targets

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,21 @@
6464
</CoreCompileDependsOn>
6565
</PropertyGroup>
6666

67-
<Target Name="XamlG" BeforeTargets="BeforeCompile" DependsOnTargets="PrepareResourceNames" Condition="'$(_XamlGAlreadyExecuted)'!='true'">
68-
<PropertyGroup>
69-
<_XamlGAlreadyExecuted>true</_XamlGAlreadyExecuted>
70-
</PropertyGroup>
67+
<Target Name="_FindXamlGFiles" DependsOnTargets="PrepareResourceNames">
68+
<ItemGroup>
69+
<_XamlGInputs Include="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs'" />
70+
<_XamlGOutputs Include="@(_XamlGInputs->'$(IntermediateOutputPath)%(TargetPath).g.cs')" />
71+
<FileWrites Include="@(_XamlGOutputs)" />
72+
<Compile Include="@(_XamlGOutputs)" />
73+
</ItemGroup>
74+
</Target>
75+
76+
<Target Name="XamlG" BeforeTargets="BeforeCompile" DependsOnTargets="_FindXamlGFiles" Inputs="@(_XamlGInputs)" Outputs="@(_XamlGOutputs)">
7177
<XamlGTask
72-
XamlFiles="@(EmbeddedResource)" Condition="'%(Extension)' == '.xaml' AND '$(DefaultLanguageSourceExtension)' == '.cs'"
73-
Language = "$(Language)"
74-
AssemblyName = "$(AssemblyName)"
75-
OutputPath = "$(IntermediateOutputPath)">
76-
<Output ItemName="FilesWrite" TaskParameter="GeneratedCodeFiles" />
77-
<Output ItemName="Compile" TaskParameter="GeneratedCodeFiles" />
78-
</XamlGTask>
78+
XamlFiles="@(_XamlGInputs)"
79+
OutputFiles="@(_XamlGOutputs)"
80+
Language="$(Language)"
81+
AssemblyName="$(AssemblyName)" />
7982
</Target>
8083

8184
<!-- XamlC -->
@@ -95,7 +98,7 @@
9598
DebugType = "$(DebugType)"
9699
KeepXamlResources = "$(XFKeepXamlResources)" />
97100
<Touch Files="$(IntermediateOutputPath)XamlC.stamp" AlwaysCreate="True">
98-
<Output TaskParameter="TouchedFiles" ItemName="FileWrites"/>
101+
<Output ItemName="FileWrites" TaskParameter="TouchedFiles" />
99102
</Touch>
100103
</Target>
101104

@@ -115,7 +118,7 @@
115118
Language = "$(Language)"
116119
AssemblyName = "$(AssemblyName)"
117120
OutputPath = "$(IntermediateOutputPath)">
118-
<Output ItemName="FilesWrite" TaskParameter="GeneratedCodeFiles" />
121+
<Output ItemName="FileWrites" TaskParameter="GeneratedCodeFiles" />
119122
<Output ItemName="Compile" TaskParameter="GeneratedCodeFiles" />
120123
</CssGTask>
121124
</Target>

Xamarin.Forms.Build.Tasks/XamlGTask.cs

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Collections.Generic;
32
using System.IO;
43
using System.Xml;
54

@@ -10,44 +9,40 @@ namespace Xamarin.Forms.Build.Tasks
109
{
1110
public class XamlGTask : Task
1211
{
13-
List<ITaskItem> _generatedCodeFiles = new List<ITaskItem>();
14-
1512
[Required]
1613
public ITaskItem[] XamlFiles { get; set; }
1714

18-
[Output]
19-
public ITaskItem[] GeneratedCodeFiles => _generatedCodeFiles.ToArray();
15+
[Required]
16+
public ITaskItem[] OutputFiles { get; set; }
2017

2118
public string Language { get; set; }
2219
public string AssemblyName { get; set; }
23-
public string OutputPath { get; set; }
2420

2521
public override bool Execute()
2622
{
2723
bool success = true;
2824
Log.LogMessage(MessageImportance.Normal, "Generating code behind for XAML files");
2925

30-
if (XamlFiles == null) {
26+
//NOTE: should not happen due to [Required], but there appears to be a place this is class is called directory
27+
if (XamlFiles == null || OutputFiles == null) {
3128
Log.LogMessage("Skipping XamlG");
3229
return true;
3330
}
3431

35-
foreach (var xamlFile in XamlFiles) {
36-
//when invoked from `UpdateDesigntimeXaml` target, the `TargetPath` isn't set, use a random one instead
37-
var targetPath = xamlFile.GetMetadata("TargetPath");
38-
if (string.IsNullOrWhiteSpace(targetPath))
39-
targetPath = $".{Path.GetRandomFileName()}";
32+
if (XamlFiles.Length != OutputFiles.Length) {
33+
Log.LogErrorWithCodeFromResources("General.TwoVectorsMustHaveSameLength", XamlFiles.Length, OutputFiles.Length, "XamlFiles", "OutputFiles");
34+
return false;
35+
}
4036

41-
var outputFile = Path.Combine(OutputPath, $"{targetPath}.g.cs");
42-
if (Path.DirectorySeparatorChar == '/' && outputFile.Contains(@"\"))
43-
outputFile = outputFile.Replace('\\','/');
44-
else if (Path.DirectorySeparatorChar == '\\' && outputFile.Contains(@"/"))
45-
outputFile = outputFile.Replace('/', '\\');
46-
47-
var generator = new XamlGenerator(xamlFile, Language, AssemblyName, outputFile, Log);
37+
for (int i = 0; i < XamlFiles.Length; i++) {
38+
var xamlFile = XamlFiles [i];
39+
var outputFile = OutputFiles [i];
40+
var generator = new XamlGenerator(xamlFile, Language, AssemblyName, outputFile.ItemSpec, Log);
4841
try {
49-
if (generator.Execute())
50-
_generatedCodeFiles.Add(new TaskItem(Microsoft.Build.Evaluation.ProjectCollection.Escape(outputFile)));
42+
if (!generator.Execute()) {
43+
//If Execute() fails, the file still needs to exist because it is added to the <Compile/> ItemGroup
44+
File.WriteAllText (outputFile.ItemSpec, string.Empty);
45+
}
5146
}
5247
catch (XmlException xe) {
5348
Log.LogError(null, null, null, xamlFile.ItemSpec, xe.LineNumber, xe.LinePosition, 0, 0, xe.Message, xe.HelpLink, xe.Source);

Xamarin.Forms.Xaml.Xamlg/Xamlg.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public static void Main(string[] args)
8080
AssemblyName = "test",
8181
Language = "C#",
8282
XamlFiles = new[] { item },
83-
OutputPath = Path.GetDirectoryName(f),
83+
OutputFiles = new[] { new TaskItem(Path.Combine(Path.GetDirectoryName(f), item.ItemSpec)) }
8484
};
8585

8686

0 commit comments

Comments
 (0)