Skip to content

feat(asap-aware-mapping): search engine over ReplacementStrategy (Cascades-style memo search) - #263

Closed
zzylol wants to merge 1 commit into
feat/replacement-strategy-251from
feat/cascades-search-252
Closed

feat(asap-aware-mapping): search engine over ReplacementStrategy (Cascades-style memo search)#263
zzylol wants to merge 1 commit into
feat/replacement-strategy-251from
feat/cascades-search-252

Conversation

@zzylol

@zzylol zzylol commented Aug 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #252. Part of #33. Stacked on #259 (issue #251) — do not merge before that one.

Why

ReplacementStrategy (#251) lets different rewrite strategies propose alternative subtrees at a given site, but nothing actually searched that space: no workload-wide discovery of sites, no deduplication of candidates, no way to converge on and pick a winner.

Separately, #237 deliberately chose a narrow, single-axis cost comparison (share vs. recompute one CSE'd subtree) instead of building real search infrastructure, because at the time there was only one decision to make. It explicitly flagged that this stops being enough once multiple interacting axes — sketch family choice, CSE sharing, future rewrite strategies — need to be considered together. This PR is that follow-up: the actual search engine, previously only sketched as pseudocode in docs/asap_aware_mapping.md.

What

Adds crates/asap-aware-mapping/src/search.rs, the fixpoint search engine over ReplacementStrategy, with public entry points:

  • search_workload / search_workload_with
  • default_strategies / default_strategies_with
  • PlanSpace, MemoGroup, RankedGroup

These are the entry points issue #257 depends on.

Also makes two existing helpers reusable instead of duplicating them: structural_hash (cse.rs) and bindable_intent (replacement.rs).

How (core algorithm)

  1. Memo structure, not flat plans. The search space is a Cascades/Volcano-style memo: each distinct replaceable subtree (identified by Rc pointer identity) becomes one MemoGroup holding every alternative replacement discovered for it. A candidate is never materialized as a standalone full plan — two candidates that differ at only one site share every other node by construction.

  2. Workload-wide site discovery. After running the existing CSE pass once, discover_sites walks every root's DAG to find every distinct site and its real consumer count — something asap-aware-mapping: generalize summary/CSE selection into a ReplacementStrategy trait with multiple candidates #251 explicitly left out of scope.

  3. Fixpoint loop. Each site is asked exactly once per registered strategy. A round can only add to the next round's frontier via a rewrite candidate's own children exposing genuinely new structure — never by re-asking a processed site. Both current strategies (SketchFamilyStrategy, SharedSubtreeStrategy) are idempotent in this sense, so real workloads converge in one round regardless of size. A documented MAX_SEARCH_ITERATIONS cap panics with a clear message if a future strategy fails to converge (covered by a dedicated test).

  4. Deduplication reuses cse.rs's own rule: structural_hash is a cheap filter, PartialEq is the actual duplicate decision — never the reverse. One explicit exception: SharedSubtreeStrategy's two candidates (share vs. recompute) are value-equal but must never be deduped against each other, since they're a genuinely distinct choice.

  5. Final ranking reuses the existing CostModel (cse_share_decision, rank_candidates) rather than inventing a new comparison — keeping CSE: decide rule-based vs. cost-based framework for whether to actually share a detected common subexpression #237's decision and this search's ranking step a single source of truth instead of two drifting implementations.

Checks

cargo build --workspace --all-targets, cargo test --workspace, cargo fmt --all -- --check, and cargo clippy --workspace --all-targets --all-features -- -D warnings all pass clean.

🤖 Generated with Claude Code

This was referenced Aug 23, 2026
Closed
Closed
@zzylol
zzylol force-pushed the feat/replacement-strategy-251 branch from ca60c0c to 66c772d Compare August 23, 2026 19:59
@zzylol @claude
feat(asap-aware-mapping): rebuild search.rs fresh on top of #259's cu…
7b14b4b 
…rrent tip

This branch had been forked at the very first draft of #251
(boundary::implementation_for_with, a ForceSketchKind CostModel-wrapping
hack to steer bind::implement_tree_with) and never rebased — completely
missing #259's subsequent history: the boundary.rs -> implementation.rs
rename, the "make binding literally a selector over ReplacementStrategy"
refactor, and this session's deletion of
bind::implement_tree/implement_tree_with. It would not compile against
the current tip of feat/replacement-strategy-251.

Rebuilt fresh: reset this branch onto the current #259 tip, then
reapplied only the genuinely new content search.rs adds on top —
nothing here re-implements replacement.rs/bind.rs, which now come
from #259 itself.

- search.rs's two implement_tree_with call sites replaced with the
  established take-the-first-candidate pattern
  (SketchFamilyStrategy::replacements(...).into_iter().next(), falling
  back to bind::logical) — the same helper shape bind.rs's own tests
  and every external caller of this crate now use.
- structural_hash's signature grew a `cache: &mut HashCache` parameter
  since this branch forked (issue #244, CSE stage 3, landed on main
  in the meantime) — is_duplicate_rewrite now threads a fresh
  HashCache through its one pairwise comparison.
- Made HashCache pub (was pub(crate)) alongside structural_hash, for
  search.rs's cross-crate reuse — same rationale already documented
  for structural_hash itself.
- lib.rs: added the `search` module doc bullet, `pub mod search;`,
  and its public re-exports; fixed two stale `boundary::` doc
  references in search.rs itself.

Verified: cargo build --workspace --all-targets, cargo test
--workspace (0 failures, including all 12 search:: tests), cargo fmt
--all -- --check, cargo clippy --workspace --all-targets --all-features
-- -D warnings — all clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/cascades-search-252 branch from 9d22b7e to 7b14b4b Compare August 24, 2026 14:03
@zzylol zzylol mentioned this pull request Aug 24, 2026
Closed
zzylol added a commit that referenced this pull request Aug 24, 2026
@zzylol
docs(asap-aware-mapping): stop calling a TargetSubDAG a "site" in app…
0dcb8f5 
…licability.rs

"Site" was an informal synonym introduced when this module's docs were
written — every occurrence names the exact same thing the type system
already calls TargetSubDAG (or, once discovered, a MemoGroup's own
`target`). Replaced every free-standing "site"/"a site's..." with
TargetSubDAG (or "the TargetSubDAG" as a phrase) throughout the module
doc and the OptimizationKind/collect_locations/test doc comments, so
the prose names the real type instead of a parallel, undefined term.
Left two references untouched: `crate::search::discover_sites` (a
real, unrenamed function name in search.rs) and the quoted section
title "Where `for site in plan.bindable_sites()` comes from" (a
verbatim quote of search.rs's own doc heading) — both are accurate
references to search.rs's own content, which this PR doesn't touch.

Also fixed a handful of stale `boundary::`-module doc links found
while in here (the module was renamed to `implementation` well before
this branch's fork point — same staleness class as PR #263/#262/#261/
#260, just not load-bearing for compilation since these were doc-only
intra-doc links).

Verified: cargo build --workspace --all-targets, cargo test
--workspace (0 failures), cargo fmt --all -- --check, cargo clippy
--workspace --all-targets --all-features -- -D warnings — all clean.
zzylol added a commit that referenced this pull request Aug 24, 2026
@zzylol @claude
feat(asap-aware-mapping): rebuild applicability.rs fresh on top of #263…
1566419 
…'s current tip

#263 (feat/cascades-search-252) was just rewritten (force-pushed) as
part of syncing every #33 sub-issue PR to #259's current tip. This
branch, stacked on #263, was still built on #263's old (pre-rewrite)
history, which made the two branches diverge and GitHub start
reporting this PR CONFLICTING against its base.

Rebuilt fresh: reset onto #263's current tip, then reapplied
applicability.rs (already updated per this PR's own review-comment
fixes: "site" -> TargetSubDAG throughout, stale boundary:: doc links
fixed) verbatim, since it depends only on crate::search/crate::replacement,
both stable and unchanged in shape across the rewrite. Wired it into
lib.rs (pub mod applicability;, its Status bullet, and its exports),
and picked up two small search.rs doc improvements this PR had made
(replacing "a hypothetical applicability::..." with real references,
now that this module exists) that were otherwise lost.

The design doc's old "## Applicability reporting" section this PR
originally added lived at docs/asap_aware_mapping.md, a path deleted
by the docs reorg (#265) that landed on main in the meantime; the
design doc's current location (docs/design_docs/asap_aware_mapping.md)
already carries its own "# Applicability Reporting" section, written
independently and already consistent with this PR's own terminology
fix (uses "Target Sub-DAG" throughout, no "site"). Left it untouched
rather than reintroducing now-orphaned content.

Verified: cargo build --workspace --all-targets, cargo test
--workspace (0 failures, including all 11 applicability:: tests),
cargo fmt --all -- --check, cargo clippy --workspace --all-targets
--all-features -- -D warnings — all clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol zzylol mentioned this pull request Aug 24, 2026
Merged
@zzylol zzylol changed the title feat(asap-aware-mapping): Cascades/Volcano-style search over ReplacementStrategy feat(asap-aware-mapping): search engine over ReplacementStrategy (Cascades-style memo search) Aug 24, 2026
zzylol added a commit that referenced this pull request Aug 24, 2026
@zzylol @claude
refactor(asap-aware-mapping): merge search.rs into replacement.rs, ad…
55077da 
…d CostModel::estimate_cost

## Why

PR #263 (feat/cascades-search-252) built a Cascades/Volcano-style MEMO
search engine (search.rs: PlanSpace/MemoGroup/RankedGroup/
search_workload/search_workload_with) but was paused for lacking a
concrete use case. That use case now exists: replacement's "always
return every candidate" principle should hold at whole-workload
granularity too, and a flat Vec of fully-materialized candidate DAGs
is combinatorially infeasible there (N independently-choosable sites
-> 2^N plans) -- search.rs already built the right MEMO structure for
this. Its base predates the implementation.rs->replacement.rs merge
and the Logical->KeepPreAsap rename, so its content is manually
re-integrated here rather than merged/cherry-picked.

## What

- bind.rs shrinks: select_and_bind and keep_pre_asap (both
  single-target-scoped helpers with no workload-level state) move into
  replacement.rs, next to construct_summary/SketchFamilyStrategy which
  are their only real callers. bind.rs keeps only ImplementError,
  implement_workload/implement_workload_with (unchanged bodies --
  these own genuine cross-root Rc-identity memoization state a
  per-target module has no business holding), and its own tests.
  select_and_bind is pub(crate) (bind.rs is its only other caller).
  keep_pre_asap stays pub and is re-exported from bind.rs
  (`pub use crate::replacement::keep_pre_asap;`) so
  `asap_aware_mapping::bind::keep_pre_asap` keeps resolving for
  existing external callers (devtools, frontend-promql tests,
  integration-tests) -- pub(crate) can't be re-exported wider than its
  own crate, so this was the only option that doesn't break the build.

- search.rs's content (MAX_SEARCH_ITERATIONS, MemoGroup, PlanSpace,
  RankedGroup, is_duplicate_rewrite/is_duplicate_summary, rank_group,
  cse_preference, default_strategies[_with], search_workload[_with],
  site discovery) moves directly into replacement.rs -- no `mod
  search`, flattened into the same namespace as TargetSubDAG/
  Replacement/ReplacementStrategy/SketchFamilyStrategy/
  SharedSubtreeStrategy. crate::bind::bindable_intent and
  crate::bind::logical (renamed keep_pre_asap on this branch's tip)
  become local references; search.rs's own bind_one helper is
  unified with select_and_bind (`select_and_bind(target,
  cost_model).ok()`) since both did near-identical
  take-first-or-fall-back-to-keep_pre_asap work. The doc-link to
  `implementation::implementations_for_with` (module-private, per the
  earlier implementation.rs merge) drops its `[...]` brackets, matching
  cost_model.rs's existing style for doc-links to private items.

  asap_types::pre_asap::cse::structural_hash/HashCache go from
  pub(crate) to pub -- search.rs's own is_duplicate_rewrite doc already
  says "made pub for exactly this reuse", assuming a change that never
  actually landed on this branch's base; landing it now is what makes
  the merged code compile.

  search.rs's own #[cfg(test)] mod tests merges into replacement.rs's
  existing test module: its metric_scan/agg fixture helpers and
  `Reduction`/`Schema`/`AggIntent`/DefaultCostModel imports were
  identical (or made redundant by `use super::*`) to what the existing
  module already had, so those are dropped and the merged tests reuse
  the existing helpers directly.

- CostModel::estimate_cost(&self, candidate: &ReplacementSubDAG,
  target: &TargetSubDAG<'_>) -> f64: a new trait method exposing an
  actual numeric cost per candidate (today PlanSpace::cost_sorted only
  exposes relative order), covering both Replacement::Summary
  (SketchFamilyStrategy) and Replacement::Rewrite (SharedSubtreeStrategy)
  shapes. Default body returns f64::NAN, documented as an explicit
  "not a real cost model" placeholder -- not a breaking change for
  existing CostModel implementors. DefaultCostModel overrides it,
  reusing (not duplicating) cse_recompute_cost/
  cse_shared_maintenance_cost -- the same arithmetic already backing
  cse_share_decision: a Summary candidate costs
  cse_recompute_cost(target) + cse_shared_maintenance_cost(candidate's
  bound family); a Rewrite candidate costs
  cse_shared_maintenance_cost (if it's the "share" candidate, i.e.
  Rc::ptr_eq to target) or cse_recompute_cost * consumer_count (if
  it's "recompute independently"), recovering a representative bound
  SummaryNode via select_and_bind when needed.

  RankedGroup gains a `costs: Vec<f64>` field, aligned index-for-index
  with `candidates`, populated by PlanSpace::cost_sorted by calling
  estimate_cost per already-ranked candidate -- purely additive, does
  not change cost_sorted's existing ranking behavior/tests.

- lib.rs: no `pub mod search`; PlanSpace/MemoGroup/RankedGroup/
  search_workload/search_workload_with/default_strategies[_with]/
  MAX_SEARCH_ITERATIONS fold into the existing `pub use replacement::{
  ... }` block. `## Status`/`## Terminology` rewritten: the
  `replacement` bullet now covers decide+construct+search-across-a-
  workload as one module's job (the old "no search/ranking-across-a-
  whole-plan logic lives here" language is gone); a new Terminology
  row covers workload-wide search, reusing search.rs's own MEMO-groups
  framing; the `bind` bullet explains the shrink and why
  implement_workload/implement_workload_with still deserve their own
  module (genuine cross-root memoization state).

- Developer guide: bind::keep_pre_asap/bind::select_and_bind
  references become replacement::keep_pre_asap/replacement::
  select_and_bind; the "not yet the whole-plan Cascades/Volcano-style
  search" paragraph is rewritten to describe search_workload/
  PlanSpace as implemented; the extension map gains rows for
  search_workload, PlanSpace::cost_sorted, and CostModel::estimate_cost.
  docs/design_docs/asap_aware_mapping.md was checked and left as-is --
  it's a timeless design document with no "not yet implemented"
  language to update, and no literal "Pseudocode for Replacement Plan
  Searching" heading to annotate.

## Output

$ cargo build --workspace --all-targets && cargo test --workspace \
    && cargo fmt --all -- --check \
    && cargo clippy --workspace --all-targets --all-features -- -D warnings
(all clean; asap-aware-mapping: 69 passed, 0 failed)

PR #263 (feat/cascades-search-252) is superseded by this merge -- its
content now lives inside this branch. Left open for the user to close.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Aug 24, 2026
@zzylol @claude
feat(asap-aware-mapping): rebuild applicability.rs as a view over Pla…
5504a2f 
…nSpace (#257, part of #33)

Absorbs PR #264 (feat/applicability-view-257) into this branch, re-derived
against the current tip rather than merged/rebased/cherry-picked (#264's
base predates the SketchFamilyStrategy->SketchAlgorithmStrategy rename, the
search.rs->replacement.rs merge, and the "site"->"target"/TargetSubDAG
rename).

Replaces the deleted PR #247 rule-based traversal with a pure view over
replacement::PlanSpace: a TargetSubDAG is applicability-worthy exactly when
its candidate list contains something beyond the trivial, no-op
realization.

- OptimizationKind::SketchApproximation — the candidate list contains a
  Replacement::Summary that realizes SummaryFamilyType::Sketch(..).
- OptimizationKind::CommonSubexpressionReuse — consumer_count >= 2 and the
  candidate list contains SharedSubtreeStrategy's "build once and share"
  candidate.

Each finding's `reason` is the matching candidate's own
ReplacementSubDAG::rationale verbatim — no new prose. No new
applicability-specific extension-point trait: a new finding needs a new
impl ReplacementStrategy wired into default_strategies/default_strategies_with
regardless, since that's the only way a candidate reaches PlanSpace at all.

Kept as its own module (crates/asap-aware-mapping/src/applicability.rs, `pub
mod applicability;` in lib.rs) rather than flattened into replacement.rs:
unlike implementation.rs/search.rs, this is a reporting/presentation
transformation (internal PlanSpace -> external ApplicabilityFinding shape
for a downstream DAG-visualization consumer), a genuinely different kind of
concern from generating or ranking candidates.

Also fixes now-stale cross-references in replacement.rs that pointed at the
deleted PR #247 ApplicabilityRule/SharedSubexpressionRule/register_site
traversal, and a stale reference in the developer guide (section 4.2) to the
same deleted trait. Adds an "Applicability reporting" section (§19) and an
extension-map row to docs/developer_docs/ASAP-aware-mapping-developer-guide.md;
docs/design_docs/asap_aware_mapping.md already carries a conceptual
"Applicability Reporting" section and, matching this session's convention of
keeping that document implementation-agnostic, is left as-is.

cargo build --workspace --all-targets, cargo test --workspace, cargo fmt
--all -- --check, and cargo clippy --workspace --all-targets --all-features
-- -D warnings all pass clean.

PR #264 (feat/applicability-view-257) and PR #263 (feat/cascades-search-252)
are both now superseded by this branch; flagged for the user to close.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol

zzylol commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Superseded — its content (PlanSpace/MemoGroup/search_workload) was folded directly into #259 (feat/replacement-strategy-251) rather than staying a separate stacked PR.

@zzylol zzylol closed this Aug 24, 2026
@zzylol zzylol mentioned this pull request Aug 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@milindsrivastava1997 milindsrivastava1997 Awaiting requested review from milindsrivastava1997

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zzylol