Various optimisations (#1878)

* Cache TimelineBubbleLayout subview sizes * Cache MessageText sizes, avoid extra updates * Only use the `CollapsibleReactionLayout` if there's more than 5 reactions on a particular message * Upgrade Sentry to 8.13.0, disable various options as they're not useful and impact performance * Address PR comments, fix unit tests
2023-10-11 16:21:45 +03:00
parent 81331aa9b2
commit 8069d36249
16 changed files with 149 additions and 45 deletions
--- a/ElementX.xcodeproj/project.pbxproj
+++ b/ElementX.xcodeproj/project.pbxproj
@@ -509,6 +509,7 @@
 		9A4E3D5AA44B041DAC3A0D81 /* OIDCAuthenticationPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 92390F9FA98255440A6BF5F8 /* OIDCAuthenticationPresenter.swift */; };
 		9AC5F8142413862A9E3A2D98 /* DeviceKit in Frameworks */ = {isa = PBXBuildFile; productRef = A7CA6F33C553805035C3B114 /* DeviceKit */; };
 		9AFEE46B03B7E995B3E1A53D /* WaitlistScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80C4927D09099497233E9980 /* WaitlistScreen.swift */; };
+		9B356742E035D90A8BB5CABE /* ProposedViewSize.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DFE0E493FB55E5A62E7852A /* ProposedViewSize.swift */; };
 		9B582B3EEFEA615D4A6FBF1A /* TimelineReactionsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 351E89CE2ED9B73C5CC47955 /* TimelineReactionsView.swift */; };
 		9B872FF37DBE6BE054903831 /* MediaUploadPreviewScreenViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = D54E12B98252F6C527E31FEE /* MediaUploadPreviewScreenViewModelProtocol.swift */; };
 		9BB91CABB10D8FE90C491BCD /* StaticLocationScreenViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C833673B334A0651AB46F30B /* StaticLocationScreenViewModelTests.swift */; };
@@ -1022,6 +1023,7 @@
 		1D56469A9EE0CFA2B7BA9760 /* SessionVerificationControllerProxyProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionVerificationControllerProxyProtocol.swift; sourceTree = "<group>"; };
 		1DB34B0C74CD242FED9DD069 /* LoginScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoginScreenUITests.swift; sourceTree = "<group>"; };
 		1DF8F7A3AD83D04C08D75E01 /* RoomDetailsEditScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomDetailsEditScreenViewModelProtocol.swift; sourceTree = "<group>"; };
+		1DFE0E493FB55E5A62E7852A /* ProposedViewSize.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProposedViewSize.swift; sourceTree = "<group>"; };
 		1E508AB0EDEE017FF4F6F8D1 /* DTHTMLElement+AttributedStringBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DTHTMLElement+AttributedStringBuilder.swift"; sourceTree = "<group>"; };
 		1F2529D434C750ED78ADF1ED /* UserAgentBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserAgentBuilder.swift; sourceTree = "<group>"; };
 		1F7C6DDBB5D12F6EF6A3D6E1 /* CollapsibleReactionLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CollapsibleReactionLayout.swift; sourceTree = "<group>"; };
@@ -2399,6 +2401,7 @@
 				F72EFC8C634469F9262659C7 /* NSItemProvider.swift */,
 				95BAC0F6C9644336E9567EE6 /* NSRegularExpresion.swift */,
 				D26813CCE39221FE30BF22CD /* PlatformViewVersionPredicate.swift */,
+				1DFE0E493FB55E5A62E7852A /* ProposedViewSize.swift */,
 				7310D8DFE01AF45F0689C3AA /* Publisher.swift */,
 				584A61D9C459FAFEF038A7C0 /* Section.swift */,
 				40B21E611DADDEF00307E7AC /* String.swift */,
