performance improvements for AuthorLabel #1458

[bryanmontz]

Issues covered

#1458

Description

This is a small step in improving the scroll performance of the app and is meant to be a pattern check for other devs. If these changes are acceptable, then we could implement similar changes widely.

My goal here was to identify and address one small performance issue. Here is the profiling scenario to repeat between changes:

How to test

1. While signed in to the app build it for profiling by click-and-hold-ing on the Play button and dragging to select Profile.
2. When Instruments opens, select the SwiftUI instrument.
3. Click the red record button to begin profiling.
4. Once the app settles on the feed, do two strong flings to move down the feed.
5. Stop the recording.
6. Click and drag left to right to select the area of time during the two flings.
7. Select the "View body" instrument, and observe the results.
   The "View body" instrument gives you the count and duration of SwiftUI view body methods for all the views in the app. If you sort the results by "Total Duration", you'll see the "worst performance offenders". The "Average Duration" is also very important to look at, but it must be considered in combination with the total number of redraws.

Here's how the ranking looks before the changes in this PR:

Here you see that AuthorLabel was drawn 29 times for a total of 33.32ms. This is miserable performance. At 60Hz, a single frame must take less than 16.67ms overall, so we want individual views' times to be MUCH less than that.

After only removing the xcstringstool usage in AuthorLabel let's run the same test again:

This time you see around the same number of redraws, but each one takes 1/10th the time! This is a lot better, but we're still adding up to milliseconds here, and we'd prefer to get it lower.

The next big performance improvement is to cache the result of the attributedAuthor method, which is taking too long, in a view-scoped cache. I also made another change to pass only the relevant properties of Author to this view, rather than the entire object. This is good practice regardless of performance.

Now we see the performance we want to get from this view. Even with 22 redraws, the total time is on the order of hundreds of microseconds.

[joshuatbrown]

👀

[joshuatbrown]

This looks good to me -- both the code and the results I saw in Instruments. Since it's such an important change, I'd like to have @mplorentz review as well.

[mplorentz]

Unfortunately it looks to me like this breaks the observation of the author by these views and makes them static. If you delete the app and launch it again with an empty Core Data database none of the names load on the home feed note cards.

I am hopeful we can get observation back without losing the performance improvements though. I think if we go back to using @ObservedObject var author (or maybe better: @Bindable var author to use @Observable) we can still set the cache up so that it only recomputes the AttributedString when the relevant properties on the author change and bust the cache when they do.

I think one way to approach it is to store the attributed string in an @State var and recompute it using onChange(of: author). This will get recomputed too often, but not every time body is called, only when author changes (and only the properties on author I think, not when core data relationships are updated).

A more advanced approach might be to pass the whole author into the Cache and wrap the attributedAuthor creation in a withObservationTracking block. Then the cache will only recompute the string when the one of the relevant stored properties on author changes (e.g. displayName, name, hexadecimalPublicKey, etc.