Dirty many widgets at once, which nothing was checking
Every generated case changes one thing: four declared sizes, or one span's children, or the output. A frame settling one dependency path says nothing about a frame settling a set of them that overlap, which is the case the settle order exists for. So two more: every declared size in the tree changing at once, and a spread of widgets marked for redraw together. The second changes nothing, which is the point -- no box may move, and the order the dirty set is taken in is all that can make one. The hundred-seed sweep is 1,000 comparisons now rather than 800, and passes. `IRIS_PHASE=many` is the same load for the diagnostics rig, with `IRIS_DIRTY` widgets marked per frame. It says what one repainting leaf cannot: at 130 of 260 widgets, choosing which dirty widget to settle next is 24.5% of the frame, because the dirty set is scanned once per widget settled and a hash set is walked by capacity rather than by length. Memoizing the depth walk inside one scan does not pay -- it trades parent lookups for memo lookups and costs 4% more instructions -- so the fix is to stop rescanning, which changes the order widgets settle in and wants agreeing first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
2525637e26
commit
77bb75e5de
2 files changed
+99
-11
No files matched your search
+79
-8
@@ -44,24 +44,37 @@ fn plant(h: &mut Harness, seed: u64, edits: &Edits) -> Tree {
|
|||||||
tree
|
tree
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn resize_one(h: &mut Harness, tree: &Tree, idx: usize, rng: &mut Rng) -> Lens {
|
||||||
|
let lens = [
|
||||||
|
Some(Len::abs(20.0 + rng.below(180) as f32)),
|
||||||
|
Some(Len::abs(20.0 + rng.below(180) as f32)),
|
||||||
|
];
|
||||||
|
let sized = &mut h.rsc[tree.sized[idx]];
|
||||||
|
sized.x = lens[0];
|
||||||
|
sized.y = lens[1];
|
||||||
|
lens
|
||||||
|
}
|
||||||
|
|
||||||
/// Changes a few of the declared sizes, and says which, so the cold tree can
|
/// Changes a few of the declared sizes, and says which, so the cold tree can
|
||||||
/// be grown with the same ones.
|
/// be grown with the same ones.
|
||||||
fn edit(h: &mut Harness, tree: &Tree, rng: &mut Rng) -> HashMap<usize, Lens> {
|
fn edit(h: &mut Harness, tree: &Tree, rng: &mut Rng) -> HashMap<usize, Lens> {
|
||||||
let mut edits = HashMap::new();
|
let mut edits = HashMap::new();
|
||||||
for _ in 0..4 {
|
for _ in 0..4 {
|
||||||
let idx = rng.below(tree.sized.len());
|
let idx = rng.below(tree.sized.len());
|
||||||
let lens = [
|
edits.insert(idx, resize_one(h, tree, idx, rng));
|
||||||
Some(Len::abs(20.0 + rng.below(180) as f32)),
|
|
||||||
Some(Len::abs(20.0 + rng.below(180) as f32)),
|
|
||||||
];
|
|
||||||
edits.insert(idx, lens);
|
|
||||||
let sized = &mut h.rsc[tree.sized[idx]];
|
|
||||||
sized.x = lens[0];
|
|
||||||
sized.y = lens[1];
|
|
||||||
}
|
}
|
||||||
edits
|
edits
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Every declared size at once, so every reader of a size in the tree has a
|
||||||
|
/// changed descendant in the same frame and the whole dirty set has to settle
|
||||||
|
/// together.
|
||||||
|
fn edit_every(h: &mut Harness, tree: &Tree, rng: &mut Rng) -> HashMap<usize, Lens> {
|
||||||
|
(0..tree.sized.len())
|
||||||
|
.map(|idx| (idx, resize_one(h, tree, idx, rng)))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
/// A way of changing what a span holds. Each is a shape worth its own case:
|
/// A way of changing what a span holds. Each is a shape worth its own case:
|
||||||
/// taking a child out of the middle is not the same as emptying a span, and
|
/// taking a child out of the middle is not the same as emptying a span, and
|
||||||
/// adding one is not the same as adding three.
|
/// adding one is not the same as adding three.
|
||||||
@@ -239,6 +252,52 @@ fn reshuffled(seed: u64, shuffle: Shuffle) {
|
|||||||
assert_same(seed, &what, (&warm, &grown), (&cold, &same));
|
assert_same(seed, &what, (&warm, &grown), (&cold, &same));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn changed_every_size(seed: u64) {
|
||||||
|
let mut warm = Harness::new((900, 1200));
|
||||||
|
let grown = plant(&mut warm, seed, &Edits::default());
|
||||||
|
if grown.sized.is_empty() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut rng = Rng::new(seed ^ 0xa11);
|
||||||
|
let sizes = edit_every(&mut warm, &grown, &mut rng);
|
||||||
|
warm.frame();
|
||||||
|
|
||||||
|
let mut cold = Harness::new((900, 1200));
|
||||||
|
let same = plant(
|
||||||
|
&mut cold,
|
||||||
|
seed,
|
||||||
|
&Edits {
|
||||||
|
sizes,
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_same(seed, "every size at once", (&warm, &grown), (&cold, &same));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Marks a spread of widgets for redraw at once. Nothing changes, so no box
|
||||||
|
/// may either; what this exercises is the order a frame settles a dirty set
|
||||||
|
/// in, which the other cases reach one dependency path at a time.
|
||||||
|
fn repainted_together(seed: u64) {
|
||||||
|
let mut warm = Harness::new((900, 1200));
|
||||||
|
let grown = plant(&mut warm, seed, &Edits::default());
|
||||||
|
for &id in grown.ids.iter().step_by(5) {
|
||||||
|
warm.rsc.widgets_mut().get_dyn_mut(id);
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
!warm.rsc.widgets().needs_redraw.is_empty(),
|
||||||
|
"seed {seed}: nothing was marked"
|
||||||
|
);
|
||||||
|
warm.frame();
|
||||||
|
|
||||||
|
let mut cold = Harness::new((900, 1200));
|
||||||
|
let same = plant(&mut cold, seed, &Edits::default());
|
||||||
|
|
||||||
|
let what = "many repaints at once";
|
||||||
|
assert_same(seed, what, (&warm, &grown), (&cold, &same));
|
||||||
|
}
|
||||||
|
|
||||||
fn resized(seed: u64) {
|
fn resized(seed: u64) {
|
||||||
let mut warm = Harness::new((1920, 1200));
|
let mut warm = Harness::new((1920, 1200));
|
||||||
let grown = plant(&mut warm, seed, &Edits::default());
|
let grown = plant(&mut warm, seed, &Edits::default());
|
||||||
@@ -280,6 +339,16 @@ fn a_changed_size_lands_where_growing_it_that_way_would() {
|
|||||||
SEEDS.into_iter().for_each(changed_size);
|
SEEDS.into_iter().for_each(changed_size);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn every_size_changing_at_once_lands_where_growing_it_that_way_would() {
|
||||||
|
SEEDS.into_iter().for_each(changed_every_size);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn many_widgets_redrawing_at_once_leaves_every_box_where_it_was() {
|
||||||
|
SEEDS.into_iter().for_each(repainted_together);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_resize_lands_where_starting_at_that_size_would() {
|
fn a_resize_lands_where_starting_at_that_size_would() {
|
||||||
SEEDS.into_iter().for_each(resized);
|
SEEDS.into_iter().for_each(resized);
|
||||||
@@ -328,6 +397,8 @@ fn a_long_run_of_seeds_agrees() {
|
|||||||
.unwrap_or(1..=100);
|
.unwrap_or(1..=100);
|
||||||
for seed in seeds {
|
for seed in seeds {
|
||||||
changed_size(seed);
|
changed_size(seed);
|
||||||
|
changed_every_size(seed);
|
||||||
|
repainted_together(seed);
|
||||||
resized(seed);
|
resized(seed);
|
||||||
resized_then_changed(seed);
|
resized_then_changed(seed);
|
||||||
for shuffle in SHUFFLES {
|
for shuffle in SHUFFLES {
|
||||||
|
|||||||
@@ -11,8 +11,9 @@
|
|||||||
//! -e cycles:u,instructions:u cargo test --release \
|
//! -e cycles:u,instructions:u cargo test --release \
|
||||||
//! --test layout_diagnostics -- --ignored --nocapture
|
//! --test layout_diagnostics -- --ignored --nocapture
|
||||||
//!
|
//!
|
||||||
//! `IRIS_PHASE` is `cold`, `repaint`, `size`, `scroll`, `resize`, or `all`.
|
//! `IRIS_PHASE` is `cold`, `repaint`, `many`, `size`, `scroll`, `resize`, or
|
||||||
//! `IRIS_SEED`, `IRIS_DEPTH`, and `IRIS_FRAMES` select the load.
|
//! `all`. `IRIS_SEED`, `IRIS_DEPTH`, and `IRIS_FRAMES` select the load, and
|
||||||
|
//! `IRIS_DIRTY` how many widgets `many` marks at once.
|
||||||
|
|
||||||
use iris::harness::Harness;
|
use iris::harness::Harness;
|
||||||
use iris::prelude::*;
|
use iris::prelude::*;
|
||||||
@@ -165,7 +166,7 @@ fn layout_cost() {
|
|||||||
assert!(frames > 0, "IRIS_FRAMES must be greater than zero");
|
assert!(frames > 0, "IRIS_FRAMES must be greater than zero");
|
||||||
let phase = env("IRIS_PHASE", String::from("all"));
|
let phase = env("IRIS_PHASE", String::from("all"));
|
||||||
assert!(
|
assert!(
|
||||||
["all", "cold", "repaint", "size", "scroll", "resize"].contains(&phase.as_str()),
|
["all", "cold", "repaint", "many", "size", "scroll", "resize"].contains(&phase.as_str()),
|
||||||
"unknown IRIS_PHASE {phase:?}"
|
"unknown IRIS_PHASE {phase:?}"
|
||||||
);
|
);
|
||||||
let selected = |name| phase == "all" || phase == name;
|
let selected = |name| phase == "all" || phase == name;
|
||||||
@@ -194,6 +195,22 @@ fn layout_cost() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if selected("many") {
|
||||||
|
let (mut harness, tree) = warm(seed, depth);
|
||||||
|
trace_selected(&tree);
|
||||||
|
// Spread through the tree rather than taken from one subtree, so the
|
||||||
|
// dependency paths the frame settles overlap.
|
||||||
|
let wanted = env("IRIS_DIRTY", 32_usize).max(1);
|
||||||
|
let step = (tree.ids.len() / wanted).max(1);
|
||||||
|
let dirty: Vec<_> = tree.ids.iter().copied().step_by(step).collect();
|
||||||
|
println!("marking {} of {} widgets", dirty.len(), tree.ids.len());
|
||||||
|
run("many", frames, &mut harness, move |harness, _| {
|
||||||
|
for &id in &dirty {
|
||||||
|
harness.rsc.widgets_mut().get_dyn_mut(id);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
if selected("size") {
|
if selected("size") {
|
||||||
let (mut harness, tree) = warm(seed, depth);
|
let (mut harness, tree) = warm(seed, depth);
|
||||||
trace_selected(&tree);
|
trace_selected(&tree);
|
||||||
|
|||||||
Reference in new issue
Block a user