Skip to content

Commit

Permalink
[xamlc] fix duplicate Mono.Cecil search paths (xamarin#12030)
Browse files Browse the repository at this point in the history 
I was building the Control Gallery and saw:

    XamlCTask
        Compiling Xaml, assembly: obj\Debug\netstandard2.0\Xamarin.Forms.Controls.dll
        Adding searchpath C:\Users\jopepper\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
        Adding searchpath C:\Users\jopepper\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
        Adding searchpath C:\Users\jopepper\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
        Adding searchpath C:\Users\jopepper\.nuget\packages\newtonsoft.json\12.0.3\lib\netstandard2.0
        Adding searchpath C:\Users\jopepper\.nuget\packages\nunit\3.12.0\lib\netstandard2.0
        Adding searchpath C:\Users\jopepper\.nuget\packages\xam.plugin.deviceinfo\3.0.2\lib\netstandard1.0
        Adding searchpath C:\Users\jopepper\.nuget\packages\xam.plugin.deviceinfo\3.0.2\lib\netstandard1.0
        Adding searchpath C:\Users\jopepper\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
        Adding searchpath C:\Users\jopepper\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
        ...

122 lines in total.

In f330652 there was an attempt to fix this in the past with a
`.Distinct()` call, but `Path.GetDirectoryName()` wasn't called first.

After reordering things, the `<XamlCTask/>` now prints 9 search paths
for the Control Gallery:

    XamlCTask
        Compiling Xaml, assembly: obj\Debug\netstandard2.0\Xamarin.Forms.Controls.dll
        Adding searchpath C:\Users\jopepper\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
        Adding searchpath C:\Users\jopepper\.nuget\packages\newtonsoft.json\12.0.3\lib\netstandard2.0
        Adding searchpath C:\Users\jopepper\.nuget\packages\nunit\3.12.0\lib\netstandard2.0
        Adding searchpath C:\Users\jopepper\.nuget\packages\xam.plugin.deviceinfo\3.0.2\lib\netstandard1.0
        Adding searchpath C:\src\Xamarin.Forms\Xamarin.Forms.Core\bin\Debug\netstandard2.0
        Adding searchpath C:\src\Xamarin.Forms\Xamarin.Forms.CustomAttributes\bin\Debug\netstandard2.0
        Adding searchpath C:\src\Xamarin.Forms\Xamarin.Forms.Maps\bin\Debug\netstandard2.0
        Adding searchpath C:\src\Xamarin.Forms\Xamarin.Forms.Platform\bin\Debug\netstandard2.0
        Adding searchpath C:\src\Xamarin.Forms\Xamarin.Forms.Xaml\bin\Debug\netstandard2.0

Other changes to `<XamlCTask/>`:

* Made `ReferencePath` a `string[]` so that MSBuild can split the item
  groups for us. This way we don't have to call `string.Split()` and
  weird (and valid!) separators work such as space characters or %3b.

* Removed `DependencyPaths`, as I could not find any usage of it.

~~ Results ~~

I could see small performance difference fixing this.

On Windows + .NET framework:

    Before:
    2140 ms  XamlCTask                                  1 calls
    After:
    2074 ms  XamlCTask                                  1 calls

On macOS + Mono:

    Before:
    7580 ms  XamlCTask                                  1 calls
    After:
    7480 ms  XamlCTask                                  1 calls

My PC is newer/faster than my Mac, but I think this probably saves
between 65ms-100ms for `<XamlCTask/>` in the Control Gallery sample.
  • Loading branch information
@jonathanpeppers
jonathanpeppers authored Sep 7, 2020
1 parent 6d90430 commit 2131ea1
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 24 deletions.
13 changes: 3 additions & 10 deletions Xamarin.Forms.Build.Tasks/DebugXamlCTask.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,9 @@ public override bool Execute(out IList<Exception> thrownExceptions)
LoggingHelper.LogMessage(Normal, $"{new string(' ', 0)}Preparing debug code for xamlc, assembly: {Assembly}");

var resolver = new DefaultAssemblyResolver();
if (!string.IsNullOrEmpty(DependencyPaths)) {
foreach (var dep in DependencyPaths.Split(';')) {
LoggingHelper.LogMessage(Low, $"{new string(' ', 2)}Adding searchpath {dep}");
resolver.AddSearchDirectory(dep);
}
}
if (!string.IsNullOrEmpty(ReferencePath)) {
var paths = ReferencePath.Replace("//", "/").Split(';');
foreach (var p in paths) {
var searchpath = IOPath.GetDirectoryName(p);
if (ReferencePath != null) {
var paths = ReferencePath.Select(p => IOPath.GetDirectoryName(p.Replace("//", "/"))).Distinct();
foreach (var searchpath in paths) {
LoggingHelper.LogMessage(Low, $"{new string(' ', 2)}Adding searchpath {searchpath}");
resolver.AddSearchDirectory(searchpath);
}
Expand Down
14 changes: 3 additions & 11 deletions Xamarin.Forms.Build.Tasks/XamlCTask.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,9 @@ public override bool Execute(out IList<Exception> thrownExceptions)
using (var fallbackResolver = DefaultAssemblyResolver == null ? new XamlCAssemblyResolver() : null) {
var resolver = DefaultAssemblyResolver ?? fallbackResolver;
if (resolver is XamlCAssemblyResolver xamlCResolver) {
if (!string.IsNullOrEmpty(DependencyPaths)) {
foreach (var dep in DependencyPaths.Split(';').Distinct()) {
LoggingHelper.LogMessage(Low, $"{new string(' ', 2)}Adding searchpath {dep}");
xamlCResolver.AddSearchDirectory(dep);
}
}

if (!string.IsNullOrEmpty(ReferencePath)) {
var paths = ReferencePath.Replace("//", "/").Split(';').Distinct();
foreach (var p in paths) {
var searchpath = IOPath.GetDirectoryName(p);
if (ReferencePath != null) {
var paths = ReferencePath.Select (p => IOPath.GetDirectoryName(p.Replace("//", "/"))).Distinct();
foreach (var searchpath in paths) {
LoggingHelper.LogMessage(Low, $"{new string(' ', 2)}Adding searchpath {searchpath}");
xamlCResolver.AddSearchDirectory(searchpath);
}
Expand Down
3 changes: 1 addition & 2 deletions Xamarin.Forms.Build.Tasks/XamlTask.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ public abstract class XamlTask : MarshalByRefObject, ITask
{
[Required]
public string Assembly { get; set; }
public string DependencyPaths { get; set; }
public string ReferencePath { get; set; }
public string[] ReferencePath { get; set; }
[Obsolete("this is no longer used")]
[EditorBrowsable(EditorBrowsableState.Never)]
public int Verbosity { get; set; }
Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Xaml.UnitTests/MockCompiler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static void Compile(Type type, out MethodDefinition methdoDefinition)

var xamlc = new XamlCTask {
Assembly = assembly,
ReferencePath = string.Join(";", refs),
ReferencePath = refs.ToArray(),
KeepXamlResources = true,
OptimizeIL = true,
DebugSymbols = false,
Expand Down

0 comments on commit 2131ea1

Please sign in to comment.