From e67b1c06994225df953cf12c9924c0468bbc0e88 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 10 Sep 2025 19:51:57 +0200 Subject: [PATCH] Improve selected nodes perf and memoize network hash computation --- .../utility_types/network_interface.rs | 83 +++++++++++++++++-- .../portfolio/document/utility_types/nodes.rs | 4 +- editor/src/node_graph_executor.rs | 2 +- node-graph/graph-craft/src/document.rs | 7 +- 4 files changed, 81 insertions(+), 15 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index a42a7c5c25..d732ca44b0 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -26,6 +26,7 @@ use graphene_std::vector::{PointId, Vector, VectorModificationType}; use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypes; use interpreted_executor::node_registry::NODE_REGISTRY; use kurbo::BezPath; +use private::MemoNetwork; use serde_json::{Value, json}; use std::collections::{HashMap, HashSet, VecDeque}; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -36,7 +37,7 @@ use std::ops::Deref; pub struct NodeNetworkInterface { /// The node graph that generates this document's artwork. It recursively stores its sub-graphs, so this root graph is the whole snapshot of the document content. /// A public mutable reference should never be created. It should only be mutated through custom setters which perform the necessary side effects to keep network_metadata in sync - network: NodeNetwork, + network: MemoNetwork, /// Stores all editor information for a NodeNetwork. Should automatically kept in sync by the setter methods when changes to the document network are made. network_metadata: NodeNetworkMetadata, // TODO: Wrap in TransientMetadata Option @@ -68,10 +69,69 @@ impl PartialEq for NodeNetworkInterface { } } +mod private { + use std::cell::RefCell; + use std::hash::{Hash, Hasher}; + + use graph_craft::document::NodeNetwork; + + #[derive(Debug, Default, Clone, PartialEq)] + pub struct MemoNetwork { + network: NodeNetwork, + hash_code: RefCell>, + } + + impl<'de> serde::Deserialize<'de> for MemoNetwork { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self::new(NodeNetwork::deserialize(deserializer)?)) + } + } + + impl serde::Serialize for MemoNetwork { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.network.serialize(serializer) + } + } + + impl Hash for MemoNetwork { + fn hash(&self, state: &mut H) { + self.current_hash().hash(state); + } + } + + impl MemoNetwork { + pub fn network(&self) -> &NodeNetwork { + &self.network + } + pub fn network_mut(&mut self) -> &mut NodeNetwork { + self.hash_code.replace(None); + &mut self.network + } + + pub fn new(network: NodeNetwork) -> Self { + Self { network, hash_code: None.into() } + } + + pub fn current_hash(&self) -> u64 { + let mut hash_code = self.hash_code.borrow_mut(); + if hash_code.is_none() { + *hash_code = Some(self.network.current_hash()); + } + hash_code.unwrap() + } + } +} + impl NodeNetworkInterface { /// Add DocumentNodePath input to the PathModifyNode protonode pub fn migrate_path_modify_node(&mut self) { - fix_network(&mut self.network); + fix_network(self.document_network_mut()); fn fix_network(network: &mut NodeNetwork) { for node in network.nodes.values_mut() { if let Some(network) = node.implementation.get_network_mut() { @@ -91,18 +151,25 @@ impl NodeNetworkInterface { impl NodeNetworkInterface { /// Gets the network of the root document pub fn document_network(&self) -> &NodeNetwork { - &self.network + self.network.network() + } + pub fn document_network_mut(&mut self) -> &mut NodeNetwork { + self.network.network_mut() } /// Gets the nested network based on network_path pub fn nested_network(&self, network_path: &[NodeId]) -> Option<&NodeNetwork> { - let Some(network) = self.network.nested_network(network_path) else { + let Some(network) = self.document_network().nested_network(network_path) else { log::error!("Could not get nested network with path {network_path:?} in NodeNetworkInterface::network"); return None; }; Some(network) } + pub fn network_hash(&self) -> u64 { + self.network.current_hash() + } + /// Get the specified document node in the nested network based on node_id and network_path pub fn document_node(&self, node_id: &NodeId, network_path: &[NodeId]) -> Option<&DocumentNode> { let network = self.nested_network(network_path)?; @@ -161,7 +228,7 @@ impl NodeNetworkInterface { .back() .cloned() .unwrap_or_default() - .filtered_selected_nodes(network_metadata.persistent_metadata.node_metadata.keys().cloned().collect()), + .filtered_selected_nodes(|node_id| network_metadata.persistent_metadata.node_metadata.contains_key(node_id)), ) } @@ -1556,7 +1623,7 @@ impl NodeNetworkInterface { log::error!("Could not get network or network_metadata in upstream_flow_back_from_nodes"); return FlowIter { stack: Vec::new(), - network: &self.network, + network: &self.document_network(), network_metadata: &self.network_metadata, flow_type: FlowType::UpstreamFlow, }; @@ -1708,7 +1775,7 @@ impl NodeNetworkInterface { } } Self { - network: node_network, + network: MemoNetwork::new(node_network), network_metadata, document_metadata: DocumentMetadata::default(), resolved_types: ResolvedDocumentNodeTypes::default(), @@ -1744,7 +1811,7 @@ fn random_protonode_implementation(protonode: &graph_craft::ProtoNodeIdentifier) // Private mutable getters for use within the network interface impl NodeNetworkInterface { fn network_mut(&mut self, network_path: &[NodeId]) -> Option<&mut NodeNetwork> { - self.network.nested_network_mut(network_path) + self.document_network_mut().nested_network_mut(network_path) } fn network_metadata_mut(&mut self, network_path: &[NodeId]) -> Option<&mut NodeNetworkMetadata> { diff --git a/editor/src/messages/portfolio/document/utility_types/nodes.rs b/editor/src/messages/portfolio/document/utility_types/nodes.rs index c120938a80..0e70f90fff 100644 --- a/editor/src/messages/portfolio/document/utility_types/nodes.rs +++ b/editor/src/messages/portfolio/document/utility_types/nodes.rs @@ -167,8 +167,8 @@ impl SelectedNodes { std::mem::replace(&mut self.0, new) } - pub fn filtered_selected_nodes(&self, node_ids: std::collections::HashSet) -> SelectedNodes { - SelectedNodes(self.0.iter().filter(|node_id| node_ids.contains(node_id)).cloned().collect()) + pub fn filtered_selected_nodes(&self, filter: impl Fn(&NodeId) -> bool) -> SelectedNodes { + SelectedNodes(self.0.iter().copied().filter(filter).collect()) } } diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index ee63999155..ff73192ef5 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -115,7 +115,7 @@ impl NodeGraphExecutor { /// Update the cached network if necessary. fn update_node_graph(&mut self, document: &mut DocumentMessageHandler, node_to_inspect: Option, ignore_hash: bool) -> Result<(), String> { - let network_hash = document.network_interface.document_network().current_hash(); + let network_hash = document.network_interface.network_hash(); // Refresh the graph when it changes or the inspect node changes if network_hash != self.node_graph_hash || self.previous_node_to_inspect != node_to_inspect || ignore_hash { let network = document.network_interface.document_network().clone(); diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index be798346a8..dc6265a2a7 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -9,7 +9,7 @@ pub use graphene_core::uuid::NodeId; pub use graphene_core::uuid::generate_uuid; use graphene_core::{Context, ContextDependencies, Cow, MemoHash, ProtoNodeIdentifier, Type}; use log::Metadata; -use rustc_hash::FxHashMap; +use rustc_hash::{FxBuildHasher, FxHashMap}; use std::collections::HashMap; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; @@ -551,9 +551,8 @@ impl PartialEq for NodeNetwork { /// Graph modification functions impl NodeNetwork { pub fn current_hash(&self) -> u64 { - let mut hasher = DefaultHasher::new(); - self.hash(&mut hasher); - hasher.finish() + use std::hash::BuildHasher; + FxBuildHasher.hash_one(self) } pub fn value_network(node: DocumentNode) -> Self {