Record the fourteenth sweep, over allocation and the cost per child
The sweep is Iris `97fca76`. Its two largest findings are both costs per thing rather than per frame: a container's draw searched a list once per child and so cost the square of its children, and a mask's rectangle was resolved once per fragment rather than once per instance. Both now have a rig that can see them -- `children_cost.rs` is the only one here that varies width, and `chain_cost.rs` has a masked fixture beside its two-pixel quads. The sweep also settles the `Fixed::div`-versus-`ratio` item this file listed, by deleting the operation nothing performed, and adds to the pre-gate review's waiting list the `Arc<AtomicU32>` every `StrongWidget` allocates for a count it can never raise.
This commit is contained in:
1 parent
ea66fc3f71
commit
d24ff83d6b
2 files changed
+214
-2
No files matched your search
+59
-2
@@ -4,6 +4,55 @@ Where the work in flight stands. The settled layout design, the vocabulary
|
|||||||
and the measurement method are in `docs/LAYOUT.md`; what the review of #19
|
and the measurement method are in `docs/LAYOUT.md`; what the review of #19
|
||||||
found is in `docs/LAYOUT_LOG.md`.
|
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:
|
||||||
|
|
||||||
|
- **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
|
||||||
|
children 70% of the redraw was those searches. A draw now takes a `DrawId`
|
||||||
|
and leaves it on every widget it asks about, which answers the question in
|
||||||
|
one read -- one note per widget suffices because the handle a container holds
|
||||||
|
a child by cannot be cloned. `tests/children_cost.rs` is new and is the only
|
||||||
|
rig here that varies width: 3.680 ms to 0.811 ms at 1,600 children, and flat
|
||||||
|
per child at every count. A transcript is exactly this shape.
|
||||||
|
- **A mask's rectangle was resolved once per fragment**, a walk up to 64 links
|
||||||
|
long, although it is the same rectangle for every fragment of an instance.
|
||||||
|
Resolved in the vertex stage now, which takes `masks` and `move_offsets` out
|
||||||
|
of the fragment stage entirely. GPU timestamps over one screenful of clipped
|
||||||
|
rows: 838.4 us to 95.7 us at chain depth 64, 141.2 to 69.4 at depth 8, and
|
||||||
|
unchanged at depth 1. `chain_cost.rs` could not see this -- its instances are
|
||||||
|
two pixels wide so vertex work dominates -- and has a masked fixture now.
|
||||||
|
|
||||||
|
The other six: `TextBuffer::shape` copying its attrs before the check that
|
||||||
|
would not need them, which was one allocation per named-family text per frame
|
||||||
|
and 800 to 0 over a measured hundred frames; `cargo test --release` failing,
|
||||||
|
because a `#[should_panic]` test asserts a `debug_assert` release does not
|
||||||
|
compile; `Fixed::div` and `Div for Fixed`, reached only by their own test,
|
||||||
|
whose deletion also settles the `div`-versus-`ratio` zero-divisor item this
|
||||||
|
file used to list; `Moves::remove` re-uploading an array it cannot have
|
||||||
|
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`.
|
||||||
|
|
||||||
|
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<AtomicU32>` 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.
|
||||||
|
|
||||||
|
`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
|
||||||
|
`cbccfb6` across all 34,986 boxes, all three seed scans, the three deferred
|
||||||
|
corpora, and `mask_clip.rs` reading the clipped pixels back off the GPU to the
|
||||||
|
same 6,000 in the same bounds before and after the shader change.
|
||||||
|
|
||||||
## Built-in bounds replace `MaxSize` on PR #19
|
## Built-in bounds replace `MaxSize` on PR #19
|
||||||
|
|
||||||
Bryan clarified that the built-in attribute should replace the wrapper.
|
Bryan clarified that the built-in attribute should replace the wrapper.
|
||||||
@@ -238,6 +287,9 @@ comparison work above, its review rounds past `cadfba0` are described in
|
|||||||
- **A full sweep of the branch**, `cbccfb6` -- a length of zero that printed
|
- **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
|
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.
|
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.
|
||||||
|
|
||||||
The settled design of the vocabulary rounds is in `docs/LAYOUT.md` under
|
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
|
"Three names, and the one argument that says them". Bryan settled the API
|
||||||
@@ -349,6 +401,13 @@ a compile error, which reads exactly like a fuzzer failure.
|
|||||||
grammars of which `core/src/util/vec2.rs` uses two, one line apart.
|
grammars of which `core/src/util/vec2.rs` uses two, one line apart.
|
||||||
`Align` now has `Index<Axis>`, so the `if let Some` each
|
`Align` now has `Index<Axis>`, so the `if let Some` each
|
||||||
`partial_align` writes twice collapses when that sweep reaches them.
|
`partial_align` writes twice collapses when that sweep reaches them.
|
||||||
|
The fourteenth sweep adds more: `StrongWidget` allocates an
|
||||||
|
`Arc<AtomicU32>` 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.
|
||||||
3. **Integrate the app's Iris capabilities before changing its pin.**
|
3. **Integrate the app's Iris capabilities before changing its pin.**
|
||||||
`32f6ad8` has 45 commits not reachable from the review branch; shared UI
|
`32f6ad8` has 45 commits not reachable from the review branch; shared UI
|
||||||
ownership, richer masks, Android support, and app-side performance work
|
ownership, richer masks, Android support, and app-side performance work
|
||||||
@@ -380,8 +439,6 @@ glyphs do not follow a shortened entry.
|
|||||||
`rel(1.0)` and shares `Painter::longer_than` with the span. Done in
|
`rel(1.0)` and shares `Painter::longer_than` with the span. Done in
|
||||||
`b295c8b`, and in `0d03267` for the root as well, which used to read it the
|
`b295c8b`, and in `0d03267` for the root as well, which used to read it the
|
||||||
old way.
|
old way.
|
||||||
- `Fixed::div` by zero answers `MIN`/`MAX` while `ratio` answers `ZERO`; both
|
|
||||||
are caller bugs under `debug_assert`, but the fallbacks differ.
|
|
||||||
- `docs/LAYOUT.md` §4, §5 and the density section name `Painter::place`,
|
- `docs/LAYOUT.md` §4, §5 and the density section name `Painter::place`,
|
||||||
`Painter::region()`, `SetSize`, `desired_width`, `apply_rest`, `Len::dp`,
|
`Painter::region()`, `SetSize`, `desired_width`, `apply_rest`, `Len::dp`,
|
||||||
`Aligned`, which no longer exist; `MaxSize` now exists with the bounds API.
|
`Aligned`, which no longer exist; `MaxSize` now exists with the bounds API.
|
||||||
|
|||||||
@@ -6,6 +6,161 @@ nothing here is rediscovered. Each entry says who found it and when.
|
|||||||
it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
|
it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
|
||||||
the current plan is in `docs/HANDOFF.md`.
|
the current plan is in `docs/HANDOFF.md`.
|
||||||
|
|
||||||
|
## Fourteenth sweep: allocation, and the cost per child (2026-09-20)
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
- **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:
|
||||||
|
`widget_at` searched `children` and then `under`, `place_at` searched
|
||||||
|
`children`, `draw_at` searched it once per old child and once per size read,
|
||||||
|
and `depend_on` searched `size_deps`. At a hundred children none of that
|
||||||
|
shows; at a thousand it is most of the frame. Profiled at 1,600 children,
|
||||||
|
70% of the redraw was in those searches -- `place_at` alone 15.6%, and it
|
||||||
|
does nothing else linear.
|
||||||
|
|
||||||
|
The question each of them asks is answered in one read now: a draw takes a
|
||||||
|
`DrawId` and leaves it on every widget it asks about, and a widget carries
|
||||||
|
the draw that last asked. One note per widget is enough because one widget
|
||||||
|
is asked about by one container -- the handle a container holds a child by
|
||||||
|
cannot be cloned. `under` is still searched, but only on the rare re-ask,
|
||||||
|
since it is added to in step with the child list. `depend_on` no longer
|
||||||
|
dedupes at all: a child that answered with a hint may have no record to note
|
||||||
|
it on until the draw ends, and the loop `size_deps` drives asks the same of a
|
||||||
|
widget twice as of it once. A debug assertion in `widget_at` checks the note
|
||||||
|
against the list it claims to be in, which is the cheap guard for the pair.
|
||||||
|
|
||||||
|
`tests/children_cost.rs` is new and is the only rig here that varies width;
|
||||||
|
every other one varies depth, the window, or what changed. 40 full redraws of
|
||||||
|
one span, per child in the last column:
|
||||||
|
|
||||||
|
| children | before | after | per child before | after |
|
||||||
|
| --- | --- | --- | --- | --- |
|
||||||
|
| 100 | 0.068 ms | 0.053 ms | 0.0007 ms | 0.0005 ms |
|
||||||
|
| 200 | 0.143 ms | 0.101 ms | 0.0007 ms | 0.0005 ms |
|
||||||
|
| 400 | 0.386 ms | 0.213 ms | 0.0010 ms | 0.0005 ms |
|
||||||
|
| 800 | 1.158 ms | 0.410 ms | 0.0014 ms | 0.0005 ms |
|
||||||
|
| 1600 | 3.680 ms | 0.811 ms | 0.0023 ms | 0.0005 ms |
|
||||||
|
|
||||||
|
Flat per child after, 4.5x at 1,600. A transcript is exactly this shape.
|
||||||
|
|
||||||
|
- **A mask's rectangle was resolved once per fragment.** `masked()` in the
|
||||||
|
prelude called `resolve_move` -- a walk up to `CHAIN_LIMIT` (64) links
|
||||||
|
long -- for every fragment of every masked primitive, although the rectangle
|
||||||
|
is the same for all of them. It is resolved in `vs_main` now and handed on as
|
||||||
|
two flat `vec2`s, which also takes `masks` and `move_offsets` out of the
|
||||||
|
fragment stage entirely: both bindings are `ShaderStages::VERTEX` now, and
|
||||||
|
the wrong visibility is what the GPU rig caught first.
|
||||||
|
|
||||||
|
`chain_cost.rs` could not see this: its instances are two pixels wide on
|
||||||
|
purpose, so vertex work dominates. It has a second fixture now -- one
|
||||||
|
screenful of rows, each clipped by a mask whose own chain is that deep --
|
||||||
|
and the two tables are what tell the stages apart. GPU timestamps, best of 8
|
||||||
|
batches, 1024x1024:
|
||||||
|
|
||||||
|
| mask chain | before | after |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| 1 | 66.5 us | 66.2 us |
|
||||||
|
| 4 | 91.4 us | 67.6 us |
|
||||||
|
| 8 | 141.2 us | 69.4 us |
|
||||||
|
| 16 | 240.8 us | 73.1 us |
|
||||||
|
| 64 | 838.4 us | 95.7 us |
|
||||||
|
|
||||||
|
8.8x at depth 64 and 2.0x at depth 8; unchanged at depth 1, which is the
|
||||||
|
check that nothing else moved. What is left at depth 64 is the instances' own
|
||||||
|
vertex walk, which this does not touch.
|
||||||
|
|
||||||
|
- **`TextBuffer::shape` copied the attrs before the check that would not need
|
||||||
|
them.** It built a whole `LayoutKey` first and compared that, so every text
|
||||||
|
draw copied its `TextAttrs` -- a heap allocation for any text naming its font
|
||||||
|
family, which is what the app does for every icon and every monospace run --
|
||||||
|
and then compared the attrs up to three times over. The comparisons are
|
||||||
|
asked of the borrowed attrs now and the key is built where it is kept.
|
||||||
|
Measured over 100 redraws of 8 named-family texts at one width: 800
|
||||||
|
allocations before, 0 after, which is one per text per frame.
|
||||||
|
|
||||||
|
`tests/allocation_cost.rs` had no text case at all, so none of this was
|
||||||
|
visible to the rig whose whole job is to say the steady state allocates
|
||||||
|
nothing. It has one now, asserting zero for a text redrawn at the width it
|
||||||
|
already has.
|
||||||
|
|
||||||
|
- **`cargo test --release` failed on this branch**, and every rig here is run
|
||||||
|
in release. `a_clipping_widget_reporting_more_than_its_box_is_caught` is
|
||||||
|
`#[should_panic]` on a `debug_assert`, which a release build does not
|
||||||
|
compile, so the test cannot pass there -- and a `should_panic` that does not
|
||||||
|
panic fails rather than passing vacuously. It is `#[cfg(debug_assertions)]`
|
||||||
|
now.
|
||||||
|
|
||||||
|
- **`Fixed::div` and `Div for Fixed` were reached only by their own test.**
|
||||||
|
Nothing in the framework divides one length by another; `div_int` and `ratio`
|
||||||
|
are what the layout uses. Deleted with the test that was its only caller,
|
||||||
|
which also settles the open item about `div` answering `MIN`/`MAX` for a zero
|
||||||
|
divisor where `ratio` answers `ZERO`: there is one of them now.
|
||||||
|
|
||||||
|
- **`Moves::remove` marked the entries changed.** Freeing a slot changes no
|
||||||
|
byte the GPU holds -- the slot keeps what it had, nothing names it until it
|
||||||
|
is handed out again, and whoever is handed it writes it then -- so every
|
||||||
|
frame that retired a region node re-uploaded the whole array for nothing.
|
||||||
|
|
||||||
|
- **A comment saying `widget_trait!` takes no attributes**, which this branch's
|
||||||
|
own `76aaf06` made false when it taught the macro to forward them to both the
|
||||||
|
trait and the impl. The comment was written on 2026-09-16 and was true then;
|
||||||
|
it is the doc comment it says it cannot be now.
|
||||||
|
|
||||||
|
- **A comment saying `Scroll` clips.** `Masked` explained reporting its own box
|
||||||
|
"for the reason `Scroll` reports the same: it clips what is inside to that
|
||||||
|
box". Nothing calls `set_mask` for a `Scroll`, and `scrollable()` only makes
|
||||||
|
its inner a region node: a scroll area positions its content by a move and
|
||||||
|
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**:
|
||||||
|
|
||||||
|
- **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
|
||||||
|
(`keep_placed`), and the other five are `place` growing its glyph list from
|
||||||
|
empty, 4 through 64 entries for a 58-glyph paragraph. Both would go with a
|
||||||
|
free list fed by the store's own evictions, which happen exactly when a
|
||||||
|
placement needs one. Measured what that would buy, with the copy removed and
|
||||||
|
the list given its capacity outright: 3,710,680,668 instructions before and
|
||||||
|
3,708,100,546 after over 500 sweeping resize frames of 40 paragraphs, which
|
||||||
|
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<AtomicU32>)` 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`.
|
||||||
|
Left out of this sweep's diff because they are not this branch's, not
|
||||||
|
because they should stay.
|
||||||
|
- **`request_readers` keeps an empty `HashSet` for a widget whose readers have
|
||||||
|
all gone.** `remove` takes the reader out of the set and leaves the set; only
|
||||||
|
freeing the widget itself takes the entry. Bounded by the number of live
|
||||||
|
widgets and one allocation each, so it is a cost that does not grow with
|
||||||
|
time; left as it stands.
|
||||||
|
- **`Nodes::graft` copies a shared sub-expression once per reference.** A node
|
||||||
|
can be named twice only where a caller reused a `SizeRequest` it cloned, so
|
||||||
|
the duplication is bounded by what the caller wrote rather than by anything
|
||||||
|
the arena does on its own.
|
||||||
|
|
||||||
|
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
|
||||||
|
dump byte-identical to `cbccfb6` across all **34,986** boxes. All three seed
|
||||||
|
scans pass -- 400 depth 5 (64.42s), 1,000 depth 6 (160.99s), 2,000 depth 4
|
||||||
|
(301.45s) -- as do 400 depth-5 trees in each of the three deferred corpora
|
||||||
|
(206.31s); the depth-5 scan was run again after the text change (91.40s). The
|
||||||
|
new `mask_clip.rs` reads the clipped pixels back off the GPU and gives the same
|
||||||
|
6,000 pixels in the same bounds before and after the shader change.
|
||||||
|
|
||||||
## Thirteenth sweep: a full pass over #19 (2026-09-20)
|
## Thirteenth sweep: a full pass over #19 (2026-09-20)
|
||||||
|
|
||||||
Over the whole branch rather than one commit, and the first review of
|
Over the whole branch rather than one commit, and the first review of
|
||||||
|
|||||||
Reference in new issue
Block a user