[essentials] use Lazy<T> for Android application-wide values#7996
Merged
mattleibow merged 2 commits intodotnet:mainfrom Jun 16, 2022
Merged
[essentials] use Lazy<T> for Android application-wide values#7996mattleibow merged 2 commits intodotnet:mainfrom
mattleibow merged 2 commits intodotnet:mainfrom
Conversation
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Jun 13, 2022
Context: dotnet#7996 Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon my changes in dotnet#7996 (sample sample), I noticed: 7.60s (14%) mono.android!Android.Views.View.get_Context() 14% of the time is literally spent calling `View.Context`! We did a little investigation on the underlying Java interop, this is basically doing: 1. Call into JNI, get an `IntPtr`. 2. See if that `IntPtr` maps to a C# object that is already alive. 3. Return the C# object, or create a new one if needed. We can actually avoid all this work, in this case. For example: 5.48s (10%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorX(Android.Views.View,Microsoft.Maui.IView) 4.28s (8.2%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorY(Android.Views.View,Microsoft.Maui.IView) These extension methods call `View.Context` twice: public static void UpdateTranslationY(this AView platformView, IView view) { if (platformView.Context == null) return; platformView.TranslationY = platformView.Context.ToPixels(view.TranslationY); } We can actually, make an overload for `ToPixels()` to where `View.Context` wouldn't be called *at all*: internal static float ToPixels (this View view, double dp) { if (s_displayDensity != float.MinValue) return (float)Math.Ceiling(dp * s_displayDensity); return view.Context.ToPixels(dp); } I used this everywhere I saw it appearing in `dotnet trace` output. Next, I saw `View.Context` being called a lot from `LayoutViewGroup` and `ContentViewGroup`. I could simply store the value in the constructor for these types, make it non-nullable, and remove null checks. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 64.23 Dopes/s After: 81.70 Dopes/s
akoeplinger
reviewed
Jun 13, 2022
mattleibow
approved these changes
Jun 14, 2022
Member
|
Can we retarget to net6.0 and these changes will flow up to main and net7 |
Member
Author
|
@mattleibow I think I want this in .NET 7-only. |
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Jun 14, 2022
Context: dotnet#7996 Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon my changes in dotnet#7996 (sample sample), I noticed: 7.60s (14%) mono.android!Android.Views.View.get_Context() 14% of the time is literally spent calling `View.Context`! We did a little investigation on the underlying Java interop, this is basically doing: 1. Call into JNI, get an `IntPtr`. 2. See if that `IntPtr` maps to a C# object that is already alive. 3. Return the C# object, or create a new one if needed. We can actually avoid all this work, in this case. For example: 5.48s (10%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorX(Android.Views.View,Microsoft.Maui.IView) 4.28s (8.2%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorY(Android.Views.View,Microsoft.Maui.IView) These extension methods call `View.Context` twice: public static void UpdateTranslationY(this AView platformView, IView view) { if (platformView.Context == null) return; platformView.TranslationY = platformView.Context.ToPixels(view.TranslationY); } We can actually, make an overload for `ToPixels()` to where `View.Context` wouldn't be called *at all*: internal static float ToPixels (this View view, double dp) { if (s_displayDensity != float.MinValue) return (float)Math.Ceiling(dp * s_displayDensity); return view.Context.ToPixels(dp); } I used this everywhere I saw it appearing in `dotnet trace` output. Next, I saw `View.Context` being called a lot from `LayoutViewGroup` and `ContentViewGroup`. I could simply store the value in the constructor for these types, make it non-nullable, and remove null checks. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 64.23 Dopes/s After: 81.70 Dopes/s
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui I was reviewing a benchmark/sample that puts lots of labels on the screen that say "dope" and measures how many per second. Personally, I would have chosen some other word like `LOL`, so you could have a `LOLs/s` average. LOL? Reviewing `dotnet trace` output, I saw: 1.06s (6.3%) Microsoft.Maui.Essentials!Microsoft.Maui.ApplicationModel.AppInfoImplementation.get_RequestedLayoutDirection() 4.46ms (0.03%) Microsoft.Maui.Essentials!Microsoft.Maui.ApplicationModel.AppInfoImplementation.get_RequestedTheme() So approximately 6.3% of the time was spent querying if layouts are RTL or not? And 0.03% checking dark mode? I went through `AppInfoImplementation` on Android, and simply used `Lazy<T>` for every value that queries `Application.Context`. These values cannot change after the app launches, so they don't need to be computed on every call. If these values ever need to match a `Context` for multiple activities, we should re-evaluate this caching. A dictionary using a key of `Context.Handle` might work for that case. We can also remove places with `#if __ANDROID_24__` as they are always true in .NET 6. Runtime checks using `OperatingSystem` should be used going forward. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 55.42 Dopes/s After: 64.23 Dopes/s This should help the performance of `Label` in any .NET MAUI application running on Android.
Minimum API level is 21 in .NET 6
1c63a3c to
a698ac7
Compare
Member
Author
|
@mattleibow ok, I understand now after asking in Teams. This can go to |
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Jun 15, 2022
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon dotnet#7996 and dotnet#8001, I noticed while profiling the sample app putting N `Label` on the screen: 405.02ms (2.3%) microsoft.maui!Microsoft.Maui.MauiContext.get_Context() 77.90ms (0.44%) microsoft.maui!Microsoft.Maui.MauiContext.get_Handlers() Meaning that around ~2.74% of the time was spent just calling these two properties. These properties hit Microsoft.Extensions each time to locate the service: public Android.Content.Context? Context => _services.GetService<Android.Content.Context>(); These are core-services that wouldn't change after startup, so should we just use `Lazy<T>` instead of doing a call like this each time? I also made use of `ANDROID`, as that is the preferred define to use in .NET 6+ instead of `__ANDROID__`: https://github.com/dotnet/designs/blob/a447d77bb53d189746ccd80c2d814064c2b6c606/accepted/2020/net5/net5.md#vary-implementation ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 81.70 Dopes/s After: 91.94 Dopes/s
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Jun 15, 2022
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon dotnet#7996, dotnet#8001, and dotnet#8033, I noticed while profiling the sample app putting N Label on the screen: 783.92ms (6.2%) microsoft.maui!Microsoft.Maui.ViewHandlerExtensions.GetDesiredSizeFromHandler(Microsoft.Maui.IViewHandler,double,double) So around %6 of the time spend just measuring. Looking through the call stack, I can see 3 JNI calls happening: 932.51ms (7.4%) mono.android!Android.Views.View.Measure(int,int) 115.53ms (0.91%) mono.android!Android.Views.View.get_MeasuredWidth() 96.97ms (0.77%) mono.android!Android.Views.View.get_MeasuredHeight() So, we could write a Java method that calls all three of these and somehow returns the `MeasuredWidth` and `MeasuredHeight`. After a little research, it seemed the best approach here was to "pack" two integers into a `long`. If we tried to return some Java object instead, then we'd have the same number of JNI calls to get the integers out. I found a couple links describing how to "pack" an `int` into a `long`: * Java: https://stackoverflow.com/a/12772968 * C#: https://stackoverflow.com/a/827267 Which after some testing, arrives at: public static long measureAndGetWidthAndHeight(View view, int widthMeasureSpec, int heightMeasureSpec) { view.measure(widthMeasureSpec, heightMeasureSpec); int width = view.getMeasuredWidth(); int height = view.getMeasuredHeight(); return ((long)width << 32) | (height & 0xffffffffL); } Unpacked in C# such as: var packed = PlatformInterop.MeasureAndGetWidthAndHeight(platformView, widthSpec, heightSpec); var measuredWidth = (int)(packed >> 32); var measuredHeight = (int)(packed & 0xffffffffL); Reducing 3 JNI calls in every `View`'s layout to 1. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 91.94 Dopes/s After: 102.45 Dopes/s After profiling again, it drops the % time spent in `GetDesiredSizeFromHandler`: 528.96ms (4.5%) microsoft.maui!Microsoft.Maui.ViewHandlerExtensions.GetDesiredSizeFromHandler So the added math is negligible compared to reduced JNI calls.
PureWeen
pushed a commit
that referenced
this pull request
Jun 15, 2022
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon #7996, #8001, and #8033, I noticed while profiling the sample app putting N Label on the screen: 783.92ms (6.2%) microsoft.maui!Microsoft.Maui.ViewHandlerExtensions.GetDesiredSizeFromHandler(Microsoft.Maui.IViewHandler,double,double) So around %6 of the time spend just measuring. Looking through the call stack, I can see 3 JNI calls happening: 932.51ms (7.4%) mono.android!Android.Views.View.Measure(int,int) 115.53ms (0.91%) mono.android!Android.Views.View.get_MeasuredWidth() 96.97ms (0.77%) mono.android!Android.Views.View.get_MeasuredHeight() So, we could write a Java method that calls all three of these and somehow returns the `MeasuredWidth` and `MeasuredHeight`. After a little research, it seemed the best approach here was to "pack" two integers into a `long`. If we tried to return some Java object instead, then we'd have the same number of JNI calls to get the integers out. I found a couple links describing how to "pack" an `int` into a `long`: * Java: https://stackoverflow.com/a/12772968 * C#: https://stackoverflow.com/a/827267 Which after some testing, arrives at: public static long measureAndGetWidthAndHeight(View view, int widthMeasureSpec, int heightMeasureSpec) { view.measure(widthMeasureSpec, heightMeasureSpec); int width = view.getMeasuredWidth(); int height = view.getMeasuredHeight(); return ((long)width << 32) | (height & 0xffffffffL); } Unpacked in C# such as: var packed = PlatformInterop.MeasureAndGetWidthAndHeight(platformView, widthSpec, heightSpec); var measuredWidth = (int)(packed >> 32); var measuredHeight = (int)(packed & 0xffffffffL); Reducing 3 JNI calls in every `View`'s layout to 1. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 91.94 Dopes/s After: 102.45 Dopes/s After profiling again, it drops the % time spent in `GetDesiredSizeFromHandler`: 528.96ms (4.5%) microsoft.maui!Microsoft.Maui.ViewHandlerExtensions.GetDesiredSizeFromHandler So the added math is negligible compared to reduced JNI calls.
mattleibow
approved these changes
Jun 16, 2022
mattleibow
pushed a commit
that referenced
this pull request
Jun 16, 2022
Context: #7996 Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon my changes in #7996 (sample sample), I noticed: 7.60s (14%) mono.android!Android.Views.View.get_Context() 14% of the time is literally spent calling `View.Context`! We did a little investigation on the underlying Java interop, this is basically doing: 1. Call into JNI, get an `IntPtr`. 2. See if that `IntPtr` maps to a C# object that is already alive. 3. Return the C# object, or create a new one if needed. We can actually avoid all this work, in this case. For example: 5.48s (10%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorX(Android.Views.View,Microsoft.Maui.IView) 4.28s (8.2%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorY(Android.Views.View,Microsoft.Maui.IView) These extension methods call `View.Context` twice: public static void UpdateTranslationY(this AView platformView, IView view) { if (platformView.Context == null) return; platformView.TranslationY = platformView.Context.ToPixels(view.TranslationY); } We can actually, make an overload for `ToPixels()` to where `View.Context` wouldn't be called *at all*: internal static float ToPixels (this View view, double dp) { if (s_displayDensity != float.MinValue) return (float)Math.Ceiling(dp * s_displayDensity); return view.Context.ToPixels(dp); } I used this everywhere I saw it appearing in `dotnet trace` output. Next, I saw `View.Context` being called a lot from `LayoutViewGroup` and `ContentViewGroup`. I could simply store the value in the constructor for these types, make it non-nullable, and remove null checks. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 64.23 Dopes/s After: 81.70 Dopes/s
PureWeen
pushed a commit
that referenced
this pull request
Jun 16, 2022
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon #7996 and #8001, I noticed while profiling the sample app putting N `Label` on the screen: 405.02ms (2.3%) microsoft.maui!Microsoft.Maui.MauiContext.get_Context() 77.90ms (0.44%) microsoft.maui!Microsoft.Maui.MauiContext.get_Handlers() Meaning that around ~2.74% of the time was spent just calling these two properties. These properties hit Microsoft.Extensions each time to locate the service: public Android.Content.Context? Context => _services.GetService<Android.Content.Context>(); These are core-services that wouldn't change after startup, so should we just use `Lazy<T>` instead of doing a call like this each time? I also made use of `ANDROID`, as that is the preferred define to use in .NET 6+ instead of `__ANDROID__`: https://github.com/dotnet/designs/blob/a447d77bb53d189746ccd80c2d814064c2b6c606/accepted/2020/net5/net5.md#vary-implementation ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 81.70 Dopes/s After: 91.94 Dopes/s
rmarinho
pushed a commit
that referenced
this pull request
Jun 23, 2022
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon #7996, #8001, and #8033, I noticed while profiling the sample app putting N Label on the screen: 783.92ms (6.2%) microsoft.maui!Microsoft.Maui.ViewHandlerExtensions.GetDesiredSizeFromHandler(Microsoft.Maui.IViewHandler,double,double) So around %6 of the time spend just measuring. Looking through the call stack, I can see 3 JNI calls happening: 932.51ms (7.4%) mono.android!Android.Views.View.Measure(int,int) 115.53ms (0.91%) mono.android!Android.Views.View.get_MeasuredWidth() 96.97ms (0.77%) mono.android!Android.Views.View.get_MeasuredHeight() So, we could write a Java method that calls all three of these and somehow returns the `MeasuredWidth` and `MeasuredHeight`. After a little research, it seemed the best approach here was to "pack" two integers into a `long`. If we tried to return some Java object instead, then we'd have the same number of JNI calls to get the integers out. I found a couple links describing how to "pack" an `int` into a `long`: * Java: https://stackoverflow.com/a/12772968 * C#: https://stackoverflow.com/a/827267 Which after some testing, arrives at: public static long measureAndGetWidthAndHeight(View view, int widthMeasureSpec, int heightMeasureSpec) { view.measure(widthMeasureSpec, heightMeasureSpec); int width = view.getMeasuredWidth(); int height = view.getMeasuredHeight(); return ((long)width << 32) | (height & 0xffffffffL); } Unpacked in C# such as: var packed = PlatformInterop.MeasureAndGetWidthAndHeight(platformView, widthSpec, heightSpec); var measuredWidth = (int)(packed >> 32); var measuredHeight = (int)(packed & 0xffffffffL); Reducing 3 JNI calls in every `View`'s layout to 1. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 91.94 Dopes/s After: 102.45 Dopes/s After profiling again, it drops the % time spent in `GetDesiredSizeFromHandler`: 528.96ms (4.5%) microsoft.maui!Microsoft.Maui.ViewHandlerExtensions.GetDesiredSizeFromHandler So the added math is negligible compared to reduced JNI calls.
rmarinho
pushed a commit
that referenced
this pull request
Jun 23, 2022
* [essentials] use Lazy<T> for Android application-wide values Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui I was reviewing a benchmark/sample that puts lots of labels on the screen that say "dope" and measures how many per second. Personally, I would have chosen some other word like `LOL`, so you could have a `LOLs/s` average. LOL? Reviewing `dotnet trace` output, I saw: 1.06s (6.3%) Microsoft.Maui.Essentials!Microsoft.Maui.ApplicationModel.AppInfoImplementation.get_RequestedLayoutDirection() 4.46ms (0.03%) Microsoft.Maui.Essentials!Microsoft.Maui.ApplicationModel.AppInfoImplementation.get_RequestedTheme() So approximately 6.3% of the time was spent querying if layouts are RTL or not? And 0.03% checking dark mode? I went through `AppInfoImplementation` on Android, and simply used `Lazy<T>` for every value that queries `Application.Context`. These values cannot change after the app launches, so they don't need to be computed on every call. If these values ever need to match a `Context` for multiple activities, we should re-evaluate this caching. A dictionary using a key of `Context.Handle` might work for that case. We can also remove places with `#if __ANDROID_24__` as they are always true in .NET 6. Runtime checks using `OperatingSystem` should be used going forward. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 55.42 Dopes/s After: 64.23 Dopes/s This should help the performance of `Label` in any .NET MAUI application running on Android. * Remove IsAndroidVersionAtLeast(17) check Minimum API level is 21 in .NET 6
rmarinho
pushed a commit
that referenced
this pull request
Jun 23, 2022
Context: #7996 Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon my changes in #7996 (sample sample), I noticed: 7.60s (14%) mono.android!Android.Views.View.get_Context() 14% of the time is literally spent calling `View.Context`! We did a little investigation on the underlying Java interop, this is basically doing: 1. Call into JNI, get an `IntPtr`. 2. See if that `IntPtr` maps to a C# object that is already alive. 3. Return the C# object, or create a new one if needed. We can actually avoid all this work, in this case. For example: 5.48s (10%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorX(Android.Views.View,Microsoft.Maui.IView) 4.28s (8.2%) microsoft.maui!Microsoft.Maui.Platform.TransformationExtensions.UpdateAnchorY(Android.Views.View,Microsoft.Maui.IView) These extension methods call `View.Context` twice: public static void UpdateTranslationY(this AView platformView, IView view) { if (platformView.Context == null) return; platformView.TranslationY = platformView.Context.ToPixels(view.TranslationY); } We can actually, make an overload for `ToPixels()` to where `View.Context` wouldn't be called *at all*: internal static float ToPixels (this View view, double dp) { if (s_displayDensity != float.MinValue) return (float)Math.Ceiling(dp * s_displayDensity); return view.Context.ToPixels(dp); } I used this everywhere I saw it appearing in `dotnet trace` output. Next, I saw `View.Context` being called a lot from `LayoutViewGroup` and `ContentViewGroup`. I could simply store the value in the constructor for these types, make it non-nullable, and remove null checks. ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 64.23 Dopes/s After: 81.70 Dopes/s
rmarinho
pushed a commit
that referenced
this pull request
Jun 23, 2022
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Building upon #7996 and #8001, I noticed while profiling the sample app putting N `Label` on the screen: 405.02ms (2.3%) microsoft.maui!Microsoft.Maui.MauiContext.get_Context() 77.90ms (0.44%) microsoft.maui!Microsoft.Maui.MauiContext.get_Handlers() Meaning that around ~2.74% of the time was spent just calling these two properties. These properties hit Microsoft.Extensions each time to locate the service: public Android.Content.Context? Context => _services.GetService<Android.Content.Context>(); These are core-services that wouldn't change after startup, so should we just use `Lazy<T>` instead of doing a call like this each time? I also made use of `ANDROID`, as that is the preferred define to use in .NET 6+ instead of `__ANDROID__`: https://github.com/dotnet/designs/blob/a447d77bb53d189746ccd80c2d814064c2b6c606/accepted/2020/net5/net5.md#vary-implementation ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 81.70 Dopes/s After: 91.94 Dopes/s
11 tasks
Member
|
Some of the APIs should not have been cached - theme and layout direction. These have been reverted in #11200 That PR was not a plain revert and instead cache the values in Controls where we get events when the theme changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui
I was reviewing a benchmark/sample that puts lots of labels on the
screen that say "dope" and measures how many per second. Personally, I
would have chosen some other word like
LOL, so you could have aLOLs/saverage. LOL?Reviewing
dotnet traceoutput, I saw:So approximately 6.3% of the time was spent querying if layouts are
RTL or not? And 0.03% checking dark mode?
I went through
AppInfoImplementationon Android, and simply usedLazy<T>for every value that queriesApplication.Context. Thesevalues cannot change after the app launches, so they don't need to be
computed on every call.
If these values ever need to match a
Contextfor multipleactivities, we should re-evaluate this caching. A dictionary using a
key of
Context.Handlemight work for that case.We can also remove places with
#if __ANDROID_24__as they are alwaystrue in .NET 6. Runtime checks using
OperatingSystemshould be usedgoing forward.
Results
A
Releasebuild on a Pixel 5 device, I was getting:This should help the performance of
Labelin any .NET MAUIapplication running on Android.