-
Notifications
You must be signed in to change notification settings - Fork 528
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] lazily populate Resource lookup #7686
[Xamarin.Android.Build.Tasks] lazily populate Resource lookup #7686
Conversation
Fixes: dotnet#7684 Comparing .NET 7 to main, I noticed: .NET 7 Task LinkAssembliesNoShrink 4ms main/.NET 8 Task LinkAssembliesNoShrink 101ms Under `dotnet trace` a lot of the time was spent in: 94.74ms MonoDroid.Tuner.FixLegacyResourceDesignerStep.LoadDesigner() Reviewing the code, I think we can "lazily" call the `LoadDesigner()` method. Now it delays until the `ProcessAssemblyDesigner()` method. I had to reorder logic slightly to do this. I also updated one log message to only log duplicates, as it was logging hundreds of lines: if (output.ContainsKey (key)) { LogMessage ($" Found duplicate {key}"); } else { output.Add (key, property.GetMethod); } Which also showed up in `dotnet trace`: 25.58ms Microsoft.Android.Build.Tasks.MSBuildExtensions.LogDebugMessage() With these changes, I instead get: Task LinkAssembliesNoShrink 5ms Which is probably ~the same performance as before or plenty good enough! Lastly, I also removed some other code & members in `FixLegacyResourceDesignerStep` that Visual Studio showed me was unused.
30501c1
to
6e09a65
Compare
src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs
Show resolved
Hide resolved
I think it's going to work this time, my first attempt triggered the failure:
So we should have decent test coverage on this. |
This might have been an ovesight on my part, but I'm not sure we have a test which specifically tests this senario on device. Problem is the test would need to be DotNet only since the new system is not supported. |
When I was debugging Because this app is Xamarin.Forms (which uses |
* main: [Mono.Android] Android.Telecom.InCallService.SetAudioRoute() + enum (dotnet#7711) [Mono.Android] Fix some incorrect enums. (dotnet#7670) [Xamarin.Android.Build.Tasks] _Microsoft.Android.Resource.Designer namespace (dotnet#7681) LEGO: Merge pull request 7713 [Xamarin.Android.Build.Tasks] lazily populate Resource lookup (dotnet#7686)
* main: [ci] Pass token when building Designer tests (dotnet#7715) [Mono.Android] Android.Telecom.InCallService.SetAudioRoute() + enum (dotnet#7711) [Mono.Android] Fix some incorrect enums. (dotnet#7670) [Xamarin.Android.Build.Tasks] _Microsoft.Android.Resource.Designer namespace (dotnet#7681) LEGO: Merge pull request 7713 [Xamarin.Android.Build.Tasks] lazily populate Resource lookup (dotnet#7686)
* main: (32 commits) [monodroid] Replace `exit()` with `abort()` in native code (dotnet#7734) Bump to xamarin/Java.Interop/main@8a1ae57 (dotnet#7738) [build] bump `$(AndroidNet7Version)` (dotnet#7737) Bump to xamarin/Java.Interop/main@1366d99 (dotnet#7718) [Xamarin.Android.Build.Tasks] fix AndroidGenerateResourceDesigner (dotnet#7721) Bump to xamarin/monodroid@50faac94 (dotnet#7725) Revert "[Xamarin.Android.Build.Tasks] fix cases of missing `@(Reference)` (dotnet#7642)" (dotnet#7726) [marshal methods] Properly support arrays of arrays (dotnet#7707) Bump to dotnet/installer@9962c6a 8.0.100-alpha.1.23063.11 (dotnet#7677) [Actions] Add action to bump the hash used for the unified pipeline (dotnet#7712) Bump to xamarin/xamarin-android-tools/main@099fd95 (dotnet#7709) [ci] Move build stages into yaml templates (dotnet#7553) [Xamarin.Android.Build.Tasks] fix NRE in `<GenerateResourceCaseMap/>` (dotnet#7716) [ci] Pass token when building Designer tests (dotnet#7715) [Mono.Android] Android.Telecom.InCallService.SetAudioRoute() + enum (dotnet#7711) [Mono.Android] Fix some incorrect enums. (dotnet#7670) [Xamarin.Android.Build.Tasks] _Microsoft.Android.Resource.Designer namespace (dotnet#7681) LEGO: Merge pull request 7713 [Xamarin.Android.Build.Tasks] lazily populate Resource lookup (dotnet#7686) [Xamarin.Android.Build.Tasks] skip XA1034 logic in some cases (dotnet#7680) ...
Fixes: #7684
Comparing .NET 7 to main, I noticed:
Under
dotnet trace
a lot of the time was spent in:Reviewing the code, I think we can "lazily" call the
LoadDesigner()
method. Now it delays until the
ProcessAssemblyDesigner()
method. Ihad to reorder logic slightly to do this.
I also updated one log message to only log duplicates, as it was
logging hundreds of lines:
Which also showed up in
dotnet trace
:With these changes, I instead get:
Which is probably ~the same performance as before or plenty good enough!
Lastly, I also removed some other code & members in
FixLegacyResourceDesignerStep
that Visual Studio showed me wasunused.