Ordering children while adding #8231

memsranga · 2019-10-25T13:48:24Z

Description of Change

Orders (Z ordering) the child soon after adding it to the view, reducing the complexity from N^2 to N

Issues Resolved

fixes [Bug] Adding children to iOS VisualElementPackager has O(N^2) performance and thrashes the native layer #8129

API Changes

None

Platforms Affected

iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Followed the same repro that was reported in the bug.
Added 2000 elements to a Stack View
Test Cases -> G8129
Should not lock the UI considerably.

PR Checklist

Targets the correct branch
Tests are passing (or failures are unrelated)

dnfclas · 2019-10-25T13:48:36Z

All CLA requirements met.

memsranga · 2019-10-26T00:54:25Z

If this looks acceptable, I'll add more test with Z-indexing

Xamarin.Forms.Platform.iOS/VisualElementPackager.cs

samhouts · 2019-12-19T23:06:31Z

Issue8129.ZIndex After Adding Contents Test is failing

samhouts · 2019-12-19T23:07:06Z

Issue8129.Add Too Many Contents Test is also failing

samhouts · 2020-01-15T23:53:10Z

@shanranm Are you able to look at the failing tests? Thanks!

memsranga · 2020-02-25T02:43:59Z

@samhouts sorry couldn't find time last quarter!
I've updated the tests to run only for iOS since the same performance can't be expected from Android

AlleSchonWeg · 2021-05-28T07:30:39Z

Hi @rmarinho
FYI: Adding 1000 Children takes 1,5 seconds on Android. On iOS 5 minutes.

It's a litte bit frustrating if a PR with a potential fix is not merged since almost 2 years.

What can i do that this fix will become a part of the next SR? Tell me and i will do my very best. Please don't let my stay in the rain.

Thank you.

AlleSchonWeg · 2021-06-18T05:46:59Z

Hi @rachelkang,

could we merge this in the next SR?

jfversluis · 2021-08-26T14:23:04Z

/azp run

azure-pipelines · 2021-08-26T14:23:19Z

Azure Pipelines successfully started running 1 pipeline(s).

jfversluis · 2021-08-27T12:22:18Z

/azp run

azure-pipelines · 2021-08-27T12:22:31Z

Azure Pipelines successfully started running 1 pipeline(s).

jfversluis · 2021-08-27T13:07:54Z

@AlleSchonWeg as per the description here, there is now a nuget available with this functionality. Could you have a quick look if this does what you expect?

AlleSchonWeg · 2021-08-30T08:37:02Z

Hi @jfversluis,
here are my test results.

	Add 1000 Labels	Add 220 Labels
Old	77191 ms	1578 ms
New	2388 ms	505 ms

Great work!
Thaank you.

jfversluis · 2021-08-30T12:25:51Z

Thanks for testing it! I will see if we can get this reviewed :)

jsuarezruiz · 2021-09-07T10:01:24Z

We should try to have this one in the upcoming SR's.

jsuarezruiz · 2021-09-07T10:03:12Z

Xamarin.Forms.Platform.iOS/VisualElementPackager.cs

@@ -141,34 +143,59 @@ protected virtual void OnChildRemoved(VisualElement view)
 			viewRenderer.Dispose();
 		}

-		void EnsureChildrenOrder()
+		void EnsureChildrenOrder(VisualElement element = null)


Could we add some tests validating the logic to order and ensure the order of the elements?

