Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove data race caused by doing sample on rum thread #1177

Merged
merged 3 commits into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Sources/Datadog/RUM/RUMVitals/VitalCPUReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<host_cpu_load_info>.stride / MemoryLayout<integer_t>.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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
6 changes: 5 additions & 1 deletion Sources/Datadog/RUM/RUMVitals/VitalInfoSampler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,17 @@ internal final class VitalInfoSampler {
self.refreshRateReader.register(self.refreshRatePublisher)
self.maximumRefreshRate = maximumRefreshRate

takeSample()
// Schedule reoccuring samples
let timer = Timer(
timeInterval: frequency,
repeats: true
) { [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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class VitalInfoSamplerTests: XCTestCase {
}
}

func testItDoesSamplePeriodicallyWithLowFrequency() {
func testItDoesInitialSample() {
let mockCPUReader = SamplingBasedVitalReaderMock()
let mockMemoryReader = SamplingBasedVitalReaderMock()

Expand All @@ -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()
}

Expand All @@ -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)
}
maciejburda marked this conversation as resolved.
Show resolved Hide resolved
}

func testItSamplesDataFromBackgroundThreads() {
// swiftlint:disable implicitly_unwrapped_optional
var sampler: VitalInfoSampler!
Expand Down