Merge pull request #11 from bookingcom/gtarasov/background_identifiers

Experiment to create view controller observers on the background

.github/workflows/tests.yml

@@ -59,7 +59,7 @@ jobs:
           destination: platform=iOS Simulator,name=iPhone 15 Pro,OS=17.5
           workspace: Project.xcworkspace
         run: |
-          xcodebuild test -scheme "$scheme" -workspace "$workspace" -destination "$destination" -test-iterations 3 -run-tests-until-failure -enableCodeCoverage YES -derivedDataPath DerivedData
+          xcodebuild test -scheme "$scheme" -workspace "$workspace" -destination "$destination" -derivedDataPath DerivedData
       
       - name: Slather
         env:

PerformanceSuite/PerformanceApp/IssuesSimulator.swift

@@ -10,7 +10,7 @@ import Foundation
 class IssuesSimulator {
     static func simulateNonFatalHang() {
         DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(500)) {
-            Thread.sleep(forTimeInterval: 4.5)
+            Thread.sleep(forTimeInterval: 6)
         }
     }
 

PerformanceSuite/PerformanceApp/MetricsConsumer.swift

@@ -74,6 +74,10 @@ class MetricsConsumer: PerformanceSuiteMetricsReceiver {
         interop?.send(message: Message.hangStarted)
     }
 
+    var hangThreshold: TimeInterval {
+        return 3
+    }
+
     private func log(_ message: String) {
         logger.info("\(message, privacy: .public)")
     }

PerformanceSuite/Sources/LoggingObserver.swift

