Skip to content

Add profiling metrics for, and speed up, Graphene graph compilation #1924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 12, 2024
Merged
Prev Previous commit
Next Next commit
Implement node input replacement batching
isometric-fountain      time:   [7.4307 ms 7.5072 ms 7.5974 ms]
                        change: [-19.302% -18.136% -16.903%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe

painted-dreams          time:   [1.8108 ms 1.8223 ms 1.8350 ms]
                        change: [-12.422% -11.524% -10.650%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  5 (5.00%) high mild
  8 (8.00%) high severe

procedural-string-lights
                        time:   [551.65 µs 560.58 µs 571.13 µs]
                        change: [-5.7783% -2.5770% +1.3136%] (p = 0.20 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) high mild
  5 (5.00%) high severe

red-dress               time:   [9.7951 ms 9.9006 ms 10.016 ms]
                        change: [-18.812% -17.558% -16.292%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild

valley-of-spires        time:   [4.7294 ms 4.7837 ms 4.8442 ms]
                        change: [-16.889% -15.712% -14.615%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 16 outliers among 100 measurements (16.00%)
  11 (11.00%) high mild
  5 (5.00%) high severe
  • Loading branch information
TrueDoctor committed Aug 12, 2024
commit 93d7f9d5c42b6c937ffa2152c34e963a6415d34a
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
target/
*.spv
*.exrc
perf.data*
profile.json
10 changes: 5 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ graphite-editor = { opt-level = 1 }
graphene-core = { opt-level = 1 }
graphene-std = { opt-level = 1 }
interpreted-executor = { opt-level = 1 } # This is a mitigation for https://github.com/rustwasm/wasm-pack/issues/981 which is needed because the node_registry function is too large
graphite-proc-macros = { opt-level = 3 }
graphite-proc-macros = { opt-level = 1 }
autoquant = { opt-level = 3 }
image = { opt-level = 3 }
image = { opt-level = 2 }
rustc-hash = { opt-level = 3 }
serde_derive = { opt-level = 3 }
specta-macros = { opt-level = 3 }
syn = { opt-level = 3 }
serde_derive = { opt-level = 1 }
specta-macros = { opt-level = 1 }
syn = { opt-level = 1 }

[profile.release]
lto = "thin"
Expand Down
3 changes: 0 additions & 3 deletions node-graph/graph-craft/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ graph-craft = { workspace = true, features = ["serde"] }
name = "compile_demo_art"
harness = false

[profile.release]
debug=true

# [[bench]]
# name = "exec_demo_art"
# harness = false
84 changes: 28 additions & 56 deletions node-graph/graph-craft/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use glam::IVec2;
use graphene_core::memo::MemoHashGuard;
pub use graphene_core::uuid::generate_uuid;
use graphene_core::{Cow, MemoHash, ProtoNodeIdentifier, Type};
use rustc_hash::FxBuildHasher;

use std::collections::hash_map::DefaultHasher;
use std::collections::HashMap;
Expand Down Expand Up @@ -772,49 +773,6 @@ impl NodeNetwork {
}
}

/// A graph with just an input node
// pub fn new_network() -> Self {
// Self {
// exports: vec![NodeInput::node(NodeId(0), 0)],
// nodes: [(
// NodeId(0),
// DocumentNode {
// name: "Input Frame".into(),
// manual_composition: Some(concrete!(u32)),
// implementation: DocumentNodeImplementation::ProtoNode("graphene_core::ops::IdentityNode".into()),
// metadata: DocumentNodeMetadata { position: (8, 4).into() },
// ..Default::default()
// },
// )]
// .into_iter()
// .collect(),
// ..Default::default()
// }
// }

/// Appends a new node to the network after the output node and sets it as the new output
// pub fn push_node_to_document_network(&mut self, mut node: DocumentNode) -> NodeId {
// let id = NodeId(self.nodes.len().try_into().expect("Too many nodes in network"));
// // Set the correct position for the new node
// if node.metadata().position == IVec2::default() {
// if let Some(pos) = self.get_root_node().and_then(|root_node| self.nodes.get(&root_node.id)).map(|n| n.metadata().position) {
// node.metadata().position = pos + IVec2::new(8, 0);
// }
// }
// if !self.exports.is_empty() {
// let input = self.exports[0].clone();
// if node.inputs.is_empty() {
// node.inputs.push(input);
// } else {
// node.inputs[0] = input;
// }
// }
// // Use node_graph.insert_node
// self.insert_node(id, node);
// self.exports = vec![NodeInput::node(id, 0)];
// id
// }

/// Get the nested network given by the path of node ids
pub fn nested_network(&self, nested_path: &[NodeId]) -> Option<&Self> {
let mut network = Some(self);
Expand Down Expand Up @@ -868,6 +826,8 @@ impl NodeNetwork {
}
}

type SubstitutionMap = HashMap<(NodeId, usize), (NodeId, usize), FxBuildHasher>;

/// Functions for compiling the network
impl NodeNetwork {
/// Replace all references in the graph of a node ID with a new node ID defined by the function `f`.
Expand Down Expand Up @@ -910,11 +870,21 @@ impl NodeNetwork {
}

/// Replace all references in any node of `old_input` with `new_input`
fn replace_node_inputs(&mut self, old_input: NodeInput, new_input: NodeInput) {
fn replace_node_inputs(&mut self, substitutions: &SubstitutionMap) {
for node in self.nodes.values_mut() {
node.inputs.iter_mut().for_each(|input| {
if *input == old_input {
*input = new_input.clone();
if let NodeInput::Node {
ref mut node_id,
ref mut output_index,
..
} = input
{
let mut lookup = (*node_id, *output_index);
lookup = loop {
let Some(new_lookup) = substitutions.get(&lookup) else { break lookup };
lookup = *new_lookup
};
(*node_id, *output_index) = lookup;
}
});
}
Expand Down Expand Up @@ -988,11 +958,13 @@ impl NodeNetwork {

/// Remove all nodes that contain [`DocumentNodeImplementation::Network`] by moving the nested nodes into the parent network.
pub fn flatten(&mut self, node_id: NodeId) {
self.flatten_with_fns(node_id, merge_ids, || NodeId(generate_uuid()))
let mut subtitutions = HashMap::with_capacity_and_hasher(self.nodes.len(), FxBuildHasher);
self.flatten_with_fns(node_id, merge_ids, || NodeId(generate_uuid()), &mut subtitutions);
self.replace_node_inputs(&subtitutions);
}

/// Remove all nodes that contain [`DocumentNodeImplementation::Network`] by moving the nested nodes into the parent network.
pub fn flatten_with_fns(&mut self, node_id: NodeId, map_ids: impl Fn(NodeId, NodeId) -> NodeId + Copy, gen_id: impl Fn() -> NodeId + Copy) {
pub fn flatten_with_fns(&mut self, node_id: NodeId, map_ids: impl Fn(NodeId, NodeId) -> NodeId + Copy, gen_id: impl Fn() -> NodeId + Copy, substitutions: &mut SubstitutionMap) {
let Some((id, mut node)) = self.nodes.remove_entry(&node_id) else {
warn!("The node which was supposed to be flattened does not exist in the network, id {node_id} network {self:#?}");
return;
Expand Down Expand Up @@ -1146,18 +1118,15 @@ impl NodeNetwork {

// Connect all nodes that were previously connected to this node to the nodes of the inner network
for (i, export) in inner_network.exports.into_iter().enumerate() {
let node_input = |node_id, output_index, lambda| NodeInput::Node { node_id, output_index, lambda };

if let NodeInput::Node { node_id, output_index, .. } = &export {
self.replace_node_inputs(node_input(id, i, false), node_input(*node_id, *output_index, false));
self.replace_node_inputs(node_input(id, i, true), node_input(*node_id, *output_index, true));
substitutions.insert((id, i), (*node_id, *output_index));
}

self.replace_network_outputs(NodeInput::node(id, i), export);
}

for node_id in new_nodes {
self.flatten_with_fns(node_id, map_ids, gen_id);
self.flatten_with_fns(node_id, map_ids, gen_id, substitutions);
}
} else {
// If the node is not a network, it is a primitive node and can be inserted into the network as is.
Expand Down Expand Up @@ -1332,6 +1301,7 @@ mod test {
use crate::proto::{ConstructionArgs, ProtoNetwork, ProtoNode, ProtoNodeInput};

use graphene_core::ProtoNodeIdentifier;
use rustc_hash::FxHashMap;

use std::sync::atomic::AtomicU64;

Expand Down Expand Up @@ -1446,7 +1416,7 @@ mod test {
..Default::default()
};
network.generate_node_paths(&[]);
network.flatten_with_fns(NodeId(1), |self_id, inner_id| NodeId(self_id.0 * 10 + inner_id.0), gen_node_id);
network.flatten_with_fns(NodeId(1), |self_id, inner_id| NodeId(self_id.0 * 10 + inner_id.0), gen_node_id, &mut FxHashMap::default());
let flat_network = flat_network();
println!("{flat_network:#?}");
println!("{network:#?}");
Expand Down Expand Up @@ -1642,8 +1612,10 @@ mod test {
..Default::default()
};
let _new_ids = 101..;
network.flatten_with_fns(NodeId(1), |self_id, inner_id| NodeId(self_id.0 * 10 + inner_id.0), || NodeId(10000));
network.flatten_with_fns(NodeId(2), |self_id, inner_id| NodeId(self_id.0 * 10 + inner_id.0), || NodeId(10001));
let mut substitutions = FxHashMap::default();
network.flatten_with_fns(NodeId(1), |self_id, inner_id| NodeId(self_id.0 * 10 + inner_id.0), || NodeId(10000), &mut substitutions);
network.flatten_with_fns(NodeId(2), |self_id, inner_id| NodeId(self_id.0 * 10 + inner_id.0), || NodeId(10001), &mut substitutions);
network.replace_node_inputs(&substitutions);
network.remove_dead_nodes(0);
network
}
Expand Down
2 changes: 1 addition & 1 deletion shell.nix
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ let
};

# Define the rustc we need
rustc-wasm = pkgs.rust-bin.nightly.latest.default.override {
rustc-wasm = pkgs.rust-bin.stable.latest.default.override {
targets = [ "wasm32-unknown-unknown" ];
# wasm-pack needs this
extensions = [ "rust-src" "rust-analyzer" "clippy"];
Expand Down