MOB-124 rounds 1-2: measure the render pipeline, then fix what it showed - #118
Merged
Conversation
Every proposal in MOB-124 is a guess without this. The pipeline has never been measured on a device, and the four candidate fixes attack four different stages — identity and laziness attack the native rebuild, retained trees attack navigation, wire patching attacks encode and decode. Picking between them on intuition is how you spend weeks on the wrong one. Mob.RenderStats records per frame: the user's render/1, tree expansion, component reconcile, the renderer's prepare walk (which includes one register_tap per interactive node), :json.encode, and set_root as seen from the BEAM — plus node count, interactive-node count, and payload bytes. Readable over dist with Mob.RenderStats.summary/0, which reports p50/p95/max rather than a mean, because frame cost is not normally distributed and the tail is what a user feels as stutter. The switch is a :persistent_term read rather than a GenServer or an ETS lookup, so the shipped path costs 49ns per call and 0.29us per frame — 0.08% of a ~378us frame. Measured, not assumed; that number matters because unlike the recording path it runs on every frame of every app. Two things the measuring found in the meter itself: - total_us was being stamped after the node-counting walk, so it inflated the number it exists to describe. Stamped before now. - interactive?/1 probed all eighteen handler names per node — ~14k map lookups on a 780-node tree. Walking the node's own props against a MapSet instead took recording overhead from 80% to 42%. Still not free, which is why the moduledoc says to read the stages rather than an enabled total_us. No version bump: nothing releases until the epic works end to end. Tests: 18, covering that it records nothing and still runs the pipeline when disabled, that stage timings and the node/tap walk are correct, that the ring buffer keeps the newest frames, and that a stage never recorded reads as nil rather than zero — zero would claim the stage is free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The instrumentation recorded nothing on device. Mob.Screen.Server.paint/4 opens the frame and times render/expand/reconcile in the SCREEN process, then hands the tree to Mob.Sender as a cast — so prepare, :json.encode and set_root run in the SENDER process, where the process-dictionary accumulator does not exist. finish/2 returned :ok without storing, every frame. I designed that cast in MOB-110 and still wrote "the whole pipeline runs in one screen process" in the moduledoc. It does not, and the moduledoc now explains why, since the split is the non-obvious thing about this module. The screen times its stages, hand_off/1 sends the partial frame to the sender, and the sender resumes it before committing. Sent as its own cast rather than threaded through Mob.Sender.render/5,6: those are shipped render entry points and one is already probed with function_exported?/3 for version skew, so widening them to carry measurement scaffolding would be the wrong trade. Frames the sender drops — superseded by a newer tree, or belonging to a screen that is not active — are now recorded with committed: false rather than discarded. A pipeline throwing away BEAM-side work is a finding, not a detail, and this epic needs to know how often it happens. Also fixed: the ETS table was created by whoever called enable/0 first. Over :rpc.call that is a transient process, so the table died the instant enabling returned and every later write went nowhere — which is why this was invisible twice over. A process owns it now. The first test I wrote for this passed with the fix reverted: it called hand_off/1 directly, so it proved the mechanism worked without proving paint/4 used it. Added a test that drives a real router in render mode with a stub NIF, and removing the hand_off call from paint/4 now fails it. Found by the subagent building the benchmark app, by checking that the meter actually recorded something before trusting it. Tests: 22 in this file, 1362 total. No version bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
prepare dominates a dense frame, and it does two unrelated jobs: pure-Elixir prop and theme resolution, and one register_tap NIF call per interactive node. Which of the two it is decides what the fix looks like, so they are timed apart. The split answered it immediately. On a 200-row screen (1627 nodes, 615 register_tap calls) register_tap is 13.0ms of a 27.4ms frame — 47%. Under the 256-handle cap it costs 0.76us per call; over it, 21us. A 28x cliff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every number MOB-124 will be decided on comes out of this module, so a meter that is subtly wrong is worse than no meter — it produces confident, specific, wrong conclusions. A review pass found five ways the first version was wrong, four of which affect numbers already reported. taps counted interactive nodes, not register_tap calls. A node carrying on_tap + on_long_press + on_double_tap makes three NIF calls and contributed one, and nine handler props the renderer registers (the swipe and scroll families) were missing from the set entirely — so any scrolling screen, the exact case MOB-128 is about, undercounted silently. A direct probe through Mob.Renderer.render/4 showed 12 calls reported as 1. taps now counts handle-valued props, which makes it the same quantity as register_tap_us_n and therefore a real cross-check: the two disagreeing now means one has a bug. at/2 used round/1 where nearest-rank wants ceil/1 - 1. Every reported p50 was one rank high — the 60th percentile at n=10 — and every p95 was literally the worst frame for any run under about 21 frames, which is the range a short measurement run lands in. The old test used [1,1,1,1,1,1,1,1,1,500] and could not fail: nine identical values hide a rank error, and p95 was not asserted. Dropped frames contaminated the percentiles. drop_frame/1 set bytes: 0 rather than nil, so uncommitted frames survived the nil filter: one committed frame at 5000 bytes among nine drops reported a byte p50 of 0. Their total_us is screen-side work plus however long the frame waited in the sender, not a render. Percentiles now come from committed frames only, with committed and dropped counts alongside so `frames: 40` can never read as 40 rendered frames. finish/2, accumulate/2 and drop_frame/1 had no enabled? guard — only time/2 checked — so recording continued after disable/0. And resume_frame(nil) was a no-op rather than an erase. finish/2 is the only thing that clears the process dictionary key and it is skipped whenever the render raises, a path Mob.Sender.commit/1 exists specifically to rescue. A leftover frame was then closed against the next unrelated tree, producing one record spanning two frames whose total_us was mostly the gap between them, marked committed: true. The structural fix is in the sender. A frame was cast separately from the render it describes and looked up again at flush time, which let anything landing in between mis-pair them — Mob.Sender.sync/1 is called from the router, a different process, so a flush can arrive between a screen's two casts and commit tree N-1 while holding frame N. The frame is now bound to its tree when the render cast is dequeued and travels with it through coalescing, so a superseded tree's frame is recorded as dropped rather than reattributed. Staged frames deliberately survive a flush: the render that pairs them may still be in the mailbox behind the :flush message. A test caught that when I first cleared them. register_tap_us also wrapped Mob.Listener.handler/1 — a whereis and a tuple allocation, not the NIF. Negligible against 13 ms, but a meaningful share of the 0.76 us/call baseline that the exhaustion cliff is measured against. Every fix has a test that fails when the fix is reverted; that was checked one at a time rather than assumed. The moduledoc's disabled-cost claim is also corrected: it said six calls per frame at 49 ns, but accumulate/2 runs once per registered handler — 615 times on the benchmark, not six. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two native changes, both in the deserialiser's neighbourhood.
## The deserialiser probed ~100 keys per node to read three
mob_node_from_dict looked up every prop key it knows into every node's props
dictionary, regardless of node type: 104 probe sites over 99 distinct keys,
with only 8 guarded by a node-type check. A 207KB payload across 1627 nodes is
127 bytes per node, of which about 39 is the {"type","props","children"}
skeleton — so a typical node carries three to five props and paid ~100 hashed
lookups to find them. Each probe hashes the literal's bytes afresh (CFString
caches nothing), probes a bucket, and on a hit runs a character compare, because
the JSON-parsed key is a different object from the literal.
That is why converting an already-parsed NSDictionary cost more than four times
what parsing the JSON did: the parse touches each byte once, the conversion did
constant work per node with no relationship to node size.
Now the node's own props are enumerated once, each key resolved to a slot
through a dispatch_once table, and the deserialiser reads slots. Statement order
is untouched, which matters: prop precedence depends on it in three places
(text before value for a text field, generic width/height before canvas,
generic corner_radius before sheet). That is the reason for an indexed array
rather than a switch inside the enumeration — a switch would have reordered
those and broken them silently.
Measured on the iOS simulator, 200 rows / 1627 nodes / 207KB:
set_root 7625us -> 4040us
whole frame 13002us -> 9403us (28% faster)
The two source-contract tests that asserted on the literal props[@"..."] text
now assert the key is in the slot table and that the node builder reads the
slot, which is the same contract in the new shape.
## clear_taps freed the wrong number of slots
Bounding nif_clear_taps by tap_table_used rather than MAX_TAP_HANDLES avoided
walking 256 slots for a frame that used four. But set_root was the only writer
of that high-water mark, and a frame can register taps and never reach set_root:
Mob.Renderer.render/4 runs clear_taps, prepare (N register_tap calls),
:json.encode, then set_root — and Mob.Sender.commit/1 rescues anything raising
in between, deliberately, so one screen's bad render cannot freeze every other
screen.
So the rescued path leaked one ErlNifEnv per tap, per failed frame, permanently,
on a path built to survive. A simulation of the verbatim logic showed 4900 live
envs after 50 failed 100-tap frames, and zero with the bound restored.
register_tap now maintains the mark — it is the thing that knows a slot was
written.
tap_exhausted_count had the same shape: reset only inside set_root's reporting
branch, so a frame that overflowed and then failed carried its count into the
next frame's report, which claims to describe "this frame". It resets in
clear_taps now, the one entry point every frame runs. The iOS increment also
moved inside the mutex to match Zig — Mob.Sender serialises callers today so it
was benign, but nothing else in that file relies on that.
The stale claim that register_tap costs 0.76us under the cap is removed rather
than corrected: it came from a measurement contaminated by the per-call NSLog
this same path removed, and it cannot be reconciled with the current numbers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first review fixed five defects. A second, run against those fixes, found four more — two of them in numbers already published to MOB-124. Both activation paths in Mob.Sender deleted a pending tree and threw its frame away with it. The tree is correctly discarded: it predates the navigation boundary and must not become the newly active screen's first frame. But the BEAM work that built it was paid for, which is exactly what committed: false is for. The meter was undercounting dropped frames precisely at navigation transitions, the case this epic cares most about. Staged frames are swept by age now. Keeping them across a flush was right — the render that pairs a staged frame may still be in the mailbox behind the :flush message — but the claim that the map was "bounded anyway, one entry per live screen ref" was wrong. A screen killed between hand_off/1 and Mob.Sender.render/5 leaves an entry nothing will ever claim. The meter's own cost drops by roughly 90%. The review measured the finish/2 walk at 120 ns per node and showed the per-node prop scan was 90% of it — all to recompute a number register_tap_us_n already held exactly, for free, as the calls happened. taps comes from that counter now. The walk survives behind verify_taps/1 as an opt-in cross-check, since the two disagreeing is how a counting bug announces itself, and the tests that exercise it use that flag. The review also disproved my guess that the ETS insert and ring trim contributed: they are 0.71us per frame, 0.3% of the overhead. Left alone. Percentiles carry the n they were computed over. The stages do not share a population — register_tap_us exists only on frames that registered a handler — so a run mixing dense and tap-free screens computed it over a much smaller sample than prepare_us and reported both as bare p50s. Forty committed frames of which three are dense reads as "register_tap costs fifty times all of prepare". register_tap_us_n also moved out of the stage map, where a count sat among microsecond durations, and the docs now state that register_tap_us is nested inside prepare_us rather than being a sibling of it. The moduledoc's total_us section is rewritten. It spans two casts and the sender's mailbox and includes the meter's own cost; on a physical device it was observed exceeding an externally measured frame, which is only possible because it covers time outside the frame. It must not be compared against a frame budget or used to compare configurations — sum the stages, or measure from outside by driving one render and blocking on Mob.Sender.sync/1. Every fix has a test that fails when the fix is reverted, checked one at a time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three decision records and a filled-in Unreleased section, so the epic's reasoning survives outside the commit log and nothing is missed at release. The decisions worth keeping are the ones where the obvious choice was wrong: a frame spans two processes so timing state cannot live in one process dictionary; the meter's ETS table needs an owner or it dies with the rpc caller; taps come from the call counter because recounting them was 90% of the meter's cost; the deserialiser keeps statement order because prop precedence depends on it in three places; and the function that writes a tap slot is the one that must record it was written, because the stage that used to do it does not always run. Also records what total_us is not, since two published sets of numbers had to be retracted over it, and the external measurement method that replaced it. No version bump — none of this has shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
case .scroll: wrapped its children in an eager VStack/HStack, so every child of a scroll was built whether or not it was on screen. Only the dedicated lazyList node type used LazyVStack. A column that is the direct content of a scroll now builds with LazyVStack. Everywhere else the stacks stay eager — laziness has setup cost and only pays when most children are off screen. Two choices worth recording. The COLUMN is made lazy, not the scroll's own stack. Mob screens are written scroll > column > rows, so the scroll's own stack has exactly one child; making it lazy buys nothing because the column underneath is the stack with 200 children. And the column is KEPT, not flattened away. Android flattens it and uses its children as list items, guarded by a check that the column's props are layout-neutral — cheap there because props are a map. On iOS MobNode exposes typed properties (padding, background, alignment, borders, corner radius, nativeViewId), so an exhaustive neutrality test would be a long list and missing one entry would silently drop something visible. Passing a lazyContainer flag down one level keeps every modifier where it was. MobEitherStack exists because SwiftUI cannot pick between VStack and LazyVStack inside one expression. Its `if` yields two view identities, which is safe here: `lazy` is fixed for a node's position in the tree and never flips for a live view. Verified correct: a 200-row screen renders identically to the eager path, including multi-line wrapping labels, text fields, toggles and buttons. The win is NOT measured on iOS. Android has dumpsys gfxinfo framestats, which reports per-frame main-thread cost; iOS has no script-readable equivalent, and set_root dispatches to the main thread asynchronously so no BEAM-side instrument can see it. The simulator is a development Mac and not representative — assuming otherwise is what made this epic's first numbers worthless. This is therefore parity-by-construction on a mechanism proven on Android, and it is labelled that way in the changelog and the decision record rather than claimed as a result. No version bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 2, 2026
…uard, safer logging A review of this PR proved the prop-dispatch rewrite behaviour-identical (a pure regex substitution; the whole diff against the substituted original is 17 added lines) and the tap-table invariant sound on both platforms. It also found real problems, addressed here. ## Lazy scroll is opt-in, and vertical only The review established that making :scroll lazy by default silently degrades the test harness. Rows below the fold are never built, so they never register a frame and Mob.Test.element_frames / tap_id cannot address them; and a LazyVStack's contentSize reflects only built rows and grows as you scroll, so nif_scroll_info's contentSize - bounds under-reports and scroll_to(:bottom) under-scrolls while screenshot_tour truncates. lazy_list already makes that trade explicitly. So a scroll now opts in with `lazy: true`, wired through MobNode as a typed property. It also passed lazyContainer to children of a HORIZONTAL scroll, where only the column branch consumed it — producing a LazyVStack lazy on the wrong axis. The vertical axis under a horizontal ScrollView is bounded and never scrolls, so rows below the fold would never be built at all rather than on demand. Now vertical only. ## The enum/table drift guard MobPropKey and names[] were two independently ordered lists of 99 strings joined by `m[names[i]] = @(i)`. Insert a key mid-enum and append it to names[] — the natural mistake when the two are a hundred lines apart — and every slot after the insertion point silently reads a different prop's value on every node. The two updated source-contract tests cannot catch it: they assert the literal and the read exist somewhere, and both pass under total misalignment. names[] now uses designated initializers, so each entry names the slot it fills and drift is unrepresentable. An NSCAssert catches a slot with no name, which would otherwise resolve to nil and read as absent forever. ## The exhaustion log no longer holds tap_mutex Both platforms logged the once-per-frame pool-exhaustion line while holding the tap mutex, blocking concurrent mob_send_* on the main thread for a synchronous system-log write — reintroducing once per frame exactly the cost this change removed from the per-call path. The count is snapshotted under the lock and logged after it is released. ## Sender survives a pre-reload pending entry discard_pending/2 and flush/1 matched only a 5-tuple. mob's dev loop reloads modules onto a running BEAM, so the sender can meet state a previous version wrote; a CaseClauseError there takes down the one process every screen renders through. Both now fall through. ## Docs corrected against the code - disable/0 also clears verify_taps; the docstring now says so. - The prop-dispatch record claimed 8 node-type guards in the old deserialiser. The real count is 11 (104 sites and 99 keys were exact). - The changelog said 141 ms where the iOS decision record said 134 ms for the same measurement; both now say 141 ms p50. - The changelog implied :scroll became lazy wholesale and quoted Android numbers in mob's changelog, where the Android work lives in mob_new. It now states the actual scope and attributes the measurement. - Mob.RenderStats is a new public documented module and was ungrouped in hexdocs; added to "Testing & Debugging". This touches mix.exs, which is the release workflow's trigger path — verified harmless: tag 0.7.38 exists and 0.7.38 is already on Hex, so every publish step short-circuits. Version unchanged. Verified on the iOS simulator that a 200-row screen with `lazy: true` renders identically to the eager path — wrapping multi-line labels, text fields, toggles and buttons all at natural height. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
MOB-124 rendering-performance epic: measurement, three native fixes, and the iOS half of lazy scroll. No version bump — nothing here releases.
Companion PR in
mob_new: GenericJam/mob_new#46 (the Android half of MOB-128).Contents
1648f27d4b87d75cc18f2c1b199090fadf6Mob.RenderStats, per-frame instrumentation readable over dist, plus nine defects two reviews found in itf2552d4d48df34register_tapno longer logs per exhausted call;ErlNifEnvleak fixedd48df34a28a508a069a2d:scrollbuilds content lazily withlazy: trueWhy the epic changed direction
On a Moto G Power, a 200-row screen costs ~45 ms of BEAM-plus-NIF and ~141 ms of main-thread work. The native rebuild dominates — the opposite of what the first (BEAM-side-only) measurements suggested, because
set_rootdispatches to the main thread asynchronously and no BEAM-side instrument can see past it.Measured wins
register_tap: 13004 µs → 81 µs. The pool caps at 256; a 200-row screen registers 615, and each of the 359 overflows calledNSLogsynchronously — 13 ms of a 27 ms frame. Counted and reported once per frame instead.set_root: 7625 → 4040 µs, whole frame 13002 → 9403 µs. The deserialiser probed ~100 prop keys into every node to read the three to five it carries (104 probe sites, 99 keys, 11 type guards). Now one enumeration resolving keys to slots.The bug I introduced, and fixed
Bounding
clear_tapsby a high-water mark that onlyset_rootwrote leaked oneErlNifEnvper tap, per frame, whenever a render raised betweenclear_tapsandset_root— a pathMob.Sender.commit/1deliberately rescues, so it accumulated silently. A simulation showed 4900 live envs after 50 failed 100-tap frames.register_tapnow maintains the mark.Changed by adversarial review
The review proved the prop-dispatch rewrite behaviour-identical — applying the regex to the original body leaves a 17-line diff, all of it the new declaration, comment and loop — and proved the tap-table invariant sound on both platforms. It also found:
element_frames/tap_idcannot address them), and aLazyVStack'scontentSizereflects only built rows, soscroll_to(:bottom)under-scrolls andscreenshot_tourtruncates. Now opt-in vialazy: true, matchinglazy_list, which already makes that trade explicitly.lazyContainerwas passed to children of a horizontal scroll, where aLazyVStackwould be lazy on a bounded, non-scrolling axis — rows below the fold never built at all rather than on demand. Vertical only now.MobPropKeyandnames[]were two independently ordered 99-string lists joined by index; inserting mid-enum and appending to the table would silently shift every prop.names[]now uses designated initializers, so drift is unrepresentable, plus anNSCAssertfor an unnamed slot. The two source-contract tests could not have caught this — they pass under total misalignment.tap_mutexacross a synchronous system-log write, blockingmob_send_*on the main thread — reintroducing once per frame the cost this PR removes from the per-call path. Snapshotted under the lock, logged after.Mob.Sendercrashed on a pre-reload pending entry. mob's dev loop reloads modules onto a running BEAM, so the sender can meet state a previous version wrote; aCaseClauseErrorthere takes down the process every screen renders through.Docs corrected against code: 8 → 11 type guards, a 141/134 ms contradiction between changelog and decision record, an overstated MOB-128 scope, and
Mob.RenderStatsadded to a hexdocs group.Release safety
mix.exsis touched (one line, the hexdocs group) and is the release workflow's trigger path. Verified harmless: tag0.7.38exists and 0.7.38 is already on Hex, so the tag, release andhex.publishsteps each short-circuit. Version unchanged.Testing
1380 tests pass,
mix credo --strictclean,zig fmt/clang-formatclean,zig test tap_handle_codec5/5. Every fix in the review-response commits has a test that fails when that fix alone is reverted, checked one at a time.Verified on the iOS simulator that a 200-row screen with
lazy: truerenders identically to the eager path.Also filed
MOB-134 (throttle config never reaches native, both platforms), MOB-136 (Android
text_fieldblows up row height under unbounded constraints — breaks the ordinary eagerscroll > column > rowsidiom), MOB-137 (Androidset_roottriple materialisation, ~39 ms of the remaining 45 ms BEAM frame), and MOB-133's remaining half (359 elements per frame still get handle-1).