@@ -5058,6 +5061,7 @@
 				153E22E8227F46545E5D681C /* PollRoomTimelineView.swift in Sources */,
 				DF504B10A4918F971A57BEF2 /* PostHogAnalyticsClient.swift in Sources */,
 				FD4DEC88210F35C35B2FB386 /* ProcessInfo.swift in Sources */,
+				9B356742E035D90A8BB5CABE /* ProposedViewSize.swift in Sources */,
 				2835FD52F3F618D07F799B3D /* Publisher.swift in Sources */,
 				9095B9E40DB5CF8BA26CE0D8 /* ReactionsSummaryView.swift in Sources */,
 				743790BF6A5B0577EA74AF14 /* ReadMarkerRoomTimelineItem.swift in Sources */,
@@ -6024,7 +6028,7 @@
 			repositoryURL = "https://github.com/getsentry/sentry-cocoa";
 			requirement = {
 				kind = upToNextMinorVersion;
-				minimumVersion = 8.6.0;
+				minimumVersion = 8.13.0;
 			};
 		};
 		AC3475112CA40C2C6E78D1EB /* XCRemoteSwiftPackageReference "matrix-analytics-events" */ = {
--- a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
+++ b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -183,8 +183,8 @@
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/getsentry/sentry-cocoa",
      "state" : {
-        "revision" : "e6dcfba32f2861438b82c7ad34e058b23c83daf6",
-        "version" : "8.6.0"
+        "revision" : "0a915b93ff3abee1a9752448e47808334d306980",
+        "version" : "8.13.0"
      }
    },
    {
--- a/ElementX/Sources/Other/Extensions/ProposedViewSize.swift
+++ b/ElementX/Sources/Other/Extensions/ProposedViewSize.swift
@@ -0,0 +1,24 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import SwiftUI
+
+extension ProposedViewSize: Hashable {
+    public func hash(into hasher: inout Hasher) {
+        hasher.combine(width)
+        hasher.combine(height)
+    }
+}
--- a/ElementX/Sources/Other/Pills/MessageText.swift
+++ b/ElementX/Sources/Other/Pills/MessageText.swift
@@ -49,7 +49,13 @@ final class MessageTextView: UITextView {
 struct MessageText: UIViewRepresentable {
    @Environment(\.openURL) private var openURLAction: OpenURLAction
    @EnvironmentObject private var viewModel: RoomScreenViewModel.Context
-    @State var attributedString: AttributedString
+    @State private var computedSizes = [Double: CGSize]()
+    
+    @State var attributedString: AttributedString {
+        didSet {
+            computedSizes.removeAll()
+        }
+    }

    func makeUIView(context: Context) -> MessageTextView {
        // Need to use TextKit 1 for mentions
@@ -81,13 +87,20 @@ struct MessageText: UIViewRepresentable {
        return textView
    }

-    func updateUIView(_ uiView: MessageTextView, context: Context) {
-        uiView.attributedText = NSAttributedString(attributedString)
-        context.coordinator.openURLAction = openURLAction
-    }
+    func updateUIView(_ uiView: MessageTextView, context: Context) { }

    func sizeThatFits(_ proposal: ProposedViewSize, uiView: MessageTextView, context: Context) -> CGSize? {
-        uiView.sizeThatFits(CGSize(width: proposal.width ?? UIView.layoutFittingExpandedSize.width, height: UIView.layoutFittingCompressedSize.height))
+        let proposalWidth = proposal.width ?? UIView.layoutFittingExpandedSize.width
+        
+        if let size = computedSizes[proposalWidth] {
+            return size
+        }
+        
+        let size = uiView.sizeThatFits(CGSize(width: proposalWidth, height: UIView.layoutFittingCompressedSize.height))
+        DispatchQueue.main.async {
+            computedSizes[proposalWidth] = size
+        }
+        return size
    }

    func makeCoordinator() -> Coordinator {
--- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineBubbleLayout.swift
+++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineBubbleLayout.swift
@@ -21,6 +21,10 @@ import SwiftUI
 /// A custom layout is required as the embedded quote bubbles should fill the entire width of
 /// the message bubble, without causing the width of the bubble to fill all of the available space.
 struct TimelineBubbleLayout: Layout {
+    struct Cache {
+        var sizes = [Int: [ProposedViewSize: CGSize]]()
+    }
+    
    /// The spacing between the components in the bubble.
    let spacing: CGFloat
    
@@ -36,13 +40,23 @@ struct TimelineBubbleLayout: Layout {
        static let regularText: Double = 1
    }
    
-    func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) -> CGSize {
+    func makeCache(subviews: Subviews) -> Cache {
+        Cache()
+    }
+    
+    func updateCache(_ cache: inout Cache, subviews: Subviews) {
+        // A subview changed, reset everything
+        cache = Cache()
+    }
+    
+    func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) -> CGSize {
        guard !subviews.isEmpty else { return .zero }
        
        // Calculate the natural size using the regular text and non-greedy quote bubbles.
        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
+
+        let subviewSizes = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache) }
        
-        let subviewSizes = layoutSubviews.map { $0.sizeThatFits(proposal) }
        let maxWidth = subviewSizes.map(\.width).reduce(0, max)
        let totalHeight = subviewSizes.map(\.height).reduce(0, +)
        let totalSpacing = CGFloat(layoutSubviews.count - 1) * spacing
@@ -50,16 +64,17 @@ struct TimelineBubbleLayout: Layout {
        return CGSize(width: maxWidth, height: totalHeight + totalSpacing)
    }
    
-    func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) {
+    func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) {
        guard !subviews.isEmpty else { return }
        
        // Calculate the width using the regular text and the non-greedy quote bubbles.
        let layoutSubviews = subviews.filter { $0.priority != Priority.visibleQuote }
-        let maxWidth = layoutSubviews.map { $0.sizeThatFits(proposal).width }.reduce(0, max)
+        let maxWidth = layoutSubviews.map { size(for: $0, subviews: subviews, proposedSize: proposal, cache: &cache).width }.reduce(0, max)
        
        // Place the regular text and greedy quote bubbles using the calculated width.
        let visibleSubviews = subviews.filter { $0.priority != Priority.hiddenQuote }
-        let subviewSizes = visibleSubviews.map { $0.sizeThatFits(ProposedViewSize(width: maxWidth, height: proposal.height)) }
+
+        let subviewSizes = visibleSubviews.map { size(for: $0, subviews: subviews, proposedSize: ProposedViewSize(width: maxWidth, height: proposal.height), cache: &cache) }
        
        var y = bounds.minY
        for index in visibleSubviews.indices {
@@ -70,4 +85,25 @@ struct TimelineBubbleLayout: Layout {
            y += height + spacing
        }
    }
+    
+    // MARK: - Private
+    
+    private func size(for subview: LayoutSubview, subviews: LayoutSubviews, proposedSize: ProposedViewSize, cache: inout Cache) -> CGSize {
+        guard let index = subviews.firstIndex(of: subview) else {
+            fatalError()
+        }
+        
+        if cache.sizes[index] == nil {
+            cache.sizes[index] = [:]
+        }
+        
+        if let cachedSize = cache.sizes[index]?[proposedSize] {
+            return cachedSize
+        }
+        
+        let size = subview.sizeThatFits(proposedSize)
+        
+        cache.sizes[index]?[proposedSize] = size
+        return size
+    }
 }
--- a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift
+++ b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift
@@ -32,9 +32,9 @@ struct TimelineReactionsView: View {
        guard isLayoutRTL else { return layoutDirection }
        return layoutDirection == .leftToRight ? .rightToLeft : .leftToRight
    }
-        
+    
    var body: some View {
-        CollapsibleReactionLayout(itemSpacing: 4, rowSpacing: 4, collapsed: collapsed, rowsBeforeCollapsible: 2) {
+        layout {
            ForEach(reactions, id: \.self) { reaction in
                TimelineReactionButton(reaction: reaction) { key in
                    feedbackGenerator.impactOccurred()
@@ -45,14 +45,18 @@ struct TimelineReactionsView: View {
                .reactionLayoutItem(.reaction)
                .environment(\.layoutDirection, layoutDirection)
            }
-            Button {
-                collapsed.toggle()
-            } label: {
-                TimelineCollapseButtonLabel(collapsed: collapsed)
-                    .transaction { $0.animation = nil }
+            
+            if isCollapsible {
+                Button {
+                    collapsed.toggle()
+                } label: {
+                    TimelineCollapseButtonLabel(collapsed: collapsed)
+                        .transaction { $0.animation = nil }
+                }
+                .reactionLayoutItem(.expandCollapse)
+                .environment(\.layoutDirection, layoutDirection)
            }
-            .reactionLayoutItem(.expandCollapse)
-            .environment(\.layoutDirection, layoutDirection)
+            
            Button {
                context.send(viewAction: .displayEmojiPicker(itemID: itemID))
            } label: {
@@ -64,6 +68,23 @@ struct TimelineReactionsView: View {
        .animation(.easeInOut(duration: 0.1).disabledDuringTests(), value: reactions)
        .padding(.leading, 4)
    }
+    
+    // MARK: - Private
+    
+    private var isCollapsible: Bool {
+        reactions.count > 5
+    }
+    
+    private var layout: AnyLayout {
+        if isCollapsible {
+            return AnyLayout(CollapsibleReactionLayout(itemSpacing: 4,
+                                                       rowSpacing: 4,
+                                                       collapsed: collapsed,
+                                                       rowsBeforeCollapsible: 2))
+        }
+        
+        return AnyLayout(HStackLayout(spacing: 4.0))
+    }
 }

 /// The pill shape for the label that surrounds both the reaction and collapse buttons.
@@ -159,10 +180,8 @@ struct TimelineReactionAddMoreButtonLabel: View {
            Image(asset: Asset.Images.addReaction)
                .resizable()
                .frame(width: addMoreButtonIconSize, height: addMoreButtonIconSize)
-                // Vertical sizing is done by the layout so that the add more button
-                // matches the height of the text based buttons.
-                .padding(.horizontal, 10)
-                .frame(maxHeight: .infinity, alignment: .center)
+                .padding(.vertical, 8)
+                .padding(.horizontal, 12)
                .foregroundColor(.compound.iconSecondary)
        }
    }
--- a/ElementX/Sources/Services/BugReport/BugReportService.swift
+++ b/ElementX/Sources/Services/BugReport/BugReportService.swift
@@ -69,6 +69,14 @@ class BugReportService: NSObject, BugReportServiceProtocol {
            
            options.dsn = self.sentryURL.absoluteString
            
+            // Sentry swizzling shows up quite often as the heaviest stack trace when profiling
+            // We don't need any of the features it powers (see docs)
+            options.enableSwizzling = false
+            
+            // WatchdogTermination is currently the top issue but we've had zero complaints
+            // so it might very well just all be false positives
+            options.enableWatchdogTerminationTracking = false
+            
            // Disabled as it seems to report a lot of false positives
            options.enableAppHangTracking = false
            
@@ -79,7 +87,7 @@ class BugReportService: NSObject, BugReportServiceProtocol {
            options.enableAutoBreadcrumbTracking = false
            
            // Experimental. Stitches stack traces of asynchronous code together
-            options.stitchAsyncCode = true
+            options.swiftAsyncStacktraces = true

            // Set tracesSampleRate to 1.0 to capture 100% of transactions for performance monitoring.
            // We recommend adjusting this value in production.
--- a/UnitTests/Snapshots/PreviewTests/test_roomScreen.1.png
+++ b/UnitTests/Snapshots/PreviewTests/test_roomScreen.1.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:2c6e64db92199ab4f6239a2ede5240f4bc22fb1f533e455d9787910c14270ec4
-size 313456
+oid sha256:eed926bb68ff4c51560219050a9c41941df446b386bef3201986a22cdcfe6d6f
+size 313571
--- a/UnitTests/Snapshots/PreviewTests/test_timelineItemBubbledStylerView.Mock-Timeline-RTL.png
+++ b/UnitTests/Snapshots/PreviewTests/test_timelineItemBubbledStylerView.Mock-Timeline-RTL.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:19348f5dc147cbd87d1fbcdb9f228f18e00b29e6f6bf0a2d4c95a19e12424655
-size 341025
+oid sha256:158acc42dea8232a7b8a24f373198f42ad00eb507c37cd7749344b06cdb7393f
+size 340751
--- a/UnitTests/Snapshots/PreviewTests/test_timelineItemBubbledStylerView.Mock-Timeline-with-read-receipts.png
+++ b/UnitTests/Snapshots/PreviewTests/test_timelineItemBubbledStylerView.Mock-Timeline-with-read-receipts.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:1241ae3a54ad3f598748c86df59853a96b6266135816c5c3213b9b45f38b3272
-size 337109
+oid sha256:a48e1672bc55f0d0c2b5f2345554e5e491f8dada688374accb9e4a63597290e3
+size 337134
--- a/UnitTests/Snapshots/PreviewTests/test_timelineItemBubbledStylerView.Mock-Timeline.png
+++ b/UnitTests/Snapshots/PreviewTests/test_timelineItemBubbledStylerView.Mock-Timeline.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:e6be01fcd027b50ca4b6eabf43c82c3530e23a939b22bd5279646d7a935f6989
-size 339436
+oid sha256:850458aa89e4cadabe9bde5b89d131c1eed38077ee1fd9e04f82aa50cf7504b2
+size 339376
--- a/UnitTests/Snapshots/PreviewTests/test_timelineItemPlainStylerView.1.png
+++ b/UnitTests/Snapshots/PreviewTests/test_timelineItemPlainStylerView.1.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:5ed2fe6d8dbec14cc7f97d0ede2b86d0c1dbbbc559cf95a9fa2fdbbdbe8bd785
-size 378187
+oid sha256:c263006ebed5bbccef51df5439d62a377abc91f6882c1f38d8f63524a55fc968
+size 371053
--- a/UnitTests/Snapshots/PreviewTests/test_timelineReactionView.1.png
+++ b/UnitTests/Snapshots/PreviewTests/test_timelineReactionView.1.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:460d1bd367245319a632b500f2f2ce24733e4070d1252f343ab3a039ede0222d
-size 288518
+oid sha256:1cb552e4238b94f765b9e27242b64daa0517a40f79bd3ef642b3c3a167e47725
+size 281715
--- a/UnitTests/Snapshots/PreviewTests/test_timelineView.1.png
+++ b/UnitTests/Snapshots/PreviewTests/test_timelineView.1.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:2c6e64db92199ab4f6239a2ede5240f4bc22fb1f533e455d9787910c14270ec4
-size 313456
+oid sha256:eed926bb68ff4c51560219050a9c41941df446b386bef3201986a22cdcfe6d6f
+size 313571
--- a/UnitTests/Snapshots/PreviewTests/test_uITimelineView.1.png
+++ b/UnitTests/Snapshots/PreviewTests/test_uITimelineView.1.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:2c6e64db92199ab4f6239a2ede5240f4bc22fb1f533e455d9787910c14270ec4
-size 313456
+oid sha256:eed926bb68ff4c51560219050a9c41941df446b386bef3201986a22cdcfe6d6f
+size 313571
--- a/project.yml
+++ b/project.yml
@@ -100,7 +100,7 @@ packages:
    minorVersion: 1.5.0
  Sentry:
    url: https://github.com/getsentry/sentry-cocoa
-    minorVersion: 8.6.0
+    minorVersion: 8.13.0
  SnapshotTesting:
    url: https://github.com/pointfreeco/swift-snapshot-testing
    minorVersion: 1.13.0