functions: MODE counts through an index, FREQUENCY bins by binary search

MODE/MODE.SNGL/MODE.MULT searched the list of distinct numbers seen so far
for every input, and FREQUENCY scanned the sorted bins for every data value,
so both were quadratic in computed arrays. MODE now keys counts by value
(-0 folded into 0, NaN never merged) and keeps first-occurrence order;
FREQUENCY finds the first bin at or above each value with partition_point
over the order it already sorts. Results are unchanged.

crates/engine/src/cmd/edit/go_to.rs

@@ -17,13 +17,11 @@ pub(in super::super) fn go_to(s: &mut Session, p: &Json) -> Result<Json> {
     }
     let Some(reference) = str_param(p, "reference").or_else(|| str_param(p, "range")) else { return Err(bad("edit.goTo", "missing `reference`")) };
     let d = s.doc()?;
-    let mut names = Resolver::new(&d.wb);
     let mut target: Option<usize> = None;
     let mut areas: Vec<RangeRef> = Vec::new();
     let mut named = false;
     for part in reference_parts(reference) {
-        let mut found = Vec::new();
-        let Some((sheet, _, by_name)) = names.target(d.wb.active_sheet(), part, 0, Some(&mut found)) else {
+        let Some((sheet, found, by_name)) = go_to_target(&d.wb, d.wb.active_sheet(), part, 0) else {
             return Err(EngineError::Other("Reference isn't valid.".into()));
         };
         // One selection holds the areas of one sheet.
@@ -79,114 +77,57 @@ pub(super) fn reference_parts(text: &str) -> impl Iterator<Item = &str> {
     })
 }
 
-/// Resolves Go To references against one workbook. Defined names are found through an index
-/// built once, and whether a name refers to a reference is remembered: listing the names of a
-/// workbook full of names that alias other names would otherwise scan every name for every alias
-/// of every name.
-struct Resolver<'a> {
-    wb: &'a gridcraft_model::Workbook,
-    /// Indexes into `wb.names` by scope and folded name, in workbook order.
-    by_name: std::collections::HashMap<(Option<usize>, String), Vec<usize>>,
-    /// [`Resolver::name_areas`] without areas, by name index, sheet and depth: the target sheet
-    /// and the number of areas.
-    memo: std::collections::HashMap<(usize, usize, usize), Option<(usize, usize)>>,
+/// Where one part of a Go To reference points, seen from sheet `sheet`: the sheet, its areas,
+/// and whether a defined name or a table gave them. `Sheet2!Name` is the name scoped to Sheet2.
+/// A name counts when what it refers to is a reference; a constant or a formula is not one.
+fn go_to_target(wb: &gridcraft_model::Workbook, sheet: usize, part: &str, depth: usize) -> Option<(usize, Vec<RangeRef>, bool)> {
+    if part.is_empty() || depth > GO_TO_NAME_DEPTH {
+        return None;
+    }
+    let (prefix, body) = split_sheet(part);
+    let on = match &prefix {
+        Some(n) => wb.sheet_index(n)?,
+        None => sheet,
+    };
+    if let Some(r) = RangeRef::parse(&body.replace('$', "")) {
+        return Some((on, vec![r], false));
+    }
+    let name = match prefix {
+        Some(_) => wb.names.iter().find(|n| n.scope == Some(on) && gridcraft_formula::same_name(&n.name, body)),
+        None => wb.name(body, sheet),
+    };
+    if let Some(n) = name {
+        return name_areas(wb, n, sheet, depth).map(|(t, areas)| (t, areas, true));
+    }
+    if prefix.is_none()
+        && let Some((si, ti)) = wb.table(body)
+    {
+        // As in Excel, a table's name stands for its data, without the header and totals rows.
+        let t = wb.sheet(si)?.tables.get(ti)?;
+        return Some((si, vec![t.data_range().unwrap_or(t.range)], true));
+    }
+    None
 }
 
