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

Improve performance of contentSize by not using a list to back it #283

Merged
merged 1 commit into from Mar 30, 2021

Conversation

kyleve
Copy link
Collaborator

@kyleve kyleve commented Mar 26, 2021

This PR refactors how ListView.contentSize is implemented, by switching to manually allocating a layout and measuring it, instead of using a static measurement / template view. This has significant performance improvement benefits on its own of about 50%, because we're not going through the entire diff and layout loop to update the measurement view – just creating a layout and measuring it.

To allow this, a few changes happened:

  1. Updated the list layout APIs to work without a needed UICollectionView – you really just need the layout objects and the information about the layout frame, so I cleaned that up. This involved removing references to UICollectionView, making the delegate optional.
  2. Move ListSizing to the BlueprintUILists module – it's only used there anyway.
  3. Some small cleanup things as I went.

@kyleve kyleve requested a review from kylebshr March 26, 2021 23:40
@kyleve kyleve changed the title [DNR WIP] Improve performance of contentSize by not using a list to back it Improve performance of contentSize by not using a list to back it Mar 26, 2021
@kyleve kyleve force-pushed the kve/contentSize-perf branch 2 times, most recently from 2438a49 to 618daf8 Compare March 26, 2021 23:43
@@ -402,14 +402,14 @@ fileprivate struct TestContent : ItemContent, Equatable
self.wasRemoved_calls.append(())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixes build warnings in tests

/// ┌───────────┐
/// │ │
/// │ │
/// │┌─────────┐│
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is pretty silly, feel free to leave as-is, but do you think top-aligning illustrates what will actually happen better?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to ignore this given it was the same as before in the old file and I want to get this in SPOS tonight

@kylebshr
Copy link
Collaborator

kylebshr commented Mar 30, 2021 via email

@kyleve kyleve merged commit 54ef237 into main Mar 30, 2021
@kyleve kyleve deleted the kve/contentSize-perf branch March 30, 2021 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants