perf(android): Defer SentryFrameMetricsCollector thread startup#5641
perf(android): Defer SentryFrameMetricsCollector thread startup#5641runningcode wants to merge 2 commits into
Conversation
SentryFrameMetricsCollector created and started its HandlerThread in the constructor, blocking the calling thread (the main thread during SDK init) on HandlerThread.getLooper(). The handler is only needed once startCollection() registers a listener, so start the thread lazily there instead. Apps that never collect frame metrics no longer start the thread at all. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 18c0bc2 | 306.73 ms | 349.77 ms | 43.03 ms |
| 0eaac1e | 316.82 ms | 357.34 ms | 40.52 ms |
| d15471f | 303.49 ms | 439.08 ms | 135.59 ms |
| fc5ccaf | 276.52 ms | 370.46 ms | 93.93 ms |
| e2dce0b | 308.96 ms | 360.10 ms | 51.14 ms |
| 5b1a06b | 352.27 ms | 413.70 ms | 61.43 ms |
| 37ec571 | 366.04 ms | 424.28 ms | 58.23 ms |
| 9fbb112 | 361.43 ms | 427.57 ms | 66.14 ms |
| bbc35bb | 324.88 ms | 425.73 ms | 100.85 ms |
| ff8eea4 | 313.42 ms | 337.08 ms | 23.66 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 18c0bc2 | 1.58 MiB | 2.13 MiB | 557.33 KiB |
| 0eaac1e | 1.58 MiB | 2.19 MiB | 619.17 KiB |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| fc5ccaf | 1.58 MiB | 2.13 MiB | 557.54 KiB |
| e2dce0b | 0 B | 0 B | 0 B |
| 5b1a06b | 0 B | 0 B | 0 B |
| 37ec571 | 0 B | 0 B | 0 B |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| bbc35bb | 1.58 MiB | 2.12 MiB | 553.01 KiB |
| ff8eea4 | 1.58 MiB | 2.28 MiB | 718.64 KiB |
| (thread, e) -> logger.log(SentryLevel.ERROR, "Error during frames measurements.", e)); | ||
| handlerThread.start(); | ||
| handler = new Handler(handlerThread.getLooper()); | ||
| // The frame metrics HandlerThread is started lazily on the first startCollection() call. |
There was a problem hiding this comment.
I feel like this comment isn't useful here since this isn't where one would look for it. I'll remove it after a review unless anyone thinks otherwise.
0xadam-brown
left a comment
There was a problem hiding this comment.
Nice! That's a real win 🧵 ⚡ 💯
One question for your consideration; otherwise lgtm.
| /** | ||
| * Lazily starts the background HandlerThread used to receive frame metrics. Deferred out of the | ||
| * constructor because {@link HandlerThread#getLooper()} blocks the caller (the main thread during | ||
| * SDK init) until the thread is ready, and the handler is only needed once collection starts. |
| (thread, e) -> logger.log(SentryLevel.ERROR, "Error during frames measurements.", e)); | ||
| handlerThread.start(); | ||
| handler = new Handler(handlerThread.getLooper()); | ||
| // The frame metrics HandlerThread is started lazily on the first startCollection() call. |
| handlerThread.start(); | ||
| handler = new Handler(handlerThread.getLooper()); | ||
| } | ||
| } |
There was a problem hiding this comment.
m: Thoughts about catching Exception? (eg, looks like there's an off chance handlerThread.getLooper() could return null + cause the Handler constructor to throw an NPE if the thread dies before its looper is prepared).
(I realize we didn't do so before, but now we'll be starting the handler thread outside of Sentry.init() at a time when developers may be less likely to have their own try-catch or when it's harder to diagnose.)
romtsn
left a comment
There was a problem hiding this comment.
love how easy and impactful these changes are 🚀
Resolves JAVA-535
📜 Description
SentryFrameMetricsCollectorcreated and started itsHandlerThreadin the constructor and then callednew Handler(handlerThread.getLooper()).getLooper()blocks the caller until the new thread's looper is ready — and the collector is constructed on the main thread duringSentryAndroid.init(loadDefaultAndMetadataOptions).The
handleris only ever used bytrackCurrentWindow(), which no-ops until a listener is registered viastartCollection(). So this starts theHandlerThreadlazily on the firststartCollection()(profiling /SpanFrameMetricsCollector). The constructor still registers the activity-lifecycle callback (current-window tracking is unchanged) but no longer blocks. Apps that never collect frame metrics never start the thread at all.Reusing the SDK's shared executor isn't viable here:
addOnFrameMetricsAvailableListenerrequires a dedicated non-main-threadHandler(Looper), which the sharedScheduledExecutorServicedoesn't provide — so the right move is to defer the dedicated thread, not share one.💡 Motivation and Context
From the customer-provided Perfetto trace (
sentryinvest): the main thread blocked ~5.75 ms onHandlerThread.getLooper()during init viaSentryFrameMetricsCollector.<init>.💚 How did you test it?
SentryFrameMetricsCollectorTest(incl. a new test assertinghandleris null after construction and non-null afterstartCollection); fullsentry-android-coresuite passes.Pixel 3 (Android 12) benchmark — ART method trace → Perfetto trace_processor:
HandlerThread.getLooper()Object.wait()The synchronous main-thread wait for the frame-metrics looper is entirely removed from construction.
Cold-start A/B —
sentry-samples-android(release, Pixel 3, 15 cold starts each viaam start -W -S,TotalTimems, first cold-disk run dropped):Directionally faster on every statistic, but within end-to-end cold-start noise. Note this sample auto-inits Sentry and calls
Sentry.startProfiler()inonCreate, so frame collection (the lazy-thread trigger) fires during startup either way — the change just moves when within startup. So this isn't a strong end-to-end showcase; the deterministic proof is the ART trace above. The real win is for apps that don't start frame collection at startup (the thread is then never created at init).📝 Checklist
sendDefaultPIIis enabled.