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

ref(profiling): put back some unnecessarily prefixed functions #3882

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
390a859
ref: move sliceGPUData to timeseries code unit; prefix C functions
armcknight Apr 23, 2024
9d0cb71
ref: move test helpers to separate code unit; prefix with _sentry
armcknight Apr 23, 2024
08e4e6a
ref: rename more functions to prefix with sentry_ or to remove leadin…
armcknight Apr 23, 2024
86bce30
ref(profiling): extract function to handle SDK startup tasks
armcknight Apr 23, 2024
cc1debd
ref(profiling): extract methods related to profile serialization
armcknight Apr 23, 2024
034fb4b
uncomment gates
armcknight Apr 23, 2024
e9147d5
remove unused imports
armcknight Apr 23, 2024
60e45a7
dead code
armcknight Apr 23, 2024
a129273
fix macos build
armcknight Apr 23, 2024
33639a8
fix xcframework build
armcknight Apr 23, 2024
e4893c2
Merge remote-tracking branch 'origin/main' into armcknight/feat/3555-…
armcknight Apr 23, 2024
2b9b637
Merge branch 'armcknight/feat/3555-continuous-profiling/4-refactoring…
armcknight Apr 23, 2024
ee1deb9
Merge branch 'armcknight/feat/3555-continuous-profiling/4-refactoring…
armcknight Apr 23, 2024
7c7aaf7
Merge remote-tracking branch 'origin/main' into armcknight/feat/3555-…
armcknight Apr 23, 2024
9a9d689
move to test util target
armcknight Apr 23, 2024
56b831e
put back some unnecessarily prefixed functions
armcknight Apr 23, 2024
e7a9688
fixup! put back some unnecessarily prefixed functions
armcknight Apr 23, 2024
0b3796f
Revert "move to test util target"
armcknight Apr 23, 2024
d2d5e03
Merge branch 'armcknight/feat/3555-continuous-profiling/4-refactoring…
armcknight Apr 23, 2024
4864579
move some C declarations to static scope
armcknight Apr 23, 2024
3ae3438
Merge remote-tracking branch 'origin/main' into armcknight/feat/3555-…
armcknight Apr 25, 2024
d4f8c74
Merge remote-tracking branch 'origin/main' into armcknight/feat/3555-…
armcknight Apr 26, 2024
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
2 changes: 1 addition & 1 deletion SentryTestUtils/ClearTestState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class TestCleanup: NSObject {

#if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)
SentryProfiler.getCurrent().stop(for: .normal)
SentryProfiler.sentry_resetConcurrencyTracking()
SentryProfiler.resetConcurrencyTracking()
#endif // os(iOS) || os(macOS) || targetEnvironment(macCatalyst)

#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
Expand Down
9 changes: 6 additions & 3 deletions Sources/Sentry/SentryProfileTimeseries.mm
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
# import "SentryTime.h"
# endif // SENTRY_HAS_UIKIT

namespace {
/**
* Print a debug log to help diagnose slicing errors.
* @param start @c YES if this is an attempt to find the start of the sliced data based on the
* transaction start; @c NO if it's trying to find the end of the sliced data based on the
* transaction's end, to accurately describe what's happening in the log statement.
*/
void
logSlicingFailureWithArray(
_sentry_logSlicingFailureWithArray(
NSArray<SentrySample *> *array, uint64_t startSystemTime, uint64_t endSystemTime, BOOL start)
{
if (!SENTRY_CASSERT_RETURN(
Expand All @@ -43,6 +44,8 @@
firstSampleRelativeToTransactionStart, lastSampleRelativeToTransactionStart);
}

} // namespace

NSArray<SentrySample *> *_Nullable sentry_slicedProfileSamples(
NSArray<SentrySample *> *samples, uint64_t startSystemTime, uint64_t endSystemTime)
{
Expand All @@ -59,7 +62,7 @@
}];

if (firstIndex == NSNotFound) {
logSlicingFailureWithArray(samples, startSystemTime, endSystemTime, /*start*/ YES);
_sentry_logSlicingFailureWithArray(samples, startSystemTime, endSystemTime, /*start*/ YES);

Check warning on line 65 in Sources/Sentry/SentryProfileTimeseries.mm

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryProfileTimeseries.mm#L65

Added line #L65 was not covered by tests
return nil;
} else {
SENTRY_LOG_DEBUG(@"Found first slice sample at index %lu", firstIndex);
Expand All @@ -74,7 +77,7 @@
}];

if (lastIndex == NSNotFound) {
logSlicingFailureWithArray(samples, startSystemTime, endSystemTime, /*start*/ NO);
_sentry_logSlicingFailureWithArray(samples, startSystemTime, endSystemTime, /*start*/ NO);

Check warning on line 80 in Sources/Sentry/SentryProfileTimeseries.mm

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryProfileTimeseries.mm#L80

Added line #L80 was not covered by tests
return nil;
} else {
SENTRY_LOG_DEBUG(@"Found last slice sample at index %lu", lastIndex);
Expand Down
15 changes: 8 additions & 7 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,18 @@
# import <UIKit/UIKit.h>
# endif // SENTRY_HAS_UIKIT

using namespace sentry::profiling;

namespace {

const int kSentryProfilerFrequencyHz = 101;
NSTimeInterval kSentryProfilerTimeoutInterval = 30;

using namespace sentry::profiling;

std::mutex _gProfilerLock;
SentryProfiler *_Nullable _gCurrentProfiler;

} // namespace

# pragma mark - Public

void
Expand Down Expand Up @@ -273,15 +277,12 @@ + (SentryProfiler *)getCurrentProfiler
return _gCurrentProfiler;
}

// this just calls through to SentryProfiledTracerConcurrency.sentry_resetConcurrencyTracking(). we
// have to do this through SentryTracer because SentryProfiledTracerConcurrency cannot be included
// in test targets via ObjC bridging headers because it contains C++.
+ (void)sentry_resetConcurrencyTracking
+ (void)resetConcurrencyTracking
{
sentry_resetConcurrencyTracking();
}

+ (NSUInteger)sentry_currentProfiledTracers
+ (NSUInteger)currentProfiledTracers
{
return sentry_currentProfiledTracers();
}
Expand Down
7 changes: 5 additions & 2 deletions Sources/Sentry/include/SentryProfiler+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ typedef NS_ENUM(NSUInteger, SentryProfilerTruncationReason) {

NS_ASSUME_NONNULL_BEGIN

SENTRY_EXTERN const int kSentryProfilerFrequencyHz;

/**
* Perform necessary profiler tasks that should take place when the SDK starts: configure the next
* launch's profiling, stop legacy profiling if no automatic performance transaction is running,
* start the continuous profiler if enabled and not profiling from launch.
*/
SENTRY_EXTERN void sentry_manageProfilerOnStartSDK(SentryOptions *options, SentryHub *hub);

/**
Expand Down
50 changes: 25 additions & 25 deletions Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class SentryProfilerSwiftTests: XCTestCase {

func createConcurrentSpansWithMetrics() throws {
XCTAssertFalse(SentryProfiler.isCurrentlyProfiling())
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))

for i in 0 ..< numberOfTransactions {
print("creating new concurrent transaction for test")
Expand All @@ -289,7 +289,7 @@ class SentryProfilerSwiftTests: XCTestCase {
fixture.systemWrapper.overrides.cpuEnergyUsage = NSNumber(value: fixture.systemWrapper.overrides.cpuEnergyUsage!.intValue + fixture.mockEnergyUsage.intValue)

XCTAssertTrue(SentryProfiler.isCurrentlyProfiling())
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(i + 1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(i + 1))
spans.append(span)
fixture.currentDateProvider.advanceBy(nanoseconds: 100)
}
Expand All @@ -301,7 +301,7 @@ class SentryProfilerSwiftTests: XCTestCase {
try fixture.gatherMockedMetrics(span: span)
XCTAssertTrue(SentryProfiler.isCurrentlyProfiling())
span.finish()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(numberOfTransactions - (i + 1)))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(numberOfTransactions - (i + 1)))

try self.assertValidProfileData(expectedThreadMetadata: [threadMetadata])

Expand All @@ -314,7 +314,7 @@ class SentryProfilerSwiftTests: XCTestCase {
}

XCTAssertFalse(SentryProfiler.isCurrentlyProfiling())
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
}

try createConcurrentSpansWithMetrics()
Expand Down Expand Up @@ -453,102 +453,102 @@ class SentryProfilerSwiftTests: XCTestCase {

/// based on ``SentryTracerTests.testFinish_WithoutHub_DoesntCaptureTransaction``
func testProfilerCleanedUpAfterTransactionDiscarded_NoHub() throws {
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
func performTransaction() {
let sut = SentryTracer(transactionContext: TransactionContext(name: fixture.transactionName, operation: fixture.transactionOperation), hub: nil)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
sut.finish()
}
performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}

/// based on ``SentryTracerTests.testFinish_WaitForAllChildren_ExceedsMaxDuration_NoTransactionCaptured``
func testProfilerCleanedUpAfterTransactionDiscarded_ExceedsMaxDuration() throws {
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
func performTransaction() throws {
let sut = try fixture.newTransaction(automaticTransaction: true)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(1))
fixture.currentDateProvider.advance(by: 500)
sut.finish()
}
try performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}

func testProfilerCleanedUpAfterInFlightTransactionDeallocated() throws {
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
func performTransaction() throws {
let sut = try fixture.newTransaction(automaticTransaction: true)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(1))
XCTAssertFalse(sut.isFinished)
}
try performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}

/// based on ``SentryTracerTests.testFinish_IdleTimeout_ExceedsMaxDuration_NoTransactionCaptured``
func testProfilerCleanedUpAfterTransactionDiscarded_IdleTimeout_ExceedsMaxDuration() throws {
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
func performTransaction() throws {
let sut = try fixture.newTransaction(automaticTransaction: true, idleTimeout: 1)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(1))
fixture.currentDateProvider.advance(by: 500)
sut.finish()
}
try performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}

/// based on ``SentryTracerTests.testIdleTimeout_NoChildren_TransactionNotCaptured``
func testProfilerCleanedUpAfterTransactionDiscarded_IdleTimeout_NoChildren() throws {
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
func performTransaction() throws {
let span = try fixture.newTransaction(automaticTransaction: true, idleTimeout: 1)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(1))
fixture.currentDateProvider.advance(by: 500)
fixture.dispatchQueueWrapper.invokeLastDispatchAfter()
XCTAssert(span.isFinished)
}
try performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}

/// based on ``SentryTracerTests.testIdleTransaction_CreatingDispatchBlockFails_NoTransactionCaptured``
func testProfilerCleanedUpAfterTransactionDiscarded_IdleTransaction_CreatingDispatchBlockFails() throws {
fixture.dispatchQueueWrapper.createDispatchBlockReturnsNULL = true
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
func performTransaction() throws {
let span = try fixture.newTransaction(automaticTransaction: true, idleTimeout: 1)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(1))
fixture.currentDateProvider.advance(by: 500)
span.finish()
}
try performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}

#if !os(macOS)
/// based on ``SentryTracerTests.testFinish_WaitForAllChildren_StartTimeModified_NoTransactionCaptured``
func testProfilerCleanedUpAfterTransactionDiscarded_WaitForAllChildren_StartTimeModified() throws {
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
let appStartMeasurement = fixture.getAppStartMeasurement(type: .cold)
SentrySDK.setAppStartMeasurement(appStartMeasurement)
fixture.currentDateProvider.advance(by: 1)
func performTransaction() throws {
let sut = try fixture.newTransaction(testingAppLaunchSpans: true, automaticTransaction: true)
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(1))
fixture.currentDateProvider.advance(by: 499)
sut.finish()
}
try performTransaction()
XCTAssertEqual(SentryProfiler.sentry_currentProfiledTracers(), UInt(0))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))
XCTAssertEqual(self.fixture.client?.captureEventWithScopeInvocations.count, 0)
}
#endif // !os(macOS)
Expand Down
16 changes: 13 additions & 3 deletions Tests/SentryTests/SentryProfiler+Test.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,19 @@ SentryProfiler ()

+ (SentryProfiler *)getCurrentProfiler;

+ (void)sentry_resetConcurrencyTracking;

+ (NSUInteger)sentry_currentProfiledTracers;
/**
* This just calls through to SentryProfiledTracerConcurrency.sentry_resetConcurrencyTracking(). we
* have to do this through SentryTracer because SentryProfiledTracerConcurrency.h cannot be included
* in test targets via ObjC bridging headers because it contains C++.
*/
+ (void)resetConcurrencyTracking;

/**
* This just calls through to SentryProfiledTracerConcurrency.sentry_currentProfiledTracers(). we
* have to do this through SentryTracer because SentryProfiledTracerConcurrency.h cannot be included
* in test targets via ObjC bridging headers because it contains C++.
*/
+ (NSUInteger)currentProfiledTracers;

@end

Expand Down
Loading