Skip to content

Commit

Permalink
fix(hydroflow_plus): rewrite IR in place to avoid stack overflow and …
Browse files Browse the repository at this point in the history
…disable cloning (#1404)

Cloning was unsafe because values behind a `Rc<RefCell<...>>` in the
case of tee would be entangled with the old IR.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/hydro-project/hydroflow/pull/1404).
* #1405
* #1398
* __->__ #1404
  • Loading branch information
shadaj authored Aug 21, 2024
1 parent b518e67 commit 1aeacb2
Show file tree
Hide file tree
Showing 7 changed files with 361 additions and 423 deletions.
4 changes: 2 additions & 2 deletions hydroflow_plus/src/builder/built.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<'a> BuiltFlow<'a> {
}
}

pub(crate) fn build_inner(ir: Vec<HfPlusLeaf>) -> BTreeMap<usize, HydroflowGraph> {
pub(crate) fn build_inner(ir: &mut Vec<HfPlusLeaf>) -> BTreeMap<usize, HydroflowGraph> {
let mut builders = BTreeMap::new();
let mut built_tees = HashMap::new();
let mut next_stmt_id = 0;
Expand All @@ -69,7 +69,7 @@ impl<'a> BuiltFlow<'a> {
self.used = true;

HfCompiled {
hydroflow_ir: build_inner(std::mem::take(&mut self.ir)),
hydroflow_ir: build_inner(&mut self.ir),
extra_stmts: BTreeMap::new(),
_phantom: PhantomData,
}
Expand Down
8 changes: 4 additions & 4 deletions hydroflow_plus/src/builder/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl<'a, D: Deploy<'a>> DeployFlow<'a, D> {
self.used = true;

let mut seen_tees: HashMap<_, _> = HashMap::new();
let ir_leaves_networked: Vec<HfPlusLeaf> = std::mem::take(&mut self.ir)
let mut ir_leaves_networked: Vec<HfPlusLeaf> = std::mem::take(&mut self.ir)
.into_iter()
.map(|leaf| leaf.compile_network::<D>(env, &mut seen_tees, &self.nodes, &self.clusters))
.collect();
Expand Down Expand Up @@ -81,7 +81,7 @@ impl<'a, D: Deploy<'a>> DeployFlow<'a, D> {
}

HfCompiled {
hydroflow_ir: build_inner(ir_leaves_networked),
hydroflow_ir: build_inner(&mut ir_leaves_networked),
extra_stmts,
_phantom: PhantomData,
}
Expand All @@ -94,7 +94,7 @@ impl<'a, D: Deploy<'a, CompileEnv = ()>> DeployFlow<'a, D> {
self.used = true;

let mut seen_tees_instantiate: HashMap<_, _> = HashMap::new();
let ir_leaves_networked: Vec<HfPlusLeaf> = std::mem::take(&mut self.ir)
let mut ir_leaves_networked: Vec<HfPlusLeaf> = std::mem::take(&mut self.ir)
.into_iter()
.map(|leaf| {
leaf.compile_network::<D>(
Expand All @@ -106,7 +106,7 @@ impl<'a, D: Deploy<'a, CompileEnv = ()>> DeployFlow<'a, D> {
})
.collect();

let mut compiled = build_inner(ir_leaves_networked.clone());
let mut compiled = build_inner(&mut ir_leaves_networked);
let mut meta = D::Meta::default();

let (mut processes, mut clusters) = (
Expand Down
Loading

0 comments on commit 1aeacb2

Please sign in to comment.