From d4cfd611d84d0ac2848abc5351077468f0279fe3 Mon Sep 17 00:00:00 2001 From: Colton Schlosser <1498529+cltnschlosser@users.noreply.github.com> Date: Fri, 24 Feb 2023 13:49:22 -0600 Subject: [PATCH 1/3] Remove data race caused by doing sample on rum thread --- Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift b/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift index 9690b15fe0..61b6e3c93a 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift @@ -63,7 +63,6 @@ internal final class VitalInfoSampler { self.refreshRateReader.register(self.refreshRatePublisher) self.maximumRefreshRate = maximumRefreshRate - takeSample() let timer = Timer( timeInterval: frequency, repeats: true From 95e0234eb76f06ef6219e1ef99cef6663b77bb5a Mon Sep 17 00:00:00 2001 From: Colton Schlosser <1498529+cltnschlosser@users.noreply.github.com> Date: Tue, 28 Feb 2023 15:32:18 -0600 Subject: [PATCH 2/3] Take initial sample. Also uses UInt64 for keeping track of background ticks. --- .../RUM/RUMVitals/VitalCPUReader.swift | 8 ++--- .../RUM/RUMVitals/VitalInfoSampler.swift | 5 ++++ .../RUM/RUMVitals/VitalInfoSamplerTests.swift | 30 +++++++++++++++++-- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/Sources/Datadog/RUM/RUMVitals/VitalCPUReader.swift b/Sources/Datadog/RUM/RUMVitals/VitalCPUReader.swift index 1ef1e61aad..7a03b6e938 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalCPUReader.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalCPUReader.swift @@ -11,8 +11,8 @@ import UIKit.UIApplication internal class VitalCPUReader: SamplingBasedVitalReader { /// host_cpu_load_info_count is 4 (tested in iOS 14.4) private static let host_cpu_load_info_count = MemoryLayout.stride / MemoryLayout.stride - private var totalInactiveTicks: natural_t = 0 - private var utilizedTicksWhenResigningActive: natural_t? = nil + private var totalInactiveTicks: UInt64 = 0 + private var utilizedTicksWhenResigningActive: UInt64? = nil init(notificationCenter: NotificationCenter = .default) { notificationCenter.addObserver(self, selector: #selector(appWillResignActive), name: UIApplication.willResignActiveNotification, object: nil) @@ -44,7 +44,7 @@ internal class VitalCPUReader: SamplingBasedVitalReader { } } - private func readUtilizedTicks() -> natural_t? { + private func readUtilizedTicks() -> UInt64? { // it must be set to host_cpu_load_info_count_size >= host_cpu_load_info_count // implementation: https://github.com/opensource-apple/xnu/blob/master/osfmk/kern/host.c#L425 var host_cpu_load_info_count_size = mach_msg_type_number_t(Self.host_cpu_load_info_count) @@ -82,6 +82,6 @@ internal class VitalCPUReader: SamplingBasedVitalReader { therefore even at the worst-case, precision isn't lost during this conversion below. */ let userTicks = cpuLoadInfo.cpu_ticks.0 - return userTicks + return UInt64(userTicks) } } diff --git a/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift b/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift index 61b6e3c93a..24e752ab22 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift @@ -63,6 +63,11 @@ internal final class VitalInfoSampler { self.refreshRateReader.register(self.refreshRatePublisher) self.maximumRefreshRate = maximumRefreshRate + // Take initial sample + RunLoop.main.perform { [weak self] in + self?.takeSample() + } + // Schedule reoccuring samples let timer = Timer( timeInterval: frequency, repeats: true diff --git a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalInfoSamplerTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalInfoSamplerTests.swift index e6bdb283c6..b4821f0de6 100644 --- a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalInfoSamplerTests.swift +++ b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalInfoSamplerTests.swift @@ -42,7 +42,7 @@ class VitalInfoSamplerTests: XCTestCase { } } - func testItDoesSamplePeriodicallyWithLowFrequency() { + func testItDoesInitialSample() { let mockCPUReader = SamplingBasedVitalReaderMock() let mockMemoryReader = SamplingBasedVitalReaderMock() @@ -57,7 +57,7 @@ class VitalInfoSamplerTests: XCTestCase { mockMemoryReader.vitalData = 321.0 let samplingExpectation = expectation(description: "sampling expectation") - DispatchQueue.global().asyncAfter(deadline: .now() + 0.6) { + DispatchQueue.global().asyncAfter(deadline: .now() + 0.1) { samplingExpectation.fulfill() } @@ -67,6 +67,32 @@ class VitalInfoSamplerTests: XCTestCase { } } + func testItDoesSamplePeriodicallyWithLowFrequency() { + let mockCPUReader = SamplingBasedVitalReaderMock() + let mockMemoryReader = SamplingBasedVitalReaderMock() + + let sampler = VitalInfoSampler( + cpuReader: mockCPUReader, + memoryReader: mockMemoryReader, + refreshRateReader: ContinuousVitalReaderMock(), + frequency: 0.5 + ) + + mockCPUReader.vitalData = 123.0 + mockMemoryReader.vitalData = 321.0 + + let samplingExpectation = expectation(description: "sampling expectation") + DispatchQueue.global().asyncAfter(deadline: .now() + 0.6) { + samplingExpectation.fulfill() + } + + waitForExpectations(timeout: 1.0) { _ in + // 2 samples, initial sample and one periodic sample + XCTAssertEqual(sampler.cpu.sampleCount, 2) + XCTAssertEqual(sampler.memory.sampleCount, 2) + } + } + func testItSamplesDataFromBackgroundThreads() { // swiftlint:disable implicitly_unwrapped_optional var sampler: VitalInfoSampler! From 24382bed85cc5c67de50d952d31605a3696e968e Mon Sep 17 00:00:00 2001 From: Maciej Burda Date: Wed, 1 Mar 2023 16:33:11 +0000 Subject: [PATCH 3/3] Change way of triggering timer's completion block --- Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift b/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift index 24e752ab22..dfee4bdbbc 100644 --- a/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift +++ b/Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift @@ -63,10 +63,6 @@ internal final class VitalInfoSampler { self.refreshRateReader.register(self.refreshRatePublisher) self.maximumRefreshRate = maximumRefreshRate - // Take initial sample - RunLoop.main.perform { [weak self] in - self?.takeSample() - } // Schedule reoccuring samples let timer = Timer( timeInterval: frequency, @@ -74,6 +70,10 @@ internal final class VitalInfoSampler { ) { [weak self] _ in self?.takeSample() } + // Take initial sample + RunLoop.main.perform { + timer.fire() + } // NOTE: RUMM-1280 based on my running Example app // non-main run loops don't fire the timer. // Although i can't catch this in unit tests