From c4e2b83031a246f34f4fb811c1bf7539b1d28e09 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Fri, 15 May 2026 15:19:20 +0530 Subject: [PATCH 01/13] docs: add design spec for Wave 2 PR 1 (predicate push-down + mixed rule optimization) Covers issues #207 and #206: extends optimizer::plan() to accept Expr clauses and interleave them at the earliest position where their variables are bound, and routes the StratifiedEvaluator mixed-rules path through the updated planner. Co-Authored-By: Claude Sonnet 4.6 --- ...5-predicate-pushdown-mixed-rules-design.md | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md diff --git a/docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md b/docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md new file mode 100644 index 000000000..9fdbb1134 --- /dev/null +++ b/docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md @@ -0,0 +1,158 @@ +# Design: Predicate Push-down + Mixed Rule Optimization (Wave 2 PR 1) + +**Issues**: #207 (predicate push-down for expression clauses), #206 (optimize mixed rule evaluation) +**Date**: 2026-05-15 +**Branch**: single worktree + PR covering both issues + +--- + +## Summary + +Two related query execution improvements delivered in one PR: + +- **#207**: Push `Expr` predicate clauses (e.g. `[(> ?age 30)]`) down to the earliest position in the execution plan where all their variables are bound, rather than applying them all in a post-pass after full pattern matching. +- **#206**: Route the mixed-rules path in `StratifiedEvaluator` through the updated planner so it benefits from both pattern selectivity ordering and predicate push-down — no separate implementation needed. + +The unifying change is extending `optimizer::plan()` to accept both `Pattern` and `Expr` clauses and return an interleaved ordered plan. + +--- + +## Current Behaviour + +**Query execution (executor.rs)**: +``` +where_clauses + → extract Pattern variants + → plan(patterns) → sorted [(Pattern, IndexHint)] # selectivity order + → PatternMatcher runs all patterns → full binding set + → apply_expr_clauses(bindings) → post-pass filter # all exprs at the end +``` + +**Mixed rules (evaluator.rs `StratifiedEvaluator`)**: +``` +rule body + → extract Pattern variants only (in original order, no plan() call) + → PatternMatcher runs patterns → bindings + → not/notjoin post-filter +``` + +Both paths leave `Expr` clauses to the end, potentially carrying large intermediate binding sets through every pattern join before any filter is applied. + +--- + +## Design + +### 1. `plan()` — new signature and algorithm + +**File**: `src/query/datalog/optimizer.rs` + +```rust +// Before +pub fn plan( + patterns: Vec, + indexes: &Indexes, +) -> Vec<(Pattern, IndexHint)> + +// After +pub fn plan( + clauses: Vec, + indexes: &Indexes, +) -> Vec<(WhereClause, Option)> +``` + +Only `WhereClause::Pattern` and `WhereClause::Expr` variants are passed in. `Not`, `NotJoin`, `Or`, `OrJoin` continue to be extracted separately at call sites and applied as post-filters — their handling is unchanged. + +**Algorithm**: + +1. Separate input into patterns (assign `IndexHint` via `select_index`) and exprs. +2. `#[cfg(not(feature = "wasm"))]`: stable-sort patterns by selectivity descending — unchanged behaviour. +3. New helper `expr_vars(expr: &Expr) -> Vec` walks the `Expr` tree and collects referenced variable names (symbols starting with `?`). +4. Walk sorted patterns left-to-right, tracking the set of variables bound so far. For each `Expr`, insert it at the position where all its variables first become bound. +5. Exprs with no variables, or variables never bound by any pattern, are placed at the end. + +**`wasm` behaviour**: Pattern reordering remains gated behind `#[cfg(not(feature = "wasm"))]`. Expr push-down (step 4) is unconditional — it applies on all targets including WASM. + +**Return type**: `Vec<(WhereClause, Option)>` where `Pattern` entries carry `Some(IndexHint)` and `Expr` entries carry `None`. + +--- + +### 2. Executor inner loop + +**File**: `src/query/datalog/executor.rs` +**Functions**: `execute_query()`, `execute_query_with_rules()` + +Both functions currently use a two-phase structure: run all patterns, then call `apply_expr_clauses()`. After this change: + +1. Collect top-level `Pattern` and `Expr` clauses into a `Vec`. +2. Call `plan(clauses, indexes)` → interleaved `Vec<(WhereClause, Option)>`. +3. Process in a single ordered loop: + +``` +bindings = [empty_binding] +for (clause, hint) in planned: + WhereClause::Pattern(p) → + bindings = PatternMatcher::match_with_hint(p, hint, facts, bindings) + WhereClause::Expr { expr, binding: out } → + bindings = filter_or_extend(bindings, expr, out) +``` + +`Not`, `NotJoin`, `Or`, `OrJoin` are extracted before the plan call and applied as post-filters afterward — no change to their handling. The **top-level** `apply_expr_clauses()` post-pass (executor.rs lines ~811, ~1094) is removed; its logic moves inline into the ordered loop. `apply_expr_clauses()` calls inside `not`/`not-join` body evaluation (lines ~1070, ~1216) are **unchanged** — those Expr clauses belong to the not-body scope and are not candidates for push-down. + +--- + +### 3. Mixed-rules path + +**File**: `src/query/datalog/evaluator.rs` +**Function**: `StratifiedEvaluator::evaluate()` (mixed-rules loop, ~line 729) + +Currently collects `positive_patterns: Vec` in original body order, bypassing the planner entirely. + +After this change: +1. Collect `Pattern` and `Expr` `WhereClause` variants from the rule body. +2. Call `plan(clauses, indexes)` → interleaved plan. +3. Process with the same ordered loop as the executor. +4. `Not` and `NotJoin` filters continue to be applied as post-filters — no change. + +Mixed rules gain both pattern selectivity ordering and predicate push-down with no separate implementation, satisfying #206. + +--- + +### 4. Testing + +**`src/query/datalog/optimizer.rs` unit tests** — `plan()` with mixed input: +- Expr with all vars bound by first pattern → positioned immediately after that pattern +- Expr with vars spread across multiple patterns → positioned after the last binding pattern +- Expr with no variables → placed at end +- Expr referencing a variable no pattern binds → placed at end +- Under `wasm` feature: patterns stay in user-written order, Exprs still pushed down + +**`src/query/datalog/executor.rs` integration tests** — semantics preserved: +- Query with `[(> ?age 30)]` predicate produces same results before and after push-down +- Selective predicate on large fact set prunes intermediate bindings (verify via result count) + +**`src/query/datalog/evaluator.rs` unit tests** — mixed-rule path: +- Rule with `not` + `Expr` in body: Expr pushed down correctly, not-filter still applied afterward +- Rule with `or` + `Expr` in body: same + +**Benchmarks** (Criterion, per #207 acceptance criteria): +- `[(> ?age 30)]` selective predicate on large fact sets: throughput comparison before/after push-down + +--- + +## Invariants Preserved + +- **Correctness**: `eval_expr` returns `Err(())` for unbound variables (silently drops row). If push-down ever positions an Expr before all its vars are bound, the silent-drop behaviour acts as a safety net — results remain correct, just not optimally filtered. The `expr_vars()` helper must be correct to get the performance benefit. +- **Stratification safety**: `Not`/`NotJoin` handling is unchanged; push-down only applies to `Expr` clauses. +- **WASM portability**: Pattern reordering stays gated; Expr push-down is always on. +- **Backwards compatibility**: No change to query semantics, file format, or public API. + +--- + +## Files Changed + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | New `plan()` signature; new `expr_vars()` helper; push-down insertion algorithm | +| `src/query/datalog/executor.rs` | Updated call sites; single ordered processing loop; remove `apply_expr_clauses` post-pass | +| `src/query/datalog/evaluator.rs` | Mixed-rules path routes through `plan()` | +| `benches/` | New Criterion bench for selective predicate push-down | From b1c139d3637bb778cc9d406d902bb94284bf1c6d Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Fri, 15 May 2026 15:37:36 +0530 Subject: [PATCH 02/13] docs: add Wave 2 PR 1 implementation plan (predicate push-down + mixed rules) TDD plan covering optimizer.rs, matcher.rs, executor.rs, evaluator.rs, and a new Criterion benchmark. 9 tasks, each with exact file paths, code blocks, and expected test output. Co-Authored-By: Claude Sonnet 4.6 --- ...26-05-15-predicate-pushdown-mixed-rules.md | 1459 +++++++++++++++++ 1 file changed, 1459 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md diff --git a/docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md b/docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md new file mode 100644 index 000000000..32b350f9d --- /dev/null +++ b/docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md @@ -0,0 +1,1459 @@ +# Wave 2 PR 1: Predicate Push-down + Mixed Rule Optimization + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extend `optimizer::plan()` to accept and interleave `Expr` predicate clauses at the earliest position where their variables are bound, and route the mixed-rules evaluator path through the updated planner. + +**Architecture:** `plan()` signature changes from `Vec → Vec<(Pattern, IndexHint)>` to `Vec → Vec<(WhereClause, Option)>`. The executor's inner join loop processes clauses in plan order (Pattern → join bindings, Expr → filter/extend inline). The `StratifiedEvaluator` mixed-rules path is updated to use the same planner. Three files modified: `optimizer.rs`, `executor.rs`, `evaluator.rs`. One new method added to `matcher.rs`. One new benchmark group. + +**Tech Stack:** Rust stable, `std::collections::HashSet` for variable tracking, Criterion for benchmarks. + +--- + +## File Map + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | New `expr_vars()`, `pattern_bound_vars()` helpers; `plan()` signature → `Vec`; update existing test | +| `src/query/datalog/matcher.rs` | New `pub(crate) match_with_hint_seeded()` method | +| `src/query/datalog/executor.rs` | Updated `execute_query()` and `execute_query_with_rules()`: inline ordered loop, registry acquired earlier, top-level `apply_expr_clauses` post-passes removed | +| `src/query/datalog/evaluator.rs` | `StratifiedEvaluator` mixed-rules loop routes through `plan()` | +| `benches/minigraf_bench.rs` | New `query/predicate_pushdown` benchmark group | + +--- + +## Task 1: Create git worktree + +**Files:** +- No file changes — shell commands only + +- [ ] **Step 1: Invoke the using-git-worktrees skill** + +``` +Use superpowers:using-git-worktrees to create a worktree for this feature. +Branch name: feature/issues-207-206-predicate-pushdown +Worktree dir: .worktrees/wave2-pr1-predicate-pushdown +``` + +- [ ] **Step 2: Verify worktree is active and clean** + +```bash +git worktree list +git status +``` + +Expected: worktree listed, status shows clean working tree on `feature/issues-207-206-predicate-pushdown`. + +--- + +## Task 2: Failing tests for `expr_vars()` and new `plan()` signature (optimizer.rs) + +**Files:** +- Modify: `src/query/datalog/optimizer.rs` (test module only) + +These tests compile-fail (new functions don't exist yet) and will pass after Task 3. + +- [ ] **Step 1: Add imports and test helpers to the test module** + +In `src/query/datalog/optimizer.rs`, extend the existing `#[cfg(test)] mod tests` block. Replace the current imports block: + +```rust +#[cfg(test)] +mod tests { + use super::*; + use crate::graph::types::Value; + use crate::query::datalog::types::{BinOp, EdnValue, Expr, Pattern, WhereClause}; + #[cfg(not(feature = "wasm"))] + use crate::storage::index::Indexes; + use uuid::Uuid; + + fn make_pattern(entity: EdnValue, attribute: EdnValue, value: EdnValue) -> Pattern { + Pattern::new(entity, attribute, value) + } + + fn var(s: &str) -> EdnValue { + EdnValue::Symbol(format!("?{s}")) + } + fn kw(s: &str) -> EdnValue { + EdnValue::Keyword(s.to_string()) + } + fn str_val(s: &str) -> EdnValue { + EdnValue::String(s.to_string()) + } + fn entity_lit() -> EdnValue { + EdnValue::Uuid(Uuid::new_v4()) + } +``` + +- [ ] **Step 2: Add failing tests for `expr_vars()`** + +Append to the test module: + +```rust + // ── expr_vars() ────────────────────────────────────────────────────────── + + #[test] + fn test_expr_vars_var() { + let e = Expr::Var("?age".to_string()); + assert_eq!(expr_vars(&e), vec!["?age".to_string()]); + } + + #[test] + fn test_expr_vars_lit_is_empty() { + let e = Expr::Lit(Value::Integer(42)); + assert!(expr_vars(&e).is_empty()); + } + + #[test] + fn test_expr_vars_binop() { + let e = Expr::BinOp( + BinOp::Gt, + Box::new(Expr::Var("?age".to_string())), + Box::new(Expr::Lit(Value::Integer(30))), + ); + assert_eq!(expr_vars(&e), vec!["?age".to_string()]); + } + + #[test] + fn test_expr_vars_nested_binop_collects_all() { + // (> (+ ?a ?b) ?c) + let e = Expr::BinOp( + BinOp::Gt, + Box::new(Expr::BinOp( + BinOp::Add, + Box::new(Expr::Var("?a".to_string())), + Box::new(Expr::Var("?b".to_string())), + )), + Box::new(Expr::Var("?c".to_string())), + ); + let vars = expr_vars(&e); + assert!(vars.contains(&"?a".to_string())); + assert!(vars.contains(&"?b".to_string())); + assert!(vars.contains(&"?c".to_string())); + assert_eq!(vars.len(), 3); + } + + #[test] + fn test_expr_vars_unary_op() { + use crate::query::datalog::types::UnaryOp; + let e = Expr::UnaryOp(UnaryOp::IntegerQ, Box::new(Expr::Var("?v".to_string()))); + assert_eq!(expr_vars(&e), vec!["?v".to_string()]); + } +``` + +- [ ] **Step 3: Add failing tests for new `plan()` return type and push-down** + +Append to the test module: + +```rust + // ── plan() — new signature and push-down ───────────────────────────────── + + #[test] + fn test_plan_pattern_carries_some_hint() { + #[cfg(not(feature = "wasm"))] + use crate::storage::index::Indexes; + #[cfg(not(feature = "wasm"))] + { + let p = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let planned = plan(vec![p], &Indexes::new()); + assert!(planned[0].1.is_some(), "Pattern entry must carry Some(IndexHint)"); + } + } + + #[test] + fn test_plan_expr_carries_none_hint() { + #[cfg(not(feature = "wasm"))] + use crate::storage::index::Indexes; + #[cfg(not(feature = "wasm"))] + { + let p = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let expr = WhereClause::Expr { + expr: Expr::Lit(Value::Boolean(true)), + binding: None, + }; + let planned = plan(vec![p, expr], &Indexes::new()); + let expr_entry = planned.iter().find(|(c, _)| matches!(c, WhereClause::Expr { .. })); + assert!(expr_entry.is_some()); + assert!(expr_entry.unwrap().1.is_none(), "Expr entry must carry None hint"); + } + } + + #[cfg(not(feature = "wasm"))] + #[test] + fn test_expr_pushed_after_binding_pattern() { + // Three patterns with equal selectivity (1 attr bound each) — stable sort preserves + // original order: [p1, p2, p3]. Expr needs ?v, bound by p2 (pos 1). + // Expected output: [p1, p2, expr, p3]. + let p1 = WhereClause::Pattern(make_pattern(var("e"), kw(":name"), var("n"))); + let p2 = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let p3 = WhereClause::Pattern(make_pattern(var("e"), kw(":dept"), var("d"))); + let expr = WhereClause::Expr { + expr: Expr::BinOp( + BinOp::Gt, + Box::new(Expr::Var("?v".to_string())), + Box::new(Expr::Lit(Value::Integer(30))), + ), + binding: None, + }; + let planned = plan(vec![p1, p2, p3, expr], &Indexes::new()); + assert_eq!(planned.len(), 4); + // Item at index 2 must be the Expr (pushed after p2 which binds ?v at index 1). + assert!( + matches!(planned[2].0, WhereClause::Expr { .. }), + "Expr must be at index 2" + ); + // Item at index 3 must be a Pattern (p3, not yet seen when ?v first bound). + assert!( + matches!(planned[3].0, WhereClause::Pattern(_)), + "p3 must be at index 3" + ); + } + + #[cfg(not(feature = "wasm"))] + #[test] + fn test_expr_no_vars_goes_to_end() { + let p1 = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let expr = WhereClause::Expr { + expr: Expr::Lit(Value::Boolean(true)), + binding: None, + }; + let planned = plan(vec![p1, expr], &Indexes::new()); + assert_eq!(planned.len(), 2); + assert!( + matches!(planned[1].0, WhereClause::Expr { .. }), + "no-var Expr must be last" + ); + } + + #[cfg(not(feature = "wasm"))] + #[test] + fn test_expr_unbound_var_goes_to_end() { + // ?x is never bound by any pattern + let p1 = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let expr = WhereClause::Expr { + expr: Expr::BinOp( + BinOp::Gt, + Box::new(Expr::Var("?x".to_string())), + Box::new(Expr::Lit(Value::Integer(0))), + ), + binding: None, + }; + let planned = plan(vec![p1, expr], &Indexes::new()); + assert_eq!(planned.len(), 2); + assert!( + matches!(planned[1].0, WhereClause::Expr { .. }), + "Expr with unbound var must be last" + ); + } +``` + +- [ ] **Step 4: Run to verify tests fail to compile** + +```bash +cargo test --lib -p minigraf -- query::datalog::optimizer 2>&1 | head -30 +``` + +Expected: compile errors — `expr_vars`, `pattern_bound_vars` not found; `plan()` type mismatch. + +--- + +## Task 3: Implement `expr_vars()`, `pattern_bound_vars()`, and new `plan()` (optimizer.rs) + +**Files:** +- Modify: `src/query/datalog/optimizer.rs` + +- [ ] **Step 1: Update imports at the top of optimizer.rs** + +Replace the existing import line: + +```rust +use crate::query::datalog::types::{AttributeSpec, EdnValue, Pattern}; +``` + +with: + +```rust +use crate::query::datalog::types::{AttributeSpec, EdnValue, Expr, Pattern, WhereClause}; +``` + +- [ ] **Step 2: Add `expr_vars()` and `pattern_bound_vars()` helpers after the existing helpers** + +Add after the `attr_is_index_bound` function (around line 38): + +```rust +/// Collect all logic-variable names (`?foo`) referenced in an Expr tree. +fn expr_vars(expr: &Expr) -> Vec { + match expr { + Expr::Var(s) => vec![s.clone()], + Expr::Lit(_) | Expr::Slot(_) => vec![], + Expr::BinOp(_, l, r) => { + let mut vars = expr_vars(l); + vars.extend(expr_vars(r)); + vars + } + Expr::UnaryOp(_, inner) => expr_vars(inner), + } +} + +/// Collect the logic-variable names bound (output) by a Pattern. +/// Only Symbol values starting with `?` count — literals never bind. +fn pattern_bound_vars(p: &Pattern) -> Vec { + let mut vars = Vec::new(); + if is_variable(&p.entity) { + if let EdnValue::Symbol(s) = &p.entity { + vars.push(s.clone()); + } + } + if let AttributeSpec::Real(attr) = &p.attribute { + if is_variable(attr) { + if let EdnValue::Symbol(s) = attr { + vars.push(s.clone()); + } + } + } + if is_variable(&p.value) { + if let EdnValue::Symbol(s) = &p.value { + vars.push(s.clone()); + } + } + vars +} +``` + +- [ ] **Step 3: Replace `plan()` with the new signature and push-down algorithm** + +Replace the entire `plan()` function (currently lines 86–108): + +```rust +/// Plan a list of where clauses: assign index hints to Pattern entries, push Expr +/// entries to the earliest position where all their variables are bound by preceding +/// patterns, and (non-wasm) sort patterns by selectivity. +/// +/// Only `WhereClause::Pattern` and `WhereClause::Expr` variants should be passed in. +/// `Not`, `NotJoin`, `Or`, `OrJoin`, and `RuleInvocation` variants are handled by +/// the executor/evaluator and must not appear here. +/// +/// Returns an interleaved `Vec<(WhereClause, Option)>` where Pattern entries +/// carry `Some(hint)` and Expr entries carry `None`. +pub fn plan( + clauses: Vec, + _indexes: &crate::storage::index::Indexes, +) -> Vec<(WhereClause, Option)> { + // Separate into patterns (with hints) and exprs. + let mut patterns: Vec<(WhereClause, IndexHint)> = Vec::new(); + let mut exprs: Vec = Vec::new(); + + for clause in clauses { + match &clause { + WhereClause::Pattern(p) => { + let hint = select_index(p); + patterns.push((clause, hint)); + } + WhereClause::Expr { .. } => exprs.push(clause), + // Other variants must not be passed to plan(); silently skip. + _ => {} + } + } + + // Stable sort patterns by selectivity descending (non-wasm only). + // Preserves original order for ties, ensuring deterministic output. + #[cfg(not(feature = "wasm"))] + patterns.sort_by_key(|(clause, _)| { + if let WhereClause::Pattern(p) = clause { + std::cmp::Reverse(selectivity_score(p)) + } else { + std::cmp::Reverse(0u8) + } + }); + + // Start with sorted patterns only. + let mut result: Vec<(WhereClause, Option)> = patterns + .into_iter() + .map(|(clause, hint)| (clause, Some(hint))) + .collect(); + + // Push each Expr to the earliest position where all its variables are bound. + for expr_clause in exprs { + let vars: std::collections::HashSet = + if let WhereClause::Expr { expr, .. } = &expr_clause { + expr_vars(expr).into_iter().collect() + } else { + Default::default() + }; + + let mut bound: std::collections::HashSet = Default::default(); + // Default: append at end (covers no-var Exprs and vars never bound by any pattern). + let mut insert_pos = result.len(); + + if !vars.is_empty() { + for (pos, (clause, _)) in result.iter().enumerate() { + if let WhereClause::Pattern(p) = clause { + bound.extend(pattern_bound_vars(p)); + if vars.is_subset(&bound) { + insert_pos = pos + 1; + break; + } + } + } + } + + result.insert(insert_pos, (expr_clause, None)); + } + + result +} +``` + +- [ ] **Step 4: Update the existing join-ordering test to use the new signature** + +Find and replace `test_join_ordering_moves_selective_pattern_first` in the test module: + +```rust + #[cfg(not(feature = "wasm"))] + #[test] + fn test_join_ordering_moves_selective_pattern_first() { + let p1 = make_pattern(var("e"), kw(":age"), var("a")); // selectivity 1 (attr only) + let p2 = make_pattern(entity_lit(), kw(":name"), var("v")); // selectivity 2 (entity + attr) + let p1_attr = p1.attribute.clone(); + let p2_attr = p2.attribute.clone(); + let planned = plan( + vec![ + WhereClause::Pattern(p1), + WhereClause::Pattern(p2), + ], + &Indexes::new(), + ); + // planned[0].0 is WhereClause::Pattern — extract the inner Pattern. + let first_attr = match &planned[0].0 { + WhereClause::Pattern(p) => p.attribute.clone(), + _ => panic!("expected Pattern at index 0"), + }; + let second_attr = match &planned[1].0 { + WhereClause::Pattern(p) => p.attribute.clone(), + _ => panic!("expected Pattern at index 1"), + }; + assert_ne!(first_attr, p1_attr, "Lower-selectivity pattern must not be first"); + assert_eq!(first_attr, p2_attr, "Higher-selectivity pattern must be first"); + assert_eq!(second_attr, p1_attr, "Lower-selectivity pattern must be second"); + } +``` + +- [ ] **Step 5: Run optimizer tests to verify they pass** + +```bash +cargo test --lib -p minigraf -- query::datalog::optimizer 2>&1 +``` + +Expected: all optimizer tests pass, no compile errors. + +- [ ] **Step 6: Commit** + +```bash +git add src/query/datalog/optimizer.rs +git commit -m "feat(optimizer): extend plan() to accept Expr clauses with push-down positioning + +Adds expr_vars() and pattern_bound_vars() helpers. plan() now accepts +Vec (Pattern + Expr) and returns Vec<(WhereClause, Option)>. +Each Expr is inserted at the earliest position after all its variables are +bound by preceding patterns. No-variable Exprs and Exprs with unbound +variables are appended at the end. + +Closes part of #207. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 4: Add `match_with_hint_seeded()` to PatternMatcher (matcher.rs) + +**Files:** +- Modify: `src/query/datalog/matcher.rs` + +- [ ] **Step 1: Write a failing test for `match_with_hint_seeded()`** + +Add to the test module in `src/query/datalog/matcher.rs`: + +```rust + #[test] + fn test_match_with_hint_seeded_unit_seed_uses_hint_path() { + // Unit seed (one empty map) — behaves like the first pattern in a query. + // Any non-empty result confirms the method returns matches (not empty). + use crate::query::datalog::optimizer::IndexHint; + let storage = FactStorage::new(); + storage.load_fact(crate::graph::types::Fact { + entity: uuid::Uuid::new_v4(), + attribute: ":val".to_string(), + value: crate::graph::types::Value::Integer(1), + tx_id: 1, + tx_count: 1, + valid_from: 0, + valid_to: i64::MAX, + asserted: true, + }).unwrap(); + let facts: Arc<[crate::graph::types::Fact]> = + Arc::from(storage.get_asserted_facts().unwrap()); + let matcher = PatternMatcher::from_slice(facts); + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":val".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + let unit_seed = vec![HashMap::new()]; + let results = matcher.match_with_hint_seeded(unit_seed, &p, &IndexHint::Aevt); + assert_eq!(results.len(), 1, "unit seed must produce one result per matching fact"); + } + + #[test] + fn test_match_with_hint_seeded_real_bindings_uses_join() { + // Non-unit seed — behaves like a subsequent pattern (join path). + use crate::query::datalog::optimizer::IndexHint; + use uuid::Uuid; + let storage = FactStorage::new(); + let e = Uuid::new_v4(); + storage.load_fact(crate::graph::types::Fact { + entity: e, + attribute: ":val".to_string(), + value: crate::graph::types::Value::Integer(42), + tx_id: 1, + tx_count: 1, + valid_from: 0, + valid_to: i64::MAX, + asserted: true, + }).unwrap(); + let facts: Arc<[crate::graph::types::Fact]> = + Arc::from(storage.get_asserted_facts().unwrap()); + let matcher = PatternMatcher::from_slice(facts); + let p = Pattern::new( + EdnValue::Uuid(e), + EdnValue::Keyword(":val".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + // Seed with a binding that already has some data (non-unit). + let seed = vec![{ + let mut m = HashMap::new(); + m.insert("?other".to_string(), crate::graph::types::Value::Integer(99)); + m + }]; + let results = matcher.match_with_hint_seeded(seed, &p, &IndexHint::Eavt); + assert_eq!(results.len(), 1, "join path must unify with existing binding"); + assert_eq!( + results[0].get("?v"), + Some(&crate::graph::types::Value::Integer(42)) + ); + } +``` + +- [ ] **Step 2: Run to confirm tests fail to compile** + +```bash +cargo test --lib -p minigraf -- query::datalog::matcher 2>&1 | head -20 +``` + +Expected: compile error — `match_with_hint_seeded` not found. + +- [ ] **Step 3: Implement `match_with_hint_seeded()`** + +Add the following method to the `impl PatternMatcher` block in `src/query/datalog/matcher.rs`, after `match_patterns_with_hints`: + +```rust + /// Match a single pattern against existing bindings, using an index hint when the + /// bindings are the unit seed (one empty map — i.e., this is the first pattern). + /// + /// When `seed` is the unit binding (`[{}]`), delegates to `match_pattern_with_hint` + /// for an indexed lookup. For all other seeds, delegates to `join_with_pattern` + /// (hash-join path) which already picks the right strategy. + /// + /// Used by the executor and evaluator incremental plan loops introduced in #207. + pub(crate) fn match_with_hint_seeded( + &self, + seed: Vec, + pattern: &Pattern, + hint: &crate::query::datalog::optimizer::IndexHint, + ) -> Vec { + if seed.len() == 1 && seed[0].is_empty() { + // First pattern in the plan — use the index hint for a targeted lookup. + self.match_pattern_with_hint(pattern, hint) + } else { + // Subsequent pattern — join_with_pattern uses hash-join when possible. + self.join_with_pattern(seed, pattern) + } + } +``` + +- [ ] **Step 4: Run matcher tests to confirm they pass** + +```bash +cargo test --lib -p minigraf -- query::datalog::matcher 2>&1 +``` + +Expected: all matcher tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/query/datalog/matcher.rs +git commit -m "feat(matcher): add match_with_hint_seeded() for incremental plan execution + +Used by the executor and evaluator plan loops added in #207: applies +an index hint for the first pattern (unit seed) and falls back to the +hash-join path for subsequent patterns. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 5: Update `execute_query()` in executor.rs + +**Files:** +- Modify: `src/query/datalog/executor.rs` + +- [ ] **Step 1: Write integration tests that assert semantics are preserved** + +Add to the `#[cfg(test)]` module in `src/query/datalog/executor.rs`: + +```rust + #[test] + fn test_expr_pushdown_preserves_query_results() { + // Query with a selective Expr predicate: results must be identical before and after + // the push-down refactor. This test serves as a regression guard. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute("(transact [[:e1 :val 10] [:e2 :val 20] [:e3 :val 30]])") + .unwrap(); + let result = executor + .execute("(query [:find ?e ?v :where [?e :val ?v] [(> ?v 15)]])") + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 2, "only :e2 and :e3 have :val > 15"); + } else { + panic!("expected QueryResults"); + } + } + + #[test] + fn test_expr_pushdown_multi_pattern_preserves_results() { + // Multi-pattern query with Expr in the middle: [?e :val ?v] [?e :name ?n] [(> ?v 10)] + // Expr should be pushed between the two patterns. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute(r#"(transact [[:e1 :val 5] [:e1 :name "a"] [:e2 :val 20] [:e2 :name "b"]])"#) + .unwrap(); + let result = executor + .execute(r#"(query [:find ?e ?n :where [?e :val ?v] [?e :name ?n] [(> ?v 10)]])"#) + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 1, "only :e2 passes the predicate"); + } else { + panic!("expected QueryResults"); + } + } + + #[test] + fn test_expr_binding_form_preserves_results() { + // Binding-form Expr: [(* ?v 2) ?doubled] — should still work after refactor. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute("(transact [[:e1 :val 5] [:e2 :val 10]])") + .unwrap(); + let result = executor + .execute("(query [:find ?e ?doubled :where [?e :val ?v] [(* ?v 2) ?doubled]])") + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 2, "both entities must appear"); + } else { + panic!("expected QueryResults"); + } + } +``` + +- [ ] **Step 2: Run to confirm tests pass against the current (pre-refactor) code** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor::tests::test_expr_pushdown 2>&1 +``` + +Expected: all three new tests PASS (they test semantics, not the push-down path). + +- [ ] **Step 3: Refactor `execute_query()` — move registry acquisition and replace pattern/expr handling** + +In `execute_query()`, make the following changes: + +**3a.** Move registry acquisition to BEFORE the pattern matching block. Replace the current sequence (starting around line 493): + +```rust + let patterns = query.get_patterns(); + + // Plan patterns: assign index hints and reorder by selectivity. + let planned_patterns = optimizer::plan(patterns, &self.indexes); + + // Match all patterns in planned order and get bindings + let bindings = matcher.match_patterns_with_hints(&planned_patterns); + + // Acquire the function registry once; used by apply_or_clauses, not_body_matches, + // apply_expr_clauses, and apply_post_processing below. + let registry = self + .functions + .read() + .map_err(|_| anyhow!("functions lock poisoned"))?; +``` + +with: + +```rust + // Acquire function registry before the plan loop — needed for inline Expr evaluation. + let registry = self + .functions + .read() + .map_err(|_| anyhow!("functions lock poisoned"))?; + + // Pre-validate UDF predicate names: surface unknown predicates as errors before + // processing any rows (matches the behaviour of the former apply_expr_clauses post-pass). + for clause in &query.where_clauses { + if let WhereClause::Expr { + expr: Expr::UnaryOp(UnaryOp::Udf(name), _), + .. + } = clause + { + if registry.get_predicate(name).is_none() { + anyhow::bail!("unknown predicate: '{}'", name); + } + } + } + + // Collect Pattern and Expr top-level clauses for the planner. + // Not/NotJoin/Or/OrJoin are extracted separately below and applied as post-filters. + let plan_clauses: Vec = query + .where_clauses + .iter() + .filter(|c| matches!(c, WhereClause::Pattern(_) | WhereClause::Expr { .. })) + .cloned() + .collect(); + + let planned = optimizer::plan(plan_clauses, &self.indexes); + + // Process planned clauses in order: Pattern → expand bindings, Expr → filter/extend. + let mut bindings: Vec> = + vec![std::collections::HashMap::new()]; + for (clause, hint) in planned { + match clause { + WhereClause::Pattern(p) => { + bindings = matcher.match_with_hint_seeded( + bindings, + &p, + hint.as_ref().unwrap_or(&optimizer::IndexHint::Eavt), + ); + } + WhereClause::Expr { expr, binding: out } => { + bindings = bindings + .into_iter() + .filter_map(|mut b| { + match eval_expr(&expr, &b, Some(®istry)) { + Ok(v) => { + if let Some(var) = &out { + b.insert(var.clone(), v); + Some(b) + } else if is_truthy(&v) { + Some(b) + } else { + None + } + } + Err(_) => None, + } + }) + .collect(); + } + _ => {} + } + } +``` + +**3b.** Remove the top-level `apply_expr_clauses` post-pass. Find and delete (around line 810): + +```rust + // Apply WhereClause::Expr clauses (filter and binding predicates) + let filtered_bindings = apply_expr_clauses(not_filtered, &query.where_clauses, ®istry)?; +``` + +Replace the reference to `filtered_bindings` in the line immediately after with `not_filtered`: + +```rust + let results = + apply_post_processing(not_filtered, &query.find, &query.with_vars, ®istry)?; +``` + +- [ ] **Step 4: Run tests to confirm they still pass** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor 2>&1 +``` + +Expected: all executor tests pass, including the three new ones. + +- [ ] **Step 5: Commit** + +```bash +git add src/query/datalog/executor.rs +git commit -m "feat(executor): inline Expr push-down in execute_query() plan loop + +Replaces the two-phase pattern-match-then-expr-filter with a single +ordered loop that processes Pattern (join) and Expr (filter/extend) +clauses in plan order. Registry is acquired before the loop for inline +Expr evaluation. Removes the top-level apply_expr_clauses post-pass. +Not/NotJoin/Or/OrJoin handling is unchanged. + +Closes part of #207. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 6: Update `execute_query_with_rules()` in executor.rs + +**Files:** +- Modify: `src/query/datalog/executor.rs` + +- [ ] **Step 1: Write integration tests for the rules path** + +Add to the test module in `src/query/datalog/executor.rs`: + +```rust + #[test] + fn test_expr_pushdown_with_rules_preserves_results() { + // Query that uses rules AND an Expr predicate — exercises execute_query_with_rules. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute("(transact [[:e1 :val 5] [:e2 :val 20] [:e3 :val 30]])") + .unwrap(); + executor + .execute("(rule [(high ?e) [?e :val ?v] [(> ?v 15)]])") + .unwrap(); + let result = executor + .execute("(query [:find ?e :where (high ?e)])") + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 2, "only :e2 and :e3 qualify"); + } else { + panic!("expected QueryResults"); + } + } +``` + +- [ ] **Step 2: Run to confirm test passes against current code** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor::tests::test_expr_pushdown_with_rules 2>&1 +``` + +Expected: PASS (semantics test; push-down not yet applied here). + +- [ ] **Step 3: Refactor `execute_query_with_rules()` — replace pattern/expr handling** + +In `execute_query_with_rules()`, replace the current sequence (around lines 881–924): + +```rust + // Convert ONLY top-level rule invocations to positive-match patterns. + // Rule invocations inside `not` bodies are handled by the not-post-filter below. + // (reachable ?x ?y) becomes [?x :reachable ?y] + let mut all_patterns = query.get_patterns(); + + for (predicate, args) in query.get_top_level_rule_invocations() { + let pattern = match args.len() { + 1 => { ... } + 2 => { ... } + n => { return Err(...); } + }; + all_patterns.push(pattern); + } + + // Compute derived_facts Arc once; reuse for or-clauses and not-post-filter. + let derived_facts: Arc<[Fact]> = ...; + + // Match all patterns against derived facts + let matcher = PatternMatcher::from_slice_with_valid_at(derived_facts.clone(), valid_at_value.clone()); + let bindings = matcher.match_patterns(&all_patterns); + + // Acquire the function registry once; ... + let registry = self.functions.read()...; +``` + +with: + +```rust + // Compute derived_facts Arc once; reuse for plan loop, or-clauses and not-post-filter. + // Must use derived_storage (includes rule-derived facts), not filtered_facts (base only). + let derived_facts: Arc<[Fact]> = + Arc::from(derived_storage.get_asserted_facts().unwrap_or_default()); + + let matcher = + PatternMatcher::from_slice_with_valid_at(derived_facts.clone(), valid_at_value.clone()); + + // Acquire function registry before the plan loop — needed for inline Expr evaluation. + let registry = self + .functions + .read() + .map_err(|_| anyhow!("functions lock poisoned"))?; + + // Pre-validate UDF predicate names. + for clause in &query.where_clauses { + if let WhereClause::Expr { + expr: Expr::UnaryOp(UnaryOp::Udf(name), _), + .. + } = clause + { + if registry.get_predicate(name).is_none() { + anyhow::bail!("unknown predicate: '{}'", name); + } + } + } + + // Collect Pattern and Expr top-level clauses for the planner. + // Rule invocations are converted to WhereClause::Pattern against derived_storage. + let mut plan_clauses: Vec = query + .where_clauses + .iter() + .filter(|c| matches!(c, WhereClause::Pattern(_) | WhereClause::Expr { .. })) + .cloned() + .collect(); + + for (predicate, args) in query.get_top_level_rule_invocations() { + let pattern = match args.len() { + 1 => { + #[allow(clippy::indexing_slicing)] + let entity = args[0].clone(); + Pattern::new( + entity, + EdnValue::Keyword(format!(":{}", predicate)), + EdnValue::Symbol("?_rule_value".to_string()), + ) + } + 2 => { + #[allow(clippy::indexing_slicing)] + let entity = args[0].clone(); + #[allow(clippy::indexing_slicing)] + let value = args[1].clone(); + Pattern::new(entity, EdnValue::Keyword(format!(":{}", predicate)), value) + } + n => { + return Err(anyhow!( + "Rule invocation '{}' must have 1 or 2 arguments, got {}", + predicate, + n + )); + } + }; + plan_clauses.push(WhereClause::Pattern(pattern)); + } + + let planned = optimizer::plan(plan_clauses, &self.indexes); + + // Process planned clauses in order: Pattern → expand, Expr → filter/extend. + let mut bindings: Vec> = + vec![std::collections::HashMap::new()]; + for (clause, hint) in planned { + match clause { + WhereClause::Pattern(p) => { + bindings = matcher.match_with_hint_seeded( + bindings, + &p, + hint.as_ref().unwrap_or(&optimizer::IndexHint::Eavt), + ); + } + WhereClause::Expr { expr, binding: out } => { + bindings = bindings + .into_iter() + .filter_map(|mut b| { + match eval_expr(&expr, &b, Some(®istry)) { + Ok(v) => { + if let Some(var) = &out { + b.insert(var.clone(), v); + Some(b) + } else if is_truthy(&v) { + Some(b) + } else { + None + } + } + Err(_) => None, + } + }) + .collect(); + } + _ => {} + } + } +``` + +**3b.** Remove the top-level `apply_expr_clauses` post-pass in this function. Find and delete (around line 1094): + +```rust + // Apply WhereClause::Expr clauses (filter and binding predicates) + let filtered_bindings = apply_expr_clauses(not_filtered, &query.where_clauses, ®istry)?; +``` + +Replace the `filtered_bindings` reference immediately after with `not_filtered`: + +```rust + let results = + apply_post_processing(not_filtered, &query.find, &query.with_vars, ®istry)?; +``` + +- [ ] **Step 4: Run all executor tests** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor 2>&1 +``` + +Expected: all tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/query/datalog/executor.rs +git commit -m "feat(executor): inline Expr push-down in execute_query_with_rules() plan loop + +Mirrors Task 5 for the rule-backed query path. Rule invocations are +converted to WhereClause::Pattern before the plan call so they +participate in selectivity ordering. Removes second apply_expr_clauses +post-pass. + +Closes part of #207. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 7: Update evaluator mixed-rules path (evaluator.rs) — closes #206 + +**Files:** +- Modify: `src/query/datalog/evaluator.rs` + +- [ ] **Step 1: Write integration test for mixed-rules Expr push-down** + +Add to the test module in `src/query/datalog/evaluator.rs`: + +```rust + #[test] + fn test_mixed_rule_with_expr_preserves_semantics() { + // Rule with not + Expr: (eligible ?x) :- [?x :val ?v] [(> ?v 10)] (not [?x :blocked true]) + // Expr should be pushed after [?x :val ?v] in the mixed-rules plan. + // Result must be the same as before the refactor. + let storage = FactStorage::new(); + let rules = Arc::new(RwLock::new(RuleRegistry::new())); + let functions = Arc::new(RwLock::new(FunctionRegistry::with_builtins())); + + let e1 = Uuid::new_v4(); + let e2 = Uuid::new_v4(); + let e3 = Uuid::new_v4(); + + storage + .transact( + vec![ + (e1, ":val".to_string(), Value::Integer(5)), + (e2, ":val".to_string(), Value::Integer(20)), + (e3, ":val".to_string(), Value::Integer(30)), + (e3, ":blocked".to_string(), Value::Boolean(true)), + ], + None, + ) + .unwrap(); + + register_test_rule( + &rules, + r#"(rule [(eligible ?x) [?x :val ?v] [(> ?v 10)] (not [?x :blocked true])])"#, + ); + + let evaluator = StratifiedEvaluator::new( + storage, + Arc::clone(&rules), + Arc::clone(&functions), + DEFAULT_MAX_ITERATIONS, + DEFAULT_MAX_DERIVED_FACTS, + DEFAULT_MAX_RESULTS, + ); + + let derived = evaluator.evaluate(&["eligible".to_string()]).unwrap(); + let facts = derived.get_asserted_facts().unwrap(); + let eligible: Vec<_> = facts + .iter() + .filter(|f| f.attribute.contains("eligible")) + .collect(); + assert_eq!(eligible.len(), 1, "only e2 is eligible (e3 is blocked)"); + } +``` + +- [ ] **Step 2: Run to confirm test passes against current code** + +```bash +cargo test --lib -p minigraf -- query::datalog::evaluator::tests::test_mixed_rule_with_expr 2>&1 +``` + +Expected: PASS (semantics test). + +- [ ] **Step 3: Refactor the mixed-rules loop in `StratifiedEvaluator::evaluate()`** + +In `src/query/datalog/evaluator.rs`, inside the mixed-rules loop (around line 729), replace the `positive_patterns` + `body_expr_clauses` extraction and the `raw_candidates` + `candidates` computation: + +**Current code to replace** (roughly lines 729–827): + +```rust + let positive_patterns: Vec = rule + .body + .iter() + .filter_map(|c| match c { + WhereClause::Pattern(p) => Some(p.clone()), + WhereClause::RuleInvocation { predicate, args } => { ... } + WhereClause::Not(_) | WhereClause::NotJoin { .. } => None, + WhereClause::Expr { .. } => None, + WhereClause::Or(_) | WhereClause::OrJoin { .. } => None, + }) + .collect(); + // ... not_clauses / not_join_clauses extraction ... + let body_expr_clauses: Vec<&WhereClause> = rule.body.iter() + .filter(|c| matches!(c, WhereClause::Expr { .. })).collect(); + let accumulated_facts: Arc<[Fact]> = ...; + let matcher = PatternMatcher::from_slice(accumulated_facts.clone()); + let raw_candidates = matcher.match_patterns(&positive_patterns); + // apply_or_clauses → or_expanded + let candidates = + apply_expr_clauses_in_evaluator(or_expanded, &body_expr_clauses, &fn_guard); +``` + +**New code**: + +```rust + // Collect Pattern and Expr clauses for the planner. + // RuleInvocations are converted to Pattern; Not/NotJoin/Or/OrJoin extracted separately below. + let mut plan_clauses: Vec = Vec::new(); + for c in &rule.body { + match c { + WhereClause::Pattern(_) | WhereClause::Expr { .. } => { + plan_clauses.push(c.clone()); + } + WhereClause::RuleInvocation { predicate, args } => { + let pattern = match args.len() { + 1 => args.first().map(|a0| { + Pattern::new( + a0.clone(), + EdnValue::Keyword(format!(":{}", predicate)), + EdnValue::Symbol("?_rule_value".to_string()), + ) + }), + 2 => args.first().and_then(|a0| { + args.get(1).map(|a1| { + Pattern::new( + a0.clone(), + EdnValue::Keyword(format!(":{}", predicate)), + a1.clone(), + ) + }) + }), + _ => None, + }; + if let Some(p) = pattern { + plan_clauses.push(WhereClause::Pattern(p)); + } + } + WhereClause::Not(_) + | WhereClause::NotJoin { .. } + | WhereClause::Or(_) + | WhereClause::OrJoin { .. } => {} + } + } + + let not_clauses: Vec> = rule + .body + .iter() + .filter_map(|c| match c { + WhereClause::Not(inner) => Some(inner.clone()), + _ => None, + }) + .collect(); + + let not_join_clauses: Vec<(Vec, Vec)> = rule + .body + .iter() + .filter_map(|c| match c { + WhereClause::NotJoin { join_vars, clauses } => { + Some((join_vars.clone(), clauses.clone())) + } + _ => None, + }) + .collect(); + + // Compute once; reuse for plan loop, apply_or_clauses, not-body matching. + let accumulated_facts: Arc<[Fact]> = + Arc::from(accumulated.get_asserted_facts().unwrap_or_default()); + + let matcher = PatternMatcher::from_slice(accumulated_facts.clone()); + + // Plan: assigns index hints + pushes Expr to earliest binding position. + let planned = crate::query::datalog::optimizer::plan( + plan_clauses, + &crate::storage::index::Indexes::new(), + ); + + // Process planned clauses in order: Pattern → join, Expr → filter/extend. + let fn_guard = self + .functions + .read() + .map_err(|_| anyhow!("function registry lock poisoned"))?; + let mut candidates: Vec = vec![Bindings::new()]; + for (clause, hint) in planned { + match clause { + WhereClause::Pattern(p) => { + candidates = matcher.match_with_hint_seeded( + candidates, + &p, + hint.as_ref() + .unwrap_or(&crate::query::datalog::optimizer::IndexHint::Eavt), + ); + } + WhereClause::Expr { expr, binding: out } => { + use crate::query::datalog::executor::{eval_expr, is_truthy}; + candidates = candidates + .into_iter() + .filter_map(|mut b| { + match eval_expr(&expr, &b, Some(&fn_guard)) { + Ok(v) => { + if let Some(var) = &out { + b.insert(var.clone(), v); + Some(b) + } else if is_truthy(&v) { + Some(b) + } else { + None + } + } + Err(_) => None, + } + }) + .collect(); + } + _ => {} + } + } + drop(fn_guard); + + // Apply Or/OrJoin clauses (mirrors top-level execute_query order). + let candidates = { + use crate::query::datalog::executor::apply_or_clauses; + let registry_guard = self + .rules + .read() + .map_err(|_| anyhow!("rule registry lock poisoned"))?; + let fn_registry = crate::query::datalog::functions::FunctionRegistry::with_builtins(); + let expanded = apply_or_clauses( + &rule.body, + candidates, + accumulated_facts.clone(), + ®istry_guard, + None, + None, + &fn_registry, + )?; + drop(registry_guard); + expanded + }; +``` + +The remaining `'binding: for binding in candidates { ... }` loop (not-body filtering + head instantiation) is **unchanged**. + +- [ ] **Step 4: Run evaluator tests** + +```bash +cargo test --lib -p minigraf -- query::datalog::evaluator 2>&1 +``` + +Expected: all tests pass. + +- [ ] **Step 5: Run all library tests** + +```bash +cargo test --lib -p minigraf 2>&1 +``` + +Expected: all tests pass. Fix any compile errors from call-site changes. + +- [ ] **Step 6: Commit** + +```bash +git add src/query/datalog/evaluator.rs +git commit -m "feat(evaluator): route mixed-rules path through plan() for Expr push-down + +StratifiedEvaluator mixed-rules loop now collects Pattern + Expr clauses +and calls optimizer::plan(), processing them in the same ordered loop as +the executor. Expr clauses are pushed down to the earliest valid position +in the rule body. Not/NotJoin filtering is unchanged. + +Closes #206. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 8: Add predicate push-down benchmark + +**Files:** +- Modify: `benches/minigraf_bench.rs` +- Modify: `benches/helpers/mod.rs` (optional — `populate_with_names` already exists) + +- [ ] **Step 1: Add a new `bench_predicate_pushdown` function in `benches/minigraf_bench.rs`** + +Add after the last existing bench function, before `criterion_group!`: + +```rust +// ── Task 8: query/predicate_pushdown ───────────────────────────────────────── + +fn bench_predicate_pushdown(c: &mut Criterion) { + // Fixture: n entities each with :val (integer) and :name (string). + // Uses helpers::populate_with_names(n). + // + // Query under test: multi-pattern with a selective Expr predicate. + // (query [:find ?e ?n :where [?e :val ?v] [?e :name ?n] [(> ?v )]]) + // + // Predicate selects 10% of entities. With push-down, [?e :name ?n] is only + // joined for the 10% that pass the filter; without push-down it would join all. + const SCALES: &[(&str, usize)] = &[ + ("1k", 1_000), + ("10k", 10_000), + ("100k", 100_000), + ]; + + let mut group = c.benchmark_group("query/predicate_pushdown"); + group.sample_size(10); + + for &(label, n) in SCALES { + let db = helpers::populate_with_names(n); + // Threshold = 90th percentile value → 10% of entities pass the predicate. + let threshold = (n as i64) * 9 / 10; + let query = format!( + "(query [:find ?e ?n :where [?e :val ?v] [?e :name ?n] [(> ?v {})]])", + threshold + ); + group.bench_with_input( + BenchmarkId::from_parameter(label), + &(db, query), + |b, (db, q)| { + b.iter(|| db.execute(q).unwrap()); + }, + ); + } + + group.finish(); +} +``` + +- [ ] **Step 2: Register the benchmark in `criterion_group!`** + +Find the `criterion_group!` macro and add `bench_predicate_pushdown`: + +```rust +criterion_group!( + benches, + bench_insert, + bench_insert_file, + // ... existing entries ... + bench_predicate_pushdown, +); +``` + +- [ ] **Step 3: Verify the benchmark compiles and runs** + +```bash +cargo bench --bench minigraf_bench -- query/predicate_pushdown 2>&1 | tail -20 +``` + +Expected: benchmark runs and prints throughput numbers, no panics. + +- [ ] **Step 4: Commit** + +```bash +git add benches/minigraf_bench.rs +git commit -m "bench: add query/predicate_pushdown benchmark group + +Measures multi-pattern query with a selective [(> ?v N)] predicate at +1K/10K/100K fact scales. Threshold set at 90th percentile (10% pass rate). +Provides baseline for evaluating push-down gains. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 9: Full test suite, final cleanup, open PR + +**Files:** +- No new file changes — verification and PR creation + +- [ ] **Step 1: Run the full test suite** + +```bash +cargo test 2>&1 +``` + +Expected: all tests pass (unit + integration + doc). Fix any remaining compile errors. + +- [ ] **Step 2: Run clippy** + +```bash +cargo clippy -- -D warnings 2>&1 +``` + +Expected: no warnings. Fix any clippy lints before continuing. + +- [ ] **Step 3: Run the full benchmark suite to confirm no panics** + +```bash +cargo bench --bench minigraf_bench 2>&1 | tail -30 +``` + +Expected: all benchmarks complete without panic. + +- [ ] **Step 4: Open the PR** + +```bash +gh pr create \ + --repo project-minigraf/minigraf \ + --title "perf: predicate push-down for Expr clauses + mixed rule optimization (#207, #206)" \ + --body "$(cat <<'EOF' +## Summary + +- **#207**: Extends `optimizer::plan()` to accept `Vec` (Pattern + Expr) and return an interleaved plan. Each `Expr` predicate clause is positioned at the earliest point where all its variables are bound by preceding patterns, rather than being applied as a post-pass over the full binding set. +- **#206**: Routes the `StratifiedEvaluator` mixed-rules path through the updated `plan()`, giving mixed rules both selectivity-based pattern ordering and Expr push-down with no separate implementation. + +## New helper + +`PatternMatcher::match_with_hint_seeded()` — used by both executor and evaluator to process the plan incrementally (first pattern uses index hint; subsequent patterns use hash-join). + +## Test plan + +- [ ] `cargo test` — all tests pass +- [ ] `cargo clippy -- -D warnings` — no warnings +- [ ] `cargo bench --bench minigraf_bench -- query/predicate_pushdown` — new benchmark runs without panic + +## Follow-up + +- Not/NotJoin push-down (same mechanism, more complex safety rules): project-minigraf/minigraf#248 + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +- [ ] **Step 5: Monitor CI and fix any failures** + +```bash +gh pr checks --watch +``` + +Expected: all checks green. If any fail, diagnose and fix before merging. From 362502ee54e442c567d00723924c546f9f96bb76 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Fri, 15 May 2026 23:07:48 +0530 Subject: [PATCH 03/13] docs: add design spec for Wave 2 PR 2 cost-based optimizer extensions (#205) Co-Authored-By: Claude Sonnet 4.6 --- .../2026-05-15-cost-based-optimizer-design.md | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md diff --git a/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md new file mode 100644 index 000000000..b6a7ffb37 --- /dev/null +++ b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md @@ -0,0 +1,143 @@ +# Design: Cost-Based Optimizer Extensions (Wave 2 PR 2) + +**Issue**: #205 (cost-based optimizer extensions for new clause types) +**Date**: 2026-05-15 +**Branch**: single worktree + PR + +--- + +## Summary + +Extend `optimizer.rs` with a static cardinality cost model and use it to order `not`/`not-join` clauses and `or`/`or-join` branches by estimated evaluation cost. Cheap filters run first; expensive ones only process the bindings that survived earlier filters. + +Aggregation post-processing cost estimation is **out of scope** — aggregation always runs last and cannot be reordered. + +Follow-up issue for Or short-circuit optimization (which builds on this cost model): #250. + +--- + +## Current Behaviour + +- `not`/`not-join` clauses are applied in source order — an expensive not-clause that scans many facts runs before a cheap one, wasting work on bindings that would have been rejected cheaply. +- `or`/`or-join` branches are evaluated in source order — no cost-based selection. +- The optimizer has no concept of cost for clause types other than `Pattern` and `Expr`. + +--- + +## Design + +### 1. Cost model — `optimizer.rs` + +#### `pattern_cost(p: &Pattern) -> u64` (private) + +Static 4-tier model derived from the existing `selectivity_score()`: + +| Bound EAV components | Cost | +|---|---| +| 3 (all bound) | 1 | +| 2 | 10 | +| 1 | 100 | +| 0 (nothing bound) | 10,000 | + +#### `pub fn clause_cost(clause: &WhereClause) -> u64` + +| Clause type | Cost formula | +|---|---| +| `Pattern(p)` | `pattern_cost(p)` | +| `Expr { .. }` | 0 (pure computation, no fact scan) | +| `Not(body)` | `min(pattern_cost(p) for p in body patterns)`, or 0 if body has no patterns | +| `NotJoin { clauses, .. }` | same as `Not` using `clauses` | +| `Or(branches)` | `sum(branch_cost(b) for b in branches)` | +| `OrJoin { branches, .. }` | same as `Or` | +| other | `u64::MAX` (never passed; defensive) | + +#### `pub fn branch_cost(branch: &[WhereClause]) -> u64` + +`min(pattern_cost(p) for p in branch patterns)`, or 0 if branch has no patterns. + +**Rationale for `min` in not/or bodies**: In a multi-pattern join, the most selective pattern (lowest cardinality) dominates — the join can never produce more rows than the smallest input. Using `min` avoids overestimating the cost of a body that contains at least one highly selective pattern. + +**WASM behaviour**: `clause_cost()` and `branch_cost()` are unconditional (available on all targets). The *sorting* call sites are gated behind `#[cfg(not(feature = "wasm"))]`, consistent with the existing pattern ordering gate. + +--- + +### 2. Executor changes — `executor.rs` + +#### `not`/`not-join` ordering + +In both `execute_query()` and `execute_query_with_rules()`, after extracting `not_clauses` and `not_join_clauses` and before the filter loop, sort each by `clause_cost()` ascending: + +```rust +#[cfg(not(feature = "wasm"))] +not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.clone()))); + +#[cfg(not(feature = "wasm"))] +not_join_clauses.sort_by_key(|(vars, body)| { + optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: body.clone(), + }) +}); +``` + +**Effect**: Cheap not-filters run first. Bindings rejected by a cheap filter never reach more expensive filters. No change to semantics. + +#### `or`/`or-join` branch ordering + +In `apply_or_clauses()`, before evaluating branches in both the fast path (no Not in branches) and the slow path (seeded), sort branches by `branch_cost()` ascending: + +```rust +#[cfg(not(feature = "wasm"))] +let sorted_branches: Vec<_> = { + let mut b = branches.to_vec(); + b.sort_by_key(|br| optimizer::branch_cost(br)); + b +}; +// iterate sorted_branches instead of branches +``` + +**Note**: In the current executor, all Or branches are always evaluated (no short-circuit). Branch ordering is a no-op for correctness and has no measurable performance impact today. It is infrastructure for #250 (short-circuit optimization). A code comment explains this. + +--- + +### 3. Evaluator changes — `evaluator.rs` + +The `StratifiedEvaluator` mixed-rules loop applies `not`/`not-join` as post-filters in the `'binding:` loop. After extracting `not_clauses` and `not_join_clauses` (which already happens before the `'binding:` loop), sort both collections by `clause_cost()` ascending, gated behind `#[cfg(not(feature = "wasm"))]`. The sort is identical to the executor call sites. + +--- + +### 4. Testing + +**`optimizer.rs` unit tests** — `clause_cost()` and `branch_cost()`: +- Fully-bound pattern → cost 1 +- 2-bound pattern → cost 10 +- 1-bound pattern → cost 100 +- Unbound pattern → cost 10,000 +- `Not` body: one selective + one full-scan pattern → cost = 1 (min) +- `Not` body with no patterns (expr-only) → cost 0 +- `Or` with two branches → cost = sum of branch min-costs +- `branch_cost` of empty branch → 0 + +**`executor.rs` integration tests** — not ordering: +- Query with two `not` clauses in expensive-first source order → results identical to cheap-first order (correctness regression guard) +- Query with two `not` clauses where cheap one eliminates most bindings → results correct + +**Validation**: The existing `bench_negation` group in `benches/minigraf_bench.rs` provides the baseline for measuring `not`-ordering improvement. No new benchmark group is added in this PR. + +--- + +## Invariants Preserved + +- **Correctness**: Sorting only changes evaluation order, never the set of bindings that satisfy a clause. The `not` filter is semantically commutative when applied to independent not-bodies over the same outer binding set. +- **WASM portability**: Cost functions unconditional; sorting gated behind `#[cfg(not(feature = "wasm"))]`. +- **Backwards compatibility**: No change to `plan()` signature or return type. `clause_cost()` and `branch_cost()` are new public additions. + +--- + +## Files Changed + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | New `pattern_cost()`, `clause_cost()`, `branch_cost()` | +| `src/query/datalog/executor.rs` | Cost-sorted `not_clauses`, `not_join_clauses` in both query functions; cost-sorted branches in `apply_or_clauses()` | +| `src/query/datalog/evaluator.rs` | Cost-sorted `not_clauses`, `not_join_clauses` in mixed-rules loop | From 6aa8466ca03fb550c5c9daf19fdd3ab295f0d378 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 08:37:22 +0530 Subject: [PATCH 04/13] docs: explain WASM ordering gate at each cfg site in optimizer and spec Add a concise 'WASM omission' comment at every existing and planned #[cfg(not(feature = "wasm"))] ordering gate explaining why sorting is disabled on WASM targets (small datasets, deterministic behaviour). The full rationale lives in the selectivity_score() doc comment; all other sites reference it with a short cross-reference. Co-Authored-By: Claude Sonnet 4.6 --- .../2026-05-15-cost-based-optimizer-design.md | 24 ++++++++++++++++--- src/query/datalog/optimizer.rs | 5 ++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md index b6a7ffb37..e972828a2 100644 --- a/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md +++ b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md @@ -68,9 +68,11 @@ Static 4-tier model derived from the existing `selectivity_score()`: In both `execute_query()` and `execute_query_with_rules()`, after extracting `not_clauses` and `not_join_clauses` and before the filter loop, sort each by `clause_cost()` ascending: ```rust +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.clone()))); +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] not_join_clauses.sort_by_key(|(vars, body)| { optimizer::clause_cost(&WhereClause::NotJoin { @@ -87,6 +89,9 @@ not_join_clauses.sort_by_key(|(vars, body)| { In `apply_or_clauses()`, before evaluating branches in both the fast path (no Not in branches) and the slow path (seeded), sort branches by `branch_cost()` ascending: ```rust +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). +// Note: all branches are still evaluated (no short-circuit); this ordering is +// infrastructure for #250. See that issue for the short-circuit optimization. #[cfg(not(feature = "wasm"))] let sorted_branches: Vec<_> = { let mut b = branches.to_vec(); @@ -96,13 +101,26 @@ let sorted_branches: Vec<_> = { // iterate sorted_branches instead of branches ``` -**Note**: In the current executor, all Or branches are always evaluated (no short-circuit). Branch ordering is a no-op for correctness and has no measurable performance impact today. It is infrastructure for #250 (short-circuit optimization). A code comment explains this. - --- ### 3. Evaluator changes — `evaluator.rs` -The `StratifiedEvaluator` mixed-rules loop applies `not`/`not-join` as post-filters in the `'binding:` loop. After extracting `not_clauses` and `not_join_clauses` (which already happens before the `'binding:` loop), sort both collections by `clause_cost()` ascending, gated behind `#[cfg(not(feature = "wasm"))]`. The sort is identical to the executor call sites. +The `StratifiedEvaluator` mixed-rules loop applies `not`/`not-join` as post-filters in the `'binding:` loop. After extracting `not_clauses` and `not_join_clauses` (which already happens before the `'binding:` loop), sort both collections by `clause_cost()` ascending: + +```rust +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). +#[cfg(not(feature = "wasm"))] +not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.clone()))); + +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). +#[cfg(not(feature = "wasm"))] +not_join_clauses.sort_by_key(|(vars, clauses)| { + optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: clauses.clone(), + }) +}); +``` --- diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index a741b984e..5648a0a53 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -39,6 +39,10 @@ fn attr_is_index_bound(a: &AttributeSpec) -> bool { /// Count the number of non-variable components in a pattern. /// Higher score = more selective. +/// +/// Gated on non-wasm: WASM/browser targets typically work with small datasets where +/// the overhead of computing scores and sorting can equal or exceed the benefit. +/// Source order is preserved on WASM for deterministic, debuggable query behaviour. #[cfg(not(feature = "wasm"))] fn selectivity_score(p: &Pattern) -> u8 { let e = !is_variable(&p.entity); @@ -145,6 +149,7 @@ pub fn plan( // Stable sort patterns by selectivity descending (non-wasm only). // Preserves original order for ties, ensuring deterministic output. + // WASM omission: see selectivity_score() — small datasets, determinism. #[cfg(not(feature = "wasm"))] patterns.sort_by_key(|(clause, _)| { if let WhereClause::Pattern(p) = clause { From f68a1df0b53777e6499d271bbb1054b9137ce590 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 11:54:33 +0530 Subject: [PATCH 05/13] docs: add design spec for SIMD benchmarking (#229) --- .../2026-05-16-simd-benchmarking-design.md | 222 ++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-16-simd-benchmarking-design.md diff --git a/docs/superpowers/specs/2026-05-16-simd-benchmarking-design.md b/docs/superpowers/specs/2026-05-16-simd-benchmarking-design.md new file mode 100644 index 000000000..cb33350a7 --- /dev/null +++ b/docs/superpowers/specs/2026-05-16-simd-benchmarking-design.md @@ -0,0 +1,222 @@ +# Design: SIMD Benchmarking (Issue #229) + +**Issue**: #229 (SIMD vectorization evaluation) +**Date**: 2026-05-16 +**Branch**: single worktree + PR +**Prerequisite**: #208 (B+Tree selective lookup) — closed + +--- + +## Summary + +Evaluate whether explicit SIMD vectorization at the three highest-impact hot paths yields meaningful performance gains over the current scalar implementation, given Minigraf's embedded/small-dataset profile. The deliverable is benchmarks + a minimal SIMD prototype (bench-only) + a written crossover analysis and recommendation. + +**What ships:** +- `benches/simd_helpers.rs` — three SIMD functions using `wide` (dev-dependency) +- `benches/minigraf_bench.rs` — three new Criterion groups at 5 dataset sizes +- `docs/simd-analysis.md` — crossover findings and final recommendation (written after running benchmarks locally) + +**What does NOT ship:** +- No changes to any `src/` file +- No production SIMD code +- No feature flags + +--- + +## Targets + +Three hot paths identified after #208 (selective lookup reduces residual filter workload): + +| Target | Location | Pattern | +|---|---|---| +| Valid-time range filter | `src/graph/storage.rs` `get_facts_valid_at` | `valid_from <= ts && ts < valid_to` per fact | +| Tx-time as-of filter | `src/graph/storage.rs` `get_facts_as_of` | `tx_count <= n` per fact | +| Aggregation sum | `src/query/datalog/functions.rs` `apply_builtin_aggregate` | horizontal `i64`/`f64` reduction | + +--- + +## SIMD Helpers — `benches/simd_helpers.rs` + +All three functions use `wide` (added to `[dev-dependencies]` only — no binary size impact). + +> **Note for implementer**: Verify the exact `wide 0.7` API before coding — specifically `cmp_le`, `cmp_gt`, mask extraction via `Into<[T; 4]>`, and whether `u64x4` is available at that version. The pseudocode below shows intent; adapt to the actual API as needed. + +### `valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> Vec` + +Processes 4 facts at a time using `i64x4`. Returns indices of facts where `valid_from[i] <= ts && ts < valid_to[i]`. Scalar tail handles remainder when `len % 4 != 0`. + +```rust +pub fn valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> Vec { + use wide::i64x4; + let ts_v = i64x4::splat(ts); + let mut result = Vec::new(); + let chunks = valid_from.len() / 4; + for i in 0..chunks { + let base = i * 4; + let vf = i64x4::new([valid_from[base], valid_from[base+1], valid_from[base+2], valid_from[base+3]]); + let vt = i64x4::new([valid_to[base], valid_to[base+1], valid_to[base+2], valid_to[base+3]]); + // vf <= ts && ts < vt ↔ vf <= ts && vt > ts + let mask_lo = vf.cmp_le(ts_v); + let mask_hi = vt.cmp_gt(ts_v); + let mask = mask_lo & mask_hi; + let bits: [i64; 4] = mask.into(); + for (j, &b) in bits.iter().enumerate() { + if b != 0 { result.push(base + j); } + } + } + // scalar tail + for i in (chunks * 4)..valid_from.len() { + if valid_from[i] <= ts && ts < valid_to[i] { result.push(i); } + } + result +} +``` + +### `as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> Vec` + +Processes 4 facts at a time using `u64x4`. Returns indices where `tx_counts[i] <= threshold`. + +```rust +pub fn as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> Vec { + use wide::u64x4; + let thr_v = u64x4::splat(threshold); + let mut result = Vec::new(); + let chunks = tx_counts.len() / 4; + for i in 0..chunks { + let base = i * 4; + let tc = u64x4::new([tx_counts[base], tx_counts[base+1], tx_counts[base+2], tx_counts[base+3]]); + let mask = tc.cmp_le(thr_v); + let bits: [u64; 4] = mask.into(); + for (j, &b) in bits.iter().enumerate() { + if b != 0 { result.push(base + j); } + } + } + for i in (chunks * 4)..tx_counts.len() { + if tx_counts[i] <= threshold { result.push(i); } + } + result +} +``` + +### `sum_simd_i64(values: &[i64]) -> i64` + +Horizontal reduction using `i64x4` with wrapping add (matching Rust's default integer overflow behaviour in release mode). Accumulates 4 lanes in parallel, reduces at the end. + +```rust +pub fn sum_simd_i64(values: &[i64]) -> i64 { + use wide::i64x4; + let mut acc = i64x4::splat(0); + let chunks = values.len() / 4; + for i in 0..chunks { + let base = i * 4; + let v = i64x4::new([values[base], values[base+1], values[base+2], values[base+3]]); + acc += v; + } + let lanes: [i64; 4] = acc.into(); + let mut sum: i64 = lanes.iter().sum(); + for i in (chunks * 4)..values.len() { + sum = sum.wrapping_add(values[i]); + } + sum +} +``` + +--- + +## Benchmark Groups — `benches/minigraf_bench.rs` + +Three new groups, each with 5 dataset sizes: 100, 1K, 10K, 100K, 1M facts. + +### Timing scope (critical) + +- **Scalar baseline**: calls the production function directly (e.g. `storage.get_facts_valid_at(ts)`). The production function's internal extraction is measured as part of its cost. +- **SIMD variant**: field extraction from `Vec` into numeric slices **plus** the SIMD helper, all timed together inside the measurement loop. This reflects the full cost a production SIMD path would pay — if extraction overhead makes SIMD slower, that is the correct finding. + +### `bench_simd_temporal` + +`helpers::populate_temporal(n)` is a new benchmark helper (added in this PR) that inserts `n` facts with staggered `valid_from`/`valid_to` windows so roughly half pass a midpoint timestamp filter — realistic selectivity. `helpers::midpoint_timestamp` returns the median `valid_from` value. + +```rust +fn bench_simd_temporal(c: &mut Criterion) { + let mut group = c.benchmark_group("simd_temporal"); + for &n in &[100usize, 1_000, 10_000, 100_000, 1_000_000] { + let storage = helpers::populate_temporal(n); // new helper: staggered valid-time windows + let ts = helpers::midpoint_timestamp(&storage); + + group.bench_with_input(BenchmarkId::new("scalar", n), &n, |b, _| { + b.iter(|| storage.get_facts_valid_at(black_box(ts))) + }); + + group.bench_with_input(BenchmarkId::new("simd", n), &n, |b, _| { + b.iter(|| { + let facts = storage.get_all_facts(); + let vf: Vec = facts.iter().map(|f| f.valid_from).collect(); + let vt: Vec = facts.iter().map(|f| f.valid_to).collect(); + simd_helpers::valid_time_filter_simd(&vf, &vt, black_box(ts)) + }) + }); + } + group.finish(); +} +``` + +### `bench_simd_as_of` + +Mirrors `bench_simd_temporal` but: +- Scalar: `storage.get_facts_as_of(AsOf::Counter(black_box(threshold)))` +- SIMD: extract `tx_count` field into `Vec` + `as_of_filter_simd` +- `threshold` = median `tx_count` value (50th percentile — realistic as-of query) + +### `bench_simd_aggregate` + +- Dataset: N facts with `Value::Integer` attributes, pre-queried to `Vec` +- Scalar: production `apply_builtin_aggregate("sum", &values)` +- SIMD: extract `i64` values from `Vec` + `sum_simd_i64` +- Both timed including extraction (same rationale as above) + +--- + +## Analysis Document — `docs/simd-analysis.md` + +Written after running benchmarks locally on the development machine. Sections: + +1. **Environment**: CPU, OS, Rust toolchain, `wide` version +2. **Results table**: ns/iter (scalar vs SIMD) at each dataset size, per target +3. **Crossover point**: first dataset size where SIMD is faster, or "none observed" +4. **Post-#208 residual analysis**: typical fact counts reaching these filters after selective lookup (from `btree_lookup` benchmark data); contextualises the crossover within real workloads +5. **Recommendation**: one of: + - **Integrate**: SIMD wins at realistic dataset sizes → follow-up issue to promote helpers into `src/` + - **Skip**: SIMD slower or negligible at all practical sizes → close as not worth it + - **Revisit with SoA layout**: SIMD wins only at large N but extraction overhead dominates → document struct-of-arrays as prerequisite for SIMD to pay off + +--- + +## Cargo.toml change + +```toml +[dev-dependencies] +wide = "0.7" +# ... existing dev-dependencies +``` + +No change to `[dependencies]` — zero binary size impact. + +--- + +## Files Changed + +| File | Change | +|---|---| +| `Cargo.toml` | Add `wide` to `[dev-dependencies]` | +| `benches/simd_helpers.rs` | New: three SIMD functions | +| `benches/minigraf_bench.rs` | Add `bench_simd_temporal`, `bench_simd_as_of`, `bench_simd_aggregate` groups | +| `docs/simd-analysis.md` | New: crossover findings and recommendation (written last) | + +--- + +## Invariants + +- **No production code changes**: all SIMD code is in `benches/` +- **No feature flags**: `wide` is a dev-dependency; it never enters the release build +- **WASM portability unaffected**: `wide` has scalar fallbacks; since it's dev-only, WASM builds are unaffected +- **Existing benchmarks unchanged**: new groups are additive only From e29f4b2146c601f022065b933f334b3fd63f6d87 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 12:05:32 +0530 Subject: [PATCH 06/13] docs: add Wave 2 PR 2 and #229 SIMD benchmarking implementation plans Co-Authored-By: Claude Sonnet 4.6 --- .../plans/2026-05-15-cost-based-optimizer.md | 678 ++++++++++++++ .../plans/2026-05-16-simd-benchmarking.md | 861 ++++++++++++++++++ 2 files changed, 1539 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-15-cost-based-optimizer.md create mode 100644 docs/superpowers/plans/2026-05-16-simd-benchmarking.md diff --git a/docs/superpowers/plans/2026-05-15-cost-based-optimizer.md b/docs/superpowers/plans/2026-05-15-cost-based-optimizer.md new file mode 100644 index 000000000..4281810d0 --- /dev/null +++ b/docs/superpowers/plans/2026-05-15-cost-based-optimizer.md @@ -0,0 +1,678 @@ +# Cost-Based Optimizer Extensions Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `pattern_cost`, `clause_cost`, and `branch_cost` to `optimizer.rs`, then use them to sort `not`/`not-join` clauses and `or`/`or-join` branches by cost ascending so cheap filters run first. + +**Architecture:** Three new cost functions land in `optimizer.rs` (unconditional — available on all targets). The sorting call-sites in `executor.rs` and `evaluator.rs` are gated behind `#[cfg(not(feature = "wasm"))]`, matching the existing pattern-ordering gate. No change to `plan()` signature or return type. + +**Tech Stack:** Rust, Cargo, `cargo test`, `cargo clippy` + +--- + +## File Structure + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | Add private `pattern_cost()`, pub `clause_cost()`, pub `branch_cost()` before the `#[cfg(test)]` block at line 199 | +| `src/query/datalog/executor.rs` | Sort `not_clauses`/`not_join_clauses` after line 592 (execute_query) and after line 1060 (execute_query_with_rules); sort branches in `apply_or_clauses()` before the `for branch in branches` loops | +| `src/query/datalog/evaluator.rs` | Sort `not_clauses`/`not_join_clauses` after line 787 (StratifiedEvaluator mixed-rules loop) | + +--- + +### Task 1: Git worktree + +**Files:** +- No file changes — worktree setup only + +- [ ] **Step 1: Invoke the using-git-worktrees skill** + +``` +Use superpowers:using-git-worktrees to create a worktree for branch feature/issue-205-cost-based-optimizer +``` + +Expected: worktree created at `.worktrees/issue-205-cost-based-optimizer`, checked out to new branch `feature/issue-205-cost-based-optimizer`. + +--- + +### Task 2: Failing unit tests for cost functions + +**Files:** +- Modify: `src/query/datalog/optimizer.rs:199` (append to existing `#[cfg(test)]` module) + +The existing test module starts at line 199. Add new tests at the end of it (before the closing `}`). + +- [ ] **Step 1: Read the end of the existing test module** + +```bash +tail -n 30 src/query/datalog/optimizer.rs +``` + +Note the exact closing `}` line number so you can insert before it. + +- [ ] **Step 2: Add failing tests for the three cost functions** + +Append the following test functions inside the existing `#[cfg(test)] mod tests { ... }` block, before its closing `}`: + +```rust + // ── cost model tests ────────────────────────────────────────────────── + + fn sym(s: &str) -> EdnValue { + EdnValue::Symbol(s.to_string()) + } + + fn lit_str(s: &str) -> EdnValue { + EdnValue::Str(s.to_string()) + } + + fn attr(s: &str) -> AttributeSpec { + use crate::query::datalog::types::AttributeSpec; + AttributeSpec::Real(EdnValue::Keyword(s.to_string())) + } + + fn attr_var() -> AttributeSpec { + use crate::query::datalog::types::AttributeSpec; + AttributeSpec::Real(EdnValue::Symbol("?a".to_string())) + } + + fn make_cost_pattern( + entity: EdnValue, + attribute: AttributeSpec, + value: EdnValue, + ) -> Pattern { + use crate::query::datalog::types::AttributeSpec; + Pattern::new(entity, attribute.into_edn(), value) + } + + #[test] + fn test_pattern_cost_fully_bound() { + // entity bound (UUID), attribute real keyword, value bound literal — 3 bound → cost 1 + let p = Pattern::new( + EdnValue::Uuid(Uuid::new_v4()), + EdnValue::Keyword(":person/name".to_string()), + lit_str("Alice"), + ); + assert_eq!(pattern_cost(&p), 1); + } + + #[test] + fn test_pattern_cost_two_bound() { + // attribute + value bound, entity variable — 2 bound → cost 10 + let p = Pattern::new( + sym("?e"), + EdnValue::Keyword(":person/name".to_string()), + lit_str("Alice"), + ); + assert_eq!(pattern_cost(&p), 10); + } + + #[test] + fn test_pattern_cost_one_bound() { + // only attribute bound — 1 bound → cost 100 + let p = Pattern::new( + sym("?e"), + EdnValue::Keyword(":person/name".to_string()), + sym("?v"), + ); + assert_eq!(pattern_cost(&p), 100); + } + + #[test] + fn test_pattern_cost_unbound() { + // all variables — 0 bound → cost 10_000 + let p = Pattern::new( + sym("?e"), + EdnValue::Symbol("?a".to_string()), + sym("?v"), + ); + assert_eq!(pattern_cost(&p), 10_000); + } + + #[test] + fn test_clause_cost_pattern() { + // clause_cost delegates to pattern_cost for Pattern variant + let p = Pattern::new( + sym("?e"), + EdnValue::Keyword(":person/name".to_string()), + lit_str("Alice"), + ); + let clause = WhereClause::Pattern(p); + assert_eq!(clause_cost(&clause), 10); // attr + value bound = 2 → 10 + } + + #[test] + fn test_clause_cost_expr() { + // Expr is pure computation — cost 0 + let clause = WhereClause::Expr { + var: "?x".to_string(), + expr: Expr::Lit(crate::graph::types::Value::Integer(42)), + }; + assert_eq!(clause_cost(&clause), 0); + } + + #[test] + fn test_clause_cost_not_body_uses_min() { + // Not body: one selective pattern (cost 10) + one full-scan pattern (cost 10_000) + // clause_cost → min = 10 + let selective = Pattern::new( + sym("?e"), + EdnValue::Keyword(":person/name".to_string()), + lit_str("Alice"), + ); + let full_scan = Pattern::new(sym("?x"), EdnValue::Symbol("?a".to_string()), sym("?v")); + let clause = WhereClause::Not(vec![ + WhereClause::Pattern(selective), + WhereClause::Pattern(full_scan), + ]); + assert_eq!(clause_cost(&clause), 10); + } + + #[test] + fn test_clause_cost_not_body_expr_only() { + // Not body with no patterns (expr only) → cost 0 + let clause = WhereClause::Not(vec![WhereClause::Expr { + var: "?x".to_string(), + expr: Expr::Lit(crate::graph::types::Value::Integer(1)), + }]); + assert_eq!(clause_cost(&clause), 0); + } + + #[test] + fn test_branch_cost_empty_branch() { + // Empty branch → 0 + assert_eq!(branch_cost(&[]), 0); + } + + #[test] + fn test_branch_cost_no_patterns() { + // Branch with only Expr clauses → 0 + let branch = vec![WhereClause::Expr { + var: "?x".to_string(), + expr: Expr::Lit(crate::graph::types::Value::Integer(99)), + }]; + assert_eq!(branch_cost(&branch), 0); + } + + #[test] + fn test_clause_cost_or_sums_branch_costs() { + // Or with two branches: + // branch 1: one pattern with cost 10 + // branch 2: one pattern with cost 100 + // clause_cost(Or) = sum = 110 + let b1 = vec![WhereClause::Pattern(Pattern::new( + sym("?e"), + EdnValue::Keyword(":person/name".to_string()), + lit_str("Alice"), + ))]; + let b2 = vec![WhereClause::Pattern(Pattern::new( + sym("?e"), + EdnValue::Keyword(":person/age".to_string()), + sym("?v"), + ))]; + let clause = WhereClause::Or(vec![b1, b2]); + assert_eq!(clause_cost(&clause), 110); // 10 + 100 + } +``` + +- [ ] **Step 3: Run the tests and verify they fail with "not found" errors** + +```bash +cargo test -p minigraf pattern_cost branch_cost clause_cost -- --nocapture 2>&1 | head -40 +``` + +Expected: compile errors — `pattern_cost`, `clause_cost`, `branch_cost` not found. This is the TDD red phase. + +--- + +### Task 3: Implement cost functions in optimizer.rs + +**Files:** +- Modify: `src/query/datalog/optimizer.rs` (insert before line 199) + +- [ ] **Step 1: Insert the three cost functions before the `#[cfg(test)]` block** + +Open `src/query/datalog/optimizer.rs`. Find line 199 (`#[cfg(test)]`). Insert the following block immediately before it (after line 197, which is the closing `}` of `plan()`): + +```rust +/// Static 4-tier cardinality estimate for a single pattern. +/// +/// Derived from selectivity_score but returns u64 cost (lower = cheaper) rather +/// than a selectivity score. Unconditional: available on all targets including WASM. +fn pattern_cost(p: &Pattern) -> u64 { + let e = !is_variable(&p.entity); + let a = attr_is_index_bound(&p.attribute); + let v = !is_variable(&p.value); + match (e as u8) + (a as u8) + (v as u8) { + 3 => 1, + 2 => 10, + 1 => 100, + _ => 10_000, + } +} + +/// Estimated cost for a body/branch slice — the minimum `pattern_cost` across all +/// Pattern clauses, or 0 if the body contains no patterns (expr-only bodies are +/// cheap pure computation). +/// +/// Rationale for `min`: In a multi-pattern join the most selective pattern dominates — +/// the join cannot produce more rows than the smallest input. +/// +/// Unconditional: available on all targets including WASM. +pub fn branch_cost(branch: &[WhereClause]) -> u64 { + branch + .iter() + .filter_map(|c| { + if let WhereClause::Pattern(p) = c { + Some(pattern_cost(p)) + } else { + None + } + }) + .min() + .unwrap_or(0) +} + +/// Estimated evaluation cost for any `WhereClause`. +/// +/// | Clause type | Cost | +/// |--------------------|------| +/// | `Pattern` | `pattern_cost(p)` | +/// | `Expr` | 0 (pure computation) | +/// | `Not(body)` | `branch_cost(body)` | +/// | `NotJoin{clauses}` | `branch_cost(clauses)` | +/// | `Or(branches)` | sum of `branch_cost` per branch | +/// | `OrJoin{branches}` | sum of `branch_cost` per branch | +/// | other | `u64::MAX` (defensive; not expected in practice) | +/// +/// Unconditional: available on all targets including WASM. The *sorting* call-sites +/// that consume this function are gated behind `#[cfg(not(feature = "wasm"))]`. +pub fn clause_cost(clause: &WhereClause) -> u64 { + match clause { + WhereClause::Pattern(p) => pattern_cost(p), + WhereClause::Expr { .. } => 0, + WhereClause::Not(body) => branch_cost(body), + WhereClause::NotJoin { clauses, .. } => branch_cost(clauses), + WhereClause::Or(branches) => branches.iter().map(|b| branch_cost(b)).sum(), + WhereClause::OrJoin { branches, .. } => branches.iter().map(|b| branch_cost(b)).sum(), + _ => u64::MAX, + } +} + +``` + +- [ ] **Step 2: Run cost-function unit tests to verify they pass** + +```bash +cargo test -p minigraf pattern_cost branch_cost clause_cost -- --nocapture 2>&1 | tail -20 +``` + +Expected: all new tests PASS. No compile errors. + +- [ ] **Step 3: Run full test suite to check for regressions** + +```bash +cargo test 2>&1 | tail -5 +``` + +Expected: all existing tests still pass. + +- [ ] **Step 4: Commit** + +```bash +git add src/query/datalog/optimizer.rs +git commit -m "feat(optimizer): add pattern_cost, branch_cost, clause_cost (#205) + +Static 4-tier cardinality cost model. Unconditional (all targets). +Sorting call-sites gated on cfg(not(feature = \"wasm\")) added in next commit." +``` + +--- + +### Task 4: Sort not-clauses in executor.rs (both query functions) + +**Files:** +- Modify: `src/query/datalog/executor.rs:592` and `src/query/datalog/executor.rs:1060` + +- [ ] **Step 1: Write integration tests for not-clause ordering** + +Add the following test at the end of `executor.rs`'s `#[cfg(test)] mod tests { ... }` block: + +```rust + #[test] + fn test_not_clause_ordering_correctness() { + // Two `not` clauses in expensive-first source order; results must be + // identical to cheap-first order — the sorting is semantics-preserving. + use crate::graph::types::Value; + use crate::storage::backend::memory::MemoryBackend; + use crate::db::Minigraf; + + let db = Minigraf::open_with_backend(MemoryBackend::new()).unwrap(); + db.execute( + r#"(transact [ + [:db/add "e1" :item/name "widget" :item/tag "cheap"] + [:db/add "e2" :item/name "gadget" :item/tag "cheap"] + [:db/add "e3" :item/name "doohickey"] + ])"#, + ) + .unwrap(); + + // Query: items that are NOT named "widget" AND NOT named "gadget" + // Only "doohickey" survives both filters. + let result = db + .execute( + r#"(query {:find [?name] + :where [[?e :item/name ?name] + (not [?e :item/name "widget"]) + (not [?e :item/name "gadget"])]})"#, + ) + .unwrap(); + assert_eq!(result.len(), 1, "expected exactly one result"); + assert_eq!( + result[0].get("name").cloned().or_else(|| result[0].get("?name").cloned()), + Some(Value::String("doohickey".to_string())) + ); + } +``` + +- [ ] **Step 2: Run the test to verify it passes (behaviour baseline)** + +```bash +cargo test -p minigraf test_not_clause_ordering_correctness -- --nocapture +``` + +Expected: PASS — this is a correctness regression guard, not a TDD new-feature test. + +- [ ] **Step 3: Add not-clause sorting after line 592 in execute_query()** + +In `src/query/datalog/executor.rs`, find the block ending at line 592 (`.collect();` closing `not_join_clauses`). Insert the following two lines immediately after it: + +```rust + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_clauses.sort_by_key(|body| optimizer::branch_cost(body)); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_join_clauses.sort_by_key(|(_, clauses)| optimizer::branch_cost(clauses)); +``` + +- [ ] **Step 4: Add not-clause sorting after line 1060 in execute_query_with_rules()** + +Find the second `.collect();` closing `not_join_clauses` (around line 1060). Insert the same block: + +```rust + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_clauses.sort_by_key(|body| optimizer::branch_cost(body)); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_join_clauses.sort_by_key(|(_, clauses)| optimizer::branch_cost(clauses)); +``` + +- [ ] **Step 5: Run full test suite** + +```bash +cargo test 2>&1 | tail -5 +``` + +Expected: all tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add src/query/datalog/executor.rs +git commit -m "feat(executor): sort not/not-join clauses by cost ascending (#205) + +Cheap filters run first; bindings rejected early never reach expensive filters. +Gated on cfg(not(feature = \"wasm\")) — small datasets + determinism on WASM." +``` + +--- + +### Task 5: Sort or/or-join branches in apply_or_clauses() + +**Files:** +- Modify: `src/query/datalog/executor.rs` — `apply_or_clauses()` function + +`apply_or_clauses()` starts at line 1821. There are two `for branch in branches` loops: +- **Slow path** (seeded, has Not): loop at line 1846 +- **Fast path** (no Not, hash-join): loop at line 1880 + +Both operate on the same `branches` binding. We introduce a sorted copy before both paths. + +- [ ] **Step 1: Find the `any_branch_has_not` binding inside the `WhereClause::Or` arm** + +Read `src/query/datalog/executor.rs` lines 1832–1845. The structure is: + +```rust +WhereClause::Or(branches) => { + let any_branch_has_not = branches.iter().any(|b| { ... }); + + if any_branch_has_not { + // slow path: for branch in branches { ... } + ... + } + // fast path: for branch in branches { ... } +``` + +- [ ] **Step 2: Add branch sorting before the `any_branch_has_not` check** + +Insert immediately after the opening `WhereClause::Or(branches) => {` line: + +```rust + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + // Note: all branches are still evaluated (no short-circuit); this ordering + // is infrastructure for issue #250. See that issue for short-circuit optimization. + #[cfg(not(feature = "wasm"))] + let branches: Vec<_> = { + let mut b: Vec<_> = branches.iter().collect(); + b.sort_by_key(|br| optimizer::branch_cost(br)); + b.into_iter().map(|br| br.as_slice()).collect() + }; + #[cfg(not(feature = "wasm"))] + let branches: &[_] = &branches; + #[cfg(feature = "wasm")] + let branches: Vec<_> = branches.iter().map(|b| b.as_slice()).collect(); + #[cfg(feature = "wasm")] + let branches: &[_] = &branches; +``` + +Wait — this approach is overly complex because `branches` is already a `&Vec>` from the match arm. A simpler approach: introduce a locally-sorted `Vec` that shadows `branches` as a `Vec<&Vec>`, and replace the two loop references. + +**Revised Step 2**: Insert immediately after `WhereClause::Or(branches) => {`: + +```rust + // Sort branches by cost ascending so cheaper branches evaluate first. + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + // Note: all branches still evaluated (no short-circuit); ordering is + // infrastructure for issue #250. + let sorted_branches: Vec<&Vec>; + #[cfg(not(feature = "wasm"))] + { + let mut b: Vec<&Vec> = branches.iter().collect(); + b.sort_by_key(|br| optimizer::branch_cost(br)); + sorted_branches = b; + } + #[cfg(feature = "wasm")] + { + sorted_branches = branches.iter().collect(); + } +``` + +Then replace `for branch in branches {` in the **slow path** (line ~1846) with `for branch in &sorted_branches {`. + +And replace `for branch in branches {` in the **fast path** (line ~1880) with `for branch in &sorted_branches {`. + +In each loop, `branch` was a `&Vec`. After the replacement it is a `&&Vec`, so add a `*` dereference or use `branch` directly — Rust auto-derefs in most call positions so it should compile unchanged, but verify. + +- [ ] **Step 3: Do the same for the `WhereClause::OrJoin` arm** + +Find the `WhereClause::OrJoin { join_vars, branches }` arm (around line 1974). The loop is `for branch in branches` (around line 1995). Apply the same sorted_branches pattern. + +Insert immediately after `WhereClause::OrJoin { join_vars, branches } => {`: + +```rust + let sorted_oj_branches: Vec<&Vec>; + #[cfg(not(feature = "wasm"))] + { + let mut b: Vec<&Vec> = branches.iter().collect(); + b.sort_by_key(|br| optimizer::branch_cost(br)); + sorted_oj_branches = b; + } + #[cfg(feature = "wasm")] + { + sorted_oj_branches = branches.iter().collect(); + } +``` + +Replace `for branch in branches {` (around line 1995) with `for branch in &sorted_oj_branches {`. + +- [ ] **Step 4: Run full test suite** + +```bash +cargo test 2>&1 | tail -5 +``` + +Expected: all tests pass. + +- [ ] **Step 5: Clippy** + +```bash +cargo clippy -- -D warnings 2>&1 | grep -v "^$" | head -30 +``` + +Expected: no warnings. If clippy flags unused variable warnings for `sorted_branches` in a cfg branch, use `let _ =` or restructure so both branches use the same name. + +- [ ] **Step 6: Commit** + +```bash +git add src/query/datalog/executor.rs +git commit -m "feat(executor): sort or/or-join branches by cost ascending (#205) + +Infrastructure for issue #250 (or short-circuit). All branches still evaluated. +Gated on cfg(not(feature = \"wasm\"))." +``` + +--- + +### Task 6: Sort not-clauses in evaluator.rs (StratifiedEvaluator) + +**Files:** +- Modify: `src/query/datalog/evaluator.rs:787` + +The `not_join_clauses` collection ends at line 787 (`.collect();`). Insert sorting immediately after. + +- [ ] **Step 1: Insert not-clause sorting after line 787** + +In `src/query/datalog/evaluator.rs`, after the `.collect();` that closes `not_join_clauses`: + +```rust + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_clauses.sort_by_key(|body| crate::query::datalog::optimizer::branch_cost(body)); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_join_clauses + .sort_by_key(|(_, clauses)| crate::query::datalog::optimizer::branch_cost(clauses)); +``` + +Note: `evaluator.rs` uses the full path `crate::query::datalog::optimizer` rather than a short `optimizer` alias (verify the existing imports in the file before deciding whether to add a `use` line or use the full path). + +- [ ] **Step 2: Run full test suite** + +```bash +cargo test 2>&1 | tail -5 +``` + +Expected: all tests pass. + +- [ ] **Step 3: Run clippy** + +```bash +cargo clippy -- -D warnings 2>&1 | grep -v "^$" | head -30 +``` + +Expected: no warnings. + +- [ ] **Step 4: Commit** + +```bash +git add src/query/datalog/evaluator.rs +git commit -m "feat(evaluator): sort not/not-join clauses by cost in StratifiedEvaluator (#205) + +Consistent with executor.rs; cheap filters run first in mixed-rules loop. +Gated on cfg(not(feature = \"wasm\"))." +``` + +--- + +### Task 7: Full verification and PR + +**Files:** +- No new file changes + +- [ ] **Step 1: Run the complete test suite** + +```bash +cargo test 2>&1 | tail -10 +``` + +Expected: all tests pass (currently ~850+). + +- [ ] **Step 2: Run clippy clean** + +```bash +cargo clippy -- -D warnings 2>&1 | grep -E "^error" | wc -l +``` + +Expected: 0. + +- [ ] **Step 3: Run cargo fmt check** + +```bash +cargo fmt --check +``` + +Expected: no diff. If there is a diff, run `cargo fmt` and commit the style fix. + +- [ ] **Step 4: Verify WASM build compiles** + +```bash +cargo build --features wasm --target wasm32-unknown-unknown 2>&1 | tail -5 +``` + +Expected: compiles without errors. (If the `wasm32-unknown-unknown` target is not installed, run `rustup target add wasm32-unknown-unknown` first.) + +- [ ] **Step 5: Open the PR** + +```bash +gh pr create \ + --title "feat: cost-based not/or ordering (#205)" \ + --body "$(cat <<'EOF' +## Summary +- Adds `pattern_cost`, `branch_cost`, `clause_cost` to `optimizer.rs` (unconditional, all targets) +- Sorts `not`/`not-join` clauses cheapest-first in `execute_query`, `execute_query_with_rules`, and `StratifiedEvaluator` +- Sorts `or`/`or-join` branches cheapest-first in `apply_or_clauses` (infrastructure for #250) +- Sorting gated on `#[cfg(not(feature = "wasm"))]` — small datasets + determinism on WASM + +Closes #205 + +## Test plan +- [ ] All existing tests pass (`cargo test`) +- [ ] New unit tests for all cost tiers in `optimizer.rs` +- [ ] New integration test: not-clause ordering produces correct results +- [ ] `cargo clippy -- -D warnings` clean +- [ ] `cargo fmt --check` clean +- [ ] WASM build compiles + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +- [ ] **Step 6: Monitor CI until green** + +```bash +gh pr checks --watch +``` + +Expected: all checks green. Fix any failures before proceeding. diff --git a/docs/superpowers/plans/2026-05-16-simd-benchmarking.md b/docs/superpowers/plans/2026-05-16-simd-benchmarking.md new file mode 100644 index 000000000..67211469b --- /dev/null +++ b/docs/superpowers/plans/2026-05-16-simd-benchmarking.md @@ -0,0 +1,861 @@ +# SIMD Benchmarking Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add three Criterion benchmark groups comparing scalar vs explicit SIMD (via `wide`) for valid-time filter, tx-time as-of filter, and aggregation sum, then write a crossover analysis and recommendation. + +**Architecture:** All SIMD code lives in `benches/simd_helpers.rs` (a new file). Both scalar and SIMD benchmarks operate on synthetic `Vec`/`Vec` data — `Fact` internals are `pub(crate)` and inaccessible from bench code, so we benchmark the kernel loops directly. `wide` is added to the non-WASM `[dev-dependencies]` block only (zero binary size impact). A single `bench_simd` function registers all three groups. + +**Tech Stack:** Rust stable, `criterion 0.8`, `wide 0.7`, `cargo bench` + +--- + +## File Structure + +| File | Change | +|---|---| +| `Cargo.toml` | Add `wide = "0.7"` to non-WASM dev-deps | +| `benches/simd_helpers.rs` | New: `valid_time_filter_simd`, `as_of_filter_simd`, `sum_simd_i64` + unit tests | +| `benches/minigraf_bench.rs` | Add `mod simd_helpers;`, `bench_simd` function, register in `criterion_group!` | +| `docs/simd-analysis.md` | New: benchmark results table, crossover analysis, recommendation | + +--- + +### Task 1: Git worktree + +**Files:** None + +- [ ] **Step 1: Create worktree** + +```bash +git worktree add .worktrees/issue-229-simd-benchmarking -b feature/issue-229-simd-benchmarking +cd .worktrees/issue-229-simd-benchmarking +``` + +Expected: `.worktrees/issue-229-simd-benchmarking` created on new branch `feature/issue-229-simd-benchmarking`. + +--- + +### Task 2: Add `wide` to Cargo.toml + +**Files:** +- Modify: `Cargo.toml` + +`wide` must live in the same cfg-gated dev-dep block as `criterion` so it is not compiled for WASM targets. + +- [ ] **Step 1: Add `wide` to the non-WASM dev-dependencies block** + +Open `Cargo.toml`. Find the block: +```toml +[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] +tempfile = "3" +criterion = { version = "0.8", features = ["html_reports"] } +``` + +Add `wide` on the line after `criterion`: +```toml +[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] +tempfile = "3" +criterion = { version = "0.8", features = ["html_reports"] } +wide = "0.7" +``` + +- [ ] **Step 2: Verify the build** + +```bash +cargo check --benches 2>&1 | tail -5 +``` + +Expected: `Finished` — no errors. `wide` resolves successfully. + +- [ ] **Step 3: Commit** + +```bash +git add Cargo.toml Cargo.lock +git commit -m "chore: add wide 0.7 to non-WASM dev-dependencies (#229)" +``` + +--- + +### Task 3: Failing unit tests for SIMD helpers + +**Files:** +- Create: `benches/simd_helpers.rs` + +Write the file skeleton and unit tests only — the three functions are declared but bodies are `todo!()`. Tests verify that SIMD output matches scalar output, so they will fail at runtime (panic on `todo!()`). The file also needs allow attributes for clippy lints that fire in benchmark/helper code. + +- [ ] **Step 1: Create `benches/simd_helpers.rs` with stubs and tests** + +```rust +//! SIMD kernel benchmarks for issue #229. +//! +//! All three functions operate on pre-extracted numeric slices. +//! `Fact` internals are `pub(crate)` and unavailable from bench code; +//! benchmarks use synthetic data to isolate the hot loop. +//! +//! `wide` API notes (verify against docs.rs/wide/0.7): +//! - `i64x4::new([a, b, c, d])` — construct from array +//! - `i64x4::splat(v)` — broadcast scalar to all lanes +//! - `a.cmp_le(b)` — returns i64x4 with all-bits-1 where ≤, 0 elsewhere +//! - `a.cmp_gt(b)` — returns i64x4 with all-bits-1 where >, 0 elsewhere +//! - `let arr: [i64; 4] = simd_val.into()` — extract lanes +//! - `u64x2` is the widest unsigned-64 type in wide 0.7 (no u64x4) + +#![allow(clippy::cast_possible_truncation)] // synthetic bench data: N ≤ 1M fits in i64 +#![allow(clippy::cast_sign_loss)] // tx_count cast: monotonic counter, always positive + +use wide::{i64x4, u64x2}; + +/// Count facts where `valid_from[i] <= ts && ts < valid_to[i]`. +/// +/// Processes 4 facts per SIMD step using `i64x4`. Scalar tail handles remainder. +/// Both slices must be the same length. +pub fn valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> usize { + todo!("implement in Task 4") +} + +/// Count facts where `tx_counts[i] <= threshold`. +/// +/// Processes 2 facts per SIMD step using `u64x2` (widest unsigned-64 in wide 0.7). +/// Scalar tail handles remainder. +pub fn as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> usize { + todo!("implement in Task 4") +} + +/// Sum all values using `i64x4` horizontal reduction. +/// +/// Accumulates 4 lanes in parallel; reduces to scalar at the end. +/// Wrapping arithmetic matches Rust's default release-mode overflow behaviour. +pub fn sum_simd_i64(values: &[i64]) -> i64 { + todo!("implement in Task 4") +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── valid_time_filter_simd ──────────────────────────────────────────────── + + #[test] + fn test_valid_time_filter_matches_scalar_partial() { + // 6 facts (non-multiple of 4 to exercise tail path) + // valid window: [i, i + 3]; ts = 2; facts 0,1,2 pass, facts 3,4,5 fail + let valid_from: Vec = (0_i64..6).collect(); + let valid_to: Vec = valid_from.iter().map(|&vf| vf + 3).collect(); + let ts = 2_i64; + + let scalar = valid_from + .iter() + .zip(valid_to.iter()) + .filter(|(&vf, &vt)| vf <= ts && ts < vt) + .count(); + + assert_eq!(valid_time_filter_simd(&valid_from, &valid_to, ts), scalar); + } + + #[test] + fn test_valid_time_filter_matches_scalar_exact_chunk() { + // 8 facts (exactly two SIMD chunks) + let valid_from: Vec = (0_i64..8).collect(); + let valid_to: Vec = valid_from.iter().map(|&vf| vf + 4).collect(); + let ts = 3_i64; + + let scalar = valid_from + .iter() + .zip(valid_to.iter()) + .filter(|(&vf, &vt)| vf <= ts && ts < vt) + .count(); + + assert_eq!(valid_time_filter_simd(&valid_from, &valid_to, ts), scalar); + } + + #[test] + fn test_valid_time_filter_empty() { + assert_eq!(valid_time_filter_simd(&[], &[], 0), 0); + } + + // ── as_of_filter_simd ───────────────────────────────────────────────────── + + #[test] + fn test_as_of_filter_matches_scalar_partial() { + // 9 tx_counts (non-multiple of 2 to exercise tail) + let tx_counts: Vec = (1..=9).collect(); + let threshold = 5_u64; + + let scalar = tx_counts.iter().filter(|&&tc| tc <= threshold).count(); + + assert_eq!(as_of_filter_simd(&tx_counts, threshold), scalar); + } + + #[test] + fn test_as_of_filter_matches_scalar_exact_chunk() { + // 10 tx_counts (exactly five 2-wide chunks) + let tx_counts: Vec = (1..=10).collect(); + let threshold = 7_u64; + + let scalar = tx_counts.iter().filter(|&&tc| tc <= threshold).count(); + + assert_eq!(as_of_filter_simd(&tx_counts, threshold), scalar); + } + + #[test] + fn test_as_of_filter_empty() { + assert_eq!(as_of_filter_simd(&[], 100), 0); + } + + // ── sum_simd_i64 ────────────────────────────────────────────────────────── + + #[test] + fn test_sum_simd_matches_scalar_partial() { + // 9 values (non-multiple of 4 to exercise tail) + let values: Vec = (1_i64..=9).collect(); + let scalar: i64 = values.iter().sum(); + + assert_eq!(sum_simd_i64(&values), scalar); + } + + #[test] + fn test_sum_simd_matches_scalar_exact_chunk() { + // 8 values (exactly two 4-wide chunks) + let values: Vec = (1_i64..=8).collect(); + let scalar: i64 = values.iter().sum(); + + assert_eq!(sum_simd_i64(&values), scalar); + } + + #[test] + fn test_sum_simd_empty() { + assert_eq!(sum_simd_i64(&[]), 0); + } + + #[test] + fn test_sum_simd_negatives() { + let values = vec![-3_i64, -2, -1, 0, 1, 2, 3]; + let scalar: i64 = values.iter().sum(); + + assert_eq!(sum_simd_i64(&values), scalar); + } +} +``` + +- [ ] **Step 2: Run the tests and confirm they panic on `todo!()`** + +```bash +cargo test -p minigraf simd_helpers 2>&1 | head -20 +``` + +Expected: tests panic with `not yet implemented: implement in Task 4`. This is the TDD red phase. + +--- + +### Task 4: Implement SIMD helper functions + +**Files:** +- Modify: `benches/simd_helpers.rs` + +Replace each `todo!()` body with the real implementation. Use `chunks_exact` for SIMD iteration (avoids `clippy::indexing_slicing`). Use `get(rem..).unwrap_or(&[])` for the scalar tail (avoids `clippy::indexing_slicing` and `clippy::unwrap_used`). + +Before writing, verify the exact `wide 0.7` API at `https://docs.rs/wide/0.7` — specifically: +- Is `cmp_le` the correct method name for ≤ comparison on `i64x4`? +- Does `cmp_gt` exist for `i64x4`? +- Does `u64x2` support `cmp_le`? +- Can you extract lanes with `let arr: [i64; 4] = v.into()` or is it `v.to_array()`? + +- [ ] **Step 1: Implement `valid_time_filter_simd`** + +Replace the `todo!()` in `valid_time_filter_simd` with: + +```rust +pub fn valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> usize { + let ts_v = i64x4::splat(ts); + let mut count = 0usize; + + for (vf_chunk, vt_chunk) in valid_from + .chunks_exact(4) + .zip(valid_to.chunks_exact(4)) + { + // Slice patterns: chunks_exact guarantees len == 4, so the else branch + // is unreachable (unreachable = "allow" in workspace lints) + let [vf0, vf1, vf2, vf3] = *vf_chunk else { unreachable!() }; + let [vt0, vt1, vt2, vt3] = *vt_chunk else { unreachable!() }; + + let vf = i64x4::new([vf0, vf1, vf2, vf3]); + let vt = i64x4::new([vt0, vt1, vt2, vt3]); + + // vf <= ts AND ts < vt (i.e., vt > ts) + // Note: verify method names against docs.rs/wide/0.7 before finalising + let lo = vf.cmp_le(ts_v); + let hi = vt.cmp_gt(ts_v); + let mask = lo & hi; + + // All-bits-1 = true lane (i64 = -1); 0 = false lane + let arr: [i64; 4] = mask.into(); + count += arr.iter().filter(|&&x| x != 0).count(); + } + + // Scalar tail for remaining facts (0–3 elements) + let rem = (valid_from.len() / 4) * 4; + for (&vf, &vt) in valid_from + .get(rem..) + .unwrap_or(&[]) + .iter() + .zip(valid_to.get(rem..).unwrap_or(&[]).iter()) + { + if vf <= ts && ts < vt { + count += 1; + } + } + + count +} +``` + +- [ ] **Step 2: Implement `as_of_filter_simd`** + +Replace the `todo!()` in `as_of_filter_simd` with: + +```rust +pub fn as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> usize { + let thr_v = u64x2::splat(threshold); + let mut count = 0usize; + + for chunk in tx_counts.chunks_exact(2) { + // Slice pattern: chunks_exact guarantees len == 2 + let [a, b] = *chunk else { unreachable!() }; + let v = u64x2::new([a, b]); + + // Note: verify u64x2::cmp_le exists in wide 0.7; if not, use manual comparison: + // let mask = u64x2::new([(if a <= threshold { u64::MAX } else { 0 }), (if b <= threshold { u64::MAX } else { 0 })]); + let mask = v.cmp_le(thr_v); + let arr: [u64; 2] = mask.into(); + count += arr.iter().filter(|&&x| x != 0).count(); + } + + let rem = (tx_counts.len() / 2) * 2; + for &tc in tx_counts.get(rem..).unwrap_or(&[]) { + if tc <= threshold { + count += 1; + } + } + + count +} +``` + +**Note:** `u64x2` is only 2-wide (wide 0.7 has no `u64x4`). If `u64x2::cmp_le` does not exist, replace the SIMD comparison with the manual mask construction shown in the comment above. + +- [ ] **Step 3: Implement `sum_simd_i64`** + +Replace the `todo!()` in `sum_simd_i64` with: + +```rust +pub fn sum_simd_i64(values: &[i64]) -> i64 { + let mut acc = i64x4::splat(0_i64); + + for chunk in values.chunks_exact(4) { + let [a, b, c, d] = *chunk else { unreachable!() }; + acc += i64x4::new([a, b, c, d]); + } + + // Horizontal reduction: sum the 4 lanes + let lanes: [i64; 4] = acc.into(); + let mut sum: i64 = lanes.iter().sum(); + + // Scalar tail + let rem = (values.len() / 4) * 4; + for &v in values.get(rem..).unwrap_or(&[]) { + sum = sum.wrapping_add(v); + } + + sum +} +``` + +- [ ] **Step 4: Run the tests — verify all pass** + +```bash +cargo test -p minigraf simd_helpers 2>&1 | tail -15 +``` + +Expected: all 10 tests PASS. If the `wide` API differs from the pseudocode (e.g., method not found), read `cargo test` errors, check `https://docs.rs/wide/0.7`, adapt accordingly. + +- [ ] **Step 5: Run clippy to confirm no new lint errors** + +```bash +cargo clippy -- -D warnings 2>&1 | grep "^error" | wc -l +``` + +Expected: 0. + +- [ ] **Step 6: Commit** + +```bash +git add benches/simd_helpers.rs +git commit -m "feat(bench): add SIMD helper kernels for temporal filter and sum (#229)" +``` + +--- + +### Task 5: Add `bench_simd` to the benchmark file + +**Files:** +- Modify: `benches/minigraf_bench.rs` + +Add `mod simd_helpers;` at the top, add the `bench_simd` function, and register it in `criterion_group!`. + +- [ ] **Step 1: Add `mod simd_helpers;` after the existing `mod helpers;` line** + +Open `benches/minigraf_bench.rs`. Line 20 is `mod helpers;`. Insert immediately after it: + +```rust +#[cfg(not(target_arch = "wasm32"))] +mod simd_helpers; +``` + +The `cfg` gate is needed because `wide` is only in the non-WASM dev-deps. + +- [ ] **Step 2: Add `use criterion::black_box;` to the imports** + +Find the existing use statement: +```rust +use criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}; +``` + +Replace with: +```rust +use criterion::{BenchmarkId, Criterion, Throughput, black_box, criterion_group, criterion_main}; +``` + +- [ ] **Step 3: Add the `bench_simd` function** + +Add the following before the `criterion_group!` macro (after `bench_predicate_pushdown`): + +```rust +// ── SIMD kernel benchmarks (Issue #229) ────────────────────────────────────── +// +// Both scalar and SIMD variants operate on synthetic Vec/Vec data. +// Fact internals (valid_from, valid_to, tx_count) are pub(crate) and unavailable +// from bench code; benchmarks isolate the hot loop rather than the full query path. +// Cross-reference the existing time_travel/ and aggregation/ groups for full-query costs. + +#[cfg(not(target_arch = "wasm32"))] +fn bench_simd(c: &mut Criterion) { + const SCALES: &[(&str, usize)] = &[ + ("100", 100), + ("1k", 1_000), + ("10k", 10_000), + ("100k", 100_000), + ("1m", 1_000_000), + ]; + + // ── simd_temporal: valid-time range filter ────────────────────────────── + // + // Synthetic data: valid_from[i] = i, valid_to[i] = i + n/2. + // ts = n/4 → ~50% of facts are valid at ts (those with valid_from ≤ n/4 < valid_to). + { + let mut group = c.benchmark_group("simd_temporal"); + group.sample_size(10); + + for &(label, n) in SCALES { + let n_i64 = i64::try_from(n).unwrap_or(i64::MAX); + let valid_from: Vec = (0_i64..n_i64).collect(); + let valid_to: Vec = valid_from.iter().map(|&vf| vf + n_i64 / 2).collect(); + let ts = n_i64 / 4; + + group.bench_with_input(BenchmarkId::new("scalar", label), &n, |b, _| { + b.iter(|| { + valid_from + .iter() + .zip(valid_to.iter()) + .filter(|(&vf, &vt)| vf <= black_box(ts) && black_box(ts) < vt) + .count() + }) + }); + + group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { + b.iter(|| { + simd_helpers::valid_time_filter_simd( + black_box(&valid_from), + black_box(&valid_to), + black_box(ts), + ) + }) + }); + } + + group.finish(); + } + + // ── simd_as_of: tx-time as-of filter ─────────────────────────────────── + // + // Synthetic data: tx_counts = 1..=n (monotonic counter). + // threshold = n/2 → 50% of facts pass the filter. + { + let mut group = c.benchmark_group("simd_as_of"); + group.sample_size(10); + + for &(label, n) in SCALES { + let n_u64 = u64::try_from(n).unwrap_or(u64::MAX); + let tx_counts: Vec = (1..=n_u64).collect(); + let threshold = n_u64 / 2; + + group.bench_with_input(BenchmarkId::new("scalar", label), &n, |b, _| { + b.iter(|| { + tx_counts + .iter() + .filter(|&&tc| tc <= black_box(threshold)) + .count() + }) + }); + + group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { + b.iter(|| { + simd_helpers::as_of_filter_simd( + black_box(&tx_counts), + black_box(threshold), + ) + }) + }); + } + + group.finish(); + } + + // ── simd_aggregate: i64 horizontal sum ───────────────────────────────── + // + // Synthetic data: values = 0..n as i64. + // Measures horizontal reduction performance. + { + let mut group = c.benchmark_group("simd_aggregate"); + group.sample_size(10); + + for &(label, n) in SCALES { + let n_i64 = i64::try_from(n).unwrap_or(i64::MAX); + let values: Vec = (0_i64..n_i64).collect(); + + group.bench_with_input(BenchmarkId::new("scalar", label), &n, |b, _| { + b.iter(|| black_box(&values).iter().copied().sum::()) + }); + + group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { + b.iter(|| simd_helpers::sum_simd_i64(black_box(&values))) + }); + } + + group.finish(); + } +} +``` + +- [ ] **Step 4: Register `bench_simd` in `criterion_group!`** + +Find the `criterion_group!` macro at the end of the file. Add `bench_simd` after `bench_predicate_pushdown`: + +```rust +criterion_group!( + benches, + bench_insert, + bench_insert_file, + bench_query, + bench_time_travel, + bench_recursion, + bench_negation, + bench_disjunction, + bench_aggregation, + bench_expr, + bench_window, + bench_temporal_metadata, + bench_udf, + bench_aggregation_extras, + bench_query_extras, + bench_open, + bench_checkpoint, + bench_concurrent, + bench_concurrent_file, + bench_concurrent_btree_scan, + bench_prepared, + bench_retract, + bench_btree_lookup, + bench_predicate_pushdown, + bench_simd, // Issue #229 +); +``` + +**Note:** `bench_simd` is defined under `#[cfg(not(target_arch = "wasm32"))]`. If the compiler complains about the `bench_simd` reference in `criterion_group!` being absent on WASM, wrap the entire criterion_group!/criterion_main! with the same cfg — but this is unlikely since `criterion` itself is already WASM-gated. + +- [ ] **Step 5: Verify the bench binary compiles** + +```bash +cargo build --benches 2>&1 | tail -5 +``` + +Expected: `Finished` with no errors. + +- [ ] **Step 6: Run just the SIMD benchmarks to verify they execute** + +```bash +cargo bench -- simd 2>&1 | grep -E "simd_|Benchmarking|error" | head -30 +``` + +Expected: Groups `simd_temporal`, `simd_as_of`, `simd_aggregate` all run and report timing numbers. No errors. + +- [ ] **Step 7: Run the full test suite to check for regressions** + +```bash +cargo test 2>&1 | tail -5 +``` + +Expected: all tests pass. + +- [ ] **Step 8: Commit** + +```bash +git add benches/minigraf_bench.rs +git commit -m "feat(bench): add simd_temporal, simd_as_of, simd_aggregate benchmark groups (#229)" +``` + +--- + +### Task 6: Run benchmarks and write `docs/simd-analysis.md` + +**Files:** +- Create: `docs/simd-analysis.md` + +Run the full benchmark suite for the new SIMD groups, collect results, and write the analysis document. This is the deliverable that closes issue #229. + +- [ ] **Step 1: Run all three SIMD benchmark groups** + +```bash +cargo bench -- simd 2>&1 | tee /tmp/simd_bench_results.txt +``` + +This will take several minutes (5 dataset sizes × 2 variants × 3 groups × ~10 samples each). Let it complete fully. + +- [ ] **Step 2: Also collect the existing time_travel results for comparison context** + +```bash +cargo bench -- time_travel 2>&1 | tee /tmp/time_travel_results.txt +``` + +These give the full-query costs for context. + +- [ ] **Step 3: Extract the mean ns/iter values from the output** + +From `/tmp/simd_bench_results.txt`, find lines like: +``` +simd_temporal/scalar/100 time: [XXX ns XXX ns XXX ns] +simd_temporal/simd/100 time: [XXX ns XXX ns XXX ns] +``` + +Record the middle (mean) value for each. Build a table with columns: group, variant, size, mean_ns. + +- [ ] **Step 4: Write `docs/simd-analysis.md`** + +Use the template below, filling in actual numbers from Step 3: + +```markdown +# SIMD Benchmarking Analysis (Issue #229) + +**Date:** 2026-05-16 +**Environment:** [fill in: CPU model, OS, Rust toolchain, wide version] +**Branch:** feature/issue-229-simd-benchmarking + +--- + +## Method + +Both scalar and SIMD benchmarks operate on synthetic numeric slices (not live DB facts — `Fact` internals are `pub(crate)`). The benchmarks isolate the hot kernel loop; extraction overhead is identical for both paths (none). For full-query costs, see the `time_travel/` group results below. + +--- + +## Results: simd_temporal (valid-time range filter) + +`valid_from[i] <= ts && ts < valid_to[i]` — ~50% selectivity + +| Size | Scalar (ns) | SIMD (ns) | Speedup | +|------|------------|-----------|---------| +| 100 | XXX | XXX | X.Xx | +| 1K | XXX | XXX | X.Xx | +| 10K | XXX | XXX | X.Xx | +| 100K | XXX | XXX | X.Xx | +| 1M | XXX | XXX | X.Xx | + +**Crossover point:** [first size where SIMD is faster, or "none observed at any tested size"] + +--- + +## Results: simd_as_of (tx-time as-of filter) + +`tx_count[i] <= threshold` — 50% selectivity. Note: uses `u64x2` (2-wide) — wide 0.7 has no `u64x4`. + +| Size | Scalar (ns) | SIMD (ns) | Speedup | +|------|------------|-----------|---------| +| 100 | XXX | XXX | X.Xx | +| 1K | XXX | XXX | X.Xx | +| 10K | XXX | XXX | X.Xx | +| 100K | XXX | XXX | X.Xx | +| 1M | XXX | XXX | X.Xx | + +**Crossover point:** [first size where SIMD is faster, or "none observed"] + +--- + +## Results: simd_aggregate (i64 horizontal sum) + +Sum of N i64 values via `i64x4` 4-wide reduction. + +| Size | Scalar (ns) | SIMD (ns) | Speedup | +|------|------------|-----------|---------| +| 100 | XXX | XXX | X.Xx | +| 1K | XXX | XXX | X.Xx | +| 10K | XXX | XXX | X.Xx | +| 100K | XXX | XXX | X.Xx | +| 1M | XXX | XXX | X.Xx | + +**Crossover point:** [first size where SIMD is faster, or "none observed"] + +--- + +## Post-#208 Residual Analysis + +After B+Tree selective lookup (#208), queries with bound entity or entity+attribute patterns +route through index-backed lookups rather than full fact scans. Typical residual fact counts +reaching the temporal filter after selective lookup (from `btree_lookup/entity_point` data): + +- Entity-point query on 1K facts: ~1 fact residual (index lookup, no filter needed) +- Entity-point query on 100K facts: ~1 fact residual +- Full-attribute scan on 100K facts: [N facts from btree_lookup/attribute_scan] + +For unbound queries (no selective lookup), all N facts reach the temporal filter. +SIMD gains apply to this case. + +--- + +## Full-Query Context + +From `time_travel/as_of_counter` and `time_travel/valid_at` (existing benchmarks): + +| Size | as_of_counter (ns) | valid_at (ns) | +|------|-------------------|---------------| +| 1K | XXX | XXX | +| 10K | XXX | XXX | +| 100K | XXX | XXX | +| 1M | XXX | XXX | + +The full query cost includes: fact loading, temporal filtering, pattern matching, result projection. +If temporal filtering is Z% of query cost, SIMD speedup of Sx translates to ~Z*(S-1)/S % query improvement. + +--- + +## Recommendation + +[Fill in ONE of the following:] + +**Integrate:** SIMD wins at sizes ≥ [N] which covers typical unbound-query workloads. Recommend +opening a follow-up issue to promote `simd_helpers` functions into production code paths in +`src/graph/storage.rs` (temporal filters) and `src/query/datalog/functions.rs` (sum aggregation). +Prerequisite: a struct-of-arrays storage layout change would allow zero-copy slice extraction. + +**Skip:** SIMD is slower or provides < 10% gain at all practical embedded sizes (≤ 100K facts). +LLVM autovectorization (enabled by `lto = true` and `codegen-units = 1` in release profile) already +captures most of the available gain. Recommend closing #229 without further SIMD work. + +**Revisit with SoA layout:** SIMD wins at ≥ [N] facts but extraction overhead (not measured here — +facts are struct-of-arrays-inaccessible from bench code) would dominate in production. A +struct-of-arrays storage layout change is a prerequisite for SIMD to pay off. Recommend opening +a separate issue to evaluate SoA storage before revisiting SIMD. +``` + +- [ ] **Step 5: Commit the analysis document** + +```bash +git add docs/simd-analysis.md +git commit -m "docs: SIMD benchmarking analysis and recommendation (#229)" +``` + +--- + +### Task 7: Full verification and PR + +**Files:** None (no new code changes unless a check fails) + +- [ ] **Step 1: Full test suite** + +```bash +cargo test 2>&1 | tail -5 +``` + +Expected: all tests pass. + +- [ ] **Step 2: Clippy clean** + +```bash +cargo clippy -- -D warnings 2>&1 | grep "^error" | wc -l +``` + +Expected: 0. + +- [ ] **Step 3: Format check** + +```bash +cargo fmt --check +``` + +Expected: no output. If there is a diff, run `cargo fmt` and commit `style: cargo fmt (#229)`. + +- [ ] **Step 4: Verify WASM build is unaffected** + +```bash +cargo build --features wasm --target wasm32-unknown-unknown 2>&1 | grep "^error" | wc -l +``` + +Expected: 0. (`wide` is in the non-WASM dev-deps only; WASM build should not even see it.) + +- [ ] **Step 5: Check commit log** + +```bash +git log main..HEAD --oneline +``` + +Expected: 5 commits (worktree excluded): +``` +docs: SIMD benchmarking analysis and recommendation (#229) +feat(bench): add simd_temporal, simd_as_of, simd_aggregate benchmark groups (#229) +feat(bench): add SIMD helper kernels for temporal filter and sum (#229) +chore: add wide 0.7 to non-WASM dev-dependencies (#229) +``` + +- [ ] **Step 6: Open PR** + +```bash +gh pr create \ + --title "feat(bench): SIMD benchmarking and crossover analysis (#229)" \ + --body "$(cat <<'EOF' +## Summary +- Adds `wide 0.7` as a non-WASM dev-dependency (zero production binary size impact) +- New `benches/simd_helpers.rs`: `valid_time_filter_simd`, `as_of_filter_simd`, `sum_simd_i64` with 10 unit tests +- Three new Criterion benchmark groups: `simd_temporal`, `simd_as_of`, `simd_aggregate` — each at 5 dataset sizes (100 → 1M), scalar vs SIMD side-by-side +- `docs/simd-analysis.md`: crossover findings and final recommendation + +Closes #229 + +## Test plan +- [ ] All existing tests pass (`cargo test`) +- [ ] 10 new unit tests for SIMD helpers (scalar equivalence) +- [ ] SIMD benchmark groups compile and run (`cargo bench -- simd`) +- [ ] `cargo clippy -- -D warnings` clean +- [ ] `cargo fmt --check` clean +- [ ] WASM build unaffected + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +- [ ] **Step 7: Monitor CI until green** + +```bash +gh pr checks --watch +``` + +Expected: all checks green. From bdde26881c185b3e7132ee8c9dfd2ac5816aae96 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 13:11:29 +0530 Subject: [PATCH 07/13] chore: add wide 0.7 to non-WASM dev-dependencies (#229) --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index c559f5856..fe4cb67e4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,6 +73,7 @@ pest_derive = "2" [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] tempfile = "3" criterion = { version = "0.8", features = ["html_reports"] } +wide = "0.7" [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dev-dependencies] wasm-bindgen-test = "0.3" From 182b7f94064cfe8b2555af37f407a68024c87a71 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 14:26:52 +0530 Subject: [PATCH 08/13] test(bench): add failing SIMD helper unit tests - TDD red phase (#229) --- benches/simd_helpers.rs | 149 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 benches/simd_helpers.rs diff --git a/benches/simd_helpers.rs b/benches/simd_helpers.rs new file mode 100644 index 000000000..64e6f2bfa --- /dev/null +++ b/benches/simd_helpers.rs @@ -0,0 +1,149 @@ +//! SIMD kernel benchmarks for issue #229. +//! +//! All three functions operate on pre-extracted numeric slices. +//! `Fact` internals are `pub(crate)` and unavailable from bench code; +//! benchmarks use synthetic data to isolate the hot loop. +//! +//! `wide` API notes (verify against docs.rs/wide/0.7): +//! - `i64x4::new([a, b, c, d])` — construct from array +//! - `i64x4::splat(v)` — broadcast scalar to all lanes +//! - `a.cmp_le(b)` — returns i64x4 with all-bits-1 where ≤, 0 elsewhere +//! - `a.cmp_gt(b)` — returns i64x4 with all-bits-1 where >, 0 elsewhere +//! - `let arr: [i64; 4] = simd_val.into()` — extract lanes +//! - `u64x2` is the widest unsigned-64 type in wide 0.7 (no u64x4) + +#![allow(clippy::cast_possible_truncation)] // synthetic bench data: N ≤ 1M fits in i64 +#![allow(clippy::cast_sign_loss)] // tx_count cast: monotonic counter, always positive + +use wide::{i64x4, u64x2}; + +/// Count facts where `valid_from[i] <= ts && ts < valid_to[i]`. +/// +/// Processes 4 facts per SIMD step using `i64x4`. Scalar tail handles remainder. +/// Both slices must be the same length. +pub fn valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> usize { + todo!("implement in Task 4") +} + +/// Count facts where `tx_counts[i] <= threshold`. +/// +/// Processes 2 facts per SIMD step using `u64x2` (widest unsigned-64 in wide 0.7). +/// Scalar tail handles remainder. +pub fn as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> usize { + todo!("implement in Task 4") +} + +/// Sum all values using `i64x4` horizontal reduction. +/// +/// Accumulates 4 lanes in parallel; reduces to scalar at the end. +/// Wrapping arithmetic matches Rust's default release-mode overflow behaviour. +pub fn sum_simd_i64(values: &[i64]) -> i64 { + todo!("implement in Task 4") +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── valid_time_filter_simd ──────────────────────────────────────────────── + + #[test] + fn test_valid_time_filter_matches_scalar_partial() { + // 6 facts (non-multiple of 4 to exercise tail path) + // valid window: [i, i + 3]; ts = 2; facts 0,1,2 pass, facts 3,4,5 fail + let valid_from: Vec = (0_i64..6).collect(); + let valid_to: Vec = valid_from.iter().map(|&vf| vf + 3).collect(); + let ts = 2_i64; + + let scalar = valid_from + .iter() + .zip(valid_to.iter()) + .filter(|&(&vf, &vt)| vf <= ts && ts < vt) + .count(); + + assert_eq!(valid_time_filter_simd(&valid_from, &valid_to, ts), scalar); + } + + #[test] + fn test_valid_time_filter_matches_scalar_exact_chunk() { + // 8 facts (exactly two SIMD chunks) + let valid_from: Vec = (0_i64..8).collect(); + let valid_to: Vec = valid_from.iter().map(|&vf| vf + 4).collect(); + let ts = 3_i64; + + let scalar = valid_from + .iter() + .zip(valid_to.iter()) + .filter(|&(&vf, &vt)| vf <= ts && ts < vt) + .count(); + + assert_eq!(valid_time_filter_simd(&valid_from, &valid_to, ts), scalar); + } + + #[test] + fn test_valid_time_filter_empty() { + assert_eq!(valid_time_filter_simd(&[], &[], 0), 0); + } + + // ── as_of_filter_simd ───────────────────────────────────────────────────── + + #[test] + fn test_as_of_filter_matches_scalar_partial() { + // 9 tx_counts (non-multiple of 2 to exercise tail) + let tx_counts: Vec = (1..=9).collect(); + let threshold = 5_u64; + + let scalar = tx_counts.iter().filter(|&&tc| tc <= threshold).count(); + + assert_eq!(as_of_filter_simd(&tx_counts, threshold), scalar); + } + + #[test] + fn test_as_of_filter_matches_scalar_exact_chunk() { + // 10 tx_counts (exactly five 2-wide chunks) + let tx_counts: Vec = (1..=10).collect(); + let threshold = 7_u64; + + let scalar = tx_counts.iter().filter(|&&tc| tc <= threshold).count(); + + assert_eq!(as_of_filter_simd(&tx_counts, threshold), scalar); + } + + #[test] + fn test_as_of_filter_empty() { + assert_eq!(as_of_filter_simd(&[], 100), 0); + } + + // ── sum_simd_i64 ────────────────────────────────────────────────────────── + + #[test] + fn test_sum_simd_matches_scalar_partial() { + // 9 values (non-multiple of 4 to exercise tail) + let values: Vec = (1_i64..=9).collect(); + let scalar: i64 = values.iter().sum(); + + assert_eq!(sum_simd_i64(&values), scalar); + } + + #[test] + fn test_sum_simd_matches_scalar_exact_chunk() { + // 8 values (exactly two 4-wide chunks) + let values: Vec = (1_i64..=8).collect(); + let scalar: i64 = values.iter().sum(); + + assert_eq!(sum_simd_i64(&values), scalar); + } + + #[test] + fn test_sum_simd_empty() { + assert_eq!(sum_simd_i64(&[]), 0); + } + + #[test] + fn test_sum_simd_negatives() { + let values = vec![-3_i64, -2, -1, 0, 1, 2, 3]; + let scalar: i64 = values.iter().sum(); + + assert_eq!(sum_simd_i64(&values), scalar); + } +} From e37511561d8bd4ea2fd03154ee62c11cb53f2dc7 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 14:32:24 +0530 Subject: [PATCH 09/13] feat(bench): add SIMD helper kernels for temporal filter and sum (#229) Replace todo!() stubs with wide 0.7 implementations: valid_time_filter_simd (i64x4 with CmpGt/Not/move_mask), as_of_filter_simd (u64x4), and sum_simd_i64 (i64x4 accumulator). All 10 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 --- benches/simd_helpers.rs | 95 +++++++++++++++++++++++++++++++++++------ 1 file changed, 83 insertions(+), 12 deletions(-) diff --git a/benches/simd_helpers.rs b/benches/simd_helpers.rs index 64e6f2bfa..4af0e8c18 100644 --- a/benches/simd_helpers.rs +++ b/benches/simd_helpers.rs @@ -4,33 +4,89 @@ //! `Fact` internals are `pub(crate)` and unavailable from bench code; //! benchmarks use synthetic data to isolate the hot loop. //! -//! `wide` API notes (verify against docs.rs/wide/0.7): +//! `wide` API notes (verified against wide 0.7.33 source): //! - `i64x4::new([a, b, c, d])` — construct from array //! - `i64x4::splat(v)` — broadcast scalar to all lanes -//! - `a.cmp_le(b)` — returns i64x4 with all-bits-1 where ≤, 0 elsewhere -//! - `a.cmp_gt(b)` — returns i64x4 with all-bits-1 where >, 0 elsewhere -//! - `let arr: [i64; 4] = simd_val.into()` — extract lanes -//! - `u64x2` is the widest unsigned-64 type in wide 0.7 (no u64x4) +//! - `a.cmp_gt(b)` — i64x4 mask: all-bits-1 (= -1) where a > b, 0 elsewhere +//! - `a.cmp_lt(b)` — i64x4 mask: all-bits-1 where a < b, 0 elsewhere +//! - `!mask` — bitwise NOT (flips -1↔0) +//! - `a.move_mask()` — i32 bitmask of sign bits; non-zero lane = matched +//! - `a.to_array()` — extract [i64; 4] lanes +//! - `u64x4` (not u64x2) is the 4-wide unsigned-64 type; no move_mask, use to_array #![allow(clippy::cast_possible_truncation)] // synthetic bench data: N ≤ 1M fits in i64 #![allow(clippy::cast_sign_loss)] // tx_count cast: monotonic counter, always positive -use wide::{i64x4, u64x2}; +use wide::{i64x4, u64x4, CmpGt}; /// Count facts where `valid_from[i] <= ts && ts < valid_to[i]`. /// /// Processes 4 facts per SIMD step using `i64x4`. Scalar tail handles remainder. /// Both slices must be the same length. pub fn valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> usize { - todo!("implement in Task 4") + let ts_v = i64x4::splat(ts); + let mut count = 0usize; + + for (vf_chunk, vt_chunk) in valid_from.chunks_exact(4).zip(valid_to.chunks_exact(4)) { + let [vf0, vf1, vf2, vf3] = *vf_chunk else { unreachable!() }; + let [vt0, vt1, vt2, vt3] = *vt_chunk else { unreachable!() }; + + let vf = i64x4::new([vf0, vf1, vf2, vf3]); + let vt = i64x4::new([vt0, vt1, vt2, vt3]); + + // vf <= ts ≡ NOT (vf > ts) + let lo = !vf.cmp_gt(ts_v); + // ts < vt ≡ vt > ts + let hi = vt.cmp_gt(ts_v); + let mask = lo & hi; + + // move_mask sets bit i if lane i has its sign bit set (i.e. value is -1) + count += mask.move_mask().count_ones() as usize; + } + + let rem = (valid_from.len() / 4) * 4; + for (&vf, &vt) in valid_from + .get(rem..) + .unwrap_or(&[]) + .iter() + .zip(valid_to.get(rem..).unwrap_or(&[]).iter()) + { + if vf <= ts && ts < vt { + count += 1; + } + } + + count } /// Count facts where `tx_counts[i] <= threshold`. /// -/// Processes 2 facts per SIMD step using `u64x2` (widest unsigned-64 in wide 0.7). +/// Processes 4 facts per SIMD step using `u64x4`. /// Scalar tail handles remainder. pub fn as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> usize { - todo!("implement in Task 4") + let thr_v = u64x4::splat(threshold); + let mut count = 0usize; + + for chunk in tx_counts.chunks_exact(4) { + let [a, b, c, d] = *chunk else { unreachable!() }; + let v = u64x4::new([a, b, c, d]); + + // tc <= threshold ≡ NOT (tc > threshold) + // cmp_gt gives u64::MAX where true, 0 where false + // !mask gives u64::MAX where tc <= threshold, 0 elsewhere + let mask = !v.cmp_gt(thr_v); + let arr: [u64; 4] = mask.to_array(); + count += arr.iter().filter(|&&x| x != 0).count(); + } + + let rem = (tx_counts.len() / 4) * 4; + for &tc in tx_counts.get(rem..).unwrap_or(&[]) { + if tc <= threshold { + count += 1; + } + } + + count } /// Sum all values using `i64x4` horizontal reduction. @@ -38,7 +94,22 @@ pub fn as_of_filter_simd(tx_counts: &[u64], threshold: u64) -> usize { /// Accumulates 4 lanes in parallel; reduces to scalar at the end. /// Wrapping arithmetic matches Rust's default release-mode overflow behaviour. pub fn sum_simd_i64(values: &[i64]) -> i64 { - todo!("implement in Task 4") + let mut acc = i64x4::splat(0_i64); + + for chunk in values.chunks_exact(4) { + let [a, b, c, d] = *chunk else { unreachable!() }; + acc = acc + i64x4::new([a, b, c, d]); + } + + let lanes: [i64; 4] = acc.to_array(); + let mut sum: i64 = lanes.iter().sum(); + + let rem = (values.len() / 4) * 4; + for &v in values.get(rem..).unwrap_or(&[]) { + sum = sum.wrapping_add(v); + } + + sum } #[cfg(test)] @@ -89,7 +160,7 @@ mod tests { #[test] fn test_as_of_filter_matches_scalar_partial() { - // 9 tx_counts (non-multiple of 2 to exercise tail) + // 9 tx_counts (non-multiple of 4 to exercise tail) let tx_counts: Vec = (1..=9).collect(); let threshold = 5_u64; @@ -100,7 +171,7 @@ mod tests { #[test] fn test_as_of_filter_matches_scalar_exact_chunk() { - // 10 tx_counts (exactly five 2-wide chunks) + // 10 tx_counts (exercises two 4-wide chunks + 2 scalar tail) let tx_counts: Vec = (1..=10).collect(); let threshold = 7_u64; From 394366a2c4fbea5a9fd6c2748a2b464780f88475 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 16:27:59 +0530 Subject: [PATCH 10/13] style(bench): use wrapping_add consistently in sum_simd_i64 horizontal reduction (#229) --- benches/simd_helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benches/simd_helpers.rs b/benches/simd_helpers.rs index 4af0e8c18..f7b7f3b54 100644 --- a/benches/simd_helpers.rs +++ b/benches/simd_helpers.rs @@ -102,7 +102,7 @@ pub fn sum_simd_i64(values: &[i64]) -> i64 { } let lanes: [i64; 4] = acc.to_array(); - let mut sum: i64 = lanes.iter().sum(); + let mut sum: i64 = lanes.iter().fold(0_i64, |acc, &x| acc.wrapping_add(x)); let rem = (values.len() / 4) * 4; for &v in values.get(rem..).unwrap_or(&[]) { From 20441f9624075947595655959a1b8f81b3fb073c Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 16:33:38 +0530 Subject: [PATCH 11/13] feat(bench): add simd_temporal, simd_as_of, simd_aggregate benchmark groups (#229) Co-Authored-By: Claude Sonnet 4.6 --- benches/minigraf_bench.rs | 120 +++++++++++++++++++++++++++++++++++++- 1 file changed, 119 insertions(+), 1 deletion(-) diff --git a/benches/minigraf_bench.rs b/benches/minigraf_bench.rs index 5936ff60c..40adef522 100644 --- a/benches/minigraf_bench.rs +++ b/benches/minigraf_bench.rs @@ -19,7 +19,10 @@ mod helpers; -use criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}; +#[cfg(not(target_arch = "wasm32"))] +mod simd_helpers; + +use criterion::{BenchmarkId, Criterion, Throughput, black_box, criterion_group, criterion_main}; use minigraf::OpenOptions; // ── Task 3: insert/ ─────────────────────────────────────────────────────────── @@ -1537,6 +1540,120 @@ fn bench_predicate_pushdown(c: &mut Criterion) { group.finish(); } +// ── SIMD kernel benchmarks (Issue #229) ────────────────────────────────────── +// +// Both scalar and SIMD variants operate on synthetic Vec/Vec data. +// Fact internals (valid_from, valid_to, tx_count) are pub(crate) and unavailable +// from bench code; benchmarks isolate the hot loop rather than the full query path. +// Cross-reference the existing time_travel/ and aggregation/ groups for full-query costs. + +fn bench_simd(c: &mut Criterion) { + const SCALES: &[(&str, usize)] = &[ + ("100", 100), + ("1k", 1_000), + ("10k", 10_000), + ("100k", 100_000), + ("1m", 1_000_000), + ]; + + // ── simd_temporal: valid-time range filter ────────────────────────────── + // + // Synthetic data: valid_from[i] = i, valid_to[i] = i + n/2. + // ts = n/4 → ~50% of facts are valid at ts (those with valid_from ≤ n/4 < valid_to). + { + let mut group = c.benchmark_group("simd_temporal"); + group.sample_size(10); + + for &(label, n) in SCALES { + let n_i64 = i64::try_from(n).unwrap_or(i64::MAX); + let valid_from: Vec = (0_i64..n_i64).collect(); + let valid_to: Vec = valid_from.iter().map(|&vf| vf + n_i64 / 2).collect(); + let ts = n_i64 / 4; + + group.bench_with_input(BenchmarkId::new("scalar", label), &n, |b, _| { + b.iter(|| { + valid_from + .iter() + .zip(valid_to.iter()) + .filter(|&(&vf, &vt)| vf <= black_box(ts) && black_box(ts) < vt) + .count() + }) + }); + + group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { + b.iter(|| { + simd_helpers::valid_time_filter_simd( + black_box(&valid_from), + black_box(&valid_to), + black_box(ts), + ) + }) + }); + } + + group.finish(); + } + + // ── simd_as_of: tx-time as-of filter ─────────────────────────────────── + // + // Synthetic data: tx_counts = 1..=n (monotonic counter). + // threshold = n/2 → 50% of facts pass the filter. + { + let mut group = c.benchmark_group("simd_as_of"); + group.sample_size(10); + + for &(label, n) in SCALES { + let n_u64 = u64::try_from(n).unwrap_or(u64::MAX); + let tx_counts: Vec = (1..=n_u64).collect(); + let threshold = n_u64 / 2; + + group.bench_with_input(BenchmarkId::new("scalar", label), &n, |b, _| { + b.iter(|| { + tx_counts + .iter() + .filter(|&&tc| tc <= black_box(threshold)) + .count() + }) + }); + + group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { + b.iter(|| { + simd_helpers::as_of_filter_simd( + black_box(&tx_counts), + black_box(threshold), + ) + }) + }); + } + + group.finish(); + } + + // ── simd_aggregate: i64 horizontal sum ───────────────────────────────── + // + // Synthetic data: values = 0..n as i64. + // Measures horizontal reduction performance. + { + let mut group = c.benchmark_group("simd_aggregate"); + group.sample_size(10); + + for &(label, n) in SCALES { + let n_i64 = i64::try_from(n).unwrap_or(i64::MAX); + let values: Vec = (0_i64..n_i64).collect(); + + group.bench_with_input(BenchmarkId::new("scalar", label), &n, |b, _| { + b.iter(|| black_box(&values).iter().copied().sum::()) + }); + + group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { + b.iter(|| simd_helpers::sum_simd_i64(black_box(&values))) + }); + } + + group.finish(); + } +} + criterion_group!( benches, bench_insert, @@ -1562,5 +1679,6 @@ criterion_group!( bench_retract, bench_btree_lookup, bench_predicate_pushdown, + bench_simd, // Issue #229 ); criterion_main!(benches); From 858816f9b90f4291d585078930c8a58b5066f281 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 16:49:37 +0530 Subject: [PATCH 12/13] docs: SIMD benchmarking analysis and recommendation (#229) Co-Authored-By: Claude Sonnet 4.6 --- docs/simd-analysis.md | 113 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 docs/simd-analysis.md diff --git a/docs/simd-analysis.md b/docs/simd-analysis.md new file mode 100644 index 000000000..5fbbe8b6a --- /dev/null +++ b/docs/simd-analysis.md @@ -0,0 +1,113 @@ +# SIMD Benchmarking Analysis (Issue #229) + +**Date:** 2026-05-16 +**Environment:** Intel Core i7-1065G7 @ 1.30GHz (Ice Lake, SSE4.2/AVX2/AVX512), Linux 6.12.85-1-MANJARO, rustc 1.94.0 (4a4ef493e 2026-03-02), wide 0.7.33 +**Branch:** feature/issue-229-simd-benchmarking + +--- + +## Method + +Both scalar and SIMD benchmarks operate on synthetic numeric slices (not live DB facts — `Fact` internals are `pub(crate)`). The benchmarks isolate the hot kernel loop; extraction overhead is identical for both paths (none). For full-query costs, see the `time_travel/` group results below. + +Three benchmark groups: +- **`simd_temporal`** — valid-time range filter: `valid_from[i] <= ts && ts < valid_to[i]`, ~50% selectivity +- **`simd_as_of`** — tx-time as-of filter: `tx_count[i] <= threshold`, 50% selectivity, using `u64x4` +- **`simd_aggregate`** — i64 horizontal sum via `i64x4` 4-wide reduction + +All sizes: 100, 1k, 10k, 100k, 1M elements. Criterion 10-sample runs, bench profile (LTO enabled). + +--- + +## Results: simd_temporal (valid-time range filter) + +`valid_from[i] <= ts && ts < valid_to[i]` — ~50% selectivity + +| Size | Scalar (ns) | SIMD (ns) | Speedup (scalar/simd) | +|--------|------------|-----------|----------------------| +| 100 | 92.9 | 126.5 | 0.73x (SIMD slower) | +| 1k | 670.9 | 1,092.6 | 0.61x (SIMD slower) | +| 10k | 7,439.8 | 10,856.0 | 0.69x (SIMD slower) | +| 100k | 73,833 | 108,850 | 0.68x (SIMD slower) | +| 1M | 860,100 | 1,175,500 | 0.73x (SIMD slower) | + +**Crossover point:** None observed at any tested size. Scalar is consistently ~30–40% faster than SIMD across all sizes. + +**Analysis:** The range filter (`valid_from[i] <= ts && ts < valid_to[i]`) involves two `i64` comparisons per element. LLVM autovectorization (enabled via `lto = true` in the bench profile) already vectorizes the scalar iterator effectively. The `wide` crate's `i64x4` path introduces overhead — likely from the count-matches bookkeeping (the benchmark counts passing elements to prevent dead-code elimination) — that outweighs the SIMD lane benefit. + +--- + +## Results: simd_as_of (tx-time as-of filter) + +`tx_count[i] <= threshold` — 50% selectivity. Uses `u64x4` (wide 0.7 `u64x4`). + +| Size | Scalar (ns) | SIMD (ns) | Speedup (scalar/simd) | +|--------|------------|-----------|----------------------| +| 100 | 54.3 | 100.7 | 0.54x (SIMD slower) | +| 1k | 464.6 | 894.2 | 0.52x (SIMD slower) | +| 10k | 4,680.9 | 8,855.0 | 0.53x (SIMD slower) | +| 100k | 46,367 | 88,418 | 0.52x (SIMD slower) | +| 1M | 611,000 | 980,130 | 0.62x (SIMD slower) | + +**Crossover point:** None observed at any tested size. Scalar is consistently ~50% faster than SIMD across all sizes. + +**Analysis:** The single-comparison filter (`tx_count[i] <= threshold`) is the simplest possible loop body. LLVM autovectorizes this aggressively. The `u64x4` path from the `wide` crate is consistently 1.9x slower than scalar — indicating the abstraction overhead of the `wide` API exceeds the SIMD lane gain for this operation on this architecture. + +--- + +## Results: simd_aggregate (i64 horizontal sum) + +Sum of N `i64` values via `i64x4` 4-wide reduction. + +| Size | Scalar (ns) | SIMD (ns) | Speedup (scalar/simd) | +|--------|------------|-----------|----------------------| +| 100 | 47.3 | 21.8 | **2.17x** | +| 1k | 419.9 | 151.5 | **2.77x** | +| 10k | 4,125.9 | 1,538.4 | **2.68x** | +| 100k | 40,863 | 16,507 | **2.47x** | +| 1M | 493,980 | 256,820 | **1.92x** | + +**Crossover point:** SIMD wins at all tested sizes (100 through 1M). The speedup is consistent and significant — approximately 2–2.8x across the range. + +**Analysis:** Horizontal sum is where SIMD excels. The `i64x4` reduction performs 4 additions per cycle vs 1 for scalar. LLVM's autovectorizer does not fully capture the same gain here because the horizontal reduction pattern is harder to autovectorize than a simple comparison filter. The `wide` crate's explicit SIMD path achieves a reliable 2–2.8x improvement at all practical sizes. + +--- + +## Post-#208 Residual Analysis + +After the B+Tree selective lookup introduced in #208, queries with bound entity or entity+attribute patterns route through index-backed lookups rather than full fact scans. This significantly reduces the number of facts reaching the temporal filter: + +- **Selective lookup queries** (entity or entity+attribute bound): residual fact counts are typically O(attributes per entity) or O(facts per entity+attribute), often 1–100 facts. At these sizes, the temporal filter benchmarks show scalar is already sub-microsecond — SIMD overhead would make things worse. +- **Unbound queries** (full scans): all N facts reach the temporal filter. These are the cases where SIMD temporal filtering would theoretically help, but the benchmark results show scalar autovectorization already wins. +- **Aggregate queries** (e.g., summing numeric values across many facts): the `simd_aggregate` results are directly applicable. A 2–2.8x improvement in the summation kernel is achievable when aggregate operations are applied to large result sets. + +The practical conclusion: for the current array-of-structs `Fact` storage layout, SIMD temporal filtering is not beneficial. SIMD aggregation is beneficial but requires extracting numeric values into a contiguous slice first — an O(N) copy that partially offsets the gain. + +--- + +## Full-Query Context + +From `time_travel/as_of_counter` and `time_travel/valid_at` (full Minigraf query stack, including parsing, transaction overhead, and fact materialization): + +| Benchmark | 1k facts | 10k facts | 100k facts | 1M facts | +|------------------------------------|------------|------------|------------|------------| +| `time_travel/as_of_counter` | 867.1 µs | 9.99 ms | 109.3 ms | 1,729 ms | +| `time_travel/valid_at` | 1,194 µs | 12.60 ms | 108.5 ms | 356.9 ms | + +The full-query cost is orders of magnitude above the raw filter kernel. For example, the `simd_as_of` scalar kernel at 10k facts takes 4.7 µs, but the full `as_of_counter` query at 10k facts costs ~10 ms — a 2000x overhead from transaction, parsing, and fact materialization layers. This confirms that optimizing the filter kernel alone would have negligible impact on end-to-end query latency. + +--- + +## Recommendation + +**Skip (filter kernels) / Revisit with SoA layout (aggregates).** + +**For temporal filtering (`simd_temporal`, `simd_as_of`):** SIMD is slower than scalar at all tested sizes (100 to 1M). LLVM autovectorization with `lto = true` already captures the available vectorization opportunity in the scalar iterator path. Integrating the `wide`-based filter helpers into production code would be a net regression. Recommend closing the filter SIMD work in #229 without further action. + +**For aggregation (`simd_aggregate`):** SIMD wins at all sizes with a consistent 2–2.8x improvement. However, the current array-of-structs `Fact` storage layout means that reaching the `i64` values requires either: +1. An O(N) extraction pass to build a contiguous slice (partially negating the gain), or +2. A struct-of-arrays storage redesign (a much larger change). + +The `pub(crate)` field restriction in `Fact` also means the SIMD helpers cannot access field data directly from production code paths without an API change. + +**Action:** Close #229 with this analysis. Open a separate issue for struct-of-arrays storage layout if aggregate performance becomes a measured bottleneck. The `wide` dependency and `simd_helpers` module introduced in this branch provide a validated foundation for future SoA work. From a61bb3db4f1f3a190b80a968704bc271b91e4d2c Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 16:52:28 +0530 Subject: [PATCH 13/13] style: cargo fmt (#229) --- benches/minigraf_bench.rs | 7 ++----- benches/simd_helpers.rs | 12 ++++++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/benches/minigraf_bench.rs b/benches/minigraf_bench.rs index 40adef522..813d859d0 100644 --- a/benches/minigraf_bench.rs +++ b/benches/minigraf_bench.rs @@ -1618,10 +1618,7 @@ fn bench_simd(c: &mut Criterion) { group.bench_with_input(BenchmarkId::new("simd", label), &n, |b, _| { b.iter(|| { - simd_helpers::as_of_filter_simd( - black_box(&tx_counts), - black_box(threshold), - ) + simd_helpers::as_of_filter_simd(black_box(&tx_counts), black_box(threshold)) }) }); } @@ -1679,6 +1676,6 @@ criterion_group!( bench_retract, bench_btree_lookup, bench_predicate_pushdown, - bench_simd, // Issue #229 + bench_simd, // Issue #229 ); criterion_main!(benches); diff --git a/benches/simd_helpers.rs b/benches/simd_helpers.rs index f7b7f3b54..fe0ec174f 100644 --- a/benches/simd_helpers.rs +++ b/benches/simd_helpers.rs @@ -15,9 +15,9 @@ //! - `u64x4` (not u64x2) is the 4-wide unsigned-64 type; no move_mask, use to_array #![allow(clippy::cast_possible_truncation)] // synthetic bench data: N ≤ 1M fits in i64 -#![allow(clippy::cast_sign_loss)] // tx_count cast: monotonic counter, always positive +#![allow(clippy::cast_sign_loss)] // tx_count cast: monotonic counter, always positive -use wide::{i64x4, u64x4, CmpGt}; +use wide::{CmpGt, i64x4, u64x4}; /// Count facts where `valid_from[i] <= ts && ts < valid_to[i]`. /// @@ -28,8 +28,12 @@ pub fn valid_time_filter_simd(valid_from: &[i64], valid_to: &[i64], ts: i64) -> let mut count = 0usize; for (vf_chunk, vt_chunk) in valid_from.chunks_exact(4).zip(valid_to.chunks_exact(4)) { - let [vf0, vf1, vf2, vf3] = *vf_chunk else { unreachable!() }; - let [vt0, vt1, vt2, vt3] = *vt_chunk else { unreachable!() }; + let [vf0, vf1, vf2, vf3] = *vf_chunk else { + unreachable!() + }; + let [vt0, vt1, vt2, vt3] = *vt_chunk else { + unreachable!() + }; let vf = i64x4::new([vf0, vf1, vf2, vf3]); let vt = i64x4::new([vt0, vt1, vt2, vt3]);