Skip to content

Commit a1fe4b2

Browse files
authored
Merge pull request #145 from ProjectASAP/docs/reconcile-optimizer-plan-with-real-implementation
docs: reconcile core::optimizer/core::plan with what's actually shipped
2 parents dd9db98 + 03396e5 commit a1fe4b2

1 file changed

Lines changed: 91 additions & 23 deletions

File tree

‎docs/design.md‎

Lines changed: 91 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,8 @@ the landed crates (§5.1) as follows:
339339
| `core::lower` (L1→L2→L3) | `asap-frontend-*` (L1→L2) + `asap-l2::convert_root` (L2→L3, with the binder + column resolution) | landed, split per language |
340340
| `core::intent_algebra` (L3) | `asap-ir::intent_algebra` | landed |
341341
| `core::sketch_algebra` (L4 IR) | `asap-sketch` | landed |
342-
| `core::optimizer` (L4 framework) | `asap-plan` | partial (CSE + the L3→L4 `implement_tree` pass (see §3 terminology) + `boundary`, #98; rule engine planned) |
343-
| `core::cost` | `asap-plan::cost_model` | stub |
342+
| `core::optimizer` (L4 framework) | `asap-plan` | landed, but not as a general rule engine — see §6.4 below for what's actually there vs. the general `OptimizerRule`/`RuleEngine` this section originally sketched (never built; superseded by the narrower `boundary`/`bind`/`CostModel` design, #98) |
343+
| `core::cost` | `asap-plan::cost_model` | landed (the `CostModel` trait + `DefaultCostModel`; deliberately narrow — see §6.4) |
344344
| `core::physical` (L5) | — | planned |
345345
| `core::pipeline`, `core::emit`, `core::registry`, `core::workload` | `asap-ir::workload` (workload types only); rest planned | partial |
346346

@@ -767,26 +767,87 @@ Core owns the driver. Deployment models own what goes into each deployment-model
767767

768768
### `core::optimizer` — Layer 4 framework
769769

770-
Core ships the **rule engine + trait surface + a shared rule library**. Deployment models pick which rules to enable.
770+
**This section originally sketched a general Cascades-style rule engine
771+
(`OptimizerRule` + `RuleEngine`, fixed-point rewriting, a shared rule
772+
library) before any of L4 was implemented.** That design was never
773+
built — what actually shipped in `asap-plan` (crate `crates/plan`) is
774+
narrower and more specific to the one decision L4 genuinely needs to
775+
make (which physical summary realizes an `AggIntent`), plus the
776+
supporting pieces that decision needs. The old sketch is kept below,
777+
struck through in spirit, so anyone who remembers it (or finds it in
778+
git blame) can see explicitly that it was superseded rather than just
779+
silently vanish — see the real architecture first.
780+
781+
**What's actually there** (see §3 for the "implementation" vs. "bind"
782+
vs. "match" terminology these use):
783+
784+
- [`boundary::implementation_for`] / [`implementation_for_with`] — the
785+
per-node decision (§3's "Implementation" row): `AggIntent →
786+
Implementation` (`Sketch { kind, params }` | `ExactAccumulator {
787+
kind, params }` | `PassThrough`). Exhaustive over the `AggIntent`
788+
vocabulary — a new variant is a compile error until given an explicit
789+
realization, so there's no silent fall-through.
790+
- [`bind::implement_tree`] / [`implement_tree_with`] — walks a whole
791+
`QueryExpr` tree calling `implementation_for` per node, emitting the
792+
complete L4 `SummaryExpr`/`L4Node` DAG (§3's `implement_tree` row).
793+
Deliberately conservative about *where* it looks for a realizable
794+
`Aggregate`: a non-`Aggregate` parent (`Filter`, `Window`, ...)
795+
wraps the whole subtree as `Logical` rather than rewriting through
796+
it — see the note below on what a deployment does about that.
797+
- [`cost_model::CostModel`] — the **one** pluggable extension point,
798+
not a general rule interface: re-ranks the candidate summary
799+
families `boundary::summary_candidates` returns for an intent (best
800+
choice first), rather than rewriting the tree arbitrarily.
801+
`DefaultCostModel` preserves the built-in static preference order;
802+
every entry point not given an explicit `&dyn CostModel` runs
803+
against it. Deliberately narrow (issues #6, #33) — "summary" already
804+
covers non-sketch realizations the day a new `SummaryKind` variant
805+
lands, so this one interface is meant to stay the only extension
806+
point rather than growing into a second `RuleEngine`.
807+
- [`cse::dedupe_subtrees`] — workload-level Common Sub-Expression
808+
Elimination: hoists sub-DAGs structurally identical across ≥2 query
809+
roots into shared `LetBinding`s so the cost model credits a shared
810+
producer once. Scoped to the basic "≥2 roots, identical
811+
`Aggregate`-child sub-trees" case; alpha-equivalence / schema-merge /
812+
nested CSE is explicitly a downstream optimization, not part of the
813+
IR contract.
814+
- [`boundary::Matcher`] — §3's "Match" row: "does an already-available
815+
`Implementation` satisfy a required one." Ships as a trait with no
816+
default implementation and no shipped instance, same shape and
817+
reasoning as `CostModel` — which `Implementation`s are actually
818+
available anywhere is an inventory only a downstream deployment has.
819+
820+
**What a deployment does beyond this**: `implement_tree`'s
821+
conservative stop at non-`Aggregate` nodes, and any additional
822+
algebraic rewriting beyond summary-candidate ranking (push-downs,
823+
fusion, dead-code elimination, …), are *not* modeled in `asap-plan` at
824+
all — they're each deployment's own problem today. ASAPQuery-backend's
825+
`control_plane` crate is the reference example: its own
826+
`sketch_algebra::rules::bind_*` modules (§3's "Bind #2") and
827+
`optimizer::` module implement exactly this kind of deployment-specific
828+
logic downstream, without `asap-plan` needing to know about it.
829+
830+
<details>
831+
<summary>Historical sketch (never built) — general rule engine + shared rule library</summary>
771832

772833
```rust
773-
// trait (in core)
834+
// trait (never built)
774835
pub trait OptimizerRule: Send + Sync {
775836
fn name(&self) -> &'static str;
776837
fn category(&self) -> RuleCategory; // PushDown | Fusion | Elim | Bind | ...
777838
fn priority(&self) -> u16;
778839
fn apply(&self, expr: &QueryExpr, c: &DeploymentConstraints) -> Option<QueryExpr>;
779840
}
780841

781-
// engine (in core) — fixed-point iteration, cycle detection, priority ordering
842+
// engine (never built) — fixed-point iteration, cycle detection, priority ordering
782843
pub struct RuleEngine { /* ... */ }
783844
impl RuleEngine {
784845
pub fn new(rules: Vec<Box<dyn OptimizerRule>>) -> Self { /* ... */ }
785846
pub fn run(&self, exprs: Vec<QueryExpr>, c: &DeploymentConstraints)
786847
-> Result<Vec<QueryExpr>, OptError>;
787848
}
788849

789-
// shared rule library (in core::optimizer::rules) — opt-in from deployment models
850+
// shared rule library (never built) — opt-in from deployment models
790851
pub mod rules {
791852
pub struct BindKllOnQuantile; // AggIntent::Quantile → bind KLL(k by accuracy)
792853
pub struct BindCmsOnCount; // AggIntent::Count → bind CMS(w, d)
@@ -798,27 +859,21 @@ pub mod rules {
798859
}
799860
```
800861

801-
Deployment models compose rule sets by picking from the shared library + adding their own:
862+
`DeploymentConstraints` and the `Executor`/`StageId` registration
863+
sketch that used to follow this belong to the L5 discussion — see
864+
`core::physical` below, also never built.
802865

803-
```rust
804-
// in deployment-model-asaplifecycle
805-
use asap_plan::optimizer::{RuleEngine, rules::*};
806-
fn rule_set() -> Vec<Box<dyn OptimizerRule>> {
807-
vec![
808-
Box::new(BindKllOnQuantile),
809-
Box::new(BindCmsOnCount),
810-
Box::new(FusionPassthrough),
811-
// DC-specific additions:
812-
Box::new(StageAwarePushDown), // pushes ops to edge when possible
813-
Box::new(TransmissionCostRewrite), // uses TCO model to defer aggregation
814-
]
815-
}
816-
```
817-
818-
Core also ships `DeploymentConstraints` as a trait object; each deployment model supplies a concrete impl with its deployment's memory budgets, network topology, available sketch backends, and the registered `Executor` list (one entry per concrete runtime instance, each tagged with its `StageId`). The executor list is populated from the deployment model's discovery channel — OpAMP for DC lifecycle, static config for single-backend query, the in-process `SessionContext` itself for fusion.
866+
</details>
819867

820868
### `core::physical` — Layer 5 framework
821869

870+
**Status: planned, not yet built.** No `asap-physical` crate (or
871+
equivalent) exists in this workspace today — the sketch below is the
872+
intended design for L5 (stage allocation + emission), not a
873+
description of shipped code. §6.0's crate map lists `core::physical`
874+
as `—` / planned for exactly this reason. Treat the trait/struct names
875+
below as a target to design against, not an API to depend on.
876+
822877
```rust
823878
pub trait PhysicalPlanner {
824879
type Topology: TopologyDescriptor;
@@ -933,6 +988,19 @@ impl PhysicalPlanner for LifecyclePlanner {
933988

934989
### `core::plan` — shared traits bridging layers
935990

991+
**Status: planned, not yet built — and a naming collision worth
992+
flagging.** This `core::plan` (a cross-layer `DeploymentModel` trait
993+
composing L4 rules + L5 topology + emission + constraints) is a
994+
*different thing* from the real, landed `asap-plan` crate
995+
(`crates/plan`), which §6.0's crate map lists under `core::optimizer`
996+
(L4 only — see that section above for what's actually there). Neither
997+
`DeploymentModel` nor the `OptimizerRule`/`PhysicalPlanner`/
998+
`PlanEmitter`/`DeploymentConstraints` types it references exist
999+
anywhere in this workspace today; this section predates the crate
1000+
split and was never reconciled with it. Read as a target design for
1001+
how a deployment model might eventually compose L4+L5+emission, not a
1002+
description of `asap-plan`.
1003+
9361004
```rust
9371005
pub trait DeploymentModel {
9381006
type Topology: TopologyDescriptor;

0 commit comments

Comments
 (0)