Revert "feat: Skip measuring of cached siblings in layout"

.github/workflows/rust.yml

@@ -5,6 +5,7 @@ on:
   push:
     branches: [ "main" ]
   pull_request:
+    branches: [ "main" ]
 
 env:
   CARGO_TERM_COLOR: always

crates/dom/src/dom_adapter.rs

@@ -1,6 +1,6 @@
 use dioxus_native_core::{prelude::NodeType, real_dom::NodeImmutable, tree::TreeRef, NodeId};
 use freya_node_state::LayoutState;
-use rustc_hash::{FxHashMap, FxHashSet};
+use rustc_hash::FxHashMap;
 use torin::prelude::*;
 
 use crate::dom::DioxusDOM;
@@ -77,23 +77,13 @@ impl DOMAdapter<NodeId> for DioxusDOMAdapter<'_> {
         is_node_valid(self.rdom, &mut self.valid_nodes_cache, node_id)
     }
 
-    fn closest_common_parent(
-        &self,
-        node_id_a: &NodeId,
-        node_id_b: &NodeId,
-        root_track_patch: &mut FxHashSet<NodeId>,
-    ) -> Option<NodeId> {
-        find_common_parent(self.rdom, *node_id_a, *node_id_b, root_track_patch)
+    fn closest_common_parent(&self, node_id_a: &NodeId, node_id_b: &NodeId) -> Option<NodeId> {
+        find_common_parent(self.rdom, *node_id_a, *node_id_b)
     }
 }
 
 /// Walk to the ancestor of `base` with the same height of `target`
