Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

fix: #13429 #15626

Closed
wants to merge 1 commit into from
Closed

fix: #13429 #15626

wants to merge 1 commit into from

Conversation

anton-yashin
Copy link

Description of Change

Because Xamarin.iOS uses a reflection to provide arguments in callbacks as decribed here. This PR provides a custom NSIndexPath wrapper to avoid reflection calling in ItemsViewDelegator.GetSizeForItem. Performance boost is 12x on Simulator or 17x on iPhone.

Issues Resolved

API Changes

Added:

  • public object IItemsViewSource.this[NSIndexPathRef indexPath] { get; }
  • internal object EmptySource.this[NSIndexPathRef indexPath] { get; }
  • internal object ListSource.this[NSIndexPathRef indexPath] { get; }
  • internal object ObservableGroupedSource.this[NSIndexPathRef indexPath] { get; }
  • internal object ObservableItemsSource.this[NSIndexPathRef indexPath] { get; }
  • public ref struct NSIndexPathRef
  • public CGSize ItemsViewDelegator.GetSizeForItem(IntPtr collectionView, IntPtr layout, IntPtr indexPath)
  • public CGSize ItemsViewDelegator.GetSizeForItem(IntPtr collectionView, IntPtr layout, NSIndexPathRef indexPath)
  • internal bool IndexPathHelpers.IsIndexPathValid(this IItemsViewSource source, NSIndexPathRef indexPath)

Changed:

  • internal CGSize ItemsViewController.GetSizeForItem(NSIndexPath indexPath) => internal CGSize ItemsViewController.GetSizeForItem(NSIndexPathRef indexPath)

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Test page for control gallery Issue13429 is provided, UITest included. You may able to run this test from VS. Because this is performance issue you may want to place some different timing depending on your hardware. Current timings is applicable for iPhone 11 and mac mini 2018 i5/16GB. See EXPECTED_TIMEOUT constant at Issue13429 test page. If you want to run this test through control gallery, then launch test, then tap "Add", "Remove" or "Replace" button. Test will show you how much time is taken by operation in milliseconds.

PR Checklist

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

@anton-yashin
Copy link
Author

@microsoft-github-policy-service agree

@jfversluis
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis
Copy link
Member

@jfversluis
Copy link
Member

Hey @anton-yashin thank you so much for this. I would LOVE to take this, but unfortunately at this point in the Xamarin.Forms lifecycle that's not possible because it adds new APIs and that's not something we still want to do.

I know it's been a while, but if you're interested, please have a look at .NET MAUI and see if this is the case there also as well, we would love to get these benefits there. Thank you again so much for your efforts. It's really appreciated!

@jfversluis jfversluis closed this Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Xamarin.Forms 5.0.0.x ObservableCollection.Add performance decrease on iOS
2 participants