perf(plan-selection): accuracy pair checks reuse built outputs; shared input contexts measured (#1095 §8)

crates/plan-selection/src/groups/accuracy_positions.rs

@@ -17,7 +17,8 @@
 //! fails through one reader is unusable through every reader (the largest
 //! weight counts, as before). The candidates of one operator node (other
 //! timings or materializations) build the same node, so they are checked
-//! once, together.
+//! once, together. What a check builds is kept for the later checks
+//! ([`BuiltBefore`]).
 use std::cell::OnceCell;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::hash::{Hash, Hasher};
@@ -77,6 +78,7 @@ pub(crate) fn accuracy_positions(
     let exact_stand_ins: Vec<OnceCell<Rc<OperatorNode>>> = (0..physical.candidates.len())
         .map(|_| OnceCell::new())
         .collect();
+    let mut built_before = BuiltBefore::default();
     let mut positions = AccuracyPositions::default();
     for (query, root) in physical.query_roots.iter().enumerate() {
         let references: Vec<usize> = (0..output_refs.len())
@@ -117,7 +119,7 @@ pub(crate) fn accuracy_positions(
         let mut weights = BTreeMap::new();
         let mut usable: Vec<&[usize]> = Vec::new();
         for group in groups.values() {
-            let weight = match checker.check(group) {
+            let weight = match checker.check(group, &mut built_before) {
                 Outcome::Exact => continue,
                 Outcome::Meets => {
                     usable.push(group);
@@ -165,7 +167,7 @@ pub(crate) fn accuracy_positions(
                 .chain(usable[second])
                 .copied()
                 .collect();
-            if let Outcome::Fails(_) = checker.check(&both) {
+            if let Outcome::Fails(_) = checker.check(&both, &mut built_before) {
                 for &one in usable[first] {
                     for &other in usable[second] {
                         excluded.push((
@@ -296,21 +298,82 @@ struct Checker<'a> {
 /// The outputs one candidate is built as, or why they are not checked.
 type Built = Result<Vec<Rc<OperatorNode>>, UnusableReason>;
 
+/// What the checks built, kept across checks and queries. A candidate's
+/// outputs depend only on the members of the approximate set at or below
+/// it, so a pair check rebuilds only the candidates above both members
+/// and reuses the rest from the single checks. Outputs built across a
+/// cycle of links depend on where the walk entered the cycle and are not
+/// kept.
+#[derive(Default)]
+struct BuiltBefore {
+    /// Per (candidate, approximate members at or below it, ascending): its
+    /// outputs.
+    outputs: HashMap<(usize, Vec<usize>), Built>,
+    /// The [`reader_key`]s of every node built so far.
+    keys: Keys,
+}
+
+#[cfg(test)]
+thread_local! {
+    /// Set by a test to check with no kept builds: every check builds
+    /// from scratch ([`BuiltBefore`] cleared) and builds every output
+    /// candidate (no exact stand-in skipped).
+    pub(crate) static FRESH_BUILDS: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
+}
+
+/// One check's walk.
+#[derive(Default)]
+struct Walk {
+    /// Per candidate built in this check.
+    built: HashMap<usize, BuiltInWalk>,
+    /// The candidates being built.
+    visiting: Vec<usize>,
+    /// How many reads of a candidate still being built the walk has made.
+    cycle_reads: usize,
+}
+
+/// A candidate's outputs in one check, and whether they were built across
+/// a cycle of links (then they are not kept in [`BuiltBefore`]).
+struct BuiltInWalk {
+    outputs: Built,
+    across_cycle: bool,
+}
+
 impl Checker<'_> {
     /// Checks the query's outputs with exactly `approximate` approximate.
-    fn check(&self, approximate: &[usize]) -> Outcome {
-        let mut memo: HashMap<usize, Built> = HashMap::new();
-        let mut keys = Keys::new();
+    /// `before` holds what earlier checks built, for every query.
+    fn check(&self, approximate: &[usize], before: &mut BuiltBefore) -> Outcome {
+        #[cfg(test)]
+        let fresh = FRESH_BUILDS.with(std::cell::Cell::get);
+        #[cfg(not(test))]
+        let fresh = false;
+        if fresh {
+            *before = BuiltBefore::default();
+        }
+        let mut walk = Walk::default();
         let mut per_reference: Vec<Vec<Rc<OperatorNode>>> = Vec::new();
         for &reference in self.references {
             let mut outputs: Vec<Rc<OperatorNode>> = Vec::new();
+            // Exact stand-ins that are not summary states are the same
+            // for readers ([`reader_key`]): only the first is kept.
+            let mut exact_value_stand_in = false;
             for &class in &self.output_candidates[reference] {
-                match self.built(class.0, approximate, &mut memo, &mut keys, &mut Vec::new()) {
+                let reaches = approximate
+                    .iter()
+                    .any(|member| self.below[class.0].contains(member));
+                if !fresh
+                    && !reaches
+                    && self.physical.physical_node(class).result_kind != OperatorResultKind::State
+                    && std::mem::replace(&mut exact_value_stand_in, true)
+                {
+                    continue;
+                }
+                match self.built(class.0, approximate, &mut walk, before) {
                     Ok(built) => outputs.extend(built),
                     Err(reason) => return Outcome::Fails(reason),
                 }
             }
-            per_reference.push(distinct_for_readers(outputs, &mut keys));
+            per_reference.push(distinct_for_readers(outputs, &mut before.keys));
         }
         let Some(roots) = self.roots(&per_reference) else {
             return Outcome::Fails(UnusableReason::NotChecked);
@@ -374,49 +437,69 @@ impl Checker<'_> {
     }
 
     /// The outputs `class` is built as with `approximate` approximate
-    /// (module documentation). `visiting` holds the candidates being built,
-    /// so a cycle of links builds nothing.
+    /// (module documentation). `walk.visiting` holds the candidates being
+    /// built, so a cycle of links builds nothing.
     fn built(
         &self,
         class: usize,
         approximate: &[usize],
-        memo: &mut HashMap<usize, Built>,
-        keys: &mut Keys,
-        visiting: &mut Vec<usize>,
+        walk: &mut Walk,
+        before: &mut BuiltBefore,
     ) -> Built {
-        if let Some(done) = memo.get(&class) {
-            return done.clone();
+        if let Some(done) = walk.built.get(&class) {
+            walk.cycle_reads += usize::from(done.across_cycle);
+            return done.outputs.clone();
         }
-        if visiting.contains(&class) {
+        if walk.visiting.contains(&class) {
+            walk.cycle_reads += 1;
             return Ok(Vec::new());
         }
-        let node = self.physical.physical_node(PhysicalCandidateId(class));
         let reaches = |candidate: usize| {
             approximate
                 .iter()
                 .any(|member| self.below[candidate].contains(member))
         };
+        if !reaches(class) {
+            return Ok(vec![self.exact_stand_in(class)]);
+        }
+        let mut members: Vec<usize> = approximate
+            .iter()
+            .copied()
+            .filter(|member| self.below[class].contains(member))
+            .collect();
+        members.sort_unstable();
+        let key = (class, members);
+        if let Some(done) = before.outputs.get(&key) {
+            walk.built.insert(
+                class,
+                BuiltInWalk {
+                    outputs: done.clone(),
+                    across_cycle: false,
+                },
+            );
+            return done.clone();
+        }
+        let cycle_reads = walk.cycle_reads;
+        let node = self.physical.physical_node(PhysicalCandidateId(class));
         let producers = &self.producer_candidates_per_input[class];
         let below_inputs = producers
             .iter()
             .flatten()
             .any(|producer| reaches(producer.0));
-        let built = if !reaches(class) {
-            Ok(vec![self.exact_stand_in(class)])
-        } else if !below_inputs {
+        let built = if !below_inputs {
             // `class` is in `approximate`, over exact inputs: its node with
             // every guarantee below it composed, as the realizer does.
             with_guarantees(node, &mut HashMap::new())
                 .map(|node| vec![node])
                 .map_err(|_| UnusableReason::NotChecked)
         } else {
-            visiting.push(class);
+            walk.visiting.push(class);
             let mut per_input = Vec::with_capacity(producers.len());
             let mut failed = None;
             for input in producers {
                 let mut outputs = Vec::new();
                 for producer in input.iter().filter(|producer| reaches(producer.0)) {
-                    match self.built(producer.0, approximate, memo, keys, visiting) {
+                    match self.built(producer.0, approximate, walk, before) {
                         Ok(built) => outputs.extend(built),
                         Err(reason) => failed = Some(reason),
                     }
@@ -430,15 +513,25 @@ impl Checker<'_> {
                 if let Some(exact) = exact {
                     outputs.push(self.exact_stand_in(exact.0));
                 }
-                per_input.push(distinct_for_readers(outputs, keys));
+                per_input.push(distinct_for_readers(outputs, &mut before.keys));
             }
-            visiting.pop();
+            walk.visiting.pop();
             match failed {
                 Some(reason) => Err(reason),
-                None => rebuilt(node, &per_input, keys),
+                None => rebuilt(node, &per_input, &mut before.keys),
             }
         };
-        memo.insert(class, built.clone());
+        let across_cycle = walk.cycle_reads != cycle_reads;
+        if !across_cycle {
+            before.outputs.insert(key, built.clone());
+        }
+        walk.built.insert(
+            class,
+            BuiltInWalk {
+                outputs: built.clone(),
+                across_cycle,
+            },
+        );
         built
     }
 }
@@ -565,3 +658,133 @@ fn key_and_settled(node: &Rc<OperatorNode>, keys: &mut Keys) -> (u64, bool) {
     keys.insert(Rc::as_ptr(node), (known.0, known.1, node.clone()));
     known
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::groups::tests::{candidate, catalog, reader_of, relative, root, values_node};
+    use asap_logical_asap_optimization::test_support::entry_with;
+    use asap_types::ir::properties::AccuracyTarget;
+    use asap_types::workload::{QueryRecurrence, RepeatedDemand, RepetitionInterval};
+
+    /// The depth of each output of `built`.
+    fn depths(built: &Built) -> Vec<usize> {
+        fn depth(node: &OperatorNode) -> usize {
+            1 + node
+                .children()
+                .into_iter()
+                .map(|child| depth(child))
+                .max()
+                .unwrap_or(0)
+        }
+        built
+            .as_ref()
+            .expect("built")
+            .iter()
+            .map(|node| depth(node))
+            .collect()
+    }
+
+    // A two-candidate link cycle above an approximate member: `A` reads
+    // the leaf or `B`; `B` reads `A` or a second leaf. Walking from the
+    // top enters the cycle at `A`, so `B` is built there with `A` cut off
+    // (a cycle read), over the second leaf only. Such outputs are not
+    // kept: a later walk that starts at `B` builds it in full, over `A`,
+    // as with no kept builds, for one member and for a pair.
+    #[test]
+    fn outputs_built_across_a_cycle_are_not_kept() {
+        let approximate_leaf = |value: f64, error: f64| {
+            Rc::new(
+                (*values_node(value))
+                    .clone()
+                    .with_guarantee(Some(relative(error, 0.01))),
+            )
+        };
+        let leaf = approximate_leaf(1.0, 0.01);
+        let other_leaf = approximate_leaf(2.0, 0.02);
+        let a = reader_of(&leaf);
+        let b = reader_of(&leaf);
+        let top = reader_of(&a);
+        let physical = catalog(
+            vec![(leaf, 0), (a, 1), (b, 2), (top, 3), (other_leaf, 4)],
+            vec![
+                candidate(0, &[], false),
+                candidate(1, &[&[0, 2]], false),
+                candidate(2, &[&[1, 4]], false),
+                candidate(3, &[&[1]], true),
+                candidate(4, &[], false),
+            ],
+            vec![root(3)],
+        );
+        let producers: Vec<Vec<Vec<PhysicalCandidateId>>> = physical
+            .candidates
+            .iter()
+            .map(|candidate| {
+                candidate
+                    .producer_candidates_per_input
+                    .iter()
+                    .map(|producers| producers.iter().copied().collect())
+                    .collect()
+            })
+            .collect();
+        // Leaves 0 and 4 stand for approximate candidates.
+        let below: Vec<BTreeSet<usize>> = vec![
+            BTreeSet::from([0]),
+            BTreeSet::from([0, 4]),
+            BTreeSet::from([0, 4]),
+            BTreeSet::from([0, 4]),
+            BTreeSet::from([4]),
+        ];
+        let exact_stand_ins: Vec<OnceCell<Rc<OperatorNode>>> = (0..physical.candidates.len())
+            .map(|_| OnceCell::new())
+            .collect();
+        let entry = entry_with(
+            AccuracyTarget::max_error(0.01).failure_probability(0.01),
+            QueryRecurrence::Repeated(RepeatedDemand::FixedInterval(RepetitionInterval(60_000))),
+        );
+        let data = DataWorkload::default();
+        let cost_inputs = asap_cost::CostInputs::default();
+        let models = SelectionModels {
+            accuracy: &asap_accuracy::BuiltinAccuracyModel,
+            evidence: &asap_accuracy::NoAccuracyEvidence,
+            cost_inputs: &cost_inputs,
+        };
+        let output_candidates = vec![vec![PhysicalCandidateId(3)]];
+        let checker = Checker {
+            physical: &physical,
+            exact_stand_ins: &exact_stand_ins,
+            producer_candidates_per_input: &producers,
+            output_candidates: &output_candidates,
+            below: &below,
+            query: 0,
+            root: &physical.query_roots[0],
+            references: &[0],
+            entry: &entry,
+            data: &data,
+            models: &models,
+        };
+        for approximate in [&[0][..], &[0, 4]] {
+            let mut kept = BuiltBefore::default();
+            // From the top: `B` is built with `A` cut off, over the second
+            // leaf alone (depth 2).
+            let mut walk = Walk::default();
+            checker
+                .built(3, approximate, &mut walk, &mut kept)
+                .expect("built");
+            assert!(walk.cycle_reads > 0);
+            assert!(walk.built[&2].across_cycle);
+            assert_eq!(depths(&walk.built[&2].outputs)[0], 2, "{approximate:?}");
+            // From `B`: in full, its first output reads `A` over the leaf
+            // (depth 3), with kept builds as with none.
+            let from_b = checker.built(2, approximate, &mut Walk::default(), &mut kept);
+            let fresh = checker.built(
+                2,
+                approximate,
+                &mut Walk::default(),
+                &mut BuiltBefore::default(),
+            );
+            assert_eq!(depths(&from_b), depths(&fresh), "{approximate:?}");
+            assert_eq!(depths(&fresh)[0], 3, "{approximate:?}");
+        }
+    }
+}