From 089990a929b560c5e3c628c27feb2682a6376893 Mon Sep 17 00:00:00 2001 From: grom Date: Sun, 18 Oct 2026 12:35:33 -0600 Subject: [PATCH] chain: the icon catalog lists its own directory, not all 102,144 names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiled off the player's own 2026-10-18 session log, where one blocked frame ran 7,725 ms: the file loading is 4,505 ms of it (pfQuest's `init\data.xml` alone 695 ms, pfQuest-octo's 311 ms, all of FrameXML 693 ms), then `load_icon_catalog` 396 ms, the random-property table 259 ms, and the first VM tick the remaining ~2.3 s. This takes the icon catalog. It walked `Chain::list()`, and the walk is what the catalog costs — the module says so itself: classifying the three buckets out of the names is 1.4 ms. Per name a full list allocates a normalized dedup key, hashes it into a 102k-entry set, resolves the winning archive and asks it for a size; the prefix this caller wants was tested *after* all of that, on every name in the chain. So `Chain::list_under(prefix)` tests it first, with the same case-insensitive, separator-blind compare the caller was doing for itself. Measured on this install, and the test prints the pair it quotes: 102,144 entries in 471 ms listed whole against 51 ms for the 3,205 under `Interface\Icons\`, the same entries and the same sizes either way. 420 ms off every login, and off every `/reload`, which rebuilds the catalog for the new VM. Nothing else in the client lists the chain; `benilla-extract` does and wants the lot, so `list()` keeps its name and its body, now shared. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-formats/src/chain.rs | 92 ++++++++++++++++++++++- crates/benilla-formats/src/macro_icons.rs | 22 ++++-- 2 files changed, 105 insertions(+), 9 deletions(-) diff --git a/crates/benilla-formats/src/chain.rs b/crates/benilla-formats/src/chain.rs index 980900b1..7306bf16 100644 --- a/crates/benilla-formats/src/chain.rs +++ b/crates/benilla-formats/src/chain.rs @@ -139,6 +139,48 @@ impl Chain { /// (most of `texture.MPQ`) is readable by name but not listed. Unions every archive's /// `(listfile)`, as each names only its own files; sizes come from the winning archive. pub fn list(&self) -> Result> { + self.list_filtered(None) + } + + /// [`Self::list`] restricted to the names under `prefix`, compared the way MPQ hashing + /// compares names: any case, `/` and `\` alike. + /// + /// Worth having because the *filter* is what the walk costs. Per name a full list allocates a + /// normalized dedup key, hashes it, resolves the winning archive and asks it for a size; the + /// prefix test is a byte compare that runs before all of it. On this install's chain — 102,144 + /// listfile entries — a full list is 471 ms where `Interface\Icons\` out of it is 51 ms, the + /// same 3,205 entries either way ([`crate::macro_icons`]'s catalog is the one caller that + /// wants a directory rather than the lot; the rest is `benilla-extract`, which wants the lot). + pub fn list_under(&self, prefix: &str) -> Result> { + let key: Vec = prefix + .bytes() + .map(|b| { + if b == b'/' { + b'\\' + } else { + b.to_ascii_lowercase() + } + }) + .collect(); + self.list_filtered(Some(&key)) + } + + /// The shared body: `prefix` is already lowercased with `\` separators. + fn list_filtered(&self, prefix: Option<&[u8]>) -> Result> { + let under = |name: &str| match prefix { + None => true, + Some(p) => { + let n = name.as_bytes(); + n.len() >= p.len() + && n[..p.len()].iter().zip(p).all(|(&a, &b)| { + (if a == b'/' { + b'\\' + } else { + a.to_ascii_lowercase() + }) == b + }) + } + }; let mut seen = HashSet::new(); let mut out = Vec::new(); for archive in &self.archives { @@ -147,8 +189,11 @@ impl Chain { }; for raw in String::from_utf8_lossy(&listfile).split([';', '\r', '\n']) { let name = raw.trim(); + if name.is_empty() || !under(name) { + continue; + } // Dedupe the way MPQ hashing compares names: any case, `/` and `\` alike. - if name.is_empty() || !seen.insert(name.replace('/', "\\").to_ascii_lowercase()) { + if !seen.insert(name.replace('/', "\\").to_ascii_lowercase()) { continue; } if let Some(a) = self.resolve(name) { @@ -234,4 +279,49 @@ mod tests { owned(&["DBC.mpq", "Model.MPQ", "PATCH.mpq"]) ); } + + /// `list_under` answers exactly what filtering a full `list` answers, and far faster: the + /// numbers this prints are the ones quoted in [`Chain::list_under`]'s doc. + #[test] + fn a_prefix_list_is_the_full_list_filtered_and_pays_for_the_prefix_only() { + let data = crate::wow_data_or_skip!(); + let chain = crate::open_chain(&data).expect("open chain"); + const DIR: &str = "Interface\\Icons\\"; + + let t0 = std::time::Instant::now(); + let all = chain.list().expect("list"); + let full_ms = t0.elapsed().as_secs_f32() * 1000.0; + let t1 = std::time::Instant::now(); + let under = chain.list_under(DIR).expect("list_under"); + let under_ms = t1.elapsed().as_secs_f32() * 1000.0; + + let norm = |n: &str| n.replace('/', "\\").to_ascii_lowercase(); + let mut want: Vec = all + .iter() + .map(|e| norm(&e.name)) + .filter(|n| n.starts_with(&norm(DIR))) + .collect(); + let mut got: Vec = under.iter().map(|e| norm(&e.name)).collect(); + want.sort(); + got.sort(); + eprintln!( + "chain listfile union {} entries in {full_ms:.0} ms; {DIR} is {} entries in \ + {under_ms:.0} ms", + all.len(), + under.len() + ); + assert_eq!(got, want, "the filtered list and the full list must agree"); + assert!( + !got.is_empty(), + "this install has icons — otherwise nothing is proven" + ); + // Sizes come from the same winning archive either way. + for e in &under { + let full = all + .iter() + .find(|f| norm(&f.name) == norm(&e.name)) + .expect("in the full list too"); + assert_eq!(e.size, full.size, "{}", e.name); + } + } } diff --git a/crates/benilla-formats/src/macro_icons.rs b/crates/benilla-formats/src/macro_icons.rs index dd1e0735..c32c3f5a 100644 --- a/crates/benilla-formats/src/macro_icons.rs +++ b/crates/benilla-formats/src/macro_icons.rs @@ -5,9 +5,14 @@ //! [`load_macro_icon_catalog`] does that one walk and splits it three ways: 1.12's own chooser //! list, plus the spell and item buckets the modern four-function enumeration surface //! (`GetMacroIcons`/`GetMacroItemIcons`/`GetLooseMacroIcons`/`GetLooseMacroItemIcons`, behind -//! `IconDataProviderMixin`) serves. One walk because the walk is the whole cost: the chain's -//! `(listfile)` union is 102,144 entries and 471 ms on the measured install, where classifying all -//! three buckets out of it is 1.4 ms. +//! `IconDataProviderMixin`) serves. One walk because the walk is the whole cost: classifying all +//! three buckets out of the names is 1.4 ms, where reaching the names is tens of milliseconds. +//! +//! And it is one walk of the **directory**, not of the chain: [`Chain::list_under`] tests the +//! prefix before the per-name resolve and size lookup a full list pays for all 102,144 of this +//! install's listfile entries. That is 471 ms listed whole against 51 ms for the 3,205 under +//! `Interface\Icons\` — 420 ms off every login and every `/reload`, which rebuilds the catalog +//! for the new VM. use anyhow::Result; @@ -58,12 +63,13 @@ pub fn load_macro_icon_catalog(chain: &mut Chain) -> Result { let mut chooser: Vec = Vec::new(); let mut spell: Vec = Vec::new(); let mut item: Vec = Vec::new(); - for entry in chain.list()? { + // `list_under`, not `list`: the chain's whole `(listfile)` union is the cost of this walk, + // and the prefix test runs before the per-name resolve that makes it expensive. Everything + // below is unchanged — the filter is the same case-insensitive, separator-blind compare the + // loop did for itself. + for entry in chain.list_under(ICON_DIR)? { let path = entry.name.replace('/', "\\"); - // Case-insensitive (`SStrCmpI`), as listfiles are not consistently cased. - if path.len() <= ICON_DIR.len() - || !path.as_bytes()[..ICON_DIR.len()].eq_ignore_ascii_case(ICON_DIR.as_bytes()) - { + if path.len() <= ICON_DIR.len() { continue; } let file = &path[ICON_DIR.len()..];