feat(ui-ios): drive onFrame from CADisplayLink and add frame-time metrics - #7754
Draft
proggeramlug wants to merge 7 commits into
Draft
feat(ui-ios): drive onFrame from CADisplayLink and add frame-time metrics#7754proggeramlug wants to merge 7 commits into
proggeramlug wants to merge 7 commits into
Conversation
…rics onFrame was pumped from the 8ms NSTimer via js_frame_pump_default(), which is neither vsync-aligned nor better than millisecond-resolution (js_timer_now truncates via as_millis). At a 8.33ms/120Hz budget that made iOS frame pacing unmeasurable rather than merely imprecise. Adds perry-ui-ios/src/frame_driver.rs: a real CADisplayLink on the main run loop in NSRunLoopCommonModes (the default mode goes silent for the whole of a UIScrollView drag), rebasing the link's CACurrentMediaTime timestamp onto onFrame's documented "ms since app start" epoch so the contract holds while gaining sub-microsecond resolution. The timer pump keeps driving timers, microtasks, the stdlib pump and the GC step; it now only reconciles the link's paused state, since a paused link cannot observe a new callback and wake itself. Adds perry-runtime/src/frame_metrics.rs (PERRY_FRAME_STATS=1, off by default): p50/p95/p99 frame times, longest frame and dropped-frame counts over a rolling 8192-frame window. Drops are derived from the display's nominal frame duration rather than a hardcoded 60Hz, so the same code is correct at 120Hz. Deliberate pauses are excluded instead of recorded as one huge frame, one hitch cannot dominate the drop count, and report() returns its sample count so "all frames fast" is distinguishable from "the link never ticked". Covered by 11 unit tests in perry-runtime --lib (per-PR CI runs --lib --bins, so coverage under crates/*/tests would not gate this). Verified: 1999 runtime tests pass; perry-ui-ios compiles clean for aarch64-apple-ios and aarch64-apple-ios-sim; clippy, fmt, file-size, addr-class and gc_runtime_root_holders gates all pass. Known limitation: iPhone caps the 120Hz request unless the bundle sets CADisableMinimumFrameDurationOnPhone=YES, which Perry's generated Info.plist does not yet emit. Documented in docs/src/ui/on-frame.md.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
added 5 commits
August 10, 2026 10:44
…le scan register_widget only ever pushed: every widget ever created stayed retained for the process lifetime, and clear_children recovered handles by scanning the whole table once per removed subview - O(children x every widget ever created) against a vector that only grew, so repeated list rebuilds were quadratic and degraded the longer the app ran. Adds a ptr->handle reverse index (O(1) lookup) and releases removed subtrees in clear_children/remove_child. Release is recursive: a list row is a container with children, and freeing only the directly-removed child would strand its descendants behind a parent nobody can reach. Slots are tombstoned, never reused. Handles must stay inside the < 0x100000 handle band, which leaves no bits for the generation counter safe reuse would need; without one a recycled index lets a stale handle drive a different widget. A tombstone makes a stale handle a safe None instead. Also unblocks device measurement: PERRY_* env vars now reach a simulator or device launch (SIMCTL_CHILD_ / devicectl --environment-variables), and perry.toml high_refresh_rate emits CADisableMinimumFrameDurationOnPhone, without which iPhone caps at 60Hz regardless of what CADisplayLink requests. Frame stats now auto-report every N frames since an iOS app has no exit at which to print a summary. perry-ui-ios is cfg'd to iOS so none of the widget-table change is host testable; verification is on-device. The config reader and metrics changes are covered by 5 + 13 unit tests.
…argets ld64 scans archives left-to-right once, and the UI lib is added after the runtime and stdlib. Anything the UI lib references but does not define is undefined for the first time only at that point, by which time those archives have been scanned and will not be revisited. This made --target ios unlinkable outright: PerryTestExitTarget::test_exit calls Stdout::flush, whose only remaining definition after strip-dedup lives in libperry_stdlib.a, and the link died on that one symbol. Reproduced with the untouched docs/examples/platforms/ui/ios_app.ts, so it was not specific to any one app. Repeating the archives is the standard remedy and is cheap - the linker extracts only members resolving a still-pending undefined. The Windows branch already solved the same problem with /WHOLEARCHIVE and documents it inline. Also adds benchmarks/ios-ui/, a frame-time benchmark cycling idle, property-updates, add-remove, animation and text-heavy phases with a printed marker per phase so [frame-stats] lines are attributable.
The CADisplayLink driver called js_frame_tick / js_frame_metrics_* as perry_runtime::… Rust paths. perry-ui-ios depends on perry-runtime as an rlib while the final binary also links libperry_runtime.a, so that instantiated a SECOND copy of the runtime inside the UI crate - its own thread-local arena, GC state and statics. Frame callbacks registered in one copy's queue while the driver drained the other's, and memory allocated by one allocator was freed by the other, aborting on device with malloc: *** error for object 0x…: pointer being freed was not allocated Every runtime entry point in app.rs was already extern "C" for exactly this reason; the convention is load-bearing, not stylistic. The failure was timing-dependent, which made it easy to misread: the first device build ran two full benchmark cycles clean before later builds aborted seconds after launch. Localized with two isolation probes - isolate_churn.ts (3,230 clear+rebuild rounds, ~323k widgets, no crash, which cleared the widget-table release path) and isolate_frame.ts (onFrame only, no structural mutation, crashed immediately). After the fix: isolate_frame ran 15,600 frames at a flat 8.33ms p99 with zero dropped, and the full 5-phase benchmark completes cleanly on device. Adds js_frame_metrics_enabled() as the C-ABI form of the predicate. Also restores the Apple archive-repeat link fix with an accurate comment: it is independently necessary (verified by reverting it - the untouched ios_app.ts still fails on Stdout::flush) and was not the cause of the crash.
…phase Releasing a widget dropped the table's last strong reference inline, so the UIView could deallocate inside CoreAnimation's pre-commit phase - onFrame runs from a CADisplayLink - while UIKit still held it in pending-layout bookkeeping. Retired views now go to a PENDING_RELEASE queue drained from the main-thread timer pump, the same quiescent point the pump already uses. Handles die immediately; only the dealloc waits. The drain swaps the queue out before dropping, because a dealloc can re-enter release_widget and would otherwise panic on the RefCell borrow. HONEST STATUS: this is motivated by correctness, NOT established as the cause of the one observed NSInvalidArgumentException 'object cannot be nil' crash. Isolation probes failed to reproduce that crash - churn from onFrame ran 690 rounds clean both with and without a UIScrollView parent - and full-bench runs are clean both before and after this change, so the change is currently unfalsified rather than verified. The single crashing run had a human touching the screen; the link is in NSRunLoopCommonModes so it keeps firing during UITrackingRunLoopMode, and no probe here injects touches. Adds isolate_churn_frame.ts alongside the existing probes.
…ified The PENDING_RELEASE deferral was added on the hypothesis that deallocating a UIView inside CoreAnimation's pre-commit phase caused the NSInvalidArgumentException seen under scroll. Device bisects disproved it: the crash reproduces with release deferred, with release disabled entirely, with the display link in NSDefaultRunLoopMode, and - decisively - with this PR's display-link driver removed altogether and onFrame back on the pre-PR NSTimer pump. The abort is a pre-existing bug this PR's benchmark merely exposes. An isolation probe released ~323k widgets inline with no fault, so immediate release is fine empirically. Shipping a queue whose justification had evaporated would have left unexplained machinery behind, so it is removed. Also restores every bisect arm (run-loop mode, driver install, pump wiring) and raises the benchmark's idle phase from 20 to 60 rows - 20 fits on screen, so that phase could not be dragged and was silently reporting the static path as though it were a scroll result.
…ted backtrace An uncaught ObjC exception aborted through SIGABRT leaving only 'libc++abi: terminating due to uncaught exception' plus raw return addresses with no load address to symbolicate against - which is exactly what made #7763 expensive to chase. NSSetUncaughtExceptionHandler now reports the exception name, reason, and [exception callStackSymbols], which NSException captures at RAISE time and the ObjC runtime symbolicates in-process. No dSYM matching, no offline .ips pass. Written to stderr because that is what devicectl --console streams back during an interactive device session. Adds isolate_autoscroll.ts: programmatic scroll (sawtooth setContentOffset) plus live label mutation. It does NOT reproduce #7763 - 13,200 frames clean - which narrows that crash to something only a real touch sequence provides, since setContentOffset runs neither the gesture recognizer nor UITrackingRunLoopMode.
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
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.
Why
Perry currently cannot answer any frame-related question about its own iOS UI, because nothing observes frames.
onFramewas driven from the 8 ms repeatingNSTimerinapp.rsviajs_frame_pump_default(). That is wrong for anything frame-shaped in two independent ways:NSTimerbeats against a 8.333 ms (120 Hz) or 16.667 ms (60 Hz) refresh instead of landing on it, and is subject to run-loop coalescing.js_timer_now()iselapsed().as_millis() as f64— whole milliseconds, ~12% of a 120 Hz frame budget. A p99 computed from that clock is noise.docs/src/ui/on-frame.mdadvertised "CADisplayLink wiring TBD" for Apple platforms. This lands it for iOS.What
perry-ui-ios/src/frame_driver.rs— a realCADisplayLinkdrivingjs_frame_tick. The timer pump keeps drivingsetTimeout/setInterval, microtasks, the stdlib pump and the GC step; it no longer drives frames.Three details are load-bearing rather than incidental:
NSRunLoopCommonModes, not the default mode. AUIScrollViewdrag switches the run loop toUITrackingRunLoopMode; a link registered only in the default mode goes silent for the entire gesture — exactly the window a scrolling benchmark exists to measure.CADisplayLink.timestampis on theCACurrentMediaTimebase (seconds since boot) whileonFrame's contract is monotonic ms since app start. Passing the raw media time would silently break every app doingt - startTime. The first tick pins the media clock againstjs_timer_now(); later ticks report an offset from that pin, keeping the epoch while gaining sub-microsecond resolution.perry-runtime/src/frame_metrics.rs—PERRY_FRAME_STATS=1, off by default. p50/p95/p99 frame times, longest frame, dropped-frame count over a rolling 8192-frame window.Drops derive from the driver-supplied nominal frame duration rather than a hardcoded 60 Hz, which is what makes the same code correct on ProMotion:
dropped = round(interval / expected) - 1, so jitter within half a frame is not a drop and a doubled interval is exactly one.Three cases that would otherwise produce confident wrong numbers are handled explicitly:
js_frame_metrics_report()returns its sample count. Zero distinguishes "every frame was fast" from "the link never ticked" — indistinguishable in a summary line otherwise, and the second is the failure mode where a probe reports success without its subject having run.Collection keeps the link awake on its own, so an app with no
onFramesubscribers — a plain scrolling list — still produces a full frame trace.Testing
11 unit tests in
perry-runtime --lib: nearest-rank percentiles, steady 120 Hz, the doubled-interval case, sub-half-frame jitter, the attribution cap, discontinuity, non-monotonic driver timestamps, ring-buffer wrap, the disabled path, report-zero.Unit tests rather than an integration suite deliberately — per-PR CI runs
--lib --bins, so coverage undercrates/*/tests/would not gate this.Sabotage-checked, not merely green: flipping the drop threshold from
spans > 1.0to> 2.0turnsa_doubled_interval_counts_as_exactly_one_dropped_framered. The tests can fail.Also verified:
perry-runtimelib tests pass, 0 failedperry-ui-ioscompiles clean foraarch64-apple-iosandaarch64-apple-ios-simcargo fmt --checkcleancheck_file_size.sh,addr_class_inventory.py,gc_runtime_root_holders.pyall passobjc2-quartz-corepinned todefault-features = false— the default set drags inobjc2-metal/-core-video/-core-graphics; narrowing toCADisplayLink+CAFrameRateRangeadds zero new packages toCargo.lockNot in this PR
CADisableMinimumFrameDurationOnPhone = YESin the bundle'sInfo.plist, which Perry's generated plist does not emit. Without it the frame-rate request is capped at 60 Hz on iPhone (iPad ProMotion has no such opt-in). Adding it touches the compiler driver and is a battery-policy decision — probably wants to be opt-in viapackage.json. Documented inon-frame.md.UIApplicationDidBecomeActiveobserver, so an app suspended without a pause transition the driver observes still shows one long interval. The attribution cap bounds the damage; treat a lone multi-secondmaxas a lifecycle artifact.onFramefrom their main loops, unchanged.Testing on device
Not yet run on hardware — this is the instrument, not a measurement. Real-device Release numbers need signing setup. The intended next steps are launch→first-frame signposts and the widget-table lifetime fix (
register_widgetonly ever pushes;clear_childrenresolves handles by linear scan over a vector that never shrinks), both of which distort list benchmarks today.