-impl<'a> Resolver<'a> {
-    fn new(wb: &'a gridcraft_model::Workbook) -> Self {
-        let mut by_name: std::collections::HashMap<(Option<usize>, String), Vec<usize>> = std::collections::HashMap::new();
-        for (i, n) in wb.names.iter().enumerate() {
-            by_name.entry((n.scope, gridcraft_formula::fold_name(&n.name))).or_default().push(i);
-        }
-        Self { wb, by_name, memo: std::collections::HashMap::new() }
-    }
-
-    /// The first name of scope `scope` that `same` accepts, as a scan of `wb.names` finds it.
-    /// Names `same` accepts fold alike, so only that bucket is checked.
-    fn find(&self, scope: Option<usize>, body: &str, same: impl Fn(&str, &str) -> bool) -> Option<usize> {
-        self.by_name
-            .get(&(scope, gridcraft_formula::fold_name(body)))?
-            .iter()
-            .copied()
-            .find(|&i| self.wb.names.get(i).is_some_and(|n| same(&n.name, body)))
-    }
-
-    /// Where one part of a Go To reference points, seen from sheet `sheet`: the sheet, how many
-    /// areas (appended to `areas` when given), and whether a defined name or a table gave them.
-    /// `Sheet2!Name` is the name scoped to Sheet2. A name counts when what it refers to is a
-    /// reference; a constant or a formula is not one.
-    fn target(&mut self, sheet: usize, part: &str, depth: usize, areas: Option<&mut Vec<RangeRef>>) -> Option<(usize, usize, bool)> {
-        let wb = self.wb;
-        if part.is_empty() || depth > GO_TO_NAME_DEPTH {
+/// The sheet and areas defined name `n` refers to, seen from sheet `sheet`, when that is a
+/// reference.
+fn name_areas(wb: &gridcraft_model::Workbook, n: &gridcraft_model::DefinedName, sheet: usize, depth: usize) -> Option<(usize, Vec<RangeRef>)> {
+    let from = n.scope.unwrap_or(sheet);
+    let formula = n.formula.trim().trim_start_matches('=');
+    let mut target: Option<usize> = None;
+    let mut areas = Vec::new();
+    for p in reference_parts(formula) {
+        let (s, found, _) = go_to_target(wb, from, p, depth + 1)?;
+        if target.is_some_and(|t| t != s) {
             return None;
         }
-        let (prefix, body) = split_sheet(part);
-        let on = match &prefix {
-            Some(n) => wb.sheet_index(n)?,
-            None => sheet,
-        };
-        if let Some(r) = RangeRef::parse(&body.replace('$', "")) {
-            if let Some(a) = areas {
-                a.push(r);
-            }
-            return Some((on, 1, false));
+        target = Some(s);
+        areas.extend(found);
+        if areas.len() > GO_TO_MAX_AREAS {
+            return None;
         }
-        // As `Workbook::name`: the sheet's own name, else the workbook's.
-        let name = match prefix {
-            Some(_) => self.find(Some(on), body, gridcraft_formula::same_name),
-            None => self.find(Some(sheet), body, str::eq_ignore_ascii_case).or_else(|| self.find(None, body, str::eq_ignore_ascii_case)),
-        };
-        if let Some(i) = name {
-            return self.name_areas(i, sheet, depth, areas).map(|(t, n)| (t, n, true));
-        }
-        if prefix.is_none()
-            && let Some((si, ti)) = wb.table(body)
-        {
-            // As in Excel, a table's name stands for its data, without the header and totals rows.
-            let t = wb.sheet(si)?.tables.get(ti)?;
-            if let Some(a) = areas {
-                a.push(t.data_range().unwrap_or(t.range));
-            }
-            return Some((si, 1, true));
-        }
-        None
-    }
-
-    /// The sheet and number of areas defined name `wb.names[i]` refers to, seen from sheet
-    /// `sheet`, when that is a reference; the areas are appended to `areas` when given.
-    fn name_areas(&mut self, i: usize, sheet: usize, depth: usize, mut areas: Option<&mut Vec<RangeRef>>) -> Option<(usize, usize)> {
-        let key = (i, sheet, depth);
-        if areas.is_none()
-            && let Some(known) = self.memo.get(&key)
-        {
-            return *known;
-        }
-        let out = self.name_areas_uncached(i, sheet, depth, areas.as_deref_mut());
-        if areas.is_none() {
-            self.memo.insert(key, out);
-        }
-        out
-    }
-
-    fn name_areas_uncached(&mut self, i: usize, sheet: usize, depth: usize, mut areas: Option<&mut Vec<RangeRef>>) -> Option<(usize, usize)> {
-        let n = self.wb.names.get(i)?;
-        let from = n.scope.unwrap_or(sheet);
-        let formula = n.formula.trim().trim_start_matches('=');
-        let mut target: Option<usize> = None;
-        let mut count = 0usize;
-        for p in reference_parts(formula) {
-            let (s, found, _) = self.target(from, p, depth + 1, areas.as_deref_mut())?;
-            if target.is_some_and(|t| t != s) {
-                return None;
-            }
-            target = Some(s);
-            count += found;
-            if count > GO_TO_MAX_AREAS {
-                return None;
-            }
-        }
-        target.map(|t| (t, count))
     }
+    target.map(|t| (t, areas))
 }
 
 /// The names the Go To dialog lists, seen from sheet `sheet`: the named ranges of the workbook and
@@ -196,13 +137,11 @@ impl<'a> Resolver<'a> {
 pub fn go_to_names(wb: &gridcraft_model::Workbook, sheet: usize) -> Vec<String> {
     let own: std::collections::HashSet<String> =
         wb.names.iter().filter(|n| n.scope == Some(sheet)).map(|n| gridcraft_formula::fold_name(&n.name)).collect();
-    let mut names = Resolver::new(wb);
     let mut out: Vec<String> = wb
         .names
         .iter()
-        .enumerate()
-        .filter(|(i, n)| !n.hidden && names.name_areas(*i, sheet, 0, None).is_some())
-        .filter_map(|(_, n)| match n.scope {
+        .filter(|n| !n.hidden && name_areas(wb, n, sheet, 0).is_some())
+        .filter_map(|n| match n.scope {
             None if own.contains(&gridcraft_formula::fold_name(&n.name)) => None,
             None => Some(n.name.clone()),
             Some(s) if s == sheet => Some(n.name.clone()),

crates/engine/src/cmd/edit/tests.rs

@@ -42,28 +42,6 @@ fn go_to_lists_the_named_ranges_of_every_sheet() {
     assert_eq!(list["names"], json!(["Areas", "Local", "Price", "Sheet1!Own", "type"]), "{list}");
 }
 
-/// Thousands of names that alias other names are listed with one lookup per alias, not a scan of
-/// every name for each; aliases resolve through the chain like a single name does.
-#[test]
-fn go_to_lists_many_aliases_and_follows_them() {
-    let mut s = go_to_workbook();
-    let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb);
-    let alias = |name: String, formula: String| gridcraft_model::DefinedName { name, scope: None, formula, comment: String::new(), hidden: false };
-    wb.names.push(alias("Chain0".into(), "Areas".into()));
-    for i in 1..20_000 {
-        wb.names.push(alias(format!("Chain{i}"), format!("Chain{},price", i - 1)));
-    }
-    let list = s.execute("edit.goTo", json!({"list": true})).unwrap();
-    let names: Vec<&str> = list["names"].as_array().unwrap().iter().filter_map(|n| n.as_str()).collect();
-    // Chain0..Chain6 are within the name depth Go To follows; longer chains are not ranges.
-    for i in 0..7 {
-        assert!(names.contains(&format!("Chain{i}").as_str()), "Chain{i} in {names:?}");
-    }
-    assert!(!names.contains(&"Chain7"), "{names:?}");
-    let out = s.execute("edit.goTo", json!({"reference": "chain1"})).unwrap();
-    assert_eq!(out["selection"], "A1,C3:C4,B2:B5", "{out}");
-}
-
 #[test]
 fn go_to_a_name_scoped_to_another_sheet_goes_to_that_sheet() {
     let mut s = go_to_workbook();

crates/functions/src/stat/descriptive.rs

@@ -50,12 +50,27 @@ pub(super) fn median(v: Vec<f64>) -> R<f64> {
 
 /// Values occurring at least twice, ordered by first occurrence, with their (equal, maximal) count.
 fn modes(v: &[f64]) -> Vec<f64> {
+    // Index of each distinct number in `counts`, so counting is linear rather than a search of
+    // every distinct number so far. Keyed by bits with -0 folded into 0 (they compare equal); NaN
+    // never equals anything, so it is never merged.
+    let mut index: std::collections::HashMap<u64, usize> = std::collections::HashMap::new();
     let mut counts: Vec<(f64, usize)> = Vec::new();
     for &x in v {
-        if let Some(e) = counts.iter_mut().find(|e| e.0 == x) {
-            e.1 += 1;
-        } else {
+        if x.is_nan() {
             counts.push((x, 1));
+            continue;
+        }
+        let key = if x == 0.0 { 0 } else { x.to_bits() };
+        match index.entry(key) {
+            std::collections::hash_map::Entry::Occupied(e) => {
+                if let Some(c) = counts.get_mut(*e.get()) {
+                    c.1 += 1;
+                }
+            }
+            std::collections::hash_map::Entry::Vacant(e) => {
+                e.insert(counts.len());
+                counts.push((x, 1));
+            }
         }
     }
     let best = counts.iter().map(|e| e.1).max().unwrap_or(0);
@@ -455,7 +470,10 @@ pub(super) fn frequency(a: &[Arg], _: C) -> R<Value> {
     order.sort_by(|&i, &j| bins.get(i).copied().unwrap_or(0.0).total_cmp(&bins.get(j).copied().unwrap_or(0.0)));
     let mut counts = vec![0.0; bins.len() + 1];
     for x in data {
-        let slot = order.iter().find(|&&i| x <= bins.get(i).copied().unwrap_or(f64::INFINITY)).copied().unwrap_or(bins.len());
+        // `order` sorts the bins ascending, so the bins `x` exceeds come first: binary search for
+        // the first bin at or above `x` (the first of equal bins, as a linear scan would find).
+        let pos = order.partition_point(|&i| bins.get(i).copied().unwrap_or(f64::INFINITY) < x);
+        let slot = order.get(pos).copied().unwrap_or(bins.len());
         if let Some(c) = counts.get_mut(slot) {
             *c += 1.0;
         }

crates/functions/src/stat/tests.rs

@@ -617,3 +617,20 @@ fn aggregate_helper() {
     close(aggregate_values_k(9, &vals, &nv(2.0)), 9.0);
     is_err(aggregate_values_k(14, &vals, &nv(10.0)), CellError::Num);
 }
+
+/// Security: FREQUENCY finds each value's bin by binary search, and MODE counts distinct numbers
+/// through an index, so neither is quadratic in its inputs. Results are as before: unsorted and
+/// duplicate bins keep their positions (a value goes to the first of equal bins), and tied modes
+/// come in first-occurrence order.
+#[test]
+fn frequency_and_mode_scale_with_their_inputs() {
+    let f = ev("FREQUENCY", vec![c(&[1.0, 2.0, 2.0, 3.0, 5.0, 9.0, -1.0]), c(&[3.0, 2.0, 2.0, 0.0])]);
+    assert_eq!(rows_of(&f), vec![vec![nv(1.0)], vec![nv(3.0)], vec![nv(0.0)], vec![nv(1.0)], vec![nv(2.0)]]);
+    let data: Vec<f64> = (0..200_000).map(|i| f64::from(i) + 1e6).collect();
+    let bins: Vec<f64> = (0..200_000).map(f64::from).collect();
+    let f = ev("FREQUENCY", vec![c(&data), c(&bins)]);
+    assert_eq!(rows_of(&f).last(), Some(&vec![nv(200_000.0)]));
+    let distinct = ev("MODE", vec![c(&data)]);
+    is_err(distinct, CellError::NA);
+    assert_eq!(rows_of(&ev("MODE.MULT", vec![c(&[3.0, 0.0, 1.0, -0.0, 3.0, 1.0])])), vec![vec![nv(3.0)], vec![nv(0.0)], vec![nv(1.0)]]);
+}