-fn balance_heights(
-    rdom: &DioxusDOM,
-    base: NodeId,
-    target: NodeId,
-    root_track_patch: &mut FxHashSet<NodeId>,
-) -> Option<NodeId> {
+fn balance_heights(rdom: &DioxusDOM, base: NodeId, target: NodeId) -> Option<NodeId> {
     let tree = rdom.tree_ref();
     let target_height = tree.height(target)?;
     let mut current = base;
@@ -105,34 +95,25 @@ fn balance_heights(
         let parent_current = tree.parent_id(current);
         if let Some(parent_current) = parent_current {
             current = parent_current;
-            root_track_patch.insert(current);
         }
     }
     Some(current)
 }
 
 /// Return the closest common ancestor of both Nodes
-fn find_common_parent(
-    rdom: &DioxusDOM,
-    node_a: NodeId,
-    node_b: NodeId,
-    root_track_patch: &mut FxHashSet<NodeId>,
-) -> Option<NodeId> {
+fn find_common_parent(rdom: &DioxusDOM, node_a: NodeId, node_b: NodeId) -> Option<NodeId> {
     let tree = rdom.tree_ref();
     let height_a = tree.height(node_a)?;
     let height_b = tree.height(node_b)?;
 
-    root_track_patch.insert(node_a);
-    root_track_patch.insert(node_b);
-
     let (node_a, node_b) = match height_a.cmp(&height_b) {
         std::cmp::Ordering::Less => (
             node_a,
-            balance_heights(rdom, node_b, node_a, root_track_patch).unwrap_or(node_b),
+            balance_heights(rdom, node_b, node_a).unwrap_or(node_b),
         ),
         std::cmp::Ordering::Equal => (node_a, node_b),
         std::cmp::Ordering::Greater => (
-            balance_heights(rdom, node_a, node_b, root_track_patch).unwrap_or(node_a),
+            balance_heights(rdom, node_a, node_b).unwrap_or(node_a),
             node_b,
         ),
     };
@@ -148,7 +129,6 @@ fn find_common_parent(
         let parent_a = tree.parent_id(currents.0);
         if let Some(parent_a) = parent_a {
             currents.0 = parent_a;
-            root_track_patch.insert(parent_a);
         } else if rdom.root_id() != currents.0 {
             // Skip unconected nodes
             break;
@@ -157,7 +137,6 @@ fn find_common_parent(
         let parent_b = tree.parent_id(currents.1);
         if let Some(parent_b) = parent_b {
             currents.1 = parent_b;
-            root_track_patch.insert(parent_b);
         } else if rdom.root_id() != currents.1 {
             // Skip unconected nodes
             break;

crates/torin/Cargo.toml

@@ -18,7 +18,7 @@ default = ["dioxus"]
 [dependencies]
 tracing = { workspace = true }
 euclid = { workspace = true }
-rustc-hash = { workspace = true }
+fxhash = { workspace = true }
 dioxus-native-core = { workspace = true, optional = true }
 dioxus-core = { workspace = true, optional = true }
 

crates/torin/benches/bench.rs

@@ -1,7 +1,6 @@
 use std::collections::HashMap;
 
 use criterion::{black_box, criterion_group, criterion_main, Criterion};
-use rustc_hash::FxHashSet;
 use torin::prelude::*;
 
 struct TestingMeasurer;
@@ -30,17 +29,6 @@ impl TestingDOM {
         self.mapper.insert(node_id, (parent, children, depth, node));
     }
 
-    fn add_with_depth(
-        &mut self,
-        node_id: usize,
-        parent: Option<usize>,
-        children: Vec<usize>,
-        node: Node,
-        depth: u16,
-    ) {
-        self.mapper.insert(node_id, (parent, children, depth, node));
-    }
-
     fn set_node(&mut self, node_id: usize, node: Node) {
         self.mapper.get_mut(&node_id).unwrap().3 = node;
     }
@@ -70,21 +58,14 @@ impl DOMAdapter<usize> for TestingDOM {
         true
     }
 
-    fn closest_common_parent(
-        &self,
-        node_id_a: &usize,
-        node_id_b: &usize,
-        root_track_patch: &mut FxHashSet<usize>,
-    ) -> Option<usize> {
-        root_track_patch.insert(*node_id_a);
-        root_track_patch.insert(*node_id_b);
-        self.parent_of(node_id_a)
+    fn closest_common_parent(&self, node_id_a: &usize, _node_id_b: &usize) -> Option<usize> {
+        Some(self.parent_of(node_id_a)?)
     }
 }
 
 fn criterion_benchmark(c: &mut Criterion) {
     let mut g = c.benchmark_group("benchmarks");
-    g.significance_level(0.05).sample_size(500);
+    g.significance_level(0.1).sample_size(500);
 
     let params = [
         ("big trees (wide) nodes=1000, depth=1", 1000, 1),
@@ -391,7 +372,7 @@ fn criterion_benchmark(c: &mut Criterion) {
             b.iter(|| {
                 black_box({
                     mocked_dom.set_node(
-                        1001,
+                        1,
                         Node::from_size_and_direction(
                             Size::Inner,
                             Size::Pixels(Length::new(10.0)),
@@ -410,86 +391,6 @@ fn criterion_benchmark(c: &mut Criterion) {
             })
         },
     );
-
-    g.bench_function(
-        "big trees (deep + branches + cached) + invalidated node in the middle",
-        |b| {
-            let mut layout = Torin::<usize>::new();
-            let mut measurer = Some(TestingMeasurer);
-            let mut mocked_dom = TestingDOM::default();
-
-            mocked_dom.add(
-                0,
-                None,
-                vec![101, 102],
-                Node::from_size_and_direction(
-                    Size::Percentage(Length::new(100.0)),
-                    Size::Percentage(Length::new(100.0)),
-                    DirectionMode::Vertical,
-                ),
-            );
-
-            const LEVELS: usize = 9;
-            const WIDE: usize = 2;
-
-            fn build_branch(mocked_dom: &mut TestingDOM, root: usize, level: usize) -> Vec<usize> {
-                if level == LEVELS {
-                    return vec![];
-                }
-
-                let nodes = (0..=WIDE)
-                    .map(|i| i + ((level + 1) * 100) + (root * 10))
-                    .into_iter()
-                    .collect::<Vec<usize>>();
-                for id in nodes.iter() {
-                    let children = build_branch(mocked_dom, *id, level + 1);
-                    mocked_dom.add_with_depth(
-                        *id,
-                        Some(root),
-                        children,
-                        Node::from_size_and_direction(
-                            Size::Pixels(Length::new(100.0)),
-                            Size::Pixels(Length::new(100.0)),
-                            DirectionMode::Vertical,
-                        ),
-                        level as u16,
-                    );
-                }
-                nodes
-            }
-
-            build_branch(&mut mocked_dom, 0, 0);
-
-            layout.find_best_root(&mut mocked_dom);
-            layout.measure(
-                0,
-                Rect::new(Point2D::new(0.0, 0.0), Size2D::new(1000.0, 1000.0)),
-                &mut measurer,
-                &mut mocked_dom,
-            );
-
-            b.iter(|| {
-                black_box({
-                    mocked_dom.set_node(
-                        1202,
-                        Node::from_size_and_direction(
-                            Size::Inner,
-                            Size::Pixels(Length::new(10.0)),
-                            DirectionMode::Vertical,
-                        ),
-                    );
-                    layout.invalidate(12456790001);
-                    layout.find_best_root(&mut mocked_dom);
-                    layout.measure(
-                        0,
-                        Rect::new(Point2D::new(0.0, 0.0), Size2D::new(1000.0, 1000.0)),
-                        &mut measurer,
-                        &mut mocked_dom,
-                    )
-                });
-            })
-        },
-    );
 }
 
 criterion_group!(benches, criterion_benchmark);

crates/torin/src/dom_adapter.rs

@@ -1,5 +1,4 @@
 pub use euclid::Rect;
-use rustc_hash::FxHashSet;
 
 use crate::{
     geometry::{Area, Size2D},
@@ -54,10 +53,5 @@ pub trait DOMAdapter<NodeKey> {
     fn is_node_valid(&mut self, node_id: &NodeKey) -> bool;
 
     /// Get the closest common parent Node of two Nodes
-    fn closest_common_parent(
-        &self,
-        node_id_a: &NodeKey,
-        node_id_b: &NodeKey,
-        root_track_patch: &mut FxHashSet<NodeKey>,
-    ) -> Option<NodeKey>;
+    fn closest_common_parent(&self, node_id_a: &NodeKey, node_id_b: &NodeKey) -> Option<NodeKey>;
 }