sentry-cocoa icon indicating copy to clipboard operation
sentry-cocoa copied to clipboard

Create the profiling envelope item on a background thread

Open philipphofmann opened this issue 1 year ago • 1 comments

Description

Currently, the tracer creates the profile envelope item, which calls slicing the profiling data on the calling thread of SentryTracer.finishInternal here https://github.com/getsentry/sentry-cocoa/blob/c02bb1f4252fce2f69a5a6e36bde70f0211663fb/Sources/Sentry/SentryTracer.m#L627-L644

When the profile lasts for a few seconds, this could noticeably block the main thread. With https://github.com/getsentry/sentry-cocoa/issues/3826 the captureTransaction in the hub already happens on a background thread. Moving createProfilingEnvelopeItemForTransaction to a background thread is a bit trickier, because if the tracer gets deallocated in the meantime, it might remove the profiling payload in dealloc: https://github.com/getsentry/sentry-cocoa/blob/c02bb1f4252fce2f69a5a6e36bde70f0211663fb/Sources/Sentry/SentryTracer.m#L208-L215

While we can somehow fix this problem with the current solution, continuous profiling (https://github.com/getsentry/sentry-cocoa/issues/3555) will change our current approach, and the solution could get outdated. @armcknight, does it make sense to consider this point while moving towards continuous profiling (https://github.com/getsentry/sentry-cocoa/issues/3555)?

philipphofmann avatar Apr 24 '24 14:04 philipphofmann

It does make sense to consider for the new implementation, definitely. For legacy profiling, I think it should be fine to move things to a bg thread once we have the transaction created by the tracer. There may be a few accesses of the tracer via the transaction from profiler code, but those could be replaced with parameters in the profiling methods/functions that are called so the values can be captured before offloading to the bg thread, risking deallocation.

armcknight avatar Apr 25 '24 00:04 armcknight