@@ -91,11 +91,23 @@ final class LoggingObserver<V: ViewControllerLoggingReceiver>: ViewControllerObs
     // MARK: - Top screen detection
 
     private func rememberOpenedScreenIfNeeded(_ viewController: UIViewController) {
-        guard isTopScreen(viewController) else {
-            return
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            DispatchQueue.main.async {
+                guard self.isTopScreen(viewController) else {
+                    return
+                }
+                PerformanceMonitoring.queue.async {
+                    let description = RootViewIntrospection.shared.description(viewController: viewController)
+                    AppInfoHolder.screenOpened(description)
+                }
+            }
+        } else {
+            guard isTopScreen(viewController) else {
+                return
+            }
+            let description = RootViewIntrospection.shared.description(viewController: viewController)
+            AppInfoHolder.screenOpened(description)
         }
-        let description = RootViewIntrospection.shared.description(viewController: viewController)
-        AppInfoHolder.screenOpened(description)
     }
 
     private func isTopScreen(_ viewController: UIViewController) -> Bool {

PerformanceSuite/Sources/PerformanceMonitoring.swift

@@ -10,7 +10,13 @@ import UIKit
 protocol AppMetricsReporter: AnyObject {}
 
 public struct Experiments {
-    public init() { }
+    public init(observersOnBackgroundQueue: Bool = false) {
+        self.observersOnBackgroundQueue = observersOnBackgroundQueue
+    }
+
+
+    /// Experiment to try to create view controller observers on the PerformanceMonitoring.queue
+    let observersOnBackgroundQueue: Bool
 }
 
 public enum PerformanceMonitoring {

PerformanceSuite/Sources/ScreenMetricsReceiver.swift

@@ -14,12 +14,15 @@ public protocol ScreenMetricsReceiver: AnyObject {
 
     /// Method converts `UIViewController` instance to `ScreenIdentifier`. It can be enum or String, which identifies your screen.
     /// Return `nil` if we shouldn't track metrics for this `UIViewController`.
+    /// This method should be as effective as possible. Slow implementation may harm app performance.
     ///
     /// This method is called on the main thread only once, during `UIViewController` initialization.
+    /// If experiment `observersOnBackgroundQueue` is turned on, this method is called on the background internal queue `PerformanceMonitoring.queue`.
+    /// Slow implementation may harm overall performance and also can affect the precision of the measurements.
     ///
-    /// Default implementation will return nil for view controllers that are not from the main bundle and return UIViewController itself for others
+    /// Default implementation will return nil for view controllers that are not from the main bundle and return `UIViewController` itself for others
     ///
-    /// - Parameter viewController: UIViewController which is being tracked
+    /// - Parameter viewController: `UIViewController` which is being tracked
     func screenIdentifier(for viewController: UIViewController) -> ScreenIdentifier?
 }
 

PerformanceSuite/Sources/ViewControllerObserver.swift

@@ -34,7 +34,11 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
     private let observerMaker: (S.ScreenIdentifier) -> T
 
     private func observer(for viewController: UIViewController) -> T? {
-        precondition(Thread.isMainThread)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            dispatchPrecondition(condition: .onQueue(PerformanceMonitoring.queue))
+        } else {
+            precondition(Thread.isMainThread)
+        }
 
         if let observer = ViewControllerObserverFactoryHelper.existingObserver(for: viewController, identifier: T.identifier) as? T {
             return observer
@@ -51,27 +55,63 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
     }
 
     func beforeInit(viewController: UIViewController) {
-        observer(for: viewController)?.beforeInit(viewController: viewController)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            PerformanceMonitoring.queue.async {
+                self.observer(for: viewController)?.beforeInit(viewController: viewController)
+            }
+        } else {
+            observer(for: viewController)?.beforeInit(viewController: viewController)
+        }
     }
 
     func beforeViewDidLoad(viewController: UIViewController) {
-        observer(for: viewController)?.beforeViewDidLoad(viewController: viewController)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            PerformanceMonitoring.queue.async {
+                self.observer(for: viewController)?.beforeViewDidLoad(viewController: viewController)
+            }
+        } else {
+            observer(for: viewController)?.beforeViewDidLoad(viewController: viewController)
+        }
     }
 
     func afterViewDidAppear(viewController: UIViewController) {
-        observer(for: viewController)?.afterViewDidAppear(viewController: viewController)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            PerformanceMonitoring.queue.async {
+                self.observer(for: viewController)?.afterViewDidAppear(viewController: viewController)
+            }
+        } else {
+            observer(for: viewController)?.afterViewDidAppear(viewController: viewController)
+        }
     }
 
     func beforeViewWillDisappear(viewController: UIViewController) {
-        observer(for: viewController)?.beforeViewWillDisappear(viewController: viewController)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            PerformanceMonitoring.queue.async {
+                self.observer(for: viewController)?.beforeViewWillDisappear(viewController: viewController)
+            }
+        } else {
+            observer(for: viewController)?.beforeViewWillDisappear(viewController: viewController)
+        }
     }
 
     func afterViewWillAppear(viewController: UIViewController) {
-        observer(for: viewController)?.afterViewWillAppear(viewController: viewController)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            PerformanceMonitoring.queue.async {
+                self.observer(for: viewController)?.afterViewWillAppear(viewController: viewController)
+            }
+        } else {
+            observer(for: viewController)?.afterViewWillAppear(viewController: viewController)
+        }
     }
 
     func beforeViewDidDisappear(viewController: UIViewController) {
-        observer(for: viewController)?.beforeViewDidDisappear(viewController: viewController)
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            PerformanceMonitoring.queue.async {
+                self.observer(for: viewController)?.beforeViewDidDisappear(viewController: viewController)
+            }
+        } else {
+            observer(for: viewController)?.beforeViewDidDisappear(viewController: viewController)
+        }
     }
 
     static var identifier: AnyObject {
@@ -134,13 +174,20 @@ class ViewControllerObserverCollection: ViewControllerObserver {
 /// Non-generic helper for generic `ViewControllerObserverFactory`. To put all the static methods and vars there.
 final class ViewControllerObserverFactoryHelper {
     static func existingObserver(for viewController: UIViewController, identifier: AnyObject) -> Any? {
-        var vc: UIViewController? = viewController
-        while let current = vc {
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
             let tPointer = unsafeBitCast(identifier, to: UnsafeRawPointer.self)
-            if let result = objc_getAssociatedObject(current, tPointer) {
+            if let result = objc_getAssociatedObject(viewController, tPointer) {
                 return result
             }
-            vc = current.parent
+        } else {
+            var vc: UIViewController? = viewController
+            while let current = vc {
+                let tPointer = unsafeBitCast(identifier, to: UnsafeRawPointer.self)
+                if let result = objc_getAssociatedObject(current, tPointer) {
+                    return result
+                }
+                vc = current.parent
+            }
         }
 
         return nil

PerformanceSuite/Tests/LoggingObserverTests.swift

@@ -71,7 +71,7 @@ final class LoggingObserverTests: XCTestCase {
             MyViewController3(rootView: MyView3()), // take
 
         ]
-        let observers = vcs.compactMap {
+        _ = vcs.compactMap {
             if let screen = stub.screenIdentifier(for: $0) {
                 let o = LoggingObserver(screen: screen, receiver: stub)
                 o.afterViewDidAppear(viewController: $0)
@@ -82,6 +82,17 @@ final class LoggingObserverTests: XCTestCase {
 
         }
 
+        let exp = expectation(description: "openedScreens")
+
+        DispatchQueue.global().async {
+            while (AppInfoHolder.appRuntimeInfo.openedScreens.count < 3) {
+                Thread.sleep(forTimeInterval: 0.001)
+            }
+            exp.fulfill()
+        }
+
+        wait(for: [exp], timeout: 5)
+
         XCTAssertEqual(AppInfoHolder.appRuntimeInfo.openedScreens, [
             "MyViewForLoggingObserverTests",
             "MyViewController1",

PerformanceSuite/Tests/PerformanceMonitoringTests.swift

@@ -15,6 +15,7 @@ final class PerformanceMonitoringTests: XCTestCase {
         continueAfterFailure = false
         try PerformanceMonitoring.disable()
         StartupTimeReporter.forgetMainStartedForTests()
+        AppInfoHolder.resetForTests()
     }
 
     override func tearDown() {
@@ -31,6 +32,16 @@ final class PerformanceMonitoringTests: XCTestCase {
         let vc = UIViewController()
         wait(for: [exp], timeout: 20) // increase timeout as it is very slow on CI
         _ = vc
+
+        // simulate vc appearance to generate more performance events
+        // checking there are no crashes
+        _ = vc.view
+        vc.beginAppearanceTransition(true, animated: false)
+        vc.endAppearanceTransition()
+        vc.beginAppearanceTransition(false, animated: false)
+        vc.endAppearanceTransition()
+        PerformanceMonitoring.queue.sync { }
+
         try PerformanceMonitoring.disable()
 
         let exp2 = expectation(description: "onInit2")
@@ -55,6 +66,16 @@ final class PerformanceMonitoringTests: XCTestCase {
         setenv("ActivePrewarm", "", 1)
     }
 
+    func testNoPrewarming() throws {
+        setenv("ActivePrewarm", "", 1)
+        PerformanceMonitoring.onMainStarted()
+        try PerformanceMonitoring.enable(config: .all(receiver: self))
+
+        XCTAssertFalse(PerformanceMonitoring.appStartInfo.appStartedWithPrewarming)
+
+        try PerformanceMonitoring.disable()
+    }
+
     private var onInitExpectation: XCTestExpectation?
 }
 

PerformanceSuite/Tests/UnitTests.xctestplan

@@ -0,0 +1,26 @@
+{
+  "configurations" : [
+    {
+      "id" : "ABDBB9DB-889D-4707-B550-881A8D2FE47B",
+      "name" : "Test Scheme Action",
+      "options" : {
+
+      }
+    }
+  ],
+  "defaultOptions" : {
+    "maximumTestRepetitions" : 3,
+    "testRepetitionMode" : "untilFailure",
+    "userAttachmentLifetime" : "keepAlways"
+  },
+  "testTargets" : [
+    {
+      "target" : {
+        "containerPath" : "container:Pods\/Pods.xcodeproj",
+        "identifier" : "E7B17CF293A0585BA47ADEED225E1659",
+        "name" : "PerformanceSuite-Unit-Tests"
+      }
+    }
+  ],
+  "version" : 1
+}

PerformanceSuite/Tests/ViewControllerObserverTests.swift

@@ -81,6 +81,8 @@ class ViewControllerObserverTests: XCTestCase {
 
         let vc1 = UIViewController()
         factory.beforeInit(viewController: vc1)
+        PerformanceMonitoring.queue.sync { }
+
         let observer = lastObserverCreated
         XCTAssertNotNil(observer)
         XCTAssertEqual(observer?.lastMethod, .beforeInit)
@@ -90,22 +92,27 @@ class ViewControllerObserverTests: XCTestCase {
         lastObserverCreated = nil
 
         factory.afterViewDidAppear(viewController: vc1)
-        XCTAssertNil(lastObserverCreated)
+        PerformanceMonitoring.queue.sync { }
 
+        XCTAssertNil(lastObserverCreated)
         XCTAssertEqual(observer?.lastMethod, .afterViewDidAppear)
         XCTAssertEqual(observer?.viewController, vc1)
         observer?.clear()
 
 
         let vc2 = UIViewController()
         factory.afterViewDidAppear(viewController: vc2)
+        PerformanceMonitoring.queue.sync { }
+
         XCTAssertNotNil(lastObserverCreated)
         XCTAssert(lastObserverCreated !== observer)
         XCTAssertEqual(lastObserverCreated?.lastMethod, .afterViewDidAppear)
         XCTAssertEqual(lastObserverCreated?.viewController, vc2)
         lastObserverCreated?.clear()
 
         factory.beforeViewWillDisappear(viewController: vc2)
+        PerformanceMonitoring.queue.sync { }
+
         XCTAssertNotNil(lastObserverCreated)
         let observer2 = lastObserverCreated
         XCTAssertEqual(observer2?.lastMethod, .beforeViewWillDisappear)
@@ -114,11 +121,15 @@ class ViewControllerObserverTests: XCTestCase {
         lastObserverCreated = nil
 
         factory.afterViewWillAppear(viewController: vc1)
+        PerformanceMonitoring.queue.sync { }
+
         XCTAssertNil(lastObserverCreated)
         XCTAssertEqual(observer?.lastMethod, .afterViewWillAppear)
         XCTAssertEqual(observer?.viewController, vc1)
 
         factory.beforeViewDidDisappear(viewController: vc2)
+        PerformanceMonitoring.queue.sync { }
+
         XCTAssertNil(lastObserverCreated)
         XCTAssertEqual(observer2?.lastMethod, .beforeViewDidDisappear)
         XCTAssertEqual(observer2?.viewController, vc2)

PerformanceSuite/UITests/TerminationTests.swift

@@ -46,7 +46,7 @@ final class TerminationTests: BaseTests {
         performFirstLaunch()
         assertNoMessages(.hangStarted, .nonFatalHang)
         app.staticTexts["Non-fatal hang"].tap()
-        waitForTimeout(4)
+        waitForTimeout(5)
         waitForMessage { $0 == .nonFatalHang }
 
         assertHasMessages(.hangStarted, .nonFatalHang)

PerformanceSuite/UITests/UITests.xctestplan

@@ -0,0 +1,30 @@
+{
+  "configurations" : [
+    {
+      "id" : "21225ED1-5F89-43DE-9076-0D7ED1174387",
+      "name" : "Test Scheme Action",
+      "options" : {
+
+      }
+    }
+  ],
+  "defaultOptions" : {
+    "environmentVariableEntries" : [
+      {
+        "key" : "OS_ACTIVITY_MODE",
+        "value" : "disable"
+      }
+    ],
+    "testRepetitionMode" : "retryOnFailure"
+  },
+  "testTargets" : [
+    {
+      "target" : {
+        "containerPath" : "container:Pods\/Pods.xcodeproj",
+        "identifier" : "B65E438620CC2DA1C6452F8C936D2E6C",
+        "name" : "PerformanceSuite-UI-UITests"
+      }
+    }
+  ],
+  "version" : 1
+}