MOB-124 rounds 1-2: measure the render pipeline, then fix what it showed

CHANGELOG.md

@@ -10,6 +10,78 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob).
 
 ## [Unreleased]
 
+Everything below is unreleased work from the MOB-124 rendering-performance
+epic. Nothing here has shipped to Hex.
+
+### Added
+- **`Mob.RenderStats` — per-frame render instrumentation** (MOB-125). Records
+  the user's `render/1`, tree expansion, component reconcile, the renderer's
+  prepare walk, `register_tap`, `:json.encode`, and `set_root` as seen from the
+  BEAM, plus node count, `register_tap` call count and payload bytes. Off by
+  default behind a `:persistent_term` flag; readable over dist with
+  `Mob.RenderStats.summary/0`, which reports p50/p95/max with the sample size
+  `n` per stage. `verify_taps/1` enables an opt-in second walk that cross-checks
+  the tap count. See `decisions/2026-09-01-render-instrumentation.md`, including
+  why `total_us` must not be compared against a frame budget.
+
+### Performance
+- **`:scroll` can build its content lazily, with `lazy: true`** (MOB-128). A
+  column that is the direct content of a **vertical** scroll uses `LazyVStack`
+  rather than `VStack`, so only the rows on screen are built. Opt-in: rows below
+  the fold are never built, so `Mob.Test.element_frames` / `tap_id` cannot
+  address them and `scroll_to(:bottom)` under-scrolls, exactly as for
+  `lazy_list`. A `row` under a horizontal scroll, and anything deeper than a
+  scroll's direct child, stay eager.
+
+  This is the **iOS** half. It is verified to render identically to the eager
+  path but its win is **not** independently measured on iOS — the equivalent
+  Android change (in `mob_new`) measures a 500-row screen going from 498.9 ms to
+  115.8 ms of main-thread work per frame. See
+  `decisions/2026-09-02-lazy-scroll-on-ios.md`.
+- **iOS `set_root` is 47% faster on a dense screen** (MOB-135). The native
+  deserialiser probed ~100 prop keys into every node's props regardless of node
+  type — 104 probe sites, 99 distinct keys, 8 type guards — to read the three to
+  five props a node actually carries. It now enumerates each node's own props
+  once and resolves keys to slots. On a 200-row screen (1627 nodes, 207 KB):
+  `set_root` 7625 → 4040 µs, whole frame 13002 → 9403 µs. Purely
+  native-internal; no wire-format change.
+- **`register_tap` no longer logs once per exhausted call** (MOB-133). On a
+  screen with more than `MAX_TAP_HANDLES` (256) interactive elements, the
+  exhaustion path called `NSLog` synchronously per overflowing node — 359 times
+  per frame on a 200-row screen, 13 ms of a 27 ms frame. The count is now
+  reported once per frame from `set_root`, taking `register_tap` from 13004 µs
+  to 81 µs.
+- **`clear_taps` frees only the slots that were used**, instead of walking all
+  256 every frame.
+
+### Fixed
+- **`ErlNifEnv` leak on the rescued render path** (MOB-133). Bounding
+  `clear_taps` by a high-water mark that only `set_root` wrote leaked one
+  `ErlNifEnv` per tap, per frame, whenever a render raised between `clear_taps`
+  and `set_root` — a path `Mob.Sender.commit/1` deliberately rescues, so it
+  accumulated silently. `register_tap` now maintains the mark. See
+  `decisions/2026-09-02-register-tap-owns-the-table-high-water-mark.md`.
+- **`tap_exhausted_count` no longer leaks across frames.** It was 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. Reset in `clear_taps`
+  now. The iOS increment also moved inside the tap mutex, matching Zig.
+- **`Mob.Sender` no longer discards render-stat frames at navigation
+  boundaries.** Both activation paths deleted a queued tree and threw its
+  measurement away with it, so dropped frames were undercounted at exactly the
+  transitions the epic measures.
+- **Staged render-stat frames are swept by age**, so a screen killed between
+  `hand_off/1` and `Mob.Sender.render/5` cannot leave an entry nothing claims.
+
+### Known issues (found while measuring, not fixed here)
+- **Throttle/debounce config never reaches native on either platform**
+  (MOB-134). iOS calls `mob_set_throttle_config` from the prop deserialiser, but
+  resolves the handle against the pre-swap tap table, so it always misses;
+  Android never calls it at all. Gestures behave as if every app used the
+  built-in defaults.
+- **256-element interactive cap still bites** (MOB-133). A 200-row screen
+  registers 615 handlers; 359 of them get handle `-1` and silently do not
+  respond. Only the logging was fixed.
+
 ## [0.7.38] - 2026-08-31
 
 ### Fixed

android/jni/mob_nif.zig

