[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

jonathanpeppers · 2019-01-17T14:53:34Z

Just like in c22475d, we can use System.Reflection.Metadata in places
over Mono.Cecil for performance benefits.

On such case is the <GetAdditionalResourcesFromAssemblies/> MSBuild
task, which only looks at custom attributes in assemblies:

Before
1st:          98 ms  GetAdditionalResourcesFromAssemblies       1 calls
Incremental: 105 ms  GetAdditionalResourcesFromAssemblies       1 calls
After
1st:          54 ms  GetAdditionalResourcesFromAssemblies       1 calls
Incremental:  53 ms  GetAdditionalResourcesFromAssemblies       1 calls

This was a straightforward port to SRM, which seems to save ~50ms on
every build.

…blies/> Just like in c22475d, we can use System.Reflection.Metadata in places over Mono.Cecil for performance benefits. On such case is the `<GetAdditionalResourcesFromAssemblies/>` MSBuild task, which only looks at custom attributes in assemblies: Before 1st: 98 ms GetAdditionalResourcesFromAssemblies 1 calls Incremental: 105 ms GetAdditionalResourcesFromAssemblies 1 calls After 1st: 54 ms GetAdditionalResourcesFromAssemblies 1 calls Incremental: 53 ms GetAdditionalResourcesFromAssemblies 1 calls This was a straightforward port to SRM, which seems to save ~50ms on every build.

dellis1972 · 2019-01-17T16:33:14Z

src/Xamarin.Android.Build.Tasks/Tasks/GetAdditionalResourcesFromAssemblies.cs

+				foreach (var assemblyItem in Assemblies) {
+					string fullPath = Path.GetFullPath (assemblyItem.ItemSpec);
+					if (DesignTimeBuild && !File.Exists (fullPath)) {
+						LogWarning ("Failed to load '{0}'. Check the file exists or the project has been built.", fullPath);


I think we need to make this a LogCodedWarning if possible

Indent some code, then you have to write docs! 😜

@dellis1972 it looks like @brendanzagaeski looked at these codes? #2258

They weren't "actionable"?

I think for this exact LogWarning() in DoExecute() I temporarily skipped further work on it when I realized that the warnings in other tasks might be a higher priority because the Xamarin-authored NuGet packages have been using the Xamarin.Build.Download mechanism for a while instead of the GetAdditionalResourcesFromAssemblies mechanism. I was going to circle back eventually to the remaining uses of LogWarning() in GetAdditionalResourcesFromAssemblies.

Ok let me go ahead and add the warning codes for these then, thanks.

jonathanpeppers requested a review from dellis1972 January 17, 2019 14:53

dellis1972 reviewed Jan 17, 2019

View reviewed changes

jonpryor merged commit afba30b into dotnet:master Jan 18, 2019

jonathanpeppers deleted the getadditionalresources-srm branch January 18, 2019 21:55

github-actions bot locked and limited conversation to collaborators Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

Uh oh!

jonathanpeppers commented Jan 17, 2019

Uh oh!

dellis1972 Jan 17, 2019

Uh oh!

jonathanpeppers Jan 17, 2019

Uh oh!

jonathanpeppers Jan 17, 2019

Uh oh!

brendanzagaeski Jan 17, 2019 •

edited

Loading

Uh oh!

jonathanpeppers Jan 18, 2019

Uh oh!

Uh oh!

[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

Uh oh!

Conversation

jonathanpeppers commented Jan 17, 2019

Uh oh!

dellis1972 Jan 17, 2019

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jan 17, 2019

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jan 17, 2019

Choose a reason for hiding this comment

Uh oh!

brendanzagaeski Jan 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Jan 18, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brendanzagaeski Jan 17, 2019 •

edited

Loading