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

Measure invalidation performance and fixes #24532

Conversation

albyrock87
Copy link
Contributor

@albyrock87 albyrock87 commented Aug 30, 2024

Description of Change

Initial proposal to solve #24551 .

I executed a speedscope on davidortinau/AllTheLists LearningPage using the latest nightly 8.0.99-ci.net8.24468.1 build.

What I noticed is that #23052 OnChildMeasureInvalidatedInternal was showing up and taking 2% of total time (recursive calls).

What I've done:

  • Avoids triggering measure invalidated completely while building the MAUI tree in memory (Handler = null)
  • Fixes some iOS SetNeedsLayout propagation issues and cleans up detection of auto SetNeedsLayout parent propagation with IPropagatesSetNeedsLayout internal interface
  • Removes "out-of-layout-pass" measurements from legacy layouts making them fast as new layouts

WinUI Speedscope

Kindly provided by @MartyIX

Before main
image
1726819823.MAIN.speedscope.json

After PR
image
1726819948.PR.speedscope.json

Before main
image
1726819865.MAIN.speedscope.json

After PR
image
1726819973.PR.speedscope.json

iOS Speedscope

Before main
image
before.speedscope.json.zip

After PR
image
after.speedscope.json.zip

Issues Fixed

Fixes #24551

@albyrock87 albyrock87 requested a review from a team as a code owner August 30, 2024 08:31
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 30, 2024
@albyrock87 albyrock87 force-pushed the improve-measure-invalidation-and-legacy-layout-performance branch from ca4b0a7 to 5b65d84 Compare August 30, 2024 08:34
@bcaceiro
Copy link

@albyrock87 I'm quite interested on your phrase: " due to legacy layouts (which includes ContentView)" , the example has a CollectionView with a DataTemplate , that has a ContentView as a parent - should we have better performance, in having for instance, DataTemplates with a Grid / Border as a parent?

@rmarinho rmarinho requested review from PureWeen and removed request for Eilon August 30, 2024 12:50
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@samhouts samhouts added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Aug 30, 2024
@albyrock87 albyrock87 force-pushed the improve-measure-invalidation-and-legacy-layout-performance branch from 5b65d84 to 2d0c694 Compare August 30, 2024 23:14
@albyrock87
Copy link
Contributor Author

@bcaceiro after this PR it shouldn't matter if you use ContentView or not.
Before this PR I can say there is a performance advantage in completely avoid views based on legacy Layout class.

@albyrock87 albyrock87 force-pushed the improve-measure-invalidation-and-legacy-layout-performance branch 4 times, most recently from f5239da to b2874d6 Compare September 6, 2024 17:11
var request = new Size(size.Request.Width + Padding.HorizontalThickness, size.Request.Height + Padding.VerticalThickness);
var minimum = new Size(size.Minimum.Width + Padding.HorizontalThickness, size.Minimum.Height + Padding.VerticalThickness);

DesiredSize = request;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Base method sets DesiredSize so the fact this was missing was simply wrong.

@PureWeen
Copy link
Member

PureWeen commented Sep 6, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87 albyrock87 force-pushed the improve-measure-invalidation-and-legacy-layout-performance branch from b2874d6 to a8780b1 Compare September 6, 2024 17:46
@PureWeen
Copy link
Member

PureWeen commented Sep 6, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87 albyrock87 force-pushed the improve-measure-invalidation-and-legacy-layout-performance branch from a8780b1 to 3153543 Compare September 7, 2024 16:40
@PureWeen
Copy link
Member

PureWeen commented Sep 9, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87 albyrock87 marked this pull request as draft September 17, 2024 15:37
@albyrock87 albyrock87 force-pushed the improve-measure-invalidation-and-legacy-layout-performance branch from 2628e98 to b9e69c7 Compare September 17, 2024 17:58
@albyrock87 albyrock87 marked this pull request as ready for review September 17, 2024 18:41
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87 albyrock87 marked this pull request as draft September 18, 2024 13:07
@albyrock87 albyrock87 marked this pull request as ready for review September 18, 2024 19:07
@albyrock87 albyrock87 changed the title Improve measure invalidation and legacy Layout(s) performance Measure invalidation performance and fixes Sep 18, 2024
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87
Copy link
Contributor Author

Closing in favor of #25664

@albyrock87 albyrock87 closed this Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve measure invalidation and legacy Layout(s) performance
6 participants