@@ -996,6 +996,12 @@ const ComponentHandle = extern struct {
 // after a row of buttons). With the swap a concurrent send always sees a
 // complete table (old or new), never a partial one.
 var tap_tables: [2][MAX_TAP_HANDLES]TapHandle = std.mem.zeroes([2][MAX_TAP_HANDLES]TapHandle);
+// How many slots of each table were actually written, so clearTaps only walks
+// those rather than the whole cap on every frame.
+var tap_table_used: [2]usize = .{ 0, 0 };
+// Exhausted registrations in the frame being built. Counted rather than logged
+// per call — see the note in register_tap.
+var tap_exhausted_count: c_int = 0;
 var tap_active: usize = 0; // index of the table readers resolve against
 var tap_active_count: c_int = 0; // committed handle count in the active table
 var tap_table_generations: [2]u32 = .{ 0, 0 };
@@ -1653,10 +1659,28 @@ export fn nif_set_root(
             }
         }
     }
+    // Snapshot now, log after the mutex is released. The log call writes
+    // synchronously, and holding tap_mutex across it would block concurrent
+    // mob_send_* — reintroducing, once per frame, exactly the cost this change
+    // removed from the per-call path.
+    const exhausted_this_frame = tap_exhausted_count;
+    tap_exhausted_count = 0;
+
     tap_active = 1 - tap_active;
     tap_active_count = tap_build_count;
     tap_table_generations[tap_active] = tap_build_generation;
     erts.enif_mutex_unlock(tap_mutex);
+
+    if (exhausted_this_frame > 0) {
+        // One line per frame rather than one per overflowing node. The count is
+        // the useful number: it says how many interactive elements are silently
+        // inert, which the per-call line never made obvious.
+        loge_nif(
+            "register_tap: pool exhausted (cap={d}) — {d} interactive element(s) in this frame have no handler and will not respond",
+            .{ MAX_TAP_HANDLES, exhausted_this_frame },
+        );
+    }
+
     const transition_cstr: [*:0]const u8 = @ptrCast(&transition);
 
     var attached: c_int = 0;
@@ -1705,7 +1729,12 @@ export fn nif_register_tap(
         // no-op on an out-of-range handle, so -1 is a safe "no handler
         // wired up" sentinel here — the interactive prop silently does
         // nothing instead of taking the screen down.
-        loge_nif("register_tap: pool exhausted (cap={d}) — returning unhandled sentinel", .{MAX_TAP_HANDLES});
+        // Deliberately not logged here. This is reached once per interactive
+        // node beyond the cap — measured on iOS at 359 times per frame on a
+        // 200-row screen — and the log call writes synchronously. On iOS that
+        // logging alone was 47% of the frame. Reported once per frame from
+        // set_root instead.
+        tap_exhausted_count += 1;
         return erts.enif_make_int(env, -1);
     }
 
@@ -1724,6 +1753,14 @@ export fn nif_register_tap(
     slot.tag = erts.enif_make_copy(slot.tag_env, tag_term);
     slot.identity_start_generation = tap_build_generation;
     tap_build_count += 1;
+    // The high-water mark has to be raised HERE, not in set_root. clear_taps
+    // frees exactly `used` slots, and a frame can register taps and then never
+    // reach set_root — Mob.Renderer.render/4 calls clear_taps, then prepare,
+    // then :json.encode, then set_root, and Mob.Sender.commit/1 rescues anything
+    // that raises in between. Recording the mark only at set_root left those
+    // slots' tag_envs uncleared and unreachable: one leaked ErlNifEnv per tap,
+    // per failed frame, forever, on a path deliberately designed to survive.
+    tap_table_used[@intCast(1 - tap_active)] = @intCast(tap_build_count);
     return erts.enif_make_int(env, handle);
 }
 
