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

Optimize SwiftUIWrapperView #320

Merged
merged 1 commit into from
Nov 10, 2024
Merged

Conversation

bryankeller
Copy link
Contributor

Details

This PR significantly reduces CPU use for people using SwiftUI views for their calendar items by making 2 optimizations:

  1. Don't add the UIHostingController as a child view controller, and just work with its hosting view directly
  2. Don't give every view a unique ID just to get onAppear and onDisappear callbacks - instead, set a hidden view's to just display an EmptyView

Together, these changes reduce CPU use by 35-40% when scrolling fast.

Related Issue

#299

Motivation and Context

Performance improvements.

How Has This Been Tested

Example app

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@bryankeller bryankeller added the enhancement New feature or request label Nov 9, 2024
@bryankeller bryankeller force-pushed the bk/optimize-SwiftUIWrapperView branch from 77e5cee to 2fce208 Compare November 9, 2024 23:15
@bryankeller bryankeller force-pushed the bk/optimize-SwiftUIWrapperView branch from 2fce208 to a8caca1 Compare November 9, 2024 23:19
public override var isHidden: Bool {
didSet {
if isHidden {
hostingController.rootView = AnyView(EmptyView())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not ideal, but way faster than the previous strategy of giving every view an ID

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also add an API in the future to opt-out of this behavior if someone doesn't need onAppear or onDisappear callbacks in their views, which would get them back ~20% CPU usage when scrolling fast 🤯

}

// MARK: Fileprivate

fileprivate var contentAndID: ContentAndID {
didSet {
hostingController.rootView = .init(content: contentAndID.content, id: contentAndID.id)
hostingController.rootView = AnyView(contentAndID.content)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I profiled and there was no perf hit from switching to AnyView

Copy link
Contributor

@brynbodayle brynbodayle left a comment

Choose a reason for hiding this comment

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

Nice findings!

@bryankeller bryankeller merged commit b7ebeee into master Nov 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants