From f156a62b5e54b88ef23e69b6f67e20fdca53e15c Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 8 Aug 2023 16:30:43 -0800 Subject: [PATCH 1/6] gather differential cpu usages between samples --- Sources/Sentry/SentryMetricProfiler.mm | 4 ++++ Sources/Sentry/SentrySystemWrapper.mm | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Sources/Sentry/SentryMetricProfiler.mm b/Sources/Sentry/SentryMetricProfiler.mm index 0b7d9cf87f5..ee96cb42279 100644 --- a/Sources/Sentry/SentryMetricProfiler.mm +++ b/Sources/Sentry/SentryMetricProfiler.mm @@ -196,6 +196,10 @@ - (void)recordCPUPercentagePerCore return; } + if (result == nil) { + return; + } + @synchronized(self) { [result enumerateObjectsUsingBlock:^( NSNumber *_Nonnull usage, NSUInteger core, BOOL *_Nonnull stop) { diff --git a/Sources/Sentry/SentrySystemWrapper.mm b/Sources/Sentry/SentrySystemWrapper.mm index 6e626700e8e..4d4c2053293 100644 --- a/Sources/Sentry/SentrySystemWrapper.mm +++ b/Sources/Sentry/SentrySystemWrapper.mm @@ -2,7 +2,9 @@ #import "SentryError.h" #import -@implementation SentrySystemWrapper +@implementation SentrySystemWrapper { + processor_info_array_t previousCPUInfo; +} - (SentryRAMBytes)memoryFootprintBytes:(NSError *__autoreleasing _Nullable *)error { @@ -43,13 +45,22 @@ - (SentryRAMBytes)memoryFootprintBytes:(NSError *__autoreleasing _Nullable *)err return nil; } + if (previousCPUInfo == NULL) { + previousCPUInfo = cpuInfo; + return nil; + } + NSMutableArray *result = [NSMutableArray arrayWithCapacity:numCPUs]; for (natural_t core = 0U; core < numCPUs; ++core) { const auto indexBase = CPU_STATE_MAX * core; - const float user = cpuInfo[indexBase + CPU_STATE_USER]; - const float sys = cpuInfo[indexBase + CPU_STATE_SYSTEM]; - const float nice = cpuInfo[indexBase + CPU_STATE_NICE]; - const float idle = cpuInfo[indexBase + CPU_STATE_IDLE]; + const float user + = cpuInfo[indexBase + CPU_STATE_USER] - previousCPUInfo[indexBase + CPU_STATE_USER]; + const float sys + = cpuInfo[indexBase + CPU_STATE_SYSTEM] - previousCPUInfo[indexBase + CPU_STATE_SYSTEM]; + const float nice + = cpuInfo[indexBase + CPU_STATE_NICE] - previousCPUInfo[indexBase + CPU_STATE_NICE]; + const float idle + = cpuInfo[indexBase + CPU_STATE_IDLE] - previousCPUInfo[indexBase + CPU_STATE_IDLE]; const auto inUse = user + sys + nice; const auto total = inUse + idle; const auto usagePercent = inUse / total * 100.f; From 760fb3bbad091f180da8895242efb65f9a390b7a Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 8 Aug 2023 16:35:38 -0800 Subject: [PATCH 2/6] DEBUG: print cpu usages --- Sources/Sentry/SentrySystemWrapper.mm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Sources/Sentry/SentrySystemWrapper.mm b/Sources/Sentry/SentrySystemWrapper.mm index 4d4c2053293..820fbbfb683 100644 --- a/Sources/Sentry/SentrySystemWrapper.mm +++ b/Sources/Sentry/SentrySystemWrapper.mm @@ -51,6 +51,7 @@ - (SentryRAMBytes)memoryFootprintBytes:(NSError *__autoreleasing _Nullable *)err } NSMutableArray *result = [NSMutableArray arrayWithCapacity:numCPUs]; + float coreTotal = 0.f; for (natural_t core = 0U; core < numCPUs; ++core) { const auto indexBase = CPU_STATE_MAX * core; const float user @@ -65,8 +66,12 @@ - (SentryRAMBytes)memoryFootprintBytes:(NSError *__autoreleasing _Nullable *)err const auto total = inUse + idle; const auto usagePercent = inUse / total * 100.f; [result addObject:@(usagePercent)]; + + coreTotal += usagePercent; } + NSLog(@"CPU usages (total: %.1f): %@", coreTotal, result); + return result; } From 63a498f4c42145555b622bff483a778057be342b Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 9 Aug 2023 21:24:33 -0800 Subject: [PATCH 3/6] collect single CPU usage instead of per core --- SentryTestUtils/TestSentrySystemWrapper.swift | 6 +- Sources/Sentry/SentryMetricProfiler.mm | 47 ++++-------- Sources/Sentry/SentrySystemWrapper.mm | 74 +++++++++++-------- Sources/Sentry/include/SentryMetricProfiler.h | 2 +- Sources/Sentry/include/SentrySystemWrapper.h | 2 +- .../SentryProfilerSwiftTests.swift | 10 +-- .../SentrySystemWrapperTests.swift | 8 +- 7 files changed, 65 insertions(+), 84 deletions(-) diff --git a/SentryTestUtils/TestSentrySystemWrapper.swift b/SentryTestUtils/TestSentrySystemWrapper.swift index 5e9bdd7bfc6..9060020b0c2 100644 --- a/SentryTestUtils/TestSentrySystemWrapper.swift +++ b/SentryTestUtils/TestSentrySystemWrapper.swift @@ -6,7 +6,7 @@ public class TestSentrySystemWrapper: SentrySystemWrapper { public var memoryFootprintBytes: SentryRAMBytes? public var cpuUsageError: NSError? - public var cpuUsagePerCore: [NSNumber]? + public var cpuUsage: NSNumber? } public var overrides = Override() @@ -19,10 +19,10 @@ public class TestSentrySystemWrapper: SentrySystemWrapper { return overrides.memoryFootprintBytes ?? super.memoryFootprintBytes(error) } - public override func cpuUsagePerCore() throws -> [NSNumber] { + public override func cpuUsage() throws -> NSNumber { if let errorOverride = overrides.cpuUsageError { throw errorOverride } - return try overrides.cpuUsagePerCore ?? super.cpuUsagePerCore() + return try overrides.cpuUsage ?? super.cpuUsage() } } diff --git a/Sources/Sentry/SentryMetricProfiler.mm b/Sources/Sentry/SentryMetricProfiler.mm index ee96cb42279..62807e130af 100644 --- a/Sources/Sentry/SentryMetricProfiler.mm +++ b/Sources/Sentry/SentryMetricProfiler.mm @@ -10,7 +10,6 @@ # import "SentryEvent+Private.h" # import "SentryFormatter.h" # import "SentryLog.h" -# import "SentryNSProcessInfoWrapper.h" # import "SentryNSTimerFactory.h" # import "SentrySystemWrapper.h" # import "SentryTime.h" @@ -28,7 +27,7 @@ @implementation SentryMetricReading @end NSString *const kSentryMetricProfilerSerializationKeyMemoryFootprint = @"memory_footprint"; -NSString *const kSentryMetricProfilerSerializationKeyCPUUsageFormat = @"cpu_usage_%d"; +NSString *const kSentryMetricProfilerSerializationKeyCPUUsage = @"cpu_usage"; NSString *const kSentryMetricProfilerSerializationUnitBytes = @"byte"; NSString *const kSentryMetricProfilerSerializationUnitPercentage = @"percent"; @@ -79,25 +78,14 @@ @implementation SentryMetricReading @implementation SentryMetricProfiler { SentryDispatchSourceWrapper *_dispatchSource; - /// arrays of readings keyed on NSNumbers representing the core number for the set of readings - NSMutableDictionary *> *_cpuUsage; - + NSMutableArray *_cpuUsage; NSMutableArray *_memoryFootprint; } - (instancetype)init { if (self = [super init]) { - _cpuUsage = - [NSMutableDictionary *> dictionary]; - const auto processorCount - = SentryDependencyContainer.sharedInstance.processInfoWrapper.processorCount; - SENTRY_LOG_DEBUG( - @"Preparing %lu arrays for CPU core usage readings", (long unsigned)processorCount); - for (NSUInteger core = 0; core < processorCount; core++) { - _cpuUsage[@(core)] = [NSMutableArray array]; - } - + _cpuUsage = [NSMutableArray array]; _memoryFootprint = [NSMutableArray array]; } return self; @@ -123,11 +111,10 @@ - (void)stop - (NSMutableDictionary *)serializeForTransaction:(SentryTransaction *)transaction { NSArray *memoryFootprint; - NSDictionary *> *cpuUsage; + NSArray *cpuUsage; @synchronized(self) { memoryFootprint = [NSArray arrayWithArray:_memoryFootprint]; - cpuUsage = [NSDictionary *> - dictionaryWithDictionary:_cpuUsage]; + cpuUsage = [NSArray arrayWithArray:_cpuUsage]; } const auto dict = [NSMutableDictionary dictionary]; @@ -137,15 +124,10 @@ - (void)stop memoryFootprint, kSentryMetricProfilerSerializationUnitBytes, transaction); } - [cpuUsage enumerateKeysAndObjectsUsingBlock:^(NSNumber *_Nonnull core, - NSArray *_Nonnull readings, BOOL *_Nonnull stop) { - if (readings.count > 0) { - dict[[NSString stringWithFormat:kSentryMetricProfilerSerializationKeyCPUUsageFormat, - core.intValue]] - = serializeValuesWithNormalizedTime( - readings, kSentryMetricProfilerSerializationUnitPercentage, transaction); - } - }]; + if (cpuUsage.count > 0) { + dict[kSentryMetricProfilerSerializationKeyCPUUsage] = serializeValuesWithNormalizedTime( + cpuUsage, kSentryMetricProfilerSerializationUnitPercentage, transaction); + } return dict; } @@ -164,7 +146,7 @@ - (void)registerSampler attributes:dispatch_queue_attr_make_with_qos_class( DISPATCH_QUEUE_CONCURRENT, QOS_CLASS_UTILITY, 0) eventHandler:^{ - [weakSelf recordCPUPercentagePerCore]; + [weakSelf recordCPUsage]; [weakSelf recordMemoryFootprint]; }]; } @@ -185,11 +167,11 @@ - (void)recordMemoryFootprint } } -- (void)recordCPUPercentagePerCore +- (void)recordCPUsage { NSError *error; const auto result = - [SentryDependencyContainer.sharedInstance.systemWrapper cpuUsagePerCore:&error]; + [SentryDependencyContainer.sharedInstance.systemWrapper cpuUsageWithError:&error]; if (error) { SENTRY_LOG_ERROR(@"Failed to read CPU usages: %@", error); @@ -201,10 +183,7 @@ - (void)recordCPUPercentagePerCore } @synchronized(self) { - [result enumerateObjectsUsingBlock:^( - NSNumber *_Nonnull usage, NSUInteger core, BOOL *_Nonnull stop) { - [_cpuUsage[@(core)] addObject:[self metricReadingForValue:usage]]; - }]; + [_cpuUsage addObject:[self metricReadingForValue:result]]; } } diff --git a/Sources/Sentry/SentrySystemWrapper.mm b/Sources/Sentry/SentrySystemWrapper.mm index 820fbbfb683..d7f702ad96b 100644 --- a/Sources/Sentry/SentrySystemWrapper.mm +++ b/Sources/Sentry/SentrySystemWrapper.mm @@ -1,9 +1,21 @@ #import "SentrySystemWrapper.h" +#import "SentryDependencyContainer.h" #import "SentryError.h" +#import "SentryNSProcessInfoWrapper.h" #import +#include @implementation SentrySystemWrapper { - processor_info_array_t previousCPUInfo; + float processorCount; +} + +- (instancetype)init +{ + if ((self = [super init])) { + processorCount + = (float)SentryDependencyContainer.sharedInstance.processInfoWrapper.processorCount; + } + return self; } - (SentryRAMBytes)memoryFootprintBytes:(NSError *__autoreleasing _Nullable *)error @@ -30,49 +42,47 @@ - (SentryRAMBytes)memoryFootprintBytes:(NSError *__autoreleasing _Nullable *)err return footprintBytes; } -- (NSArray *)cpuUsagePerCore:(NSError **)error +- (NSNumber *)cpuUsageWithError:(NSError **)error { - natural_t numCPUs = 0U; - processor_info_array_t cpuInfo; - mach_msg_type_number_t numCPUInfo; - const auto status = host_processor_info( - mach_host_self(), PROCESSOR_CPU_LOAD_INFO, &numCPUs, &cpuInfo, &numCPUInfo); - if (status != KERN_SUCCESS) { + mach_msg_type_number_t count; + thread_act_array_t list; + + const auto taskThreadsStatus = task_threads(mach_task_self(), &list, &count); + if (taskThreadsStatus != KERN_SUCCESS) { if (error) { *error = NSErrorFromSentryErrorWithKernelError( - kSentryErrorKernel, @"host_processor_info reported an error.", status); + kSentryErrorKernel, @"task_threads reported an error.", taskThreadsStatus); } return nil; } - if (previousCPUInfo == NULL) { - previousCPUInfo = cpuInfo; - return nil; - } + auto usage = 0.f; + const auto threads = [NSMutableArray array]; + for (decltype(count) i = 0; i < count; i++) { + const auto thread = list[i]; - NSMutableArray *result = [NSMutableArray arrayWithCapacity:numCPUs]; - float coreTotal = 0.f; - for (natural_t core = 0U; core < numCPUs; ++core) { - const auto indexBase = CPU_STATE_MAX * core; - const float user - = cpuInfo[indexBase + CPU_STATE_USER] - previousCPUInfo[indexBase + CPU_STATE_USER]; - const float sys - = cpuInfo[indexBase + CPU_STATE_SYSTEM] - previousCPUInfo[indexBase + CPU_STATE_SYSTEM]; - const float nice - = cpuInfo[indexBase + CPU_STATE_NICE] - previousCPUInfo[indexBase + CPU_STATE_NICE]; - const float idle - = cpuInfo[indexBase + CPU_STATE_IDLE] - previousCPUInfo[indexBase + CPU_STATE_IDLE]; - const auto inUse = user + sys + nice; - const auto total = inUse + idle; - const auto usagePercent = inUse / total * 100.f; - [result addObject:@(usagePercent)]; + mach_msg_type_number_t infoSize = THREAD_BASIC_INFO_COUNT; + thread_basic_info_data_t data; + // MACH_SEND_INVALID_DEST is returned when the thread no longer exists + const auto threadInfoStatus = thread_info( + thread, THREAD_BASIC_INFO, reinterpret_cast(&data), &infoSize); + if (threadInfoStatus != KERN_SUCCESS) { + if (error) { + *error = NSErrorFromSentryErrorWithKernelError( + kSentryErrorKernel, @"thread_info reported an error.", taskThreadsStatus); + } + return nil; + } - coreTotal += usagePercent; + [threads addObject:@(data.cpu_usage)]; + usage += data.cpu_usage / processorCount; } - NSLog(@"CPU usages (total: %.1f): %@", coreTotal, result); + vm_deallocate(mach_task_self(), reinterpret_cast(list), sizeof(*list) * count); + + NSLog(@"CPU usage: %f; per thread: %@", usage, [threads componentsJoinedByString:@", "]); - return result; + return @(usage); } @end diff --git a/Sources/Sentry/include/SentryMetricProfiler.h b/Sources/Sentry/include/SentryMetricProfiler.h index cdb0041732e..24fb7bc328b 100644 --- a/Sources/Sentry/include/SentryMetricProfiler.h +++ b/Sources/Sentry/include/SentryMetricProfiler.h @@ -9,7 +9,7 @@ NS_ASSUME_NONNULL_BEGIN SENTRY_EXTERN NSString *const kSentryMetricProfilerSerializationKeyMemoryFootprint; -SENTRY_EXTERN NSString *const kSentryMetricProfilerSerializationKeyCPUUsageFormat; +SENTRY_EXTERN NSString *const kSentryMetricProfilerSerializationKeyCPUUsage; SENTRY_EXTERN NSString *const kSentryMetricProfilerSerializationUnitBytes; SENTRY_EXTERN NSString *const kSentryMetricProfilerSerializationUnitPercentage; diff --git a/Sources/Sentry/include/SentrySystemWrapper.h b/Sources/Sentry/include/SentrySystemWrapper.h index 09d01a67d14..61c056e07d2 100644 --- a/Sources/Sentry/include/SentrySystemWrapper.h +++ b/Sources/Sentry/include/SentrySystemWrapper.h @@ -24,7 +24,7 @@ typedef mach_vm_size_t SentryRAMBytes; * returned by the underlying system call, e.g. @c @[ @c , @c , * @c ...] . */ -- (nullable NSArray *)cpuUsagePerCore:(NSError **)error; +- (nullable NSNumber *)cpuUsageWithError:(NSError **)error; @end diff --git a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift index 30e025cfbcf..13e24de0052 100644 --- a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift +++ b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift @@ -52,8 +52,7 @@ class SentryProfilerSwiftTests: XCTestCase { SentryDependencyContainer.sharedInstance().dispatchFactory = dispatchFactory SentryDependencyContainer.sharedInstance().timerFactory = timeoutTimerFactory - systemWrapper.overrides.cpuUsagePerCore = mockCPUUsages.map { NSNumber(value: $0) } - processInfoWrapper.overrides.processorCount = UInt(mockCPUUsages.count) + systemWrapper.overrides.cpuUsage = NSNumber(value: mockCPUUsage) systemWrapper.overrides.memoryFootprintBytes = mockMemoryFootprint #if !os(macOS) @@ -87,7 +86,7 @@ class SentryProfilerSwiftTests: XCTestCase { // mocking - let mockCPUUsages = [12.4, 63.5, 1.4, 4.6] + let mockCPUUsage = 66.6 let mockMemoryFootprint: SentryRAMBytes = 123_455 let mockUsageReadingsPerBatch = 2 @@ -597,10 +596,7 @@ private extension SentryProfilerSwiftTests { let expectedUsageReadings = fixture.mockUsageReadingsPerBatch * metricsBatches - for (i, expectedUsage) in fixture.mockCPUUsages.enumerated() { - let key = NSString(format: kSentryMetricProfilerSerializationKeyCPUUsageFormat as NSString, i) as String - try assertMetricValue(measurements: measurements, key: key, numberOfReadings: expectedUsageReadings, expectedValue: expectedUsage, transaction: transaction) - } + try assertMetricValue(measurements: measurements, key: kSentryMetricProfilerSerializationKeyCPUUsage, numberOfReadings: expectedUsageReadings, expectedValue: fixture.mockCPUUsage, transaction: transaction) try assertMetricValue(measurements: measurements, key: kSentryMetricProfilerSerializationKeyMemoryFootprint, numberOfReadings: expectedUsageReadings, expectedValue: fixture.mockMemoryFootprint, transaction: transaction) diff --git a/Tests/SentryProfilerTests/SentrySystemWrapperTests.swift b/Tests/SentryProfilerTests/SentrySystemWrapperTests.swift index 2607ecba33d..a72130187f0 100644 --- a/Tests/SentryProfilerTests/SentrySystemWrapperTests.swift +++ b/Tests/SentryProfilerTests/SentrySystemWrapperTests.swift @@ -8,12 +8,8 @@ class SentrySystemWrapperTests: XCTestCase { func testCPUUsageReportsData() throws { XCTAssertNoThrow({ - let cpuUsages = try self.fixture.systemWrapper.cpuUsagePerCore() - XCTAssertGreaterThan(cpuUsages.count, 0) - let range = 0.0 ... 100.0 - cpuUsages.forEach { - XCTAssert(range.contains($0.doubleValue)) - } + let cpuUsage = try XCTUnwrap(self.fixture.systemWrapper.cpuUsage()) + XCTAssert((0.0 ... 100.0).contains(cpuUsage.doubleValue)) }) } From 1069e5ceb3d3c0b3adb481267538eeb010943052 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 9 Aug 2023 21:49:25 -0800 Subject: [PATCH 4/6] clean up --- Sources/Sentry/SentrySystemWrapper.mm | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Sources/Sentry/SentrySystemWrapper.mm b/Sources/Sentry/SentrySystemWrapper.mm index d7f702ad96b..7c5525b01b5 100644 --- a/Sources/Sentry/SentrySystemWrapper.mm +++ b/Sources/Sentry/SentrySystemWrapper.mm @@ -53,34 +53,35 @@ - (NSNumber *)cpuUsageWithError:(NSError **)error *error = NSErrorFromSentryErrorWithKernelError( kSentryErrorKernel, @"task_threads reported an error.", taskThreadsStatus); } + vm_deallocate( + mach_task_self(), reinterpret_cast(list), sizeof(*list) * count); return nil; } auto usage = 0.f; - const auto threads = [NSMutableArray array]; for (decltype(count) i = 0; i < count; i++) { const auto thread = list[i]; mach_msg_type_number_t infoSize = THREAD_BASIC_INFO_COUNT; thread_basic_info_data_t data; - // MACH_SEND_INVALID_DEST is returned when the thread no longer exists const auto threadInfoStatus = thread_info( thread, THREAD_BASIC_INFO, reinterpret_cast(&data), &infoSize); if (threadInfoStatus != KERN_SUCCESS) { if (error) { *error = NSErrorFromSentryErrorWithKernelError( - kSentryErrorKernel, @"thread_info reported an error.", taskThreadsStatus); + kSentryErrorKernel, @"task_threads reported an error.", taskThreadsStatus); } + vm_deallocate( + mach_task_self(), reinterpret_cast(list), sizeof(*list) * count); return nil; } - [threads addObject:@(data.cpu_usage)]; usage += data.cpu_usage / processorCount; } vm_deallocate(mach_task_self(), reinterpret_cast(list), sizeof(*list) * count); - NSLog(@"CPU usage: %f; per thread: %@", usage, [threads componentsJoinedByString:@", "]); + NSLog(@"CPU usage: %f", usage); return @(usage); } From abe533a79262041f2d7c01f974a3f26d49fc5ec6 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 9 Aug 2023 21:50:49 -0800 Subject: [PATCH 5/6] changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61b7f79cc1d..75d34529f4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +### Fixes + +- Fix CPU usage collection for upcoming visualization in profiling flamecharts (#3214) ## 8.9.4 From 6e9ef71848fdb34145ef946f8cf5d9addf93d88d Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 15 Aug 2023 10:50:53 -0800 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17c8fa4a5e9..f57ec8c584d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,6 @@ - Allow profiling from hybrid SDKs (([#3194]https://github.com/getsentry/sentry-cocoa/pull/3194)) - ## 8.9.4 ### Fixes