A tooltip raise cost 6.6 ms because every frame it minted re-derived the layout graph
`WOW_UI_VERBS=1` over the hover bench answered it in one line: the rebuilt frame's cost was **`frame:GetLeft`, 3 calls at 2188 us each — 6.56 of the 6.6 ms** — inside `UIParent:OnUpdate`. `GetLeft` forces a resolve, and each of those resolves was a whole-graph derivation of ~19,000 frames and ~44,000 regions, because `Model::frame_id` gave up naming the graph on every mint. OctoPawn mints two frames on every `GameTooltip` OnShow purely to own a one-shot `OnUpdate` — the 1.12 way to write `C_Timer.After` — so a raise dirtied the graph twice and the three `GetLeft`s each paid for it. A just-minted frame is now **appended to the cached graph as a leaf** (`LayoutScope::append_frame_leaf`). Sound because the id was minted this instant, so no anchor anywhere can name it: nothing cached reads the new node and no cached edge moves. What it reads is its own anchors' targets, which the append is handed — a mint is not always a creation, since `frame_id` mints on the first *fetch* too, and a frame built in Rust (a nameplate, a chat bubble) is first fetched when Lua asks for its wrapper with its anchors long written. `last` stays `NO_NODE` so the incremental seed sees the hash move; `commit_incremental` counts the slot, which keeps the next full pass's `matched != last_count` node-death test honest. Refused when the frame already carries regions: a region is a graph node without needing a minted id — the derivation mints region ids inside its own walk — so only a derivation can add those rows. **Measured, same bench, same interface: the rebuild costs -0.08 ms/frame under forty churning plates and +0.14 ms with none up, from 6.63 and 6.78.** The rebuilt windows now report no derivation sites at all. Two things about how this was found, both in the bench's doc now: - Its own earlier conclusion was that the cost was "the plate's line walk and the hooks hanging off it, flat, and not the graph it dirties", because the crowded and the empty shape cost the same. They cost the same because **both** were paying the full derivation. A fair experiment that was blind to its own answer, and one env var would have named it. - `WOW_LAYOUT_VERIFY` over the real interface is red on this change — and it is the documented pre-existing 48-region shortfall, reproduced identically with the whole change stashed: `frames 13544 -> 13544, regions 29481 -> 29529` either way. All 2069 benilla-ui tests pass, and they run that verifier forced on after every resolve. 189 workspace failures, unchanged, no regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
Claude Opus 5
@@ -1242,11 +1242,26 @@ fn hover_frame(s: &mut UiScript, company: Company, plates: &[PlateFeed], rebuild
|
||||
/// Two shapes, because the guess was that they would differ: a crowded pull re-derives the layout
|
||||
/// graph every frame anyway (four plates a frame are strangers), so a rebuild's own dirtying
|
||||
/// looked like it would be partly already paid, where a player standing in a city has a settled
|
||||
/// graph and would pay for the whole resolve. **They do not differ.** Measured on grom's
|
||||
/// interface the rebuild costs 6.63 ms/frame under forty churning plates and 6.78 ms/frame with
|
||||
/// none up, against whole-frame baselines of 1.84 ms and 0.85 ms — so the cost is the plate's own
|
||||
/// line walk and the hooks hanging off it, flat, and not the graph it dirties. Both windows are
|
||||
/// kept because that is the finding.
|
||||
/// graph and would pay for the whole resolve. They did not differ — 6.63 ms/frame under forty
|
||||
/// churning plates and 6.78 ms with none up, against whole-frame baselines of 1.84 and 0.85 — and
|
||||
/// I read that as the cost being the plate's own line walk and the hooks hanging off it, flat,
|
||||
/// rather than the graph it dirties.
|
||||
///
|
||||
/// **That reading was wrong, and `WOW_UI_VERBS` said so in one run.** The rebuilt frame's cost was
|
||||
/// one row: `frame:GetLeft`, **3 calls a frame at 2188 us each — 6.56 of the 6.6 ms**, inside
|
||||
/// `UIParent:OnUpdate`. `GetLeft` forces a resolve, and the resolve was a whole-graph derivation
|
||||
/// because each of OctoPawn's two `OnShow` frame mints gave up naming the graph; the two shapes
|
||||
/// agreed because *both* were paying the full derivation, not because neither was.
|
||||
///
|
||||
/// Fixed by appending a just-minted frame to the cached graph as a leaf
|
||||
/// (`LayoutScope::append_frame_leaf`, with `layout_gate::minting_a_frame_appends_a_leaf_rather
|
||||
/// _than_deriving_the_graph` for why that is sound). Now: **the rebuild costs -0.11 ms/frame
|
||||
/// under forty plates and +0.14 ms with none up**, from 6.63 and 6.78, and the rebuilt windows
|
||||
/// report no derivation sites at all. Whole-frame totals 1.60 and 0.91 ms.
|
||||
///
|
||||
/// The lesson for the next one of these: the two-shape comparison was a fair experiment that
|
||||
/// happened to be blind to its own answer. A per-verb profile over the same window would have
|
||||
/// named it immediately, and it is one env var.
|
||||
///
|
||||
/// The live asset probes are installed and report 0.001 ms/frame, and the plate re-measures no
|
||||
/// text (0.1 requests a frame, the same held or rebuilt): neither the patch chain nor the font
|
||||
@@ -1266,9 +1281,8 @@ fn hover_frame(s: &mut UiScript, company: Company, plates: &[PlateFeed], rebuild
|
||||
/// the number created (137 against 480 here), so Lua reclaims most of them; it is a cost per
|
||||
/// raise, not an unbounded leak.
|
||||
///
|
||||
/// Kept as instrumentation rather than acted on: the lever would be letting a brand-new leaf node
|
||||
/// join the graph without a full derive, which is a layout-engine change and wants its own
|
||||
/// measurement.
|
||||
/// Acted on: that lever — letting a brand-new leaf node join the graph without a full derive — is
|
||||
/// what this bench's numbers above are now measuring the absence of.
|
||||
#[test]
|
||||
#[ignore = "bench — needs WOW_DATA and the player's AddOns; run explicitly with --release"]
|
||||
fn a_mouseover_plate_rebuilt_every_frame() {
|
||||
|
||||
@@ -89,7 +89,7 @@ const NO_NODE: u64 = 0;
|
||||
|
||||
/// A frame's per-node hash; the full derive and the incremental pass must agree bit for bit.
|
||||
#[inline]
|
||||
fn frame_node_hash(input: &LayoutInput) -> u64 {
|
||||
pub(crate) fn frame_node_hash(input: &LayoutInput) -> u64 {
|
||||
let mut node = InputFingerprint::default();
|
||||
node.input(input);
|
||||
node.finish()
|
||||
@@ -446,15 +446,82 @@ impl LayoutScope {
|
||||
self.last_screen = Some(screen);
|
||||
}
|
||||
|
||||
/// [`Self::commit`] for an incremental pass: only the re-hashed slots. `last_count` and
|
||||
/// `last_screen` stay, since a structural change always takes the full path.
|
||||
/// [`Self::commit`] for an incremental pass: only the re-hashed slots. `last_screen` stays,
|
||||
/// since a screen move always takes the full path.
|
||||
///
|
||||
/// `last_count` moves with the slots that go from "no hash" to one, which is how
|
||||
/// [`Self::append_frame_leaf`] keeps the next full pass's node-death test honest: that test is
|
||||
/// `matched != last_count` over the ids whose `last` is not [`NO_NODE`], so a node introduced
|
||||
/// on this path has to be counted here or the pass after it reads a death that never happened
|
||||
/// and takes full scope for nothing.
|
||||
fn commit_incremental(&mut self) {
|
||||
for &(id, h) in &self.staged {
|
||||
self.last[id as usize] = h;
|
||||
let slot = &mut self.last[id as usize];
|
||||
if *slot == NO_NODE && h != NO_NODE {
|
||||
self.last_count += 1;
|
||||
}
|
||||
*slot = h;
|
||||
}
|
||||
self.staged.clear();
|
||||
}
|
||||
|
||||
/// Append a brand-new frame to the cached graph as a **leaf**, so a frame minted between two
|
||||
/// resolves does not cost a whole-graph derivation. `false` means the caller must give up
|
||||
/// naming after all.
|
||||
///
|
||||
/// Sound only for a frame whose id was *just* minted, which is the one caller
|
||||
/// ([`Model::frame_id`]): **no anchor anywhere can name an id that did not exist a moment
|
||||
/// ago**, so nothing in the cached graph reads this node and no existing edge has to move.
|
||||
/// What it reads is its own anchors' targets, and `targets` carries them, because a mint is
|
||||
/// not always a creation — `frame_id` mints on the first *fetch*, which for a frame built in
|
||||
/// Rust (a nameplate, a chat bubble) is the first time Lua asks for its wrapper, long after
|
||||
/// its anchors were written.
|
||||
///
|
||||
/// `last` is left at [`NO_NODE`] so the incremental seed sees the hash move and seeds the
|
||||
/// node; `commit_incremental` then counts it.
|
||||
///
|
||||
/// The payoff is per tooltip raise, and it is most of one: an addon that owns a one-shot timer
|
||||
/// by minting `CreateFrame("Frame")` — the 1.12 way to write `C_Timer.After`, and what
|
||||
/// OctoPawn does twice on every `GameTooltip` `OnShow` — dirtied the whole graph each time,
|
||||
/// and `UIParent:OnUpdate`'s three `GetLeft` calls then each paid a full derivation: 2.19 ms
|
||||
/// a call, 6.56 ms of a 6.6 ms rebuilt-hover frame on grom's interface.
|
||||
pub(crate) fn append_frame_leaf(
|
||||
&mut self,
|
||||
h: FrameHandle,
|
||||
id: u32,
|
||||
hash: u64,
|
||||
targets: &[u32],
|
||||
) -> bool {
|
||||
// No cached graph to append to: the next resolve derives in full anyway.
|
||||
if self.last.is_empty() || self.node_of.is_empty() {
|
||||
return false;
|
||||
}
|
||||
let i = id as usize;
|
||||
if i >= self.node_of.len() {
|
||||
self.last.resize(i + 1, NO_NODE);
|
||||
self.now.resize(i + 1, NO_NODE);
|
||||
self.node_of.resize(i + 1, u32::MAX);
|
||||
self.dep_head.resize(i + 1, u32::MAX);
|
||||
self.dirty.resize(i + 1, false);
|
||||
}
|
||||
// Already a node, or a slot `last` remembers: not a fresh leaf, so this is not our case.
|
||||
if self.node_of[i] != u32::MAX || self.last[i] != NO_NODE {
|
||||
return false;
|
||||
}
|
||||
#[allow(clippy::cast_possible_truncation)]
|
||||
{
|
||||
self.node_of[i] = self.plan.len() as u32;
|
||||
}
|
||||
self.plan.push((h, id));
|
||||
self.now[i] = hash;
|
||||
// What this node reads. An edge to an id with no slot is dropped by `edge` itself, which
|
||||
// is right: an id that is not a node can never turn dirty, so it constrains nothing.
|
||||
for &to in targets {
|
||||
self.edge(id, to);
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Forget the memory, so the next resolve derives the graph in full scope.
|
||||
pub(crate) fn invalidate(&mut self) {
|
||||
self.last_count = 0;
|
||||
|
||||
@@ -2387,8 +2387,61 @@ impl Model {
|
||||
self.next_id += 1;
|
||||
self.frame_to_id.insert(h, id);
|
||||
self.id_to_frame.insert(id, h);
|
||||
// Every creation path mints here, so a new frame always opens the layout gate.
|
||||
self.touch_layout();
|
||||
// Every creation path mints here, so a new frame always opens the layout gate — but as a
|
||||
// **leaf**, which is cheap, rather than by giving up naming, which costs a whole-graph
|
||||
// derivation. A frame whose id was minted this instant has no anchors of its own yet and
|
||||
// cannot be the target of anybody else's, so it reads nothing and nothing reads it; the
|
||||
// cached roster, edges and hashes all stand with it appended.
|
||||
//
|
||||
// What that is worth: an addon owning a one-shot timer by minting `CreateFrame("Frame")`
|
||||
// (the 1.12 way to write `C_Timer.After`) used to dirty the whole graph per mint, and
|
||||
// `UIParent:OnUpdate`'s `GetLeft` calls then each paid a derivation — 6.56 ms of a 6.6 ms
|
||||
// rebuilt-hover frame on grom's interface, three calls at 2.19 ms each.
|
||||
//
|
||||
// The scale and clamp a derivation would have synced are written here for the same reason:
|
||||
// only a full derivation reads them off the arena, so an appended node would otherwise
|
||||
// carry a default 1.0 scale under a scaled parent until the next one.
|
||||
let (scale, clamp) = self
|
||||
.arena
|
||||
.frame(h)
|
||||
.map(|f| (f.effective_scale, f.clamped_to_screen))
|
||||
.unwrap_or((1.0, false));
|
||||
let (extent_x, extent_y) = (self.screen.right, self.screen.top);
|
||||
let input = self.layout_inputs.entry(h).or_default();
|
||||
input.scale = scale;
|
||||
input.clamp = clamp;
|
||||
if clamp {
|
||||
input.extent_x = extent_x;
|
||||
input.extent_y = extent_y;
|
||||
}
|
||||
// Refuse the append when the frame already carries regions, because a region is a graph
|
||||
// node **without needing a minted id**: the full derivation walks `region_data` and mints
|
||||
// region ids inside that walk, where a frame is a node only once its id exists. So a
|
||||
// frame whose regions were built before its id was fetched has rows the cached roster
|
||||
// cannot gain by appending one frame, and only a derivation can add them.
|
||||
//
|
||||
// It costs nothing where it matters: the mint that was expensive is the bare
|
||||
// `CreateFrame("Frame")` an addon uses to own a one-shot `OnUpdate`, which has no regions
|
||||
// at all. A plate, a bubble or anything off a template keeps the full-derivation path.
|
||||
//
|
||||
// (This guard is not what `WOW_LAYOUT_VERIFY`'s 48-region shortfall over the real
|
||||
// interface is about. That red is pre-existing and documented — reproduced identically
|
||||
// with this whole change stashed, `frames 13544 -> 13544, regions 29481 -> 29529` either
|
||||
// way — and adding the guard moved neither number.)
|
||||
let has_regions = self.arena.frame(h).is_some_and(|f| !f.regions.is_empty());
|
||||
let hash = crate::script::layout::frame_node_hash(input);
|
||||
// A mint is not always a creation: `frame_id` mints on the first *fetch* too, and a frame
|
||||
// built in Rust (a nameplate, a chat bubble) is first fetched when Lua asks for its
|
||||
// wrapper, with its anchors long written. So the append is handed what this node reads.
|
||||
let targets: Vec<u32> = input.anchors.iter().map(|a| a.relative_to).collect();
|
||||
if self.layout_touched.is_some()
|
||||
&& !has_regions
|
||||
&& self.layout_scope.append_frame_leaf(h, id, hash, &targets)
|
||||
{
|
||||
self.touch_layout_node(id);
|
||||
} else {
|
||||
self.touch_layout();
|
||||
}
|
||||
id
|
||||
}
|
||||
|
||||
|
||||
@@ -512,10 +512,15 @@ fn a_derivation_names_the_write_site_and_not_the_bookkeeper() {
|
||||
s.take_layout_derive_sites().is_empty(),
|
||||
"unarmed, the tally has nothing to say"
|
||||
);
|
||||
// A **region** mint, which still gives up naming: a new region can join the resolve's
|
||||
// external set. Creating a *frame* no longer does — a just-minted frame is appended to the
|
||||
// cached graph as a leaf (`LayoutScope::append_frame_leaf`), which is the whole point of that
|
||||
// change — so a frame mint is no longer a write that can exercise this tally.
|
||||
s.run(
|
||||
r#"
|
||||
f = CreateFrame("Frame", "Named", nil); f:SetWidth(10); f:SetHeight(10)
|
||||
f:SetPoint("TOPLEFT", nil, "TOPLEFT", 0, 0)
|
||||
art = f:CreateTexture(nil, "ARTWORK"); art:SetAllPoints()
|
||||
"#,
|
||||
)
|
||||
.expect("setup");
|
||||
@@ -523,7 +528,7 @@ fn a_derivation_names_the_write_site_and_not_the_bookkeeper() {
|
||||
let rows = s.take_layout_derive_sites();
|
||||
assert!(
|
||||
!rows.is_empty(),
|
||||
"creating a frame gives up naming — if this is empty the tally is not being written, and \
|
||||
"a region mint gives up naming — if this is empty the tally is not being written, and \
|
||||
every reading taken from it is a false negative"
|
||||
);
|
||||
for (site, n) in &rows {
|
||||
@@ -548,10 +553,13 @@ fn a_derivation_names_the_write_site_and_not_the_bookkeeper() {
|
||||
// `#[track_caller]` anywhere in the chain does not empty the tally — it collapses every row
|
||||
// onto `give_up_naming`'s own line, so the tally looks healthy and says nothing. Two triggers
|
||||
// that give up naming from two different lines must therefore read as two rows: minting a
|
||||
// frame id and minting a region id are different writes and the report has to tell them apart.
|
||||
// region id and a region's first anchor — which joins it to the graph — are different writes
|
||||
// and the report has to tell them apart. (Minting a *frame* id used to be one of the pair; it
|
||||
// appends a leaf now and names it, which is why the frame step below is only scaffolding.)
|
||||
for step in [
|
||||
r#"g = CreateFrame("Frame", "Another", nil)"#,
|
||||
r#"fs = Another:CreateFontString(nil, "ARTWORK")"#,
|
||||
r#"fs:SetPoint("TOPLEFT", nil, "TOPLEFT", 1, -1)"#,
|
||||
] {
|
||||
s.run(step).expect("trigger");
|
||||
s.resolve();
|
||||
@@ -576,6 +584,115 @@ fn derives(s: &UiScript) -> u64 {
|
||||
.layout_derives
|
||||
}
|
||||
|
||||
/// **Minting a frame appends a leaf instead of deriving the whole graph**, and the rects it leaves
|
||||
/// behind are the ones a full derivation would have produced.
|
||||
///
|
||||
/// This was the tooltip-raise cost, nearly whole. An addon that wants a one-shot delay in 1.12
|
||||
/// writes `CreateFrame("Frame")` and hangs an `OnUpdate` on it — there is no `C_Timer.After` — and
|
||||
/// OctoPawn does exactly that twice on every `GameTooltip` `OnShow`
|
||||
/// (`Scoring/ScoringCore.lua:540`, `UI/Paperdoll/Pdmo.lua:101`). Each mint gave up naming, so
|
||||
/// `UIParent:OnUpdate`'s three `GetLeft` calls each paid a derivation of all ~19,000 frames and
|
||||
/// ~44,000 regions: **2.19 ms a call, 6.56 ms of a 6.6 ms frame** on grom's interface
|
||||
/// (`benilla-app`'s `tick_bench`, `[ui-verbs]` `frame:GetLeft`). After this it is 0.14 ms, and the
|
||||
/// rebuilt window reports no derivations at all.
|
||||
///
|
||||
/// Why a leaf is sound: the id was minted this instant, so **no anchor anywhere can name it** —
|
||||
/// nothing in the cached graph reads the new node and no cached edge has to move. What it reads is
|
||||
/// its own anchors' targets, which the append is handed, because a mint is not always a creation:
|
||||
/// `frame_id` mints on the first *fetch* too, and a frame built in Rust (a nameplate, a chat
|
||||
/// bubble) is first fetched when Lua asks for its wrapper, with its anchors long written. That
|
||||
/// case is the second half of this test.
|
||||
#[test]
|
||||
fn minting_a_frame_appends_a_leaf_rather_than_deriving_the_graph() {
|
||||
let mut s = script();
|
||||
s.run(
|
||||
r#"root = CreateFrame("Frame", "Root", nil)
|
||||
root:SetWidth(100) root:SetHeight(50)
|
||||
root:SetPoint("TOPLEFT", nil, "TOPLEFT", 10, -10)"#,
|
||||
)
|
||||
.expect("a graph to append to");
|
||||
s.resolve();
|
||||
s.resolve();
|
||||
let before = derives(&s);
|
||||
|
||||
// A bare mint, the `C_Timer.After` shape: no anchors, no art, just a frame to hang an
|
||||
// `OnUpdate` on.
|
||||
s.run(r#"timer = CreateFrame("Frame")"#).expect("mint");
|
||||
s.resolve();
|
||||
assert_eq!(
|
||||
derives(&s),
|
||||
before,
|
||||
"minting a frame must not derive the graph from scratch"
|
||||
);
|
||||
// And the graph it was appended to still answers, which is what the append must not break.
|
||||
assert_eq!(
|
||||
s.eval::<f64>("return Root:GetLeft()").expect("root left"),
|
||||
10.0
|
||||
);
|
||||
|
||||
// A mint that is a *fetch*: a frame whose anchors were written before Lua ever asked for its
|
||||
// id. The anchor is to `Root`, so the new node reads a node already in the graph and the
|
||||
// append has to carry that edge — without it, moving `Root` would leave this one behind.
|
||||
let anchored = {
|
||||
let mut model = s.lua().app_data_mut::<Model>().expect("model app_data");
|
||||
let h = model
|
||||
.arena
|
||||
.create(crate::widget::FrameKind::Frame, Some("Late".into()), None);
|
||||
let root = model.arena.lookup("Root").expect("Root");
|
||||
let root_id = model.frame_id(root);
|
||||
let input = model.layout_inputs.entry(h).or_default();
|
||||
input.width = 20.0;
|
||||
input.height = 20.0;
|
||||
input.anchors = vec![crate::layout::Anchor::new(
|
||||
crate::layout::Point::TopLeft,
|
||||
root_id,
|
||||
crate::layout::Point::TopRight,
|
||||
0.0,
|
||||
0.0,
|
||||
)];
|
||||
h
|
||||
};
|
||||
s.resolve();
|
||||
s.resolve();
|
||||
let before = derives(&s);
|
||||
// The first fetch of its id, which is the mint: an append with one edge.
|
||||
let late_id = {
|
||||
let mut model = s.lua().app_data_mut::<Model>().expect("model app_data");
|
||||
model.frame_id(anchored)
|
||||
};
|
||||
assert!(late_id > 0);
|
||||
s.resolve();
|
||||
assert_eq!(
|
||||
derives(&s),
|
||||
before,
|
||||
"a first fetch of an already-anchored frame's id must append, not derive"
|
||||
);
|
||||
// Read off the model, not `_G`: a frame built in the arena is never published under a name,
|
||||
// which is exactly the kind this half of the test is about.
|
||||
let left_of = |s: &UiScript, h| {
|
||||
s.lua()
|
||||
.app_data_ref::<Model>()
|
||||
.expect("model app_data")
|
||||
.resolved
|
||||
.get(&h)
|
||||
.map(|r| r.left)
|
||||
};
|
||||
assert_eq!(
|
||||
left_of(&s, anchored),
|
||||
Some(110.0),
|
||||
"the appended node resolved against its anchor target: Root's left 10 + width 100"
|
||||
);
|
||||
// The edge is live: moving the target must move the appended node too.
|
||||
s.run(r#"Root:SetPoint("TOPLEFT", nil, "TOPLEFT", 30, -10)"#)
|
||||
.expect("move Root");
|
||||
s.resolve();
|
||||
assert_eq!(
|
||||
left_of(&s, anchored),
|
||||
Some(130.0),
|
||||
"without the appended edge this would still read 110"
|
||||
);
|
||||
}
|
||||
|
||||
/// A `SetScale` names its subtree instead of giving up on the whole graph.
|
||||
///
|
||||
/// It used to give up. `[ui-phases]`'s per-write-site tally put **232 of the 463** whole-graph
|
||||
|
||||
Reference in New Issue
Block a user