Skip to content

Commit cc133f7

Browse files
committed
Use RefCell to allow path compression in union-find
When I wrote the original version I didn't understand the interior mutability pattern, but now I do! With this commit, we should have a more optimal union-find implementation.
1 parent f55138c commit cc133f7

1 file changed

Lines changed: 13 additions & 13 deletions

File tree

zjit/src/hir.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ impl<T: Copy + Into<usize> + PartialEq> UnionFind<T> {
619619
}
620620

621621
/// Find the set representative for `insn` without doing path compression.
622-
pub fn find_const(&self, insn: T) -> T {
622+
fn find_const(&self, insn: T) -> T {
623623
let mut result = insn;
624624
loop {
625625
match self.at(result) {
@@ -645,7 +645,7 @@ pub struct Function {
645645
// TODO: get method name and source location from the ISEQ
646646

647647
insns: Vec<Insn>,
648-
union_find: UnionFind<InsnId>,
648+
union_find: std::cell::RefCell<UnionFind<InsnId>>,
649649
insn_types: Vec<Type>,
650650
blocks: Vec<Block>,
651651
entry_block: BlockId,
@@ -657,7 +657,7 @@ impl Function {
657657
iseq,
658658
insns: vec![],
659659
insn_types: vec![],
660-
union_find: UnionFind::new(),
660+
union_find: UnionFind::new().into(),
661661
blocks: vec![Block::default()],
662662
entry_block: BlockId(0),
663663
}
@@ -740,7 +740,7 @@ impl Function {
740740
macro_rules! find {
741741
( $x:expr ) => {
742742
{
743-
self.union_find.find_const($x)
743+
self.union_find.borrow_mut().find($x)
744744
}
745745
};
746746
}
@@ -749,12 +749,12 @@ impl Function {
749749
{
750750
BranchEdge {
751751
target: $edge.target,
752-
args: $edge.args.iter().map(|x| self.union_find.find_const(*x)).collect(),
752+
args: $edge.args.iter().map(|x| find!(*x)).collect(),
753753
}
754754
}
755755
};
756756
}
757-
let insn_id = self.union_find.find_const(insn_id);
757+
let insn_id = self.union_find.borrow_mut().find(insn_id);
758758
use Insn::*;
759759
match &self.insns[insn_id.0] {
760760
result@(PutSelf | Const {..} | Param {..} | NewArray {..} | GetConstantPath {..}
@@ -822,12 +822,12 @@ impl Function {
822822
/// Replace `insn` with the new instruction `replacement`, which will get appended to `insns`.
823823
fn make_equal_to(&mut self, insn: InsnId, replacement: InsnId) {
824824
// Don't push it to the block
825-
self.union_find.make_equal_to(insn, replacement);
825+
self.union_find.borrow_mut().make_equal_to(insn, replacement);
826826
}
827827

828828
fn type_of(&self, insn: InsnId) -> Type {
829829
assert!(self.insns[insn.0].has_output());
830-
self.insn_types[self.union_find.find_const(insn).0]
830+
self.insn_types[self.union_find.borrow_mut().find(insn).0]
831831
}
832832

833833
/// Check if the type of `insn` is a subtype of `ty`.
@@ -1963,19 +1963,19 @@ mod union_find_tests {
19631963
}
19641964

19651965
#[test]
1966-
fn test_find_const_returns_target() {
1966+
fn test_find_returns_target() {
19671967
let mut uf = UnionFind::new();
19681968
uf.make_equal_to(3, 4);
1969-
assert_eq!(uf.find_const(3usize), 4);
1969+
assert_eq!(uf.find(3usize), 4);
19701970
}
19711971

19721972
#[test]
1973-
fn test_find_const_returns_transitive_target() {
1973+
fn test_find_returns_transitive_target() {
19741974
let mut uf = UnionFind::new();
19751975
uf.make_equal_to(3, 4);
19761976
uf.make_equal_to(4, 5);
1977-
assert_eq!(uf.find_const(3usize), 5);
1978-
assert_eq!(uf.find_const(4usize), 5);
1977+
assert_eq!(uf.find(3usize), 5);
1978+
assert_eq!(uf.find(4usize), 5);
19791979
}
19801980

19811981
#[test]

0 commit comments

Comments
 (0)