fix(android): Defer frame metrics reflection during init - #5886
Open
0xadam-brown wants to merge 2 commits into
Open
fix(android): Defer frame metrics reflection during init#58860xadam-brown wants to merge 2 commits into
0xadam-brown wants to merge 2 commits into
Conversation
0xadam-brown
force-pushed
the
fix/no-reflection-on-main-thread
branch
from
August 4, 2026 11:48
e23fd78 to
1420ac9
Compare
📲 Install BuildsAndroid
|
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 455eb6e | 341.51 ms | 428.96 ms | 87.45 ms |
| d15471f | 343.13 ms | 361.47 ms | 18.34 ms |
| b193867 | 331.08 ms | 397.06 ms | 65.98 ms |
| 5b1a06b | 315.40 ms | 353.33 ms | 37.94 ms |
| 62b579c | 312.88 ms | 361.57 ms | 48.70 ms |
| 85d7417 | 347.21 ms | 394.35 ms | 47.15 ms |
| 6dff1c9 | 298.04 ms | 327.43 ms | 29.39 ms |
| 5b1a06b | 310.56 ms | 362.79 ms | 52.22 ms |
| ee747ae | 386.94 ms | 431.43 ms | 44.49 ms |
| 462dea2 | 322.40 ms | 370.06 ms | 47.66 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 455eb6e | 0 B | 0 B | 0 B |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| b193867 | 1.58 MiB | 2.19 MiB | 620.00 KiB |
| 5b1a06b | 0 B | 0 B | 0 B |
| 62b579c | 0 B | 0 B | 0 B |
| 85d7417 | 1.58 MiB | 2.10 MiB | 533.44 KiB |
| 6dff1c9 | 0 B | 0 B | 0 B |
| 5b1a06b | 0 B | 0 B | 0 B |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 462dea2 | 0 B | 0 B | 0 B |
Moves Choreographer private field lookup out of the frame metrics collector constructor so SDK init does not synchronously perform framework reflection on the calling thread. Helps us reduce the likelihood of another common class of Sentry.init() ANRs (see [here](https://sentry.sentry.io/issues/6138715212/?project=4506812075540480&referrer=seer.agent.in-chat-link)). Behavior change from the user's perspective should usually be non-existant, and minor in the worst case. The choreographer and choreographerLastFrameTimeField properties are still initialized by a main-thread Handler post made during collector construction, before later startCollection() calls post frame-listener registration work to the same main looper. Since those main-looper tasks run in order, the Choreographer fallback should be populated before any collected frame or pending-frame interpolation normally needs it. If it's not ready yet, the failure mode is a missed/less precise first pending-frame calculation rather than a crash. Co-Authored-By: OpenCode <noreply@opencode.ai>
0xadam-brown
force-pushed
the
fix/no-reflection-on-main-thread
branch
from
August 4, 2026 12:02
1420ac9 to
ec477f3
Compare
0xadam-brown
marked this pull request as ready for review
August 4, 2026 12:06
0xadam-brown
requested review from
adinauer,
markushi,
romtsn and
runningcode
as code owners
August 4, 2026 12:06
0xadam-brown
added
the
sanity-check
runningcode
approved these changes
Aug 4, 2026
runningcode
left a comment
runningcode
left a comment
Contributor
There was a problem hiding this comment.
Looks good with one nit! Thanks for taking this further!
| private @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener; | ||
| private @Nullable Choreographer choreographer; | ||
| private @Nullable Field choreographerLastFrameTimeField; | ||
| private volatile @Nullable Choreographer choreographer; |
Contributor
There was a problem hiding this comment.
If we only ever access these from the main thread why do we need to add volatile here?
Member
Author
There was a problem hiding this comment.
Good question! Because after this PR we now assign those fields on the main thread but read from another thread (via getLastKnownFrameStartTimeNanos(), which is reached from the frame metrics callback registered with our background handler). (Before they were assigned and read from the same.)
I've updated the inline comment to be more precise about the fact that we're initializing the choreographer out of the init path, rather than using the previous "accessed on the main thread" language 👍
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Moves Choreographer private field lookup out of the frame metrics collector constructor synchronously framework reflection on the calling (i.e., main) thread during
Sentry.init(). Helps us reduce the likelihood of another common class of init ANRs (see here).💡 Motivation and Context
We've seen 583k ANR events involving the SentryFrameMetricsCollector across 257 affected users since November 2024 (link), making it the highest-volume SDK-caused ANR we track.
Those numbers got lots better thanks to @runningcode's fix in #5641: events peaked at ~12k per 3-day window in June 2026, then dropped sharply to (a fairly consistent) ~4k/3d. But the reflection this PR addresses continues to contribute to those remaining ANRs across all SDK versions, including the newest ones.
Any behavior changes?
Behavior change from the user's perspective should be nonexistant in most cases, and minor in the worst case.
The choreographer and choreographerLastFrameTimeField properties are still initialized by a main-thread Handler post made during collector construction, before later startCollection() calls post frame-listener registration work to the same main looper. Since those main looper tasks run in order, the Choreographer fallback should be populated before any collected frame or pending-frame interpolation normally needs it. If it's not ready yet, the failure mode is a missed/less precise first pending-frame calculation rather than a crash.
💚 How did you test it?
Unit tests + manual smoke tests on Android sample app.
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps