-
Notifications
You must be signed in to change notification settings - Fork 533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Xamarin.Android.Build.Tasks] optimize $(AssemblySearchPaths) #2223
[Xamarin.Android.Build.Tasks] optimize $(AssemblySearchPaths) #2223
Conversation
Log files: AssemblySearchPaths.zip |
</AssemblySearchPaths> | ||
<AllowedReferenceAssemblyFileExtensions> | ||
.dll; | ||
.exe; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we need .exe
for XA. Since none of our apps or projects use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we can drop .exe
. This might improve build times a bit further.
Context: https://docs.microsoft.com/en-us/visualstudio/msbuild/resolveassemblyreference-task?view=vs-2017 Context: https://twitter.com/KirillOsenkov/status/1043725067081764865 Kirill added a new analyzer to the Windows binlog viewer, noting `$(AssemblySearchPaths)` that are unused during a build. Looking at what we are using currently, it reports: Unused AssemblySearchPaths locations {AssemblyFolders} {AssemblyFoldersFromConfig:C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\AssemblyFolders.config,v9.0} {CandidateAssemblyFiles} {Registry:Software\Microsoft\MonoAndroid,v9.0,AssemblyFoldersEx} bin\Debug\ Luckily, we already had MSBuild logic in place to remove `{GAC}` from this list. But we certainly are probing search paths that are nonsensical for Xamarin.Android: - `{AssemblyFolders}`: Specifies the task will use the Visual Studio.NET 2003 finding-assemblies-from-registry scheme. - `{AssemblyFoldersFromConfig:*}`: this probes the system-wide config file in the MSBuild / Visual Studio install - `{CandidateAssemblyFiles}`: The `CandidateAssemblyFiles` property on `ResolveAssemblyReferences` is blank. Docs say: Assemblies in this list will be considered when the SearchPaths parameter contains `{CandidateAssemblyFiles}` as one of the paths to consider. - `{Registry:*}`: probe the Windows registry... - `bin\Debug` or `$(OutputPath)`: not needed, and a potential cause of confusion. This would be if there was an arbitrary extra assembly sitting in `$(OutputPath)`. And if we look further, we see odd things: Primary reference "System, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e". Resolved file path is "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Control\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v1.0\System.dll". Reference found at search path location "{TargetFrameworkDirectory}". For SearchPath "{TargetFrameworkDirectory}". Considered "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Control\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v9.0\System.winmd", but it didn't exist. Considered "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Control\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v9.0\System.dll", but it didn't exist. Considered "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Control\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v9.0\System.exe", but it didn't exist. Considered "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Control\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v1.0\System.winmd", but it didn't exist. Why are we looking for `.winmd` files? This is a UWP thing... And then apparently, our build also will look for `.pri` files? Another UWP thing... AllowedRelatedFileExtensions .pdb .xml .pri .dll.config .exe.config .dll.mdb .exe.mdb So the first fix is to trim `$(AssemblySearchPaths)` to precisely what Xamarin.Android needs: <AssemblySearchPaths> {HintPathFromItem}; {TargetFrameworkDirectory}; {RawFileName}; </AssemblySearchPaths> We shouldn't be concerned about other system-wide places. Next, we should only be looking at these file extensions: <AllowedReferenceAssemblyFileExtensions> .dll; </AllowedReferenceAssemblyFileExtensions> <AllowedReferenceRelatedFileExtensions> .pdb; .xml; .dll.config; .dll.mdb; </AllowedReferenceRelatedFileExtensions> Removing `.winmd` from `$(AllowedReferenceAssemblyFileExtensions)` and `.pri` from `$(AllowedReferenceRelatedFileExtensions)`. We can also ignore `.exe*` files, since Xamarin.Android projects only use `.dll` files. This makes the log probing for `System.dll` much cleaner: Primary reference "System, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e". Resolved file path is "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Release\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v1.0\System.dll". Reference found at search path location "{TargetFrameworkDirectory}". For SearchPath "{TargetFrameworkDirectory}". Considered "C:\Users\myuser\Desktop\Git\xamarin-android\bin\Release\lib\xamarin.android\xbuild-frameworks\MonoAndroid\v9.0\System.dll", but it didn't exist. Here is the log of changes made to these properties here at MSBuild evaluation time: Property reassignment: $(AssemblySearchPaths)=" {HintPathFromItem}; {TargetFrameworkDirectory}; {RawFileName}; " (previous value: " {CandidateAssemblyFiles}; ; {HintPathFromItem}; {TargetFrameworkDirectory}; {AssemblyFoldersFromConfig:C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\AssemblyFolders.config,v9.0}; {Registry:Software\Microsoft\MonoAndroid,v9.0,AssemblyFoldersEx}; {AssemblyFolders}; {GAC}; {RawFileName}; bin\Debug\ ") at xamarin-android\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets (338,2) Property reassignment: $(AllowedReferenceAssemblyFileExtensions)=" .dll; " (previous value: " .winmd; .dll; .exe ") at xamarin-android\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets (343,2) Property reassignment: $(AllowedReferenceRelatedFileExtensions)=" .pdb; .xml; .dll.config; .dll.mdb; " (previous value: " .pdb; .xml; .pri; .dll.config; .exe.config ") at xamarin-android\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets (347,2) ~~ Results ~~ I timed the Xamarin.Forms Control Gallery project, since I felt this change would improve larger projects the most. Project here: https://github.com/jonathanpeppers/Xamarin.Forms/tree/msbuild-timing Before: 1106 ms ResolveAssemblies 2 calls 1401 ms ResolveAssemblyReference 18 calls After: 909 ms ResolveAssemblies 2 calls 1088 ms ResolveAssemblyReference 18 calls This seems to directly save ~500ms to these targets in incremental builds for this project. This project references ~7 other Xamarin.Android projects. ~~ Upstream ~~ On MacOS, I also see probing for `.winmd` files! Considered "xamarin-android/bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v9.0/System.winmd", but it didn't exist. Perhaps we need some upstream changes to MSBuild as well...
4933c1c
to
283fe8d
Compare
FYI I didn't see a significant performance difference in also removing I think the biggest win was removing these:
|
Context: https://docs.microsoft.com/en-us/visualstudio/msbuild/resolveassemblyreference-task?view=vs-2017
Context: https://twitter.com/KirillOsenkov/status/1043725067081764865
Kirill added a new analyzer to the Windows binlog viewer, noting
$(AssemblySearchPaths)
that are unused during a build.Looking at what we are using currently, it reports:
Luckily, we already had MSBuild logic in place to remove
{GAC}
fromthis list.
But we certainly are probing search paths that are nonsensical for
Xamarin.Android:
{AssemblyFolders}
: Specifies the task will use the VisualStudio.NET 2003 finding-assemblies-from-registry scheme.
{AssemblyFoldersFromConfig:*}
: this probes the system-wide configfile in the MSBuild / Visual Studio install
{CandidateAssemblyFiles}
: TheCandidateAssemblyFiles
property onResolveAssemblyReferences
is blank. Docs say: Assemblies in thislist will be considered when the SearchPaths parameter contains
{CandidateAssemblyFiles}
as one of the paths to consider.{Registry:*}
: probe the Windows registry...bin\Debug
or$(OutputPath)
: not needed, and a potential cause ofconfusion. This would be if there was an arbitrary extra assembly
sitting in
$(OutputPath)
.And if we look further, we see odd things:
Why are we looking for
.winmd
files? This is a UWP thing...And then apparently, our build also will look for
.pri
files?Another UWP thing...
So the first fix is to trim
$(AssemblySearchPaths)
to precisely whatXamarin.Android needs:
We shouldn't be concerned about other system-wide places.
Next, we should only be looking at these file extensions:
Removing
.winmd
from$(AllowedReferenceAssemblyFileExtensions)
and.pri
from$(AllowedReferenceRelatedFileExtensions)
. We can alsoignore
.exe*
files, since Xamarin.Android projects only use.dll
files.
This makes the log probing for
System.dll
much cleaner:Here is the log of changes made to these properties here at MSBuild
evaluation time:
Results
I timed the Xamarin.Forms Control Gallery project, since I felt this
change would improve larger projects the most.
Project here:
https://github.com/jonathanpeppers/Xamarin.Forms/tree/msbuild-timing
Before:
After:
This seems to directly save ~500ms to these targets in incremental
builds for this project. This project references ~7 other Xamarin.Android
projects.
Upstream
On MacOS, I also see probing for
.winmd
files!Perhaps we need some upstream changes to MSBuild as well...