-
Notifications
You must be signed in to change notification settings - Fork 543
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
[android] eliminate lots of fields in Resource.designer.cs #1812
[android] eliminate lots of fields in Resource.designer.cs #1812
Conversation
Context: https://github.com/jonathanpeppers/CustomResourceDesigner Context: dotnet/android#6310 We found a case where a .NET MAUI app that included SkiaSharp and Telerik controls was setting ~90,000 fields at startup in the `Resource.designer.cs` file via the `UpdateIdValues()` method. This also would happen in any Xamarin.Android class library: * Include AndroidX & Google Material * Include at least one `@(AndroidResource)` and use the ID from C# * `Resource.designer.cs` has 2,700+ fields So SkiaSharp is not unique to the problem here... Reviewing the SkiaSharp fields, I found: source\SkiaSharp.Views.Maui\SkiaSharp.Views.Maui.Controls\obj\Debug\net6.0-android\Resource.designer.cs 5335 fields source\SkiaSharp.Views.Maui\SkiaSharp.Views.Maui.Core\obj\Debug\net6.0-android\Resource.designer.cs 5313 fields source\SkiaSharp.Views.Maui\SkiaSharp.Views.Maui.Controls.Compatibility\obj\Debug\net6.0-android\Resource.designer.cs 5335 fields We can simply set `$(AndroidGenerateResourceDesigner)` to `false` in these projects, as they do not use any of the fields. This will prevent .NET 6 apps using these libraries from setting 16,005 fields at startup. I also did a spot check on `SkiaSharp.Views.Android`, but it's OK -- it only has 3 fields: public class Resource { public class Attribute { public static int ignorePixelScaling; //... } public class Styleable { public static int[] SKCanvasView; public static int SKCanvasView_ignorePixelScaling; //... } //... } We are working on a long-term solution for this issue in Xamarin.Android, but we can do a simple workaround in SkiaSharp for now.
/cc @mattleibow I guess I don't have permission to ask for a review? |
Thanks for the PR. Looks good and once CI is green I'll merge. |
@mattleibow |
Oh looks like you're already all set: SkiaSharp/scripts/azure-pipelines.yml Lines 29 to 30 in 68e80fa
|
@mattleibow are my test failures here OK?
|
I think that is just a flakey test. I spin up like 100 threads and sometimes it fails. It used to always fail so I fixed a bug and now it is at 1% failure. So a win? |
Description of Change
Context: https://github.com/jonathanpeppers/CustomResourceDesigner
Context: dotnet/android#6310
We found a case where a .NET MAUI app that included SkiaSharp and
Telerik controls was setting ~90,000 fields at startup in the
Resource.designer.cs
file via theUpdateIdValues()
method.This also would happen in any Xamarin.Android class library:
@(AndroidResource)
and use the ID from C#Resource.designer.cs
has 2,700+ fieldsSo SkiaSharp is not unique to the problem here...
Reviewing the SkiaSharp fields, I found:
We can simply set
$(AndroidGenerateResourceDesigner)
tofalse
inthese projects, as they do not use any of the fields. This will
prevent .NET 6 apps using these libraries from setting 16,005 fields
at startup.
I also did a spot check on
SkiaSharp.Views.Android
, but it's OK --it only has 3 fields:
We are working on a long-term solution for this issue in
Xamarin.Android, but we can do a simple workaround in SkiaSharp for
now.
Bugs Fixed
n/a
API Changes
None
Behavioral Changes
If the three projects require usage of
Resource.designer.cs
in the future, we could follow the pattern here:https://github.com/jonathanpeppers/CustomResourceDesigner
PR Checklist