From 67f4237190548caf712c8f41654b0ef1f82de96a Mon Sep 17 00:00:00 2001 From: Mert Buran Date: Thu, 17 Jun 2021 18:52:56 +0200 Subject: [PATCH 1/4] RUMM-1278 VitalRefreshRateReader is added --- Datadog/Datadog.xcodeproj/project.pbxproj | 8 ++ .../Datadog/RUM/RUMVitals/VitalObserver.swift | 2 +- .../RUMVitals/VitalRefreshRateReader.swift | 112 ++++++++++++++++++ .../VitalRefreshRateReaderTests.swift | 104 ++++++++++++++++ 4 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift create mode 100644 Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift diff --git a/Datadog/Datadog.xcodeproj/project.pbxproj b/Datadog/Datadog.xcodeproj/project.pbxproj index 719ca43dcd..9c9a2de093 100644 --- a/Datadog/Datadog.xcodeproj/project.pbxproj +++ b/Datadog/Datadog.xcodeproj/project.pbxproj @@ -461,7 +461,9 @@ 9E58E8E324615EDA008E5063 /* JSONEncoderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E58E8E224615EDA008E5063 /* JSONEncoderTests.swift */; }; 9E68FB55244707FD0013A8AA /* ObjcExceptionHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 9E68FB53244707FD0013A8AA /* ObjcExceptionHandler.m */; }; 9E68FB56244707FD0013A8AA /* ObjcExceptionHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = 9E68FB54244707FD0013A8AA /* ObjcExceptionHandler.h */; settings = {ATTRIBUTES = (Public, ); }; }; + 9E986C302677B91400D62490 /* VitalRefreshRateReaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E986C2F2677B91400D62490 /* VitalRefreshRateReaderTests.swift */; }; 9E989A4225F640D100235FC3 /* AppStateListenerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E989A4125F640D100235FC3 /* AppStateListenerTests.swift */; }; + 9EA3CA6926775A3500B16871 /* VitalRefreshRateReader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EA3CA6826775A3500B16871 /* VitalRefreshRateReader.swift */; }; 9EC8B5DA2668197B000F7529 /* VitalCPUReader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EC8B5D92668197B000F7529 /* VitalCPUReader.swift */; }; 9EC8B5EE2668E4DB000F7529 /* VitalCPUReaderTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EC8B5ED2668E4DB000F7529 /* VitalCPUReaderTest.swift */; }; 9ED6A6B425F2901800CB2E29 /* AppStateListener.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9ED6A6B325F2901800CB2E29 /* AppStateListener.swift */; }; @@ -1074,8 +1076,10 @@ 9E58E8E224615EDA008E5063 /* JSONEncoderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JSONEncoderTests.swift; sourceTree = ""; }; 9E68FB53244707FD0013A8AA /* ObjcExceptionHandler.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ObjcExceptionHandler.m; sourceTree = ""; }; 9E68FB54244707FD0013A8AA /* ObjcExceptionHandler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ObjcExceptionHandler.h; sourceTree = ""; }; + 9E986C2F2677B91400D62490 /* VitalRefreshRateReaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VitalRefreshRateReaderTests.swift; sourceTree = ""; }; 9E989A4125F640D100235FC3 /* AppStateListenerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppStateListenerTests.swift; sourceTree = ""; }; 9E9EB37624468CE90002C80B /* Datadog.modulemap */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.module-map"; path = Datadog.modulemap; sourceTree = ""; }; + 9EA3CA6826775A3500B16871 /* VitalRefreshRateReader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VitalRefreshRateReader.swift; sourceTree = ""; }; 9EC8B5D92668197B000F7529 /* VitalCPUReader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VitalCPUReader.swift; sourceTree = ""; }; 9EC8B5ED2668E4DB000F7529 /* VitalCPUReaderTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VitalCPUReaderTest.swift; sourceTree = ""; }; 9ED6A6B325F2901800CB2E29 /* AppStateListener.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppStateListener.swift; sourceTree = ""; }; @@ -3070,6 +3074,7 @@ B3FC3C0426526EE900DEED9E /* RUMVitals */ = { isa = PBXGroup; children = ( + 9EA3CA6826775A3500B16871 /* VitalRefreshRateReader.swift */, 9EC8B5D92668197B000F7529 /* VitalCPUReader.swift */, B3BBBCB0265E71C600943419 /* VitalMemoryReader.swift */, B3BBBCB1265E71C600943419 /* VitalReader.swift */, @@ -3086,6 +3091,7 @@ 9EC8B5ED2668E4DB000F7529 /* VitalCPUReaderTest.swift */, B3BBBCBB265E71D100943419 /* VitalMemoryReaderTest.swift */, B3FC3C3B2653A97700DEED9E /* VitalObserverTest.swift */, + 9E986C2F2677B91400D62490 /* VitalRefreshRateReaderTests.swift */, ); path = RUMVitals; sourceTree = ""; @@ -3783,6 +3789,7 @@ 614E9EB3244719FA007EE3E1 /* BundleType.swift in Sources */, 61F3CDA72512144600C816E5 /* UIKitRUMViewsPredicate.swift in Sources */, 61133BCE2423979B00786299 /* BatteryStatusProvider.swift in Sources */, + 9EA3CA6926775A3500B16871 /* VitalRefreshRateReader.swift in Sources */, E13A880C257922EC004FB174 /* EnvironmentSpanIntegration.swift in Sources */, 61B038602527247200518F3C /* URLSessionTracingHandler.swift in Sources */, 61122ED425B1B84D00F9C7F5 /* RUMEventSanitizer.swift in Sources */, @@ -3834,6 +3841,7 @@ 61411B1024EC15AC0012EAB2 /* Casting+RUM.swift in Sources */, 61133C622423990D00786299 /* InternalLoggersTests.swift in Sources */, 61FF283024BC5E2D000B3D9B /* RUMEventFileOutputTests.swift in Sources */, + 9E986C302677B91400D62490 /* VitalRefreshRateReaderTests.swift in Sources */, 61133C582423990D00786299 /* FileWriterTests.swift in Sources */, 61E917D3246546BF00E6C631 /* TracerConfigurationTests.swift in Sources */, 61C5A89D24509C1100DA608C /* DDSpanTests.swift in Sources */, diff --git a/Sources/Datadog/RUM/RUMVitals/VitalObserver.swift b/Sources/Datadog/RUM/RUMVitals/VitalObserver.swift index e48991c457..c0dd2d60e0 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalObserver.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalObserver.swift @@ -10,7 +10,7 @@ import Foundation internal class VitalObserver: ValueObserver { let listener: VitalListener - private var vitalInfo = VitalInfo( + private(set) var vitalInfo = VitalInfo( sampleCount: 0, minValue: Double.greatestFiniteMagnitude, maxValue: -Double.greatestFiniteMagnitude, diff --git a/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift new file mode 100644 index 0000000000..62375fba46 --- /dev/null +++ b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift @@ -0,0 +1,112 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2019-2020 Datadog, Inc. + */ + +import Foundation +import UIKit + +/// A class reading the refresh rate (frames per second) of the main screen +internal class VitalRefreshRateReader { + private var observers = [VitalObserver]() + private var displayLink: CADisplayLink? + private var lastFrameTimestamp: CFTimeInterval? + private(set) var isRunning = false + + init(notificationCenter: NotificationCenter = .default) { + notificationCenter.addObserver( + self, + selector: #selector(appWillResignActive), + name: UIApplication.willResignActiveNotification, + object: nil + ) + notificationCenter.addObserver( + self, + selector: #selector(appDidBecomeActive), + name: UIApplication.didBecomeActiveNotification, + object: nil + ) + } + + deinit { + stop() + } + + /// `VitalRefreshRateReader` keeps pushing data to its `observers` at every new frame. + /// - Parameter observer: receiver of refresh rate per frame. + func register(_ observer: VitalObserver) { + DispatchQueue.main.async { + self.observers.append(observer) + } + } + + /// `VitalRefreshRateReader` stops pushing data to `observer` once unregistered. + /// - Parameter observer: already added observer; otherwise nothing happens. + func unregister(_ observer: VitalObserver) { + DispatchQueue.main.async { + self.observers.removeAll { existingObserver in + return existingObserver === observer + } + } + } + + /// Starts listening to frame paints. + /// - Throws: only if `UIScreen.main` cannot generate its `CADisplayLink` + func start() throws { + try private_start() + isRunning = true + } + + /// Stops listening frame paints. Automatically called at `deinit()`. + func stop() { + private_stop() + isRunning = false + } + + // MARK: - Private + + @objc + private func displayTick(link: CADisplayLink) { + if let lastTimestamp = self.lastFrameTimestamp { + let frameDuration = link.timestamp - lastTimestamp + let currentFPS = 1.0 / frameDuration + // NOTE: RUMM-1278 `oldValue` is not used + observers.forEach { + $0.onValueChanged(oldValue: 0.0, newValue: currentFPS) + } + } + lastFrameTimestamp = link.timestamp + } + + @objc + private func appWillResignActive() { + private_stop() + } + + @objc + private func appDidBecomeActive() { + if isRunning { + try? private_start() + } + } + + private func private_start() throws { + stop() + + guard let link = UIScreen.main.displayLink( + withTarget: self, + selector: #selector(displayTick(link:)) + ) else { + throw InternalError(description: "CADisplayLink could not be created!") + } + link.add(to: .main, forMode: .default) + self.displayLink = link + } + + private func private_stop() { + displayLink?.invalidate() + displayLink = nil + lastFrameTimestamp = nil + } +} diff --git a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift new file mode 100644 index 0000000000..2655d587d3 --- /dev/null +++ b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift @@ -0,0 +1,104 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2019-2020 Datadog, Inc. + */ + +import XCTest +import UIKit +@testable import Datadog + +class VitalRefreshRateReaderTests: XCTestCase { + private let mockNotificationCenter = NotificationCenter() + + func testRefreshRateReader() throws { + let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter) + XCTAssertFalse(reader.isRunning) + + let observer_view1 = VitalObserver(listener: VitalListenerMock()) + let observer_view2 = VitalObserver(listener: VitalListenerMock()) + + XCTAssertNoThrow(try reader.start()) + XCTAssertTrue(reader.isRunning) + + reader.register(observer_view1) + + let expectation1 = expectation(description: "async expectation for first observer") + DispatchQueue.global().async { + Thread.sleep(forTimeInterval: 1.0) + expectation1.fulfill() + } + + waitForExpectations(timeout: 3.0) { _ in + XCTAssertGreaterThan(observer_view1.vitalInfo.sampleCount, 0) + XCTAssertGreaterThan(UIScreen.main.maximumFramesPerSecond, Int(observer_view1.vitalInfo.maxValue)) + XCTAssertGreaterThan(observer_view1.vitalInfo.minValue, 0.0) + } + + reader.register(observer_view1) + + let expectation2 = expectation(description: "async expectation for second observer") + DispatchQueue.global().async { + Thread.sleep(forTimeInterval: 1.0) + expectation2.fulfill() + } + + waitForExpectations(timeout: 3.0) { _ in + XCTAssertGreaterThan(observer_view1.vitalInfo.sampleCount, observer_view2.vitalInfo.sampleCount) + } + } + + func testAppStateHandling() throws { + let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter) + XCTAssertFalse(reader.isRunning) + + let observer = VitalObserver(listener: VitalListenerMock()) + + XCTAssertNoThrow(try reader.start()) + XCTAssertTrue(reader.isRunning) + + mockNotificationCenter.post(name: UIApplication.willResignActiveNotification, object: nil) + reader.register(observer) + + let expectation1 = expectation(description: "async expectation for first observer") + DispatchQueue.global().async { + Thread.sleep(forTimeInterval: 1.0) + expectation1.fulfill() + } + + waitForExpectations(timeout: 3.0) { _ in + XCTAssertEqual(observer.vitalInfo.sampleCount, 0) + } + + mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) + + let expectation2 = expectation(description: "async expectation for second observer") + DispatchQueue.global().async { + Thread.sleep(forTimeInterval: 1.0) + expectation2.fulfill() + } + + waitForExpectations(timeout: 3.0) { _ in + XCTAssertGreaterThan(observer.vitalInfo.sampleCount, 0) + } + } + + func testReaderNotRestartIfNotAlreadyRunning() throws { + let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter) + XCTAssertFalse(reader.isRunning) + + let observer = VitalObserver(listener: VitalListenerMock()) + + mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) + + let expectation1 = expectation(description: "async expectation for second observer") + DispatchQueue.global().async { + Thread.sleep(forTimeInterval: 1.0) + expectation1.fulfill() + } + + waitForExpectations(timeout: 3.0) { _ in + XCTAssertEqual(observer.vitalInfo.sampleCount, 0) + } + } +} From c7f03103f133ce7621c00394452143a967fdb224 Mon Sep 17 00:00:00 2001 From: Mert Buran Date: Thu, 24 Jun 2021 11:34:57 +0200 Subject: [PATCH 2/4] RUMM-1278 PR comments addressed --- .../RUMVitals/VitalRefreshRateReader.swift | 54 ++++++---------- .../VitalRefreshRateReaderTests.swift | 64 +++++++------------ 2 files changed, 40 insertions(+), 78 deletions(-) diff --git a/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift index 62375fba46..6df4b099c7 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift @@ -12,7 +12,6 @@ internal class VitalRefreshRateReader { private var observers = [VitalObserver]() private var displayLink: CADisplayLink? private var lastFrameTimestamp: CFTimeInterval? - private(set) var isRunning = false init(notificationCenter: NotificationCenter = .default) { notificationCenter.addObserver( @@ -27,6 +26,8 @@ internal class VitalRefreshRateReader { name: UIApplication.didBecomeActiveNotification, object: nil ) + + start() } deinit { @@ -51,19 +52,6 @@ internal class VitalRefreshRateReader { } } - /// Starts listening to frame paints. - /// - Throws: only if `UIScreen.main` cannot generate its `CADisplayLink` - func start() throws { - try private_start() - isRunning = true - } - - /// Stops listening frame paints. Automatically called at `deinit()`. - func stop() { - private_stop() - isRunning = false - } - // MARK: - Private @objc @@ -79,34 +67,28 @@ internal class VitalRefreshRateReader { lastFrameTimestamp = link.timestamp } - @objc - private func appWillResignActive() { - private_stop() - } - - @objc - private func appDidBecomeActive() { - if isRunning { - try? private_start() + private func start() { + if displayLink != nil { + return } - } - private func private_start() throws { - stop() - - guard let link = UIScreen.main.displayLink( - withTarget: self, - selector: #selector(displayTick(link:)) - ) else { - throw InternalError(description: "CADisplayLink could not be created!") - } - link.add(to: .main, forMode: .default) - self.displayLink = link + displayLink = CADisplayLink(target: self, selector: #selector(displayTick(link:))) + displayLink?.add(to: .main, forMode: .default) } - private func private_stop() { + private func stop() { displayLink?.invalidate() displayLink = nil lastFrameTimestamp = nil } + + @objc + private func appWillResignActive() { + stop() + } + + @objc + private func appDidBecomeActive() { + start() + } } diff --git a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift index 2655d587d3..cc5b272b08 100644 --- a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift +++ b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift @@ -11,62 +11,61 @@ import UIKit class VitalRefreshRateReaderTests: XCTestCase { private let mockNotificationCenter = NotificationCenter() - func testRefreshRateReader() throws { + func testHighAndLowRefreshRates() { let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter) - XCTAssertFalse(reader.isRunning) - let observer_view1 = VitalObserver(listener: VitalListenerMock()) let observer_view2 = VitalObserver(listener: VitalListenerMock()) - XCTAssertNoThrow(try reader.start()) - XCTAssertTrue(reader.isRunning) - + // View1 has simple UI, high FPS expected reader.register(observer_view1) + // Wait without blocking UI thread let expectation1 = expectation(description: "async expectation for first observer") DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 1.0) + Thread.sleep(forTimeInterval: 0.5) expectation1.fulfill() } - waitForExpectations(timeout: 3.0) { _ in + waitForExpectations(timeout: 1.0) { _ in XCTAssertGreaterThan(observer_view1.vitalInfo.sampleCount, 0) XCTAssertGreaterThan(UIScreen.main.maximumFramesPerSecond, Int(observer_view1.vitalInfo.maxValue)) XCTAssertGreaterThan(observer_view1.vitalInfo.minValue, 0.0) } - reader.register(observer_view1) + reader.unregister(observer_view1) + // View2 has complex UI, lower FPS expected + reader.register(observer_view2) + // Wait without blocking UI thread let expectation2 = expectation(description: "async expectation for second observer") DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 1.0) + Thread.sleep(forTimeInterval: 0.5) expectation2.fulfill() } + waitForExpectations(timeout: 1.0) { _ in } - waitForExpectations(timeout: 3.0) { _ in - XCTAssertGreaterThan(observer_view1.vitalInfo.sampleCount, observer_view2.vitalInfo.sampleCount) - } + // Block UI thread + Thread.sleep(forTimeInterval: 0.5) + + XCTAssertGreaterThan(observer_view2.vitalInfo.sampleCount, 0) + XCTAssertGreaterThan(observer_view1.vitalInfo.meanValue, observer_view2.vitalInfo.meanValue) } - func testAppStateHandling() throws { + func testAppStateHandling() { let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter) - XCTAssertFalse(reader.isRunning) - let observer = VitalObserver(listener: VitalListenerMock()) - XCTAssertNoThrow(try reader.start()) - XCTAssertTrue(reader.isRunning) - + mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) mockNotificationCenter.post(name: UIApplication.willResignActiveNotification, object: nil) reader.register(observer) let expectation1 = expectation(description: "async expectation for first observer") DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 1.0) + Thread.sleep(forTimeInterval: 0.5) expectation1.fulfill() } - waitForExpectations(timeout: 3.0) { _ in + waitForExpectations(timeout: 1.0) { _ in XCTAssertEqual(observer.vitalInfo.sampleCount, 0) } @@ -74,31 +73,12 @@ class VitalRefreshRateReaderTests: XCTestCase { let expectation2 = expectation(description: "async expectation for second observer") DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 1.0) + Thread.sleep(forTimeInterval: 0.5) expectation2.fulfill() } - waitForExpectations(timeout: 3.0) { _ in + waitForExpectations(timeout: 1.0) { _ in XCTAssertGreaterThan(observer.vitalInfo.sampleCount, 0) } } - - func testReaderNotRestartIfNotAlreadyRunning() throws { - let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter) - XCTAssertFalse(reader.isRunning) - - let observer = VitalObserver(listener: VitalListenerMock()) - - mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) - - let expectation1 = expectation(description: "async expectation for second observer") - DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 1.0) - expectation1.fulfill() - } - - waitForExpectations(timeout: 3.0) { _ in - XCTAssertEqual(observer.vitalInfo.sampleCount, 0) - } - } } From 13d46014325697bc0f9c3fd040ebf7a83e0b2e02 Mon Sep 17 00:00:00 2001 From: Mert Buran Date: Fri, 25 Jun 2021 18:24:16 +0200 Subject: [PATCH 3/4] RUMM-1278 PR comments addressed #2 --- .../Datadog/Core/Utils/ValuePublisher.swift | 15 ++++++++++-- .../CrashContext/CrashContextProvider.swift | 9 +++++-- .../RUMVitals/VitalRefreshRateReader.swift | 18 ++++---------- .../VitalRefreshRateReaderTests.swift | 24 +++++++------------ 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/Sources/Datadog/Core/Utils/ValuePublisher.swift b/Sources/Datadog/Core/Utils/ValuePublisher.swift index 62a5b1287f..7425356b5e 100644 --- a/Sources/Datadog/Core/Utils/ValuePublisher.swift +++ b/Sources/Datadog/Core/Utils/ValuePublisher.swift @@ -7,7 +7,7 @@ import Foundation /// An observer subscribing to a `ValuePublisher`. -internal protocol ValueObserver { +internal protocol ValueObserver: AnyObject { associatedtype ObservedValue /// Notifies this observer on the value change. Called on the publisher's queue. @@ -23,8 +23,10 @@ internal class ValuePublisher { /// Type erasure for `ValueObserver` type. private struct AnyObserver { let notifyValueChanged: (ObservedValue, ObservedValue) -> Void + let object: AnyObject init(wrapped: Observer) where Observer.ObservedValue == ObservedValue { + self.object = wrapped self.notifyValueChanged = wrapped.onValueChanged } } @@ -82,12 +84,21 @@ internal class ValuePublisher { self.unsafeObservers.append(AnyObserver(wrapped: distinctObserver)) } } + + /// Removes an observer so that it will not be notified on all value changes anymore. + func unsubscribe(_ observer: Observer) where Observer.ObservedValue == Value { + concurrentQueue.async(flags: .barrier) { + self.unsafeObservers.removeAll { existingObserver in + return existingObserver.object === observer + } + } + } } // MARK: - Helpers /// `ValueObserver` wrapper which notifies the wrapped observer only on distinct changes of the `Equatable` value. -private struct DistinctValueObserver: ValueObserver { +private class DistinctValueObserver: ValueObserver { private let wrappedOnValueChanged: (EquatableValue, EquatableValue) -> Void init(wrapped: WrappedObserver) where WrappedObserver.ObservedValue == EquatableValue { diff --git a/Sources/Datadog/CrashReporting/CrashContext/CrashContextProvider.swift b/Sources/Datadog/CrashReporting/CrashContext/CrashContextProvider.swift index 7b3780f417..9b435d520e 100644 --- a/Sources/Datadog/CrashReporting/CrashContext/CrashContextProvider.swift +++ b/Sources/Datadog/CrashReporting/CrashContext/CrashContextProvider.swift @@ -24,12 +24,17 @@ internal class CrashContextProvider: CrashContextProviderType { } /// Observes changes to a particular `Value` in the `CrashContext` and manages its updates. - private struct ContextValueUpdater: ValueObserver { + private class ContextValueUpdater: ValueObserver { let queue: DispatchQueue let update: (Value) -> Void + init(queue: DispatchQueue, update: @escaping (Value) -> Void) { + self.queue = queue + self.update = update + } + func onValueChanged(oldValue: Value, newValue: Value) { - queue.async { update(newValue) } + queue.async { self.update(newValue) } } } diff --git a/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift index 6df4b099c7..65caa42d73 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift @@ -9,7 +9,8 @@ import UIKit /// A class reading the refresh rate (frames per second) of the main screen internal class VitalRefreshRateReader { - private var observers = [VitalObserver]() + private let valuePublisher = ValuePublisher(initialValue: 0.0) + private var displayLink: CADisplayLink? private var lastFrameTimestamp: CFTimeInterval? @@ -37,19 +38,13 @@ internal class VitalRefreshRateReader { /// `VitalRefreshRateReader` keeps pushing data to its `observers` at every new frame. /// - Parameter observer: receiver of refresh rate per frame. func register(_ observer: VitalObserver) { - DispatchQueue.main.async { - self.observers.append(observer) - } + valuePublisher.subscribe(observer) } /// `VitalRefreshRateReader` stops pushing data to `observer` once unregistered. /// - Parameter observer: already added observer; otherwise nothing happens. func unregister(_ observer: VitalObserver) { - DispatchQueue.main.async { - self.observers.removeAll { existingObserver in - return existingObserver === observer - } - } + valuePublisher.unsubscribe(observer) } // MARK: - Private @@ -59,10 +54,7 @@ internal class VitalRefreshRateReader { if let lastTimestamp = self.lastFrameTimestamp { let frameDuration = link.timestamp - lastTimestamp let currentFPS = 1.0 / frameDuration - // NOTE: RUMM-1278 `oldValue` is not used - observers.forEach { - $0.onValueChanged(oldValue: 0.0, newValue: currentFPS) - } + valuePublisher.publishAsync(currentFPS) } lastFrameTimestamp = link.timestamp } diff --git a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift index cc5b272b08..7f1584edd5 100644 --- a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift +++ b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift @@ -21,8 +21,7 @@ class VitalRefreshRateReaderTests: XCTestCase { // Wait without blocking UI thread let expectation1 = expectation(description: "async expectation for first observer") - DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 0.5) + DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) { expectation1.fulfill() } @@ -38,14 +37,13 @@ class VitalRefreshRateReaderTests: XCTestCase { // Wait without blocking UI thread let expectation2 = expectation(description: "async expectation for second observer") - DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 0.5) + DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) { expectation2.fulfill() } waitForExpectations(timeout: 1.0) { _ in } // Block UI thread - Thread.sleep(forTimeInterval: 0.5) + Thread.sleep(forTimeInterval: 1.5) XCTAssertGreaterThan(observer_view2.vitalInfo.sampleCount, 0) XCTAssertGreaterThan(observer_view1.vitalInfo.meanValue, observer_view2.vitalInfo.meanValue) @@ -60,25 +58,21 @@ class VitalRefreshRateReaderTests: XCTestCase { reader.register(observer) let expectation1 = expectation(description: "async expectation for first observer") - DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 0.5) + DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) { expectation1.fulfill() } - waitForExpectations(timeout: 1.0) { _ in - XCTAssertEqual(observer.vitalInfo.sampleCount, 0) - } + waitForExpectations(timeout: 1.0) { _ in } + XCTAssertEqual(observer.vitalInfo.sampleCount, 0) mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) let expectation2 = expectation(description: "async expectation for second observer") - DispatchQueue.global().async { - Thread.sleep(forTimeInterval: 0.5) + DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) { expectation2.fulfill() } - waitForExpectations(timeout: 1.0) { _ in - XCTAssertGreaterThan(observer.vitalInfo.sampleCount, 0) - } + waitForExpectations(timeout: 1.0) { _ in } + XCTAssertGreaterThan(observer.vitalInfo.sampleCount, 0) } } From 674157d3545ed0d1b832c78bbf34ae83512e577f Mon Sep 17 00:00:00 2001 From: Mert Buran Date: Mon, 28 Jun 2021 15:06:28 +0200 Subject: [PATCH 4/4] RUMM-1278 Flaky FPS reader test fixed After blocking UI thread, we must wait a bit in order for the reader to read once again before assertions --- .../RUM/RUMVitals/VitalRefreshRateReaderTests.swift | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift index 7f1584edd5..25920b298a 100644 --- a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift +++ b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift @@ -32,18 +32,19 @@ class VitalRefreshRateReaderTests: XCTestCase { } reader.unregister(observer_view1) + // View2 has complex UI, lower FPS expected reader.register(observer_view2) - // Wait without blocking UI thread + // Block UI thread + Thread.sleep(forTimeInterval: 1.0) + + // Wait after blocking UI thread so that reader will read refresh rate before assertions let expectation2 = expectation(description: "async expectation for second observer") - DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) { + DispatchQueue.global().asyncAfter(deadline: .now() + 0.1) { expectation2.fulfill() } - waitForExpectations(timeout: 1.0) { _ in } - - // Block UI thread - Thread.sleep(forTimeInterval: 1.5) + waitForExpectations(timeout: 0.5) { _ in } XCTAssertGreaterThan(observer_view2.vitalInfo.sampleCount, 0) XCTAssertGreaterThan(observer_view1.vitalInfo.meanValue, observer_view2.vitalInfo.meanValue)