-
Notifications
You must be signed in to change notification settings - Fork 538
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] filter @(ReferencePath) for MonoAndroid assemblies #2934
Conversation
I love the idea! Alas, macOS PR Release Pipeline Build is failing:
|
… assemblies We have a couple MSBuild targets that need to only operate on `MonoAndroid` assemblies: * `_BuildAdditionalResourcesCache` is the precursor to Xamarin.Build.Download * `_ResolveLibraryProjectImports` unzips `__AndroidLibrariesProjects__.zip`, .jar/.aar files, etc. Both of these targets all looking at *all* assemblies, so we could make a new item group `@(_MonoAndroidReferencePath)` and use this instead. This would both allow the tasks inside these targets to operate on less assemblies. It would also allow them to skip for changes in NetStandard projects. I added a new `<FilterAssemblies/>` MSBuild task to filter based on the presence of this attribute in an assembly: [assembly: System.Runtime.Versioning.TargetFrameworkAttribute ("MonoAndroid,Version=v8.1")] MSBuild/Roslyn populate this attribute based on the `$(TargetFrameworkIdentifier)` of the project. One complication is that during design-time builds, some assemblies may not exist. I made `<FilterAssemblies/>` just skip files in this case--using the same logic that `<ResolveLibraryProjectImports/>` uses. (I also fixed some wording/misspelling) ~~ Results ~~ I tested the Xamarin.Forms project in this repo. Initial build: Before: 78 ms _BuildAdditionalResourcesCache 1 calls 1678 ms _ResolveLibraryProjectImports 1 calls After: 47 ms FilterAssemblies 1 calls 23 ms _BuildAdditionalResourcesCache 1 calls 1120 ms _ResolveLibraryProjectImports 1 calls Incremental build with XAML change: Before: 62 ms _BuildAdditionalResourcesCache 1 calls 300 ms _ResolveLibraryProjectImports 1 calls After: 62 ms FilterAssemblies 1 calls 0 ms _BuildAdditionalResourcesCache 1 calls 16 ms _ResolveLibraryProjectImports 1 calls Note that during the incremental build, since only a NetStandard assembly was updated the following targets are skipped: _BuildAdditionalResourcesCache: Skipping target "_BuildAdditionalResourcesCache" because all output files are up-to-date with respect to the input files. ... _ResolveLibraryProjectImports: Skipping target "_ResolveLibraryProjectImports" because all output files are up-to-date with respect to the input files. Overall I would say this saves ~500ms on initial build, and ~250ms on incremental builds.
6687464
to
06e4fe4
Compare
Nice idea. Just a head up this might impact this PR #2899 for the AndroidX support. Especially if we apply this to other tasks. |
It's only true for "official" nuget packages and how they distribute libraries... nothing is stopping anyone to distribute embedded resources differently. |
@erikpowa are you using a NetStandard library that contains |
@jonathanpeppers yeah, I have binding libraries's native stuph (only) in netstandard libs and packed targeting multiple monoandroid like this. Guess forced to use Damn multitargeting 😂 |
If you are using This PR looks for:
(v8.1 could be any number, it just looks before the |
Fixes: dotnet/android#3343 Context: dotnet/android#3343 (comment) Context: dotnet/android#2934 In Visual Studio 2019 16.1, we added some performance improvements in Xamarin.Android. In cases of .NET assemblies that: * Do not have `[assembly: TargetFramework ("MonoAndroid")]` * Have no reference to `Mono.Android.dll` Unfortunately, these two Guava projects fit into this category! They include `<EmbeddedJar/>` but otherwise have no markings at all that would indicate they are Xamarin.Android assemblies. Reading the source code for MSBuild, I found that including a single `@(Compile)` item solves the issue: https://github.com/microsoft/msbuild/blob/0cd0e92a7243088977d31b56626b56d6116de016/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3341-L3344 <ItemGroup Condition="'@(Compile)' != '' and '$(TargetFrameworkMonikerAssemblyAttributeText)' != ''"> <Compile Include="$(TargetFrameworkMonikerAssemblyAttributesPath)"/> </ItemGroup> As soon as I added an empty `*.cs` file to the projects, `[assembly:TargetFramework]` appeared in the output assembly with the necessary info. This must be a weird corner case for MSBuild... I am not sure if there is anything we can do on the Xamarin.Android side yet.
We have a couple MSBuild targets that need to only operate on
MonoAndroid
assemblies:_BuildAdditionalResourcesCache
is the precursor toXamarin.Build.Download
_ResolveLibraryProjectImports
unzips__AndroidLibrariesProjects__.zip
, .jar/.aar files, etc.Both of these targets all looking at all assemblies, so we could
make a new item group
@(_MonoAndroidReferencePath)
and use thisinstead. This would both allow the tasks inside these targets to
operate on less assemblies. It would also allow them to skip for
changes in NetStandard projects.
I added a new
<FilterAssemblies/>
MSBuild task to filter based onthe presence of this attribute in an assembly:
MSBuild/Roslyn populate this attribute based on the
$(TargetFrameworkIdentifier)
of the project.One complication is that during design-time builds, some assemblies
may not exist. I made
<FilterAssemblies/>
just skip files in thiscase--using the same logic that
<ResolveLibraryProjectImports/>
uses. (I also fixed some wording/misspelling)
Results
I tested the Xamarin.Forms project in this repo.
Initial build:
Incremental build with XAML change:
Note that during the incremental build, since only a NetStandard
assembly was updated the following targets are skipped:
Overall I would say this saves ~500ms on initial build, and ~250ms on
incremental builds.