diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index 9ce7c25..4037a03 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -6,10 +6,10 @@ found is in `docs/LAYOUT_LOG.md`. ## A sweep for allocation, and the cost per child, is on #19 -The fourteenth sweep landed at `97fca76`. It read the whole branch again, -aimed first at what allocates and then at whatever the reading turned up. -Eight findings, the largest two both about cost per thing rather than per -frame: +The fourteenth sweep landed at `97fca76`, with the refcount removal below at +`429a4f1`. It read the whole branch again, aimed first at what allocates and +then at whatever the reading turned up. Nine findings, the largest two both +about cost per thing rather than per frame: - **A container's draw was quadratic in its children.** Five per-child steps each asked "have I done this one already?" by searching a list; at 1,600 @@ -38,14 +38,21 @@ changed; and two comments this branch's own commits falsified -- one saying `widget_trait!` takes no attributes after `76aaf06` taught it to, one saying `Scroll` clips when masking is a capability a caller opts into with `Masked`. +The ninth is the one Bryan asked for on 2026-09-21, having first been left for +the pre-gate pass: `StrongWidget` allocated a `RefCounter(Arc)` it +could never use, since it deliberately has no `Clone`, so every widget paid a +heap allocation and two atomic read-modify-writes for a count that stayed at +zero. It is the id, the sender and the type now, and `Drop` sends; +`size_of::()` is 32 rather than 40. `RefCounter` stays for +`TextureHandle`, which does clone, trimmed to what that needs. + It also left five things, with the numbers. Text allocates about six times per re-broken paragraph per frame, and removing all of them is **0.07%** of instructions (3.7107B to 3.7081B over 500 sweeping resize frames of 40 paragraphs), so the count is recorded rather than the machinery built -- allocation count and cost are not the same quantity, and this is where they -part. `StrongWidget` carries an `Arc` it can never use, since it has -no `Clone`; that and seven dead items older than #19 are listed under the -pre-gate review below rather than fixed here. +part. Seven dead items older than #19 are listed under the pre-gate review +below rather than fixed here. `docs/LAYOUT_LOG.md` has all of it. Verified: format, clippy both ways, 190 ordinary, 193 diagnostic and 189 release tests, the cold dump byte-identical to @@ -287,9 +294,10 @@ comparison work above, its review rounds past `cadfba0` are described in - **A full sweep of the branch**, `cbccfb6` -- a length of zero that printed as nothing, a ceiling that stepped off the top of the grid, and a harness setter that dropped the bound beside the length it set. -- **A sweep for allocation and the cost per child**, `97fca76` -- a - container's draw quadratic in its children, and a mask's rectangle resolved - once per fragment. Described at the top of this file. +- **A sweep for allocation and the cost per child**, `97fca76` and `429a4f1` + -- a container's draw quadratic in its children, a mask's rectangle resolved + once per fragment, and the refcount a widget handle can never raise. + Described at the top of this file. The settled design of the vocabulary rounds is in `docs/LAYOUT.md` under "Three names, and the one argument that says them". Bryan settled the API @@ -401,13 +409,9 @@ a compile error, which reads exactly like a fuzzer failure. grammars of which `core/src/util/vec2.rs` uses two, one line apart. `Align` now has `Index`, so the `if let Some` each `partial_align` writes twice collapses when that sweep reaches them. - The fourteenth sweep adds more: `StrongWidget` allocates an - `Arc` refcount it can never use, because it deliberately has no - `Clone`, so every widget pays a heap allocation and two atomic - read-modify-writes for a count that stays at zero, and `StrongWidget::refs` - has no caller; and `Size::to_uivec2`, `Size::rel`, `Size::leftover`, - `Len::to_uivec2`, `Vec2::with_x`, `Vec2::with_y`, `Vec2::ceil` and - `Layers::iter_orderless_mut` are all dead. + The fourteenth sweep adds more, all dead: `Size::to_uivec2`, `Size::rel`, + `Size::leftover`, `Len::to_uivec2`, `Vec2::with_x`, `Vec2::with_y`, + `Vec2::ceil` and `Layers::iter_orderless_mut`. 3. **Integrate the app's Iris capabilities before changing its pin.** `32f6ad8` has 45 commits not reachable from the review branch; shared UI ownership, richer masks, Android support, and app-side performance work diff --git a/docs/LAYOUT_LOG.md b/docs/LAYOUT_LOG.md index 1c3a1dd..ce82560 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -10,7 +10,7 @@ the current plan is in `docs/HANDOFF.md`. Over the whole branch again, aimed first at what allocates -- `Vec`s, `Arc`s, capacity dropped and re-grown -- and then at whatever else the reading turned -up. Eight findings. +up. Nine findings. - **A container's draw was quadratic in its children.** Every per-child step in one draw asked "have I done this one already?" by searching a list: @@ -116,7 +116,24 @@ up. Eight findings. does not clip it, because masking is a capability a caller opts into by putting a `Masked` around it. The code is right; the reason was not. -Five things the sweep **looked at and left**: +- **`StrongWidget` carried a `RefCounter(Arc)` it could never + use.** It deliberately has no `Clone`, so the count never rose above zero, + `RefCounter::drop` always answered true, and `StrongWidget::refs` had no + caller -- while every widget made paid a heap allocation and every one + created or dropped paid two atomic read-modify-writes. Removed: the handle is + the id, the sender and the type, and `Drop` sends. It is 32 bytes rather than + 40, and 40 rather than 48 for a `dyn` one. Raised as something to leave for + the pre-gate pass, `handle.rs` being outside #19, and Bryan asked for it here + (2026-09-21): not being `Clone` is what makes one handle the only one, which + is the same fact the child note above rests on. + + `RefCounter` stays for `TextureHandle`, which does clone -- several widgets + showing one picture share its slot -- and is down to what that needs: + `quiet_clone` and `refs` had no callers at all, and `new` was `Default` spelt + out, so the default is derived now and says in one line that zero means one + handle. + +Four things the sweep **looked at and left**: - **Text allocates about six times per re-broken paragraph per frame**, and it is not worth removing. One is the whole text copied into the placement store @@ -129,13 +146,6 @@ Five things the sweep **looked at and left**: is **0.07%**. The machinery is two pools and a changed `place` signature for that, so the count is recorded here instead. Allocation count is not the same quantity as cost, and this is where the two part. -- **`StrongWidget` carries a `RefCounter(Arc)` it can never use.** - `StrongWidget` deliberately has no `Clone`, so the count never rises above - zero and `RefCounter::drop` always answers true -- but every widget still - allocates an `Arc`, and creating and dropping one is two atomic - read-modify-writes. `StrongWidget::refs` has no caller. `handle.rs` is - untouched by #19, so this belongs to the review of the code written before - the gate; recorded there. - **More for that same pass, all dead and all older than #19**: `Size::to_uivec2`, `Size::rel`, `Size::leftover`, `Len::to_uivec2`, `Vec2::with_x`, `Vec2::with_y`, `Vec2::ceil`, `Layers::iter_orderless_mut`. @@ -151,6 +161,11 @@ Five things the sweep **looked at and left**: the duplication is bounded by what the caller wrote rather than by anything the arena does on its own. +One more thing **looked at and left** in passing: `RefCounter` decrements with +`Ordering::Release` and has no acquire on the last drop, which is the shape an +`Arc` gets wrong when handles cross threads. Left because nothing here is +threaded and the orderings are not this sweep's to guess at. + Verified at the sweep's tip: format, workspace clippy under `-D warnings` with and without `layout-diagnostics`, **190** ordinary, **193** diagnostic and **189** release tests (188 and 191 before, with release failing), and the cold