search

xamarin / AndroidSupportComponents

Xamarin bindings for Android Support libraries - For AndroidX see https://github.com/xamarin/AndroidX
MIT License
146 stars 56 forks source link

[msbuild] make _VerifyXamarinAndroidSupportVersions incremental #167

Closed jonathanpeppers closed 5 years ago

jonathanpeppers commented 5 years ago

Support Libraries Version

28.x

Does this change any of the generated binding API's?

No

Describe your contribution

When reviewing build logs from 2019 GA, I noticed an MSBuild target from the support libraries taking some time:

377 ms  _VerifyXamarinAndroidSupportVersions       1 calls

In a build with no changes, it was the 5th longest duration:

  463 ms  _SetLatestTargetFrameworkVersion                1 calls
  554 ms  _ResolveAssemblies                              1 calls
  606 ms  _GetProjectReferenceTargetFrameworkProperties   2 calls
  682 ms  ResolveProjectReferences                        2 calls

NOTE: you can do msbuild foo.csproj /clp:performancesummary for this breakdown.

It looks like there are a couple things we can improve here:

  1. This target runs on design-time builds.
  2. This target always runs on every build, even when there is no changes.

No. 1 we can add a Condition, as I don't think it is worth the build time--DTBs are supposed to be as quick as possible. I don't think it's needed during DTBs at all?

For No. 2, we can also skip this target after the first build unless:

So I setup a simple .stamp file, that will allow these target to skip unless one of the inputs changes.

Details about how we do this in Xamarin.Android here:

https://github.com/xamarin/xamarin-android/blob/master/Documentation/guides/MSBuildBestPractices.md#stamp-files

jonathanpeppers commented 5 years ago

@Redth let me know if this makes sense, I can backport this for 28.x or something. I guess master is AndroidX?

mattleibow commented 5 years ago

@Redth does this even need to run for Android X? We might be able to skip this since we are not using any of the actual files...

Redth commented 5 years ago

Yeah we could skip this if the right AndroidX msbuild property is set...

jonathanpeppers commented 5 years ago

@mattleibow @Redth do I need to change anything here? thanks.

Redth commented 5 years ago

FWIW we don't need this in AndroidX since we aren't running this target anymore there: https://github.com/xamarin/AndroidSupportComponents/blob/AndroidX/source/androidx.annotation/annotation/merge.targets#L4