[xamlc] new <XamlCReadOnly> MSBuild property for Debug builds

XamlC is currently enabled for most Xamarin.Forms projects in `Debug` and `Release` configurations. It enables faster startup/runtime performance, XAML-syntax checking at build time--both quiet useful! However, there is a build-time cost to using XamlC: each assembly is loaded via Mono.Cecil, IL generated, and saved back to disk as an additional step after Rosyln has compiled the assembly. The proposal would be to add a new experimental MSBuild property, that can be enabled for `Debug` mode such as: <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">  <XamlCReadOnly>True</XamlCReadOnly> </PropertyGroup> This would do the following: * `OptimizeIL` is skipped. * Assemblies are analyzed, but no changes written to disk. * No symbols are loaded via Mono.Cecil, we don't need them if we aren't writing back to disk! This will improve build performance, and we don't lose the build-time error checking for XAML. Down the road, we could consider moving the MSBuild property to be the default for `Debug` builds or change the Xamarin templates. I think it would be wise to require developers to opt-in to try this out. ~~ Results ~~ I tested this change by building the ControlGallery after changing one line of XAML: msbuild Xamarin.Forms.ControlGallery.Android\Xamarin.Forms.ControlGallery.Android.csproj /clp:performancesummary /p:XamlCReadOnly=True Before: 1979 ms XamlCTask 1 calls After: 923 ms XamlCTask 1 calls I *only* tested `Debug` builds. Right, so it's faster. But let's keep the entire developer loop in mind, how much slower is startup? Before: 09-05 14:37:32.274 1169 1192 I ActivityManager: Displayed AndroidControlGallery.AndroidControlGallery/md546303760447087909496d02dc7b17ae8.Activity1: +3s890ms 09-05 14:38:30.178 1169 1192 I ActivityManager: Displayed AndroidControlGallery.AndroidControlGallery/md546303760447087909496d02dc7b17ae8.Activity1: +3s848ms 09-05 14:38:40.300 1169 1192 I ActivityManager: Displayed AndroidControlGallery.AndroidControlGallery/md546303760447087909496d02dc7b17ae8.Activity1: +3s848ms After: 09-05 14:40:38.512 1169 1192 I ActivityManager: Displayed AndroidControlGallery.AndroidControlGallery/md546303760447087909496d02dc7b17ae8.Activity1: +3s894ms 09-05 14:40:55.497 1169 1192 I ActivityManager: Displayed AndroidControlGallery.AndroidControlGallery/md546303760447087909496d02dc7b17ae8.Activity1: +3s856ms 09-05 14:41:03.754 1169 1192 I ActivityManager: Displayed AndroidControlGallery.AndroidControlGallery/md546303760447087909496d02dc7b17ae8.Activity1: +3s897ms After three runs, it seems this app suffers 25-50ms slowdown to startup, and gains 1 second of build time improvement. A good net-win! Other apps that have significantly more XAML will have different results. I suspect the build time improvement will be even better, but the hit to startup could be worse. With this setting opt-in, we can experiment and find out.
xamarin · Sep 5, 2019 · 54d9a9a · 54d9a9a
1 parent 8d790b3
commit 54d9a9a
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 27 deletions.
diff --git a/.nuspec/Xamarin.Forms.targets b/.nuspec/Xamarin.Forms.targets
@@ -110,6 +110,7 @@
  OptimizeIL = "true"
  DebugSymbols = "$(DebugSymbols)"
  DebugType = "$(DebugType)"
+ ReadOnly = "$(XamlCReadOnly)"
  KeepXamlResources = "$(XFKeepXamlResources)" />
  <Touch Files="$(IntermediateOutputPath)XamlC.stamp" AlwaysCreate="True" />
  <ItemGroup>

diff --git a/Xamarin.Forms.Build.Tasks/XamlCTask.cs b/Xamarin.Forms.Build.Tasks/XamlCTask.cs
@@ -33,7 +33,12 @@ public class XamlCTask : XamlTask
 
  internal string Type { get; set; }
  internal MethodDefinition InitCompForType { get; private set; }