* Fix 4143: improved Span region calculation (#13348) fixes #4143 fixes #6992 fixes #11650 fixes #7655 fixes #11657 fixes #10520 fixes #4829 * improved span region calculation on android * Remove unused code * Nit: clean up comments * Wrap in ScrollView for small screen compatibility Co-authored-by: Tim Dittmar <Tim.Dittmar@Exxeta.com> Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com> Co-authored-by: Rachel Kang <rachelkang@microsoft.com> * Only check for netfx when not in a wapproj (#14402) fixes #13957 * [macOS] Update Switch renderer (#14334) fixes #14313 * [macOS] Update Switch renderer #14313 * [macOS] Update Switch renderer #14313 * [Android] Fix to Issue Java.Lang.IndexOutOfBoundsException: setSpan (#12764) Fix to Issue #12762 * Update EntryRenderer.cs Fix to Issue #12762 Java.Lang.IndexOutOfBoundsException: setSpan (nn ... nn) ends beyond length n-1 * Fix to EntryRenderer.cs * Update EntryRenderer.cs * Update EntryRenderer.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * [Android] Fix ClearButton not working when changing the ClearButtonVisibility while the Entry field is focused (#13820) Fixes #13819 Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> * Automated dotnet-format update (#14404) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Automated dotnet-format update (#14405) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix memory leak (and some crashes) in ItemsViewController (iOS) (#14111) * add unit test * fix memory leak (and some crashes) in ItemsViewController (iOS) * Update ItemsViewController.cs Co-authored-by: Rui Marinho <me@ruimarinho.net> * Enable ScrollTo tests for Android, UWP; implement ScrollTo with group index for Android (#13007) * Remove inaccurate category * Enable ScrollTo tests for Android/UWP; Implement grouped ScrollTo for Android * Implement missing ScrollTo grouped item by index * Exempt 3788 test from UWP * Ignore test on UWP Co-authored-by: Rui Marinho <me@ruimarinho.net> * Fix type for case blueviolet (#14434) The 'v' in this case was uppercase causing this typeconverter to reject BlueViolet as a valid color value. Fixes #AB1286706 [XVS]XLS0431: Invalid value for property 'BackgroundColor': 'BlueViolet' * [iOS] Fix: CollectionView was not updating when it was invisible (#14384) * [iOS] Fix: CollectionView was not updating when it was invisible * [iOS] Fix: NSInternalInconsistencyException Reason: request for number of items in section X when there are only X sections in the collection view when NumberOfItemsInSection is invoked * Revert "[iOS] Fix: NSInternalInconsistencyException Reason: request for number of items in section X when there are only X sections in the collection view when NumberOfItemsInSection is invoked" This reverts commit 2343f15. * [iOS] Fix: Prevent NumberOfItemsInSection invoking if CollectionView is hidded to avoid "NSInternalInconsistencyException Reason: request for number of items in section X when there are only X sections" * [iOS] Scroll locked issue using SwipeView (#12758) * Fixed issue re-enabling the scroll * Added repro sample * Fixed also the swipe sensitive issue on iOS Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * Fix removed shadow in Thumb setting a custom color (#13166) * [iOS] Fix crash/closing wrong modal with FormSheet and tap outside (#14527) * Add repro + fix * Update Issue12300.cs * [Core] SwipeItem Parent using SwipeView in DataTemplate (#13385) * Added repro sample * Fix the issue Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * Fix Frame Background issue with transparent color (#14565) * Fix broken disabled Button visual state in UWP (#14567) Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * Update Device.Idiom when flipping tablet mode state (#13150) * [Android] Fix occasional wrong touch interception in SwipeView Content (#13732) * Fix wrong touch interception in SwipeView Content on Android * Updated swipe delta * Fix Entry issue using TextColor and ClearButton in iOS < 13 (#14566) Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * Fix issue using FormattedString and LineBreakMode on iOS (#14572) Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * Align BarBackground behavior between Android and iOS. (#13503) * Automated dotnet-format update (#14570) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * [Android] Update gradients based on offset changes in Frame and BoxView (#11812) * Update gradients based on offset changes in Frame and BoxView on Android * Updated the sample to test also the BoxView Brush offsets * Fix NRE in ListView Command validation (#14580) * [iOS] Fix inability to check an initially disabled RadioButton after enabling it (#14545) * Listen for updates to IsEnabled on RadioButton to ensure that the tap gesture recogniser is added when the control is enabled * Update Issue14544.cs * Update test case Co-authored-by: Gerald Versluis <gerald@verslu.is> * Fix crash navigating in Shell (#14577) * Androidx bumps (#14506) * Update to Latest Android X Libraries * - fix android forwarders * Moar updates Co-authored-by: Gerald Versluis <gerald@verslu.is> * Validate issue 14433 (#14576) * Added repro sample * Updated sample Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * [iOS] Remove usage of System.Drawing types (#14571) * Run nightly also for 5.0.0 branch * Automated dotnet-format update (#14581) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Ordering children while adding (#8231) * Ordering children while adding * Abstract OrderElement * Re-order on runtime child addition and Tests * Runtime addition ensures Z Index * Stop performance measurement on return * Update VisualElementPackager.cs Co-authored-by: Gerald Versluis <gerald@verslu.is> Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> * If stroke is null avoid render a shape stroke (#14587) * Fix broken Android platform tests (#14590) * Automated dotnet-format update (#14595) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * [macOS] Fix RadioButton activated behaviour (#14139) * Respond to Activated event * Set initial control state based on element state * Revert "Set initial control state based on element state" This reverts commit 1bd1238. * CollectionView RemainingItemsThreshold implementation for UWP (#14202) * [Android] Fix building with Android stable bits (#14608) * Set java sdk path * Use class-parse Co-authored-by: Braini01 <T_Dittmar@web.de> Co-authored-by: Tim Dittmar <Tim.Dittmar@Exxeta.com> Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com> Co-authored-by: Rachel Kang <rachelkang@microsoft.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: MH Rastegari <42671084+mhrastegary77@users.noreply.github.com> Co-authored-by: Felipe Silveira <felipe@transis.com.br> Co-authored-by: Rui Marinho <me@ruimarinho.net> Co-authored-by: Jonathan Goyvaerts <jonathan.goyvaerts@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Thomas Mijieux <tmijieux@users.noreply.github.com> Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com> Co-authored-by: Marco Goertz <mgoertz@microsoft.com> Co-authored-by: Kirill <rotorgames@bk.ru> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: Lee Millward <lee.millward@gmail.com> Co-authored-by: Gerald Versluis <gerald@verslu.is> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Filip Navara <filip.navara@gmail.com> Co-authored-by: Shanmukha Ranganath <shanmukharanganath@gmail.com> Co-authored-by: Julio César Rocha <JunielKatarn@users.noreply.github.com> Co-authored-by: nk221 <kirill.n@gmail.com>

samhouts added p/iOS 🍎 hacktoberfest 🍻 t/bug 🐛 labels Oct 25, 2019

samhouts requested review from jfversluis and kingces95 October 28, 2019 18:16

samhouts assigned jfversluis and kingces95 Oct 28, 2019

jfversluis reviewed Oct 29, 2019

View reviewed changes

Xamarin.Forms.Platform.iOS/VisualElementPackager.cs Outdated Show resolved Hide resolved

memsranga requested a review from jfversluis November 1, 2019 08:25

samhouts added this to In Review in v4.5.0 Nov 2, 2019

samhouts changed the base branch from master to 4.5.0 January 7, 2020 00:57

samhouts added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jan 11, 2020

samhouts unassigned jfversluis Jan 14, 2020

samhouts removed request for jfversluis and kingces95 January 15, 2020 23:53

samhouts unassigned kingces95 Jan 15, 2020

samhouts added the a/performance label Jan 15, 2020

samhouts moved this from In Review to In Progress in v4.5.0 Feb 25, 2020

memsranga force-pushed the Issue8129 branch from 698cbd2 to bacbaf9 Compare February 25, 2020 03:42

memsranga requested a review from jfversluis February 25, 2020 04:08

samhouts removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Feb 25, 2020

samhouts moved this from In Progress to In Review in v4.5.0 Feb 25, 2020

samhouts changed the base branch from 4.5.0 to master February 27, 2020 22:54

samhouts added this to In Review in v4.6.0 Feb 27, 2020

AlleSchonWeg mentioned this pull request Aug 26, 2021

[Question] Xamarin.Forms end of life? #14205

Closed

memsranga added 4 commits August 27, 2021 14:09

Ordering children while adding

3d88f4f

Abstract OrderElement

c70f783

Re-order on runtime child addition and Tests

3c8d1cc

Runtime addition ensures Z Index

dbb71a4

jfversluis force-pushed the Issue8129 branch from 47d2eef to dbb71a4 Compare August 27, 2021 12:10

memsranga requested a review from StephaneDelcroix as a code owner August 27, 2021 12:10

jfversluis changed the base branch from main to 5.0.0 August 27, 2021 12:10

jfversluis added 2 commits August 30, 2021 14:18

Stop performance measurement on return

4e1c18a

Update VisualElementPackager.cs

d1c95f6

jfversluis added needs-maui-validation and removed hacktoberfest 🍻 labels Aug 30, 2021

jfversluis added this to PR Needs Review in 5.0.0 SR 5 (Planning) Aug 30, 2021

Merge branch '5.0.0' into Issue8129

b7a1449

jsuarezruiz reviewed Sep 7, 2021

View reviewed changes

jsuarezruiz requested a review from hartez September 7, 2021 10:13

jfversluis merged commit 5f391f0 into xamarin:5.0.0 Sep 9, 2021

v5.0.1 automation moved this from Review in progress to Done Sep 9, 2021

5.0.0 SR 5 (Planning) automation moved this from PR Needs Review to Done Sep 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ordering children while adding #8231

Ordering children while adding #8231

memsranga commented Oct 25, 2019 •

edited

dnfclas commented Oct 25, 2019 •

edited

memsranga commented Oct 26, 2019

samhouts commented Dec 19, 2019

samhouts commented Dec 19, 2019

samhouts commented Jan 15, 2020

memsranga commented Feb 25, 2020

AlleSchonWeg commented May 28, 2021

AlleSchonWeg commented Jun 18, 2021

jfversluis commented Aug 26, 2021

azure-pipelines bot commented Aug 26, 2021

jfversluis commented Aug 27, 2021

azure-pipelines bot commented Aug 27, 2021

jfversluis commented Aug 27, 2021

AlleSchonWeg commented Aug 30, 2021

jfversluis commented Aug 30, 2021

jsuarezruiz commented Sep 7, 2021

jsuarezruiz Sep 7, 2021

Ordering children while adding #8231

Ordering children while adding #8231

Conversation

memsranga commented Oct 25, 2019 • edited

Description of Change

Issues Resolved

API Changes

Platforms Affected

Behavioral/Visual Changes

Before/After Screenshots

Testing Procedure

PR Checklist

dnfclas commented Oct 25, 2019 • edited

memsranga commented Oct 26, 2019

samhouts commented Dec 19, 2019

samhouts commented Dec 19, 2019

samhouts commented Jan 15, 2020

memsranga commented Feb 25, 2020

AlleSchonWeg commented May 28, 2021

AlleSchonWeg commented Jun 18, 2021

jfversluis commented Aug 26, 2021

azure-pipelines bot commented Aug 26, 2021

jfversluis commented Aug 27, 2021

azure-pipelines bot commented Aug 27, 2021

jfversluis commented Aug 27, 2021

AlleSchonWeg commented Aug 30, 2021

jfversluis commented Aug 30, 2021

jsuarezruiz commented Sep 7, 2021

jsuarezruiz Sep 7, 2021

Choose a reason for hiding this comment

memsranga commented Oct 25, 2019 •

edited

dnfclas commented Oct 25, 2019 •

edited