Skip to content
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] MauiTextView doesn't need ViewAttachedToWindow #14833

Merged

Conversation

jonathanpeppers
Copy link
Member

Context: #12130
Context: https://github.com/angelru/CvSlowJittering

Profiling a customer sample, I noticed that ~2.4% of of the time was spent subscribing to ViewAttachedToWindow:

278.55ms (2.4%) mono.android!Android.Views.View.add_ViewAttachedToWindow(System.EventHandler`1<Android.Views.View/ViewAttachedToWindowEv

It appeared that for every Label on Android, we setup this event. Additionally, Java would have to call into C# when the event fires:

30.55ms (0.26%) mono.android!Android.Views.View.IOnAttachStateChangeListenerInvoker.n_OnViewAttachedToWindow_Landroid_view_View__mm_wra

However, we don't actually do anything in the event (added in b6c3b53), so we should be able to just delete it?

this.ViewAttachedToWindow += MauiTextView_ViewAttachedToWindow;
//...
private void MauiTextView_ViewAttachedToWindow(object? sender, ViewAttachedToWindowEventArgs e)
{
}

Doing this, dropped the call to ~0.02%:

2.76ms (0.02%) mono.android!Android.Views.View.add_ViewAttachedToWindow(System.EventHandler`1<Android.Views.View/ViewAttachedToWindowEv

And the above IOnAttachStateChangeListenerInvoker call is gone completely.

This should improve the performance of every Label on Android.

Context: dotnet#12130
Context: https://github.com/angelru/CvSlowJittering

Profiling a customer sample, I noticed that ~2.4% of of the time was
spent subscribing to `ViewAttachedToWindow`:

    278.55ms (2.4%) mono.android!Android.Views.View.add_ViewAttachedToWindow(System.EventHandler`1<Android.Views.View/ViewAttachedToWindowEv

It appeared that for every `Label` on Android, we setup this event.
Additionally, Java would have to call into C# when the event fires:

    30.55ms (0.26%) mono.android!Android.Views.View.IOnAttachStateChangeListenerInvoker.n_OnViewAttachedToWindow_Landroid_view_View__mm_wra

However, we don't actually *do* anything in the event (added in
b6c3b53), so we should be able to just delete it?

    this.ViewAttachedToWindow += MauiTextView_ViewAttachedToWindow;
    //...
    private void MauiTextView_ViewAttachedToWindow(object? sender, ViewAttachedToWindowEventArgs e)
    {
    }

Doing this, dropped the call to ~0.02%:

    2.76ms (0.02%) mono.android!Android.Views.View.add_ViewAttachedToWindow(System.EventHandler`1<Android.Views.View/ViewAttachedToWindowEv

And the above `IOnAttachStateChangeListenerInvoker` call is gone
completely.

This should improve the performance of every `Label` on Android.
@jonathanpeppers jonathanpeppers added the legacy-area-perf Startup / Runtime performance label Apr 28, 2023
@rmarinho rmarinho requested a review from PureWeen April 28, 2023 16:37
@jsuarezruiz jsuarezruiz added the area-controls-label Label, Span label Apr 28, 2023
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Apr 28, 2023
@jonathanpeppers jonathanpeppers marked this pull request as ready for review April 28, 2023 18:04
@PureWeen PureWeen enabled auto-merge (squash) April 28, 2023 23:26
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙀

@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@PureWeen PureWeen merged commit f6d0fea into dotnet:main Apr 29, 2023
@jonathanpeppers jonathanpeppers deleted the MauiTextView.ViewAttachedToWindow branch May 1, 2023 13:31
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2023
@Eilon Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) and removed legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor legacy-area-perf Startup / Runtime performance labels May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-preview.4.8333 Look for this fix in 8.0.0-preview.4.8333! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-label Label, Span fixed-in-8.0.0-preview.4.8333 Look for this fix in 8.0.0-preview.4.8333! platform/android 🤖 t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants