From 40e3f4ee7232100d2abc0bce8cc411e926b7b2d6 Mon Sep 17 00:00:00 2001 From: Gleb Tarasov Date: Mon, 3 Jun 2024 12:05:56 +0200 Subject: [PATCH 1/3] Add experiment to create view controller observers on the background queue It will reduce precision of measurements, but will decrease performance impact from the library on the overall app performance. --- .../Sources/LoggingObserver.swift | 20 ++++-- .../Sources/PerformanceMonitoring.swift | 8 ++- .../Sources/ScreenMetricsReceiver.swift | 7 +- .../Sources/ViewControllerObserver.swift | 69 ++++++++++++++++--- .../Tests/LoggingObserverTests.swift | 13 +++- .../Tests/PerformanceMonitoringTests.swift | 8 +++ .../Tests/ViewControllerObserverTests.swift | 13 +++- 7 files changed, 118 insertions(+), 20 deletions(-) diff --git a/PerformanceSuite/Sources/LoggingObserver.swift b/PerformanceSuite/Sources/LoggingObserver.swift index b08edf4..f602bed 100644 --- a/PerformanceSuite/Sources/LoggingObserver.swift +++ b/PerformanceSuite/Sources/LoggingObserver.swift @@ -91,11 +91,23 @@ final class LoggingObserver: 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 { diff --git a/PerformanceSuite/Sources/PerformanceMonitoring.swift b/PerformanceSuite/Sources/PerformanceMonitoring.swift index 5c4ccce..c8f061c 100644 --- a/PerformanceSuite/Sources/PerformanceMonitoring.swift +++ b/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 { diff --git a/PerformanceSuite/Sources/ScreenMetricsReceiver.swift b/PerformanceSuite/Sources/ScreenMetricsReceiver.swift index 779aaa7..deafa28 100644 --- a/PerformanceSuite/Sources/ScreenMetricsReceiver.swift +++ b/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? } diff --git a/PerformanceSuite/Sources/ViewControllerObserver.swift b/PerformanceSuite/Sources/ViewControllerObserver.swift index 66e42f7..df22198 100644 --- a/PerformanceSuite/Sources/ViewControllerObserver.swift +++ b/PerformanceSuite/Sources/ViewControllerObserver.swift @@ -34,7 +34,11 @@ final class ViewControllerObserverFactory 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 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 diff --git a/PerformanceSuite/Tests/LoggingObserverTests.swift b/PerformanceSuite/Tests/LoggingObserverTests.swift index 73a6e61..6aefe2d 100644 --- a/PerformanceSuite/Tests/LoggingObserverTests.swift +++ b/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", diff --git a/PerformanceSuite/Tests/PerformanceMonitoringTests.swift b/PerformanceSuite/Tests/PerformanceMonitoringTests.swift index a26430f..ecbf432 100644 --- a/PerformanceSuite/Tests/PerformanceMonitoringTests.swift +++ b/PerformanceSuite/Tests/PerformanceMonitoringTests.swift @@ -31,6 +31,14 @@ final class PerformanceMonitoringTests: XCTestCase { let vc = UIViewController() wait(for: [exp], timeout: 20) // increase timeout as it is very slow on CI _ = vc + + _ = 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") diff --git a/PerformanceSuite/Tests/ViewControllerObserverTests.swift b/PerformanceSuite/Tests/ViewControllerObserverTests.swift index f4c17b8..958c21a 100644 --- a/PerformanceSuite/Tests/ViewControllerObserverTests.swift +++ b/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,8 +92,9 @@ 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() @@ -99,6 +102,8 @@ class ViewControllerObserverTests: XCTestCase { let vc2 = UIViewController() factory.afterViewDidAppear(viewController: vc2) + PerformanceMonitoring.queue.sync { } + XCTAssertNotNil(lastObserverCreated) XCTAssert(lastObserverCreated !== observer) XCTAssertEqual(lastObserverCreated?.lastMethod, .afterViewDidAppear) @@ -106,6 +111,8 @@ class ViewControllerObserverTests: XCTestCase { 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) From 3fefa217902edab93b8f2ab6309941f6ef2aa0f0 Mon Sep 17 00:00:00 2001 From: Gleb Tarasov Date: Sat, 8 Jun 2024 11:22:42 +0200 Subject: [PATCH 2/3] Increase hang threshold on CI as CI machines are slow --- PerformanceSuite/PerformanceApp/IssuesSimulator.swift | 2 +- PerformanceSuite/PerformanceApp/MetricsConsumer.swift | 4 ++++ PerformanceSuite/UITests/TerminationTests.swift | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/PerformanceSuite/PerformanceApp/IssuesSimulator.swift b/PerformanceSuite/PerformanceApp/IssuesSimulator.swift index dfa534b..f771081 100644 --- a/PerformanceSuite/PerformanceApp/IssuesSimulator.swift +++ b/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) } } diff --git a/PerformanceSuite/PerformanceApp/MetricsConsumer.swift b/PerformanceSuite/PerformanceApp/MetricsConsumer.swift index 53b6fc8..b497b4e 100644 --- a/PerformanceSuite/PerformanceApp/MetricsConsumer.swift +++ b/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)") } diff --git a/PerformanceSuite/UITests/TerminationTests.swift b/PerformanceSuite/UITests/TerminationTests.swift index 2ce9415..eb2c33d 100644 --- a/PerformanceSuite/UITests/TerminationTests.swift +++ b/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) From b7ad18aba3dc7e1715af469cd693fa8d5e8feb79 Mon Sep 17 00:00:00 2001 From: Gleb Tarasov Date: Sat, 8 Jun 2024 11:37:35 +0200 Subject: [PATCH 3/3] Reduce amount of logs in UITests --- .github/workflows/tests.yml | 2 +- .../Tests/PerformanceMonitoringTests.swift | 13 ++++++++ PerformanceSuite/Tests/UnitTests.xctestplan | 26 ++++++++++++++++ PerformanceSuite/UITests/UITests.xctestplan | 30 +++++++++++++++++++ Project.xcworkspace/contents.xcworkspacedata | 6 ++++ .../xcshareddata/xcschemes/UITests.xcscheme | 9 ++++-- .../xcshareddata/xcschemes/UnitTests.xcscheme | 9 ++++-- 7 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 PerformanceSuite/Tests/UnitTests.xctestplan create mode 100644 PerformanceSuite/UITests/UITests.xctestplan diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 14e168c..8d6b798 100644 --- a/.github/workflows/tests.yml +++ b/.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: diff --git a/PerformanceSuite/Tests/PerformanceMonitoringTests.swift b/PerformanceSuite/Tests/PerformanceMonitoringTests.swift index ecbf432..ad65785 100644 --- a/PerformanceSuite/Tests/PerformanceMonitoringTests.swift +++ b/PerformanceSuite/Tests/PerformanceMonitoringTests.swift @@ -15,6 +15,7 @@ final class PerformanceMonitoringTests: XCTestCase { continueAfterFailure = false try PerformanceMonitoring.disable() StartupTimeReporter.forgetMainStartedForTests() + AppInfoHolder.resetForTests() } override func tearDown() { @@ -32,6 +33,8 @@ final class PerformanceMonitoringTests: XCTestCase { 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() @@ -63,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? } diff --git a/PerformanceSuite/Tests/UnitTests.xctestplan b/PerformanceSuite/Tests/UnitTests.xctestplan new file mode 100644 index 0000000..3912d48 --- /dev/null +++ b/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 +} diff --git a/PerformanceSuite/UITests/UITests.xctestplan b/PerformanceSuite/UITests/UITests.xctestplan new file mode 100644 index 0000000..77dc9de --- /dev/null +++ b/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 +} diff --git a/Project.xcworkspace/contents.xcworkspacedata b/Project.xcworkspace/contents.xcworkspacedata index cf572cd..6982feb 100644 --- a/Project.xcworkspace/contents.xcworkspacedata +++ b/Project.xcworkspace/contents.xcworkspacedata @@ -7,4 +7,10 @@ + + + + diff --git a/Project.xcworkspace/xcshareddata/xcschemes/UITests.xcscheme b/Project.xcworkspace/xcshareddata/xcschemes/UITests.xcscheme index f77a742..2317598 100644 --- a/Project.xcworkspace/xcshareddata/xcschemes/UITests.xcscheme +++ b/Project.xcworkspace/xcshareddata/xcschemes/UITests.xcscheme @@ -10,8 +10,13 @@ buildConfiguration = "Release" selectedDebuggerIdentifier = "" selectedLauncherIdentifier = "Xcode.IDEFoundation.Launcher.PosixSpawn" - shouldUseLaunchSchemeArgsEnv = "YES" - shouldAutocreateTestPlan = "YES"> + shouldUseLaunchSchemeArgsEnv = "YES"> + + + + diff --git a/Project.xcworkspace/xcshareddata/xcschemes/UnitTests.xcscheme b/Project.xcworkspace/xcshareddata/xcschemes/UnitTests.xcscheme index 388a3a7..d0d9b71 100644 --- a/Project.xcworkspace/xcshareddata/xcschemes/UnitTests.xcscheme +++ b/Project.xcworkspace/xcshareddata/xcschemes/UnitTests.xcscheme @@ -26,8 +26,13 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - shouldUseLaunchSchemeArgsEnv = "YES" - shouldAutocreateTestPlan = "YES"> + shouldUseLaunchSchemeArgsEnv = "YES"> + + + +