#3087: count every parameter a placeholder walks, and popup names by length

A definition of parameters that are not kept, shared by many instances, was
walked again for each one without spending the budget; long popup item
names were kept per instance with each item counted once.

crates/aep/src/import/placeholder.rs

@@ -11,8 +11,9 @@
 //! does). Layer parameters name their layer (their keys and expression are reported). Paths and
 //! arbitrary data are not kept (reported when they hold something).
 //!
-//! The parameters a placeholder holds are bounded per import ([`MAX_COST`]): definitions can be
-//! shared by many instances, so a small file could otherwise ask for very many properties. Once
+//! The parameters placeholders walk, kept or not, and their popup items (by name length) are
+//! bounded per import ([`MAX_COST`]): definitions can be shared by many instances, so a small file
+//! could otherwise ask for very many properties. Once
 //! the budget is spent, later effects without a counterpart are reported without a placeholder.
 
 use effectcraft_keyframe::Value;
@@ -24,12 +25,16 @@ use crate::import::props::{Kind, import_expression, import_keys};
 use crate::import::report::{Collector, LossCategory};
 use crate::{Effect as AepEffect, EffectParam, ParamKind, ParamValue};
 
-/// Most placeholder properties and popup items one import creates.
+/// Most placeholder parameters (kept or not) and popup items one import walks.
 pub(crate) const MAX_COST: usize = 200_000;
 
 /// Most items read from one placeholder popup.
 const MAX_POPUP_ITEMS: usize = 1024;
 
+/// A popup item costs one more for each of these bytes in its name, so the names kept stay
+/// bounded too.
+const NAME_BYTES: usize = 16;
+
 /// The placeholder of `fx`, with `params` (its parameter definitions) as properties.
 pub(crate) fn placeholder(
     fx: &AepEffect<'_, '_>,
@@ -45,26 +50,33 @@ pub(crate) fn placeholder(
     let mut group = ids.group(&segment(fx.match_name()), shown);
     group.kind = GroupKind::Effect { effect: fx.match_name().to_string() };
     let here = format!("{}: '{}' effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name());
-    for p in params.iter().filter(|p| !carries_nothing(p)) {
+    for p in params {
+        // Every parameter walked counts, kept or not: a definition shared by many instances is
+        // otherwise walked again for each one.
+        if carries_nothing(p) {
+            col.placeholder_cost = col.placeholder_cost.saturating_add(1).min(MAX_COST);
+            continue;
+        }
         if col.placeholder_cost >= MAX_COST {
             col.lose(LossCategory::Effects, format!("{here}: parameters past the import's {MAX_COST} placeholder parameters not kept"));
             break;
         }
         let label = param_label(p);
         let Some((kind, ui, default)) = shape(p, cx.size) else {
+            col.placeholder_cost = col.placeholder_cost.saturating_add(1);
             if p.stored().is_some() {
                 col.lose(LossCategory::Effects, format!("{here}: parameter {label} not kept (type {:?})", p.kind));
             }
             continue;
         };
-        let items = match &ui {
-            ParamUi::Popup { options } => options.len(),
-            _ => 0,
+        let (items, names) = match &ui {
+            ParamUi::Popup { options } => (options.len(), options.iter().map(|o| 1 + o.len() / NAME_BYTES).fold(0, usize::saturating_add)),
+            _ => (0, 0),
         };
         if p.kind == ParamKind::Popup && popup_len(p).is_some_and(|n| n > items) {
             col.lose(LossCategory::Effects, format!("{here}: parameter {label}: the items past the first {MAX_POPUP_ITEMS} not kept"));
         }
-        let cost = col.placeholder_cost.saturating_add(items).saturating_add(1);
+        let cost = col.placeholder_cost.saturating_add(names).saturating_add(1);
         if cost > MAX_COST {
             // The budget is spent: nothing more is built, here or in later effects (definitions
             // shared by many instances would otherwise still be walked item by item).

crates/aep/tests/it/import/placeholders.rs

@@ -190,6 +190,29 @@ fn an_effect_past_the_spent_budget_gets_no_placeholder() {
     assert!(found.iter().any(|d| d.contains("kept without rendering") && !d.contains("more like it")), "the kept instance alone: {found:?}");
 }
 
+/// Parameters a placeholder does not keep (markers, arbitrary data) still spend the budget: a
+/// definition of very many of them, shared by many instances, is not walked again for each one.
+#[test]
+fn parameters_not_kept_still_spend_the_budget() {
+    let defs = (0..200_000).map(|i| other(if i % 2 == 0 { K::NoData } else { K::ArbitraryData }, "X", Default::None)).collect();
+    let fx = Fx { match_name: "Odd Fx", name: "Odd", defs, values: vec![], data: vec![], keys: vec![] };
+    let (_, r) = import_bytes(&twice(&bytes_with(&fx), "Odd Fx"), "Odd Fx");
+    let found = losses(&r, LossCategory::Effects, "Odd Fx");
+    assert!(found.iter().any(|d| d.contains("not kept (past the import's placeholder parameters)")), "{found:?}");
+}
+
+/// Long popup item names spend the budget by their length, so the names kept stay bounded.
+#[test]
+fn long_popup_item_names_spend_the_budget() {
+    let long = "x".repeat(1600);
+    let items: &'static [&'static str] =
+        Box::leak((0..1024).map(|i| &*Box::leak(format!("{long}{i}").into_boxed_str())).collect::<Vec<_>>().into_boxed_slice());
+    let fx = Fx { match_name: "Long Fx", name: "Long", defs: vec![popup("A", 1, items), popup("B", 1, items)], values: vec![], data: vec![], keys: vec![] };
+    let (g, r) = placeholder_of(&fx);
+    assert_eq!(g.props().count(), 1);
+    assert!(losses(&r, LossCategory::Effects, "Long Fx").iter().any(|d| d.contains("placeholder parameters")));
+}
+
 /// `bytes` with the layer's instance of `match_name` (its `tdmn` and `sspc`) repeated after it.
 fn twice(bytes: &[u8], match_name: &str) -> Vec<u8> {
     let mut file = parse(bytes).unwrap();

docs/aep-import.md

@@ -333,7 +333,8 @@ parameter gets the file's value:
   pseudo effect's `Pseudo/0.25-0001`). Paths and arbitrary data are not kept; a popup whose
   definition is borrowed, or whose default slot holds no item, and that has no value record takes
   its first item (reported). One import keeps at
-  most 200 000 placeholder parameters and popup items (a popup at most 1024 items); the rest is
+  most 200 000 placeholder parameters and popup items (a popup at most 1024 items; parameters it
+  does not keep count, and an item one more per 16 bytes of its name); the rest is
   reported, and once that is spent a later effect without a counterpart arrives without a
   placeholder (reported). A placeholder is still reported in `effects` and counted in
   `imported.placeholderEffects`; .aep export reports it and leaves it out.