TaffyTree mark_dirty optimization (#801)
* Mark dirty optimization
* Cleaning up mark dirty optimization experiment
* Cache clear operation improvements
---------
Co-authored-by: Nico Burns <nico@nicoburns.com>
9da7a72b99c45df07457259f84fb483d9de22b07 · ruwaq Pēteris Pakalns · (1 wata ñawpaq) · tayta 71793a3ab · kay commitpi sach'ata qhaway
Semántico sayk'uy · 13
examples/custom_tree_owned_partial.rs
examples/custom_tree_owned_unsafe.rs
examples/custom_tree_vec.rs
src/tree/cache.rs
- silq'uyninpi t'ikrakurqan struct Cache pub
- musuq enum ClearState pub
- silq'uyninpi t'ikrakurqan fn clear pub
- ukhunpi t'ikrakurqan fn new pub
- ukhunpi t'ikrakurqan fn store pub
diff --git a/examples/custom_tree_owned_partial.rs b/examples/custom_tree_owned_partial.rs
index ac070e257..a53af338e 100644
--- a/examples/custom_tree_owned_partial.rs
+++ b/examples/custom_tree_owned_partial.rs
@@ -207,7 +207,7 @@ impl CacheTree for Node {
}
fn cache_clear(&mut self, node_id: NodeId) {
- self.node_from_id_mut(node_id).cache.clear()
+ self.node_from_id_mut(node_id).cache.clear();
}
}
diff --git a/examples/custom_tree_owned_unsafe.rs b/examples/custom_tree_owned_unsafe.rs
index ed13d653a..c5fa77ad1 100644
--- a/examples/custom_tree_owned_unsafe.rs
+++ b/examples/custom_tree_owned_unsafe.rs
@@ -204,7 +204,7 @@ impl CacheTree for StatelessLayoutTree {
}
fn cache_clear(&mut self, node_id: NodeId) {
- unsafe { node_from_id_mut(node_id) }.cache.clear()
+ unsafe { node_from_id_mut(node_id) }.cache.clear();
}
}
diff --git a/examples/custom_tree_vec.rs b/examples/custom_tree_vec.rs
index a7125cb94..5a9db60ae 100644
--- a/examples/custom_tree_vec.rs
+++ b/examples/custom_tree_vec.rs
@@ -213,7 +213,7 @@ impl CacheTree for Tree {
}
fn cache_clear(&mut self, node_id: NodeId) {
- self.node_from_id_mut(node_id).cache.clear()
+ self.node_from_id_mut(node_id).cache.clear();
}
}
diff --git a/src/tree/cache.rs b/src/tree/cache.rs
index b52e14f07..cff245d5e 100644
--- a/src/tree/cache.rs
+++ b/src/tree/cache.rs
@@ -26,6 +26,8 @@ pub struct Cache {
final_layout_entry: Option<CacheEntry<LayoutOutput>>,
/// The cache entries for the node's preliminary size measurements
measure_entries: [Option<CacheEntry<Size<f32>>>; CACHE_SIZE],
+ /// Tracks if all cache entries are empty
+ is_empty: bool,
}
impl Default for Cache {
@@ -37,7 +39,7 @@ impl Default for Cache {
impl Cache {
/// Create a new empty cache
pub const fn new() -> Self {
- Self { final_layout_entry: None, measure_entries: [None; CACHE_SIZE] }
+ Self { final_layout_entry: None, measure_entries: [None; CACHE_SIZE], is_empty: true }
}
/// Return the cache slot to cache the current computed result in
@@ -160,9 +162,11 @@ impl Cache {
) {
match run_mode {
RunMode::PerformLayout => {
+ self.is_empty = false;
self.final_layout_entry = Some(CacheEntry { known_dimensions, available_space, content: layout_output })
}
RunMode::ComputeSize => {
+ self.is_empty = false;
let cache_slot = Self::compute_cache_slot(known_dimensions, available_space);
self.measure_entries[cache_slot] =
Some(CacheEntry { known_dimensions, available_space, content: layout_output.size });
@@ -171,10 +175,15 @@ impl Cache {
}
}
- /// Clear all cache entries
- pub fn clear(&mut self) {
+ /// Clear all cache entries and reports clear operation outcome ([`ClearState`])
+ pub fn clear(&mut self) -> ClearState {
+ if self.is_empty {
+ return ClearState::AlreadyEmpty;
+ }
+ self.is_empty = true;
self.final_layout_entry = None;
self.measure_entries = [None; CACHE_SIZE];
+ ClearState::Cleared
}
/// Returns true if all cache entries are None, else false
@@ -182,3 +191,11 @@ impl Cache {
self.final_layout_entry.is_none() && !self.measure_entries.iter().any(|entry| entry.is_some())
}
}
+
+/// Clear operation outcome. See [`Cache::clear`]
+pub enum ClearState {
+ /// Cleared some values
+ Cleared,
+ /// Everything was already cleared
+ AlreadyEmpty,
+}
diff --git a/src/tree/mod.rs b/src/tree/mod.rs
index 637136bd7..2096b6f3b 100644
--- a/src/tree/mod.rs
+++ b/src/tree/mod.rs
@@ -9,7 +9,7 @@ mod layout;
mod node;
pub mod traits;
-pub use cache::Cache;
+pub use cache::{Cache, ClearState};
pub use layout::{CollapsibleMarginSet, Layout, LayoutInput, LayoutOutput, RequestedAxis, RunMode, SizingMode};
pub use node::NodeId;
pub(crate) use traits::LayoutPartialTreeExt;
diff --git a/src/tree/taffy_tree.rs b/src/tree/taffy_tree.rs
index f39b9c61e..014c49952 100644
--- a/src/tree/taffy_tree.rs
+++ b/src/tree/taffy_tree.rs
@@ -8,7 +8,7 @@ use slotmap::{DefaultKey, SlotMap};
use crate::geometry::Size;
use crate::style::{AvailableSpace, Display, Style};
use crate::tree::{
- Cache, Layout, LayoutInput, LayoutOutput, LayoutPartialTree, NodeId, PrintTree, RoundTree, RunMode,
+ Cache, ClearState, Layout, LayoutInput, LayoutOutput, LayoutPartialTree, NodeId, PrintTree, RoundTree, RunMode,
TraversePartialTree, TraverseTree,
};
use crate::util::debug::{debug_log, debug_log_node};
@@ -129,8 +129,9 @@ impl NodeData {
/// Marks a node and all of its ancestors as requiring relayout
///
/// This clears any cached data and signals that the data must be recomputed.
+ /// If the node was already marked as dirty, returns true
#[inline]
- pub fn mark_dirty(&mut self) {
+ pub fn mark_dirty(&mut self) -> ClearState {
self.cache.clear()
}
}
@@ -227,7 +228,7 @@ impl<NodeContext> CacheTree for TaffyTree<NodeContext> {
}
fn cache_clear(&mut self, node_id: NodeId) {
- self.nodes[node_id.into()].cache.clear()
+ self.nodes[node_id.into()].cache.clear();
}
}
@@ -420,7 +421,7 @@ where
}
fn cache_clear(&mut self, node_id: NodeId) {
- self.taffy.nodes[node_id.into()].cache.clear()
+ self.taffy.nodes[node_id.into()].cache.clear();
}
}
@@ -863,19 +864,23 @@ impl<NodeContext> TaffyTree<NodeContext> {
}
/// Marks the layout of this node and its ancestors as outdated
- ///
- /// WARNING: this may stack-overflow if the tree contains a cycle
pub fn mark_dirty(&mut self, node: NodeId) -> TaffyResult<()> {
- /// WARNING: this will stack-overflow if the tree contains a cycle
fn mark_dirty_recursive(
nodes: &mut SlotMap<DefaultKey, NodeData>,
parents: &SlotMap<DefaultKey, Option<NodeId>>,
node_key: DefaultKey,
) {
- nodes[node_key].mark_dirty();
-
- if let Some(Some(node)) = parents.get(node_key) {
- mark_dirty_recursive(nodes, parents, (*node).into());
+ match nodes[node_key].mark_dirty() {
+ ClearState::AlreadyEmpty => {
+ // Node was already marked as dirty.
+ // No need to visit ancestors
+ // as they should be marked as dirty already.
+ }
+ ClearState::Cleared => {
+ if let Some(Some(node)) = parents.get(node_key) {
+ mark_dirty_recursive(nodes, parents, (*node).into());
+ }
+ }
}
}