- internal bool ReadOnly { get; set; }
+
+ /// <summary>
+ /// Enable to optimize for shorter build time
+ /// e.g. OptimizeIL unused, Debug symbols not loaded, no assemblies written
+ /// </summary>
+ public bool ReadOnly { get; set; }
 
  public override bool Execute(out IList<Exception> thrownExceptions)
  {
@@ -73,7 +78,7 @@ public override bool Execute(out IList<Exception> thrownExceptions)
  var readerParameters = new ReaderParameters {
  AssemblyResolver = resolver,
  ReadWrite = !ReadOnly,
- ReadSymbols = debug,
+ ReadSymbols = debug && !ReadOnly, // We don't need symbols for ReadOnly, since we won't be writing
  };
 
  using (var assemblyDefinition = AssemblyDefinition.ReadAssembly(Path.GetFullPath(Assembly),readerParameters)) {
@@ -147,24 +152,29 @@ public override bool Execute(out IList<Exception> thrownExceptions)
  (string)xamlFilePathAttr.ConstructorArguments[0].Value :
  resource.Name;
 
- var initCompRuntime = typeDef.Methods.FirstOrDefault(md => md.Name == "__InitComponentRuntime");
- if (initCompRuntime != null)
- LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}__InitComponentRuntime already exists... not creating");
- else {
- LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Creating empty {typeDef.Name}.__InitComponentRuntime");
- initCompRuntime = new MethodDefinition("__InitComponentRuntime", initComp.Attributes, initComp.ReturnType);
- initCompRuntime.Body.InitLocals = true;
- LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done.");
- LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Copying body of InitializeComponent to __InitComponentRuntime");
- initCompRuntime.Body = new MethodBody(initCompRuntime);
- var iCRIl = initCompRuntime.Body.GetILProcessor();
- foreach (var instr in initComp.Body.Instructions)
- iCRIl.Append(instr);
- initComp.Body.Instructions.Clear();
- initComp.Body.GetILProcessor().Emit(OpCodes.Ret);
- initComp.Body.InitLocals = true;
- typeDef.Methods.Add(initCompRuntime);
- LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done.");
+ MethodDefinition initCompRuntime = null;
+ if (!ReadOnly)
+ {
+ initCompRuntime = typeDef.Methods.FirstOrDefault(md => md.Name == "__InitComponentRuntime");
+ if (initCompRuntime != null)
+ LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}__InitComponentRuntime already exists... not creating");
+ else
+ {
+ LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Creating empty {typeDef.Name}.__InitComponentRuntime");
+ initCompRuntime = new MethodDefinition("__InitComponentRuntime", initComp.Attributes, initComp.ReturnType);
+ initCompRuntime.Body.InitLocals = true;
+ LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done.");
+ LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Copying body of InitializeComponent to __InitComponentRuntime");
+ initCompRuntime.Body = new MethodBody(initCompRuntime);
+ var iCRIl = initCompRuntime.Body.GetILProcessor();
+ foreach (var instr in initComp.Body.Instructions)
+ iCRIl.Append(instr);
+ initComp.Body.Instructions.Clear();
+ initComp.Body.GetILProcessor().Emit(OpCodes.Ret);
+ initComp.Body.InitLocals = true;
+ typeDef.Methods.Add(initCompRuntime);
+ LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done.");
+ }
  }
 
  LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Parsing Xaml");
@@ -197,7 +207,7 @@ public override bool Execute(out IList<Exception> thrownExceptions)
 
  LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done.");
 
- if (OptimizeIL) {
+ if (OptimizeIL && !ReadOnly) {
  LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Optimizing IL");
  initComp.Body.Optimize();
  LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done.");

diff --git a/Xamarin.Forms.Xaml.UnitTests/MSBuild/MSBuildTests.cs b/Xamarin.Forms.Xaml.UnitTests/MSBuild/MSBuildTests.cs
@@ -1,9 +1,11 @@
 using Microsoft.Build.Locator;
+using Mono.Cecil;
 using NUnit.Framework;
 using System;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
+using System.Text;
 using System.Xml.Linq;
 using static Xamarin.Forms.MSBuild.UnitTests.MSBuildXmlExtensions;
 
@@ -117,7 +119,6 @@ public void TearDown ()
  /// <param name="sdkStyle">If true, uses a new SDK-style project</param>
  XElement NewProject (bool sdkStyle)
  {
- var path = Path.GetTempFileName ();
  var project = NewElement ("Project");
 
  var propertyGroup = NewElement ("PropertyGroup");
@@ -136,6 +137,7 @@ XElement NewProject (bool sdkStyle)
  propertyGroup.Add (NewElement ("OutputType").WithValue ("Library"));
  propertyGroup.Add (NewElement ("OutputPath").WithValue ("bin\\Debug"));
  propertyGroup.Add (NewElement ("TargetFrameworkVersion").WithValue ("v4.7"));
+ propertyGroup.Add(NewElement("RootNamespace").WithValue("test"));
  }
  propertyGroup.Add(NewElement("_XFBuildTasksLocation").WithValue($"{testDirectory}\\"));
 
@@ -195,8 +197,19 @@ void RestoreIfNeeded (string projectFile, bool sdkStyle)
  }
  }
 
- void Build (string projectFile, string target = "Build", string additionalArgs = "")
+ string Build (string projectFile, string target = "Build", string additionalArgs = "", bool shouldSucceed = true)
  {
+ var builder = new StringBuilder();
+ void onData(object s, DataReceivedEventArgs e)
+ {
+ lock (builder)
+ if (e.Data != null)
+ {
+ builder.AppendLine(e.Data);
+ Console.WriteLine(e.Data);
+ }
+ };
+
  var psi = new ProcessStartInfo {
  FileName = FindMSBuild (),
  Arguments = $"/v:normal /nologo {projectFile} /t:{target} /bl {additionalArgs}",
@@ -208,14 +221,19 @@ void Build (string projectFile, string target = "Build", string additionalArgs =
  WorkingDirectory = tempDirectory,
  };
  using (var p = new Process { StartInfo = psi }) {
- p.ErrorDataReceived += (s, e) => Console.Error.WriteLine (e.Data);
- p.OutputDataReceived += (s, e) => Console.WriteLine (e.Data);
+ p.ErrorDataReceived += onData;
+ p.OutputDataReceived += onData;
 
  p.Start ();
  p.BeginErrorReadLine ();
  p.BeginOutputReadLine ();
  p.WaitForExit ();
- Assert.AreEqual (0, p.ExitCode, "MSBuild exited with {0}", p.ExitCode);
+ if (shouldSucceed)
+ Assert.AreEqual(0, p.ExitCode, "MSBuild exited with {0}", p.ExitCode);
+ else
+ Assert.AreNotEqual(0, p.ExitCode, "MSBuild exited with {0}", p.ExitCode);
+
+ return builder.ToString();
  }
  }
 
@@ -250,6 +268,40 @@ public void BuildAProject ([Values (false, true)] bool sdkStyle)
  AssertExists (Path.Combine (intermediateDirectory, "XamlC.stamp"));
  }
 
+ // Tests the XamlCReadOnly=True MSBuild property
+ [Test]
+ public void Readonly([Values(false, true)] bool sdkStyle)
+ {
+ var project = NewProject(sdkStyle);
+ project.Add(AddFile("MainPage.xaml", "EmbeddedResource", Xaml.MainPage));
+ var projectFile = Path.Combine(tempDirectory, "test.csproj");
+ project.Save(projectFile);
+ RestoreIfNeeded(projectFile, sdkStyle);
+ Build(projectFile, additionalArgs: "/p:XamlCReadonly=True");
+
+ var testDll = Path.Combine(intermediateDirectory, "test.dll");
+ AssertExists(testDll, nonEmpty: true);
+ using (var assembly = AssemblyDefinition.ReadAssembly(testDll))
+ {
+ // XAML files should remain as EmbeddedResource
+ var resources = assembly.MainModule.Resources.OfType<EmbeddedResource>().Select(e => e.Name).ToArray();
+ CollectionAssert.Contains(resources, "test.MainPage.xaml");
+ }
+ }
+
+ [Test]
+ public void Readonly_WithErrors([Values(false, true)] bool sdkStyle)
+ {
+ var project = NewProject(sdkStyle);
+ project.Add(AddFile("MainPage.xaml", "EmbeddedResource", Xaml.MainPage.Replace ("</ContentPage>", "<NotARealThing/></ContentPage>")));
+ var projectFile = Path.Combine(tempDirectory, "test.csproj");
+ project.Save(projectFile);
+ RestoreIfNeeded(projectFile, sdkStyle);
+
+ string log = Build(projectFile, additionalArgs: "/p:XamlCReadonly=True", shouldSucceed: false);
+ StringAssert.Contains("MainPage.xaml(7,6): error : Position 7:6. Type NotARealThing not found", log);
+ }
+
  /// <summary>
  /// Tests that XamlG and XamlC targets skip, as well as checking IncrementalClean doesn't delete generated files
  /// </summary>

diff --git a/Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj b/Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj
@@ -20,7 +20,7 @@
  </ItemGroup>
  <ItemGroup>
  <PackageReference Include="NUnit" Version="2.6.4" />
- <PackageReference Include="Microsoft.Build.Locator" Version="1.0.31" />
+ <PackageReference Include="Microsoft.Build.Locator" Version="1.2.6" />
  </ItemGroup>
  <ItemGroup>
  <Compile Include="..\Xamarin.Forms.Core.UnitTests\MockDispatcherProvider.cs">