perf(call): cut HeaderCallButton mount cost by consolidating call hooks#7361
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
WalkthroughRefactors video call hooks to use the direct Camera API for permissions, consolidate Jitsi/VideoConf settings into one selector, and switch HeaderCallButton to use the unified isInActiveCall state. ChangesVideo Call Hook Infrastructure Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7bce08d to
1ca2f38
Compare
HeaderCallButton ran ~12 useSelector calls (8 one-per-setting), a native expo-camera permission hook, and a duplicated zustand selector on every mount. Consolidate this per-mount work (behavior-preserving) in the shared call hooks: - useVideoConfCall: 8 useSetting -> 1 useAppSelector(..., shallowEqual) - useVideoConf: reactive useCameraPermissions() -> imperative Camera.get/requestCameraPermissionsAsync() at press time - HeaderCallButton: drop duplicate useIsInActiveVoipCall() Argent React Profiler (warm interleaved A/B), HeaderCallButton self-time in the critical room-open commit: develop ~6.14ms -> consolidated ~1.97ms (-68%), on the critical path; larger absolute saving on cold opens.
1ca2f38 to
918d393
Compare
Proposed changes
HeaderCallButton(rendered in the RoomView navigation header) ran a heavy hook stack on every mount: ~12 reduxuseSelectorcalls (8 of them one-per-setting), a nativeexpo-camerapermission hook, and a duplicated zustand selector — none of which is needed in the first frame. Because React Navigation renders the header twice during the push transition, this work runs ×2 in the critical room-open commit.This PR consolidates that per-mount work in the shared call hooks (behavior-preserving — same return values; every consumer benefits: RoomView header, Room Info, Room Actions, Sidebar, VideoConferenceEnded):
useVideoConfCall: 8 separateuseSettingreads → 1useAppSelector(..., shallowEqual)(−7 store subscriptions)useVideoConf: reactiveuseCameraPermissions()→ imperativeCamera.getCameraPermissionsAsync()/requestCameraPermissionsAsync()at press time (removes a native-backed hook from every render; reads fresh permission at press instead of a reactive snapshot)HeaderCallButton: drop the duplicateuseIsInActiveVoipCall()(reuseisInActiveCallfromuseNewMediaCall)Benchmark (iOS sim, Argent React Profiler; warm, interleaved A/B) —
HeaderCallButtonself-time in the critical room-open commit, withMessageTouchableas a warmth control (flat across runs):HeaderCallButtonself→ −68% (~4.2ms warm) in the component's own self-time, on the critical path; larger absolute saving on cold opens. (Profiling warm-up varies the commit total ~2.4× run-to-run, so per-component self-time is the trustworthy metric.)
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1195
Follow-up to NATIVE-1184 (markdown perf, #7348) from the same RoomView cold-open profiling.
How to test or reproduce
Screenshots
No visual change — header/call button render identically.
Types of changes
Checklist
Further comments
The hooks are shared, so the change is internal and preserves each hook's public API and return values — verified by the existing
CallSectionandRoomInfoButtonssnapshot tests (no snapshot changes) and on-device (VoIP call sheet still opens).An alternative was considered: deferring
HeaderCallButton's mount past the room-open transition withInteractionManager. That only relocated the work (no net CPU saving) and added a late-appearing button plus an extra re-render commit, with a warm benefit within measurement noise. Consolidation was chosen instead because it reduces the work itself, stays on the critical path, and benefits every consumer.Summary by CodeRabbit