Take the deepest dirty widget from an ordered queue, not by scanning
The walk found the next widget to settle with `max_by_key` over the whole `needs_redraw` set, and `depth` is a hash lookup, so a frame did a lookup per marked widget per pop -- 131 depth reads for nine marks at seed 1 depth 8, 1,314 for 34, and 14,611 for 145. The set is scanned once now and kept in a `BTreeSet` keyed by depth, and every mark made while the walk runs goes through `mark`, which puts itself in place. The same three counts become 57, 160 and 436. Two things the scan gave for free are paid for explicitly: a widget that was settled inside an ancestor's draw, or deferred to one, is dropped when its entry comes up, and an entry whose widget has since changed depth -- a subtree that moved under a new parent -- is re-queued at the depth it now has. What is drawn does not change: widget draws are identical at every load measured. Median frame at seed 1, depth 8: 0.955 -> 0.843 ms with 145 marks, 0.668 -> 0.666 with 34, and seed 13's default load 5.19 -> 4.86 ms. Ties between equal depths now break by widget id rather than by hash order, which makes the walk deterministic; nothing in the order within one depth was ever relied on, since a widget at the same depth as another cannot contain it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e6ba570d07
commit
3bf22935ce
2 files changed
+40
-15
No files matched your search
+39
-14
@@ -69,6 +69,10 @@ pub struct UiRenderState {
|
||||
/// Widgets waiting for an ancestor to draw them, so the walk down the
|
||||
/// depths does not pick one up again at its own depth.
|
||||
deferred: crate::util::HashSet<WidgetId>,
|
||||
/// What the walk has left to settle, deepest last. Ordered rather than
|
||||
/// searched: the set is scanned once a frame, and every mark made while
|
||||
/// the walk runs puts itself in place.
|
||||
pending: std::collections::BTreeSet<(usize, WidgetId)>,
|
||||
pub moves: Moves,
|
||||
}
|
||||
|
||||
@@ -81,6 +85,7 @@ impl UiRenderState {
|
||||
old_root: None,
|
||||
slots: Default::default(),
|
||||
deferred: Default::default(),
|
||||
pending: Default::default(),
|
||||
moves: Default::default(),
|
||||
resized: false,
|
||||
}
|
||||
@@ -994,15 +999,25 @@ impl UiRenderState {
|
||||
// something below is about to change it -- which is the whole class
|
||||
// of defect where a widget settles inside its parent's draw, clears
|
||||
// its mark there, and tells nobody its answer moved.
|
||||
loop {
|
||||
let next = rsc
|
||||
.widgets()
|
||||
.needs_redraw
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|id| !self.deferred.contains(id))
|
||||
.max_by_key(|&id| self.depth(id));
|
||||
let Some(id) = next else { break };
|
||||
let marked: Vec<WidgetId> = rsc.widgets().needs_redraw.iter().copied().collect();
|
||||
for id in marked {
|
||||
let depth = self.depth(id);
|
||||
self.pending.insert((depth, id));
|
||||
}
|
||||
while let Some(&(depth, id)) = self.pending.last() {
|
||||
self.pending.remove(&(depth, id));
|
||||
// A widget settled inside an ancestor's draw, or deferred to one,
|
||||
// is left here by the mark that queued it.
|
||||
if self.deferred.contains(&id) || !rsc.widgets().needs_redraw.contains(&id) {
|
||||
continue;
|
||||
}
|
||||
// A subtree that changed hands takes its descendants' depths with
|
||||
// it, so an entry queued before that move names the wrong one.
|
||||
let now = self.depth(id);
|
||||
if now != depth {
|
||||
self.pending.insert((now, id));
|
||||
continue;
|
||||
}
|
||||
#[cfg(feature = "layout-diagnostics")]
|
||||
diag::bump(Counter::QueuePops);
|
||||
if !self.redraw(id, rsc) {
|
||||
@@ -1012,6 +1027,16 @@ impl UiRenderState {
|
||||
self.deferred.clear();
|
||||
}
|
||||
|
||||
/// Marks a widget for the walk to settle. Every mark made while a frame
|
||||
/// is being laid out goes through here, so the queue holds what the set
|
||||
/// holds without being searched again.
|
||||
fn mark(&mut self, id: WidgetId, widgets: &mut Widgets) {
|
||||
if widgets.needs_redraw.insert(id) && !self.deferred.contains(&id) {
|
||||
let depth = self.depth(id);
|
||||
self.pending.insert((depth, id));
|
||||
}
|
||||
}
|
||||
|
||||
fn depth(&self, id: WidgetId) -> usize {
|
||||
#[cfg(feature = "layout-diagnostics")]
|
||||
diag::bump(Counter::DepthReads);
|
||||
@@ -1115,8 +1140,8 @@ impl UiRenderState {
|
||||
// Both stay marked: the parent because it has this to draw, and
|
||||
// this because the parent must draw it rather than keep what it
|
||||
// has. The mark comes off in `draw_at`, where the parent draws.
|
||||
rsc.widgets_mut().needs_redraw.insert(id);
|
||||
rsc.widgets_mut().needs_redraw.insert(parent);
|
||||
self.mark(id, rsc.widgets_mut());
|
||||
self.mark(parent, rsc.widgets_mut());
|
||||
return false;
|
||||
}
|
||||
if !active.drawn {
|
||||
@@ -1144,8 +1169,8 @@ impl UiRenderState {
|
||||
// mark left on. Lengths and not whole boxes: what a drawing depends
|
||||
// on is its lengths, so the same lengths elsewhere is one question.
|
||||
if given_px != offered_px {
|
||||
rsc.widgets_mut().needs_redraw.insert(id);
|
||||
rsc.widgets_mut().needs_redraw.insert(parent);
|
||||
self.mark(id, rsc.widgets_mut());
|
||||
self.mark(parent, rsc.widgets_mut());
|
||||
return false;
|
||||
}
|
||||
let info = DrawInfo {
|
||||
@@ -1200,7 +1225,7 @@ impl UiRenderState {
|
||||
diag::bump(Counter::SizeChanges);
|
||||
diag::bump(Counter::ReaderEdges);
|
||||
}
|
||||
rsc.widgets_mut().needs_redraw.insert(parent);
|
||||
self.mark(parent, rsc.widgets_mut());
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
|
||||
pub struct SlotId {
|
||||
idx: u32,
|
||||
genr: u32,
|
||||
|
||||
Reference in new issue
Block a user