@@ -1746,7 +1783,8 @@ export fn nif_clear_taps(
     // frame. The freshly built table is swapped in at set_root.
     const build = &tap_tables[1 - tap_active];
     var i: usize = 0;
-    while (i < MAX_TAP_HANDLES) : (i += 1) {
+    const used = tap_table_used[@intCast(1 - tap_active)];
+    while (i < used) : (i += 1) {
         const h = &build[i];
         if (h.tag_env != null) {
             erts.enif_free_env(h.tag_env);
@@ -1763,7 +1801,13 @@ export fn nif_clear_taps(
         h.last_y = 0;
         h.seq = 0;
     }
+    tap_table_used[@intCast(1 - tap_active)] = 0;
     tap_build_count = 0;
+    // Reset here, not only in set_root. set_root reports and clears the count,
+    // but a frame that overflows and then never reaches set_root would otherwise
+    // carry its overflow into the next frame's report — which claims to describe
+    // "this frame". clear_taps is the one entry point every frame runs.
+    tap_exhausted_count = 0;
     return erts.ok(env);
 }
 

decisions/2026-09-01-render-instrumentation.md

@@ -0,0 +1,70 @@
+# Measuring the render pipeline before changing it
+
+- Date: 2026-09-01
+- Status: accepted
+- Implements: MOB-125, first step of MOB-124
+- Builds on: `2026-08-28-sender-serialises-render.md`
+
+## Context
+
+MOB-124 proposed four fixes for rendering performance — retained native trees,
+stable identity, lazy scroll containers, and LiveView-style wire patching. They
+attack four different stages, and the pipeline had never been measured on a
+device. Picking between them on intuition is how weeks go into the wrong one.
+
+## Decision
+
+`Mob.RenderStats` records per-frame stage timings, readable over dist. It is off
+by default behind a `:persistent_term` flag, stores into a bounded ETS ring
+owned by a GenServer, and reports p50/p95/max with the sample size `n` — not a
+mean, because frame cost is not normally distributed and the tail is what a user
+feels as stutter.
+
+Three things about its design were not obvious and were each got wrong first.
+
+**A frame spans two processes.** `Mob.Screen.Server.paint/4` runs `render/1`,
+expansion and reconcile in the screen process, then casts to `Mob.Sender`, which
+runs prepare, encode and `set_root`. Timing state in the process dictionary
+therefore cannot span a frame. The screen hands its partial frame to the sender
+with `hand_off/1`, and the sender resumes it before committing. The frame is
+**paired with its tree when the render cast is dequeued**, not looked up again at
+flush time: `Mob.Sender.sync/1` is called from the router, a different process,
+so a flush can land between a screen's two casts and would otherwise commit tree
+N-1 while holding frame N.
+
+**The ETS table needs an owner process.** Creating it inside `enable/0` makes it
+owned by whoever called — over `:rpc.call/4` that is a transient process, so the
+table dies the instant enabling returns and every later write goes nowhere.
+
+**`taps` comes from the call counter, not a tree walk.** Recounting handle-valued
+props on the finished tree cost 120 ns per node — about 90% of the meter's whole
+overhead — to reproduce a number `accumulate/2` already had exactly, for free, as
+the calls happened. The walk survives behind `verify_taps/1` as an opt-in
+cross-check, because the two disagreeing is how a counting bug announces itself.
+
+## What `total_us` is not
+
+It is stamped in the screen process and closed in the sender, so it spans two
+casts and the sender's mailbox, and it includes the meter's own cost. On a
+physical device it was observed **exceeding an externally measured frame by
+several milliseconds** — only possible because it covers time outside the frame.
+
+It must not be compared against a frame budget or used to compare
+configurations. For that, sum the stages, or measure from outside: drive one
+render and block on `Mob.Sender.sync/1`. That external method is what the epic
+now uses for verification, and it is the reason two published sets of numbers
+had to be retracted.
+
+## Consequences
+
+The instrument was wrong in nine ways across two adversarial reviews before it
+was right, and four of those defects had already produced published conclusions:
+percentiles one rank high (`round/1` where nearest-rank wants `ceil/1 - 1`, which
+made p95 the single worst frame for any run under 20), `taps` undercounting
+12-fold, dropped frames polluting the byte and duration percentiles, and stage
+percentiles computed over different populations without saying so.
+
+The lesson worth keeping is that a meter used to rank work needs the same
+adversarial treatment as the work — and that every fix needs a test which fails
+when that fix alone is reverted. Checking that one at a time caught two "fixes"
+that changed nothing.

decisions/2026-09-02-lazy-scroll-on-ios.md

@@ -0,0 +1,73 @@
+# :scroll builds its content lazily (iOS)
+
+- Date: 2026-09-02
+- Status: accepted
+- Implements: MOB-128, part of MOB-124
+- Companion: `mob_new/decisions/2026-09-02-lazy-scroll-on-android.md`
+
+## Context
+
+The Android half of MOB-128 measured a 200-row screen at 134 ms of main-thread
+work per update, 107 ms of it recomposition, against 45 ms for the whole
+BEAM-plus-NIF pipeline. Making `:scroll` lazy took the main-thread cost to 82 ms
+and made it flat in list length.
+
+iOS has the same shape: `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`.
+
+## Decision
+
+A column that is the direct content of a **vertical** `scroll` builds its
+children with `LazyVStack` instead of `VStack`, **when that scroll opted in with
+`lazy: true`**. Everywhere else the stacks stay eager.
+
+**Opt-in**, matching Android, because laziness has observable consequences
+beyond speed. 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: `scroll_to(:bottom)`
+under-scrolls and `screenshot_tour/3` truncates. `lazy_list` already makes that
+trade explicitly; applying it silently to every scroll would change harness
+behaviour under apps that never asked for it.
+
+**Vertical only.** A `LazyVStack` under a horizontal `ScrollView` would be lazy
+on the wrong axis — the vertical axis there is bounded and never scrolls, so
+rows below the fold would never be built at all rather than built on demand.
+
+**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 and
+making it lazy would buy nothing — the column underneath is the stack with 200
+children.
+
+**And the column is kept, not flattened away.** Android flattens the column and
+uses its children as the list items, guarded by a check that the column's props
+are layout-neutral. That guard is cheap there because props are a map. On iOS
+`MobNode` exposes typed properties — padding, background, alignment, borders,
+corner radius, `nativeViewId` — so an exhaustive "is this column neutral" test
+would be a long list, and **missing one entry would silently drop something the
+user can see**. Passing a `lazyContainer` flag down one level instead keeps every
+modifier on the column exactly where it was.
+
+`MobEitherStack` exists because SwiftUI cannot choose between `VStack` and
+`LazyVStack` inside one expression. The `if` produces two different view
+identities, which is fine here: `lazy` is fixed for a given node's position in
+the tree, so it never flips for a live view.
+
+## Status of the evidence
+
+The change is **verified correct** — a 200-row screen renders identically to the
+eager path on the simulator, 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 directly; iOS has no equivalent that can
+be read from a script, and `set_root` dispatches to the main thread
+asynchronously, so no BEAM-side instrument can see the work. The simulator runs
+on a development Mac and is not representative of a phone, which is exactly the
+mistake that made the first round of this epic's numbers worthless.
+
+So this is parity-by-construction on the mechanism proven on Android, not an
+independently measured iOS result. Measuring it needs main-thread instrumentation
+on a physical device — a `CATransaction` completion timer around `setRoot` would
+do it — and that is worth doing before claiming an iOS number.

decisions/2026-09-02-prop-key-dispatch.md

@@ -0,0 +1,62 @@
+# The native deserialiser resolves prop keys in one pass
+
+- Date: 2026-09-02
+- Status: accepted
+- Implements: MOB-135, arising from MOB-124 measurement
+- Builds on: `2026-09-01-render-instrumentation.md`
+
+## Context
+
+Measured on a 200-row screen (1627 nodes, 207 KB payload), `set_root` was the
+largest single stage of a frame on both platforms. Splitting it open on iOS:
+
+| phase | µs |
+|---|---|
+| `NSData` copy of the payload | 22 |
+| `NSJSONSerialization` parse | 1345 |
+| **`mob_node_from_dict`** | **5900** |
+| frame-id collect + adopt | 70 |
+
+Converting an already-parsed `NSDictionary` into `MobNode` objects cost 4.4x
+what parsing the JSON did. That ratio was the anomaly.
+
+The cause: `mob_node_from_dict` probed every prop key it knows into every node's
+`props`, regardless of node type — **104 probe sites over 99 distinct keys, with
+only 8 guarded by a node-type check**. At 207 KB across 1627 nodes a node
+averages 127 bytes, roughly 39 of which 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 rehashes the literal's bytes (CFString caches
+nothing) and, on a hit, runs a character compare because the parsed key is a
+different object from the literal.
+
+The parse touches each byte once. The conversion did constant work per node with
+no relationship to node size. That is the whole gap.
+
+## Decision
+
+Enumerate the node's own props once, resolve each key to a slot through a
+`dispatch_once` table, and let the deserialiser read slots.
+
+**Statement order is preserved, and that is the reason for an indexed array
+rather than a switch inside the enumeration.** Prop precedence depends on order
+in three places — `text` before `value` for a text field, generic `width`/`height`
+before canvas, generic `corner_radius` before sheet. A switch would have
+reordered those and broken them silently.
+
+Measured, iOS simulator, same screen: `set_root` 7625 → 4040 µs, whole frame
+13002 → 9403 µs. A 28% frame reduction, purely native-internal — no wire-format
+change and no Elixir coordination.
+
+## Not done here
+
+A single-pass SAX parse straight into `MobNode`, skipping the intermediate
+`NSDictionary` entirely, has a higher ceiling (an estimated 0.9-1.2 ms replacing
+7.25 ms) but means owning a JSON parser — escapes, surrogate pairs, number forms,
+UTF-8 validation, depth limits on adversarial input. Its marginal gain over this
+change is about 1.1 ms, so it is not worth the risk yet.
+
+Android has the same root cause by a different mechanism: no probe amplification,
+but triple materialisation (Java `String`, an `org.json` tree, a second map copy
+per node, then the node class). One principle fixes both — never materialise a
+generic key/value object graph; scan the wire bytes once and dispatch each key by
+its bytes into the typed node representation. Android remains unfixed.