fix(android): encode replay screenshots off the main thread#494
fix(android): encode replay screenshots off the main thread#494turnipdabeets wants to merge 3 commits into
Conversation
Each replay frame ran BitmapFactory.decodeByteArray plus a JPEG re-encode and base64 synchronously in the method-channel handler on the platform main thread - 25-75ms per frame on midrange hardware, up to once per second while recording. iOS routes the identical work through a background serial queue. Meta and frame sends share one single-threaded executor so a meta event can never be overtaken by the frame it describes; submissions after engine detach are dropped.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
This review was skipped because it would exceed your organization's monthly flex usage limit. Raise the limit in billing settings or wait until the next billing period resets limits.
posthog-flutter Compliance ReportDate: 2026-07-17 22:36:24 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
Covers success-before-work and the post-detach drop (a rejected submission must never escape to the channel).
There was a problem hiding this comment.
The core performance diagnosis is corroborated: Android currently performs bitmap decode, JPEG re-encoding, and Base64 work on the platform thread, and this change moves it to a serial worker while preserving meta → frame ordering. The remaining comments are non-blocking follow-ups.
Human-driven, agent-assisted review.
| val y = call.argument<Int>("y") ?: 0 | ||
| if (imageBytes != null) { | ||
| snapshotSender.sendFullSnapshot(imageBytes, id, x, y) | ||
| submitSnapshotWork { snapshotSender.sendFullSnapshot(imageBytes, id, x, y) } |
There was a problem hiding this comment.
[P2] Consider capturing the replay timestamp before submitting the encoding work
SnapshotSender currently reads the SDK clock after bitmap decoding and JPEG/Base64 encoding. Flutter iOS instead captures Date before dispatching to its serial queue.
At the default capture interval this should only introduce tens of milliseconds of skew, so I don’t think it needs to block this improvement. Capturing the SDK-backed timestamp here and passing it into SnapshotSender would nevertheless provide closer iOS parity and prevent larger skew if the executor ever becomes backlogged.
There was a problem hiding this comment.
Agreed — our pre-PR pass flagged the same thing. Fixed in a11ae47: the handler captures the SDK-clock timestamp at enqueue and passes it into SnapshotSender, matching the iOS shape.
| stopOcclusionDetector() | ||
| channel.setMethodCallHandler(null) | ||
| bitmapExportExecutor.shutdown() | ||
| snapshotSendExecutor.shutdown() |
There was a problem hiding this comment.
[P2] Reattaching the same plugin instance leaves snapshot delivery disabled
snapshotSendExecutor is created once, shut down here, and not recreated in onAttachedToEngine. If Flutter removes and later re-adds the same plugin instance, both snapshot handlers report success while their work is rejected by the terminated executor.
Generated registration normally creates a fresh instance, so this is an uncommon lifecycle edge. Consider recreating attachment-scoped executors on attach, with coverage for attach → send → detach → reattach → send.
There was a problem hiding this comment.
Fair — generated registration makes this unreachable today, but it's cheap insurance. Fixed in a11ae47: both attachment-scoped executors are recreated on attach, with the attach → send → detach → reattach → send coverage you suggested.
| // Frame sends decode and re-encode the image — too slow for the main | ||
| // thread. Single-threaded so a meta event can never be overtaken by the | ||
| // frame it describes. | ||
| private val snapshotSendExecutor = Executors.newSingleThreadExecutor() |
There was a problem hiding this comment.
[P2/design] Consider whether the snapshot queue needs backpressure
This executor has an unbounded queue, while the method-channel call now succeeds immediately after submission. With the default one-second capture interval the worker has ample headroom, but a very low custom throttleDelay could enqueue full-screen image buffers faster than they are encoded.
Consider completing the channel result after worker completion, or bounding/coalescing queued frames, if very low capture intervals are intended to be supported safely.
There was a problem hiding this comment.
Good catch — with the result completing at enqueue, a throttleDelay below the encode time does grow the queue without bound. Fixed in a11ae47 with your first option: the reply completes after the worker finishes (posted via the main handler), restoring the Dart-side one-in-flight backpressure while keeping the encode off the main thread. E2E-verified frames still flow at the throttle cadence.
…revive executors on reattach - Complete the channel reply after the worker finishes (posted via the main handler): Dart's await then spans the encode, so a throttleDelay below the encode time self-regulates instead of growing the queue of full-screen buffers without bound. - Capture the SDK-clock timestamp in the handler and pass it into SnapshotSender, matching iOS and capping skew under any backlog. - Recreate shut-down executors on engine reattach so a reused plugin instance does not silently drop snapshots.
2 participants
Problem
While session replay records, every snapshot frame runs
BitmapFactory.decodeByteArray(full screen-size PNG decode), a JPEG re-encode (quality 30), and Base64 synchronously on the platform main thread inside the method-channel handler — roughly 25–75ms per frame on midrange hardware, up to once per second for the duration of every recorded session. That is a rhythmic 2–4 dropped-frame hitch, most visible mid-scroll.iOS already routes the identical work through a background serial queue (
PosthogFlutterPlugin.swifteven carries the comment: "otherwise we are doing slow operations in the main thread") — this brings Android to parity.Fix
sendMetaEventandsendFullSnapshotare submitted to a dedicated single-threaded executor:SnapshotSender.Evidence
Choreographer main-thread gap probe (>17ms) during an identical 20-swipe scripted scroll with replay recording, emulator (Pixel 9, Apple-silicon host — best-case CPU, so deltas understate real devices):
On midrange ARM devices the per-frame cost is several times larger, so the removed stall is proportionally bigger there.
Verified: ktlint clean, Android unit tests pass, replay frames flow end-to-end after the change (sends unchanged).
🤖 Generated with Claude Code