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

Auto Performance Monitoring for SwiftUI #1737

Closed
1 task
Tracked by #1814
philipphofmann opened this issue Apr 5, 2022 · 6 comments
Closed
1 task
Tracked by #1814

Auto Performance Monitoring for SwiftUI #1737

philipphofmann opened this issue Apr 5, 2022 · 6 comments
Assignees

Comments

@philipphofmann
Copy link
Member

philipphofmann commented Apr 5, 2022

Description

With #1242 we disabled swizzling UIViewControllers generated by SwiftUI because they have long confusing names. @brustolin had an idea of how to make sense of these names. If we can extract some meaningful information out of these names, we could enable APM for SwiftUI again.

  • Collect Slow and Frozen frames

This is part of a general SwiftUI support improvement:

@philipphofmann
Copy link
Member Author

philipphofmann commented Jun 28, 2022

SwiftUI generates the UIViewControllers for SwiftUI views during runtime. It does that between UIApplicationDidFinishLaunchingNotification and UIApplicationDidBecomeActiveNotification. In order to swizzle these UIViewControllers, we need to go through all classes with objc_getClassList. Looking into classes of the SwiftUI.framework image with copyClassNamesForImage doesn't work as these generated UIViewControllers don't end up in the SwiftUI.framework image.
The UIViewControllers lifecycle seems to be a bit different. It seems like SwiftUI skips a few. That's why we need to create a transaction in different lifecycle methods.

For a work in progress branch checkout https://github.com/getsentry/sentry-cocoa/tree/feat/swift-ui-performance.

Internal Notion page.

@brustolin
Copy link
Contributor

SwiftUI view are not really UI elements, its more like a description on what the screen should looks like. Thats why we can't manipulate Views directly. And all the code responsible to convert View into UIViews is private. If we try to manipulate it, we may cause apps to be rejected during review.

The alternative is to create a Root View or a view modifier that tracks its children performance. Of course, this will no be an automatic solution, the user must actively use this.

Also, we need to create an extra package for this. Im convinced that we should not include Swift code in the SDK.

@brustolin
Copy link
Contributor

An update on some findings

  1. Getting a meaningful name out of SwiftUI-generated UIViewControllers seems almost impossible. We have a somewhat working solution, but it is fragile and only works for a specific use case.
  2. Combining Swift-UI view spans and SwiftUI-generated UIViewControllers spans in one transaction would be fantastic as it gives you the full picture.
  3. SwiftUI only has a callback for onAppear , which doesn’t tell you when the view finished rendering. There seems to be no proper API for when a SwiftUI view finished appearing.
  4. SwiftUI doesn’t provide a global way or hook for instrumenting all views. We have to use a wrapper for every view we want to instrument. SentryWrapper for SwiftUI views (we have to come up with a better name of course). Users can pass in a name, so we can properly name transactions.
  5. The first iteration could only support plain SwiftUI views.
  6. The first iteration could only have UIViewController spans, but the users need to identify the screen name with a SentryWrapper in SwiftUI.
  7. We still need to figure out how this would work in combination with UIViewControllers.

@brustolin
Copy link
Contributor

brustolin commented Oct 13, 2022

Current Challenges

While working on this issue, we identified many new challenges that we did not face before.

Adding Swift

First, to be able to build SwiftUI performance monitoring, we need Swift code on the Sentry SDK. Challenges we face for that:

  1. Minimum breaking changes (none If possible)
  2. Only one import for the SDK in Swift and ObjC code
  3. Allow customers to target Swift API in an Objc project and vice-versa
  4. Same imports for all different package managers
  5. Distribution with package mangers. More details are below.
  6. Write new code in Swift (this would be nice, but not really necessary) without breaking changes
  7. Moving exiting classes from ObjC to Swift without breaking changes (nice to have)

Distribution with Package Managers

We use 3 different ways to distribute the Sentry framework: Swift Package Manager (SPM), CocoaPods, and Carthage.

  • SPM does not allow mixed languages in the same target; we need multiple targets. One of these targets will depend on the other; most likely ObjC target depends on the Swift target.
  • CocoaPods does not allow multiple targets in the same podspec.
  • Carthage has no problems but is the least used package manager.

SwiftUI Performance Monitoring

Once we have Swift in our codebase, we can talk about how we gonna introduce SwiftUI performance monitoring. While there are multiple ideas on what we can achieve, the first step is a wrapper view that will be explicitly used in the UI for tracking the performance of SwiftUI views.

On a pair programming session with @philipphofmann, we identified that by including this wrapper view in our Sentry SDK, every project that uses the Sentry Cocoa SDK would also load the SwiftUI framework at runtime, even if they don't use it. This is unacceptable, as the SwiftUI framework is quite large (around 25 MB); we don't want to do this.

The proposal is to create another library for those project that uses SwiftUI.

This extra library introduces more challenges.

  1. This new library needs to access Sentry SDK private classes to create transactions that play nicely with the rest of the SDK.
  2. How does the release process look like for the extra SwiftUI package?
  3. Should we use the same version number?
  4. How to make this available using CocoaPods

@github-project-automation github-project-automation bot moved this from In Progress to Done in Mobile & Cross Platform SDK Jan 3, 2023
@philipphofmann
Copy link
Member Author

@brustolin, please explain why you closed this issue and add the PR(s) completing this issue.

@armcknight
Copy link
Member

Looks like it was added in #2271?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: In Progress 📈
Development

No branches or pull requests

3 participants