Coalesce graph editor view invalidations (#6786)

Only invalidate the graph editor view at most once per frame. On develop, this saves about 70ms (2%). Testing a recent backend without #6755 as a stress-test, this saves about 5s (45%). This reflects better scalability to large numbers of `SuggestionUpdate` messages.

Fixes #6630.

# Important Notes
- Also fix intermittent profiling failures occurring since the introduction of microtasks.
This commit is contained in:
Kaz Wesley 2023-05-29 07:39:20 -07:00 committed by GitHub
parent 6eb4737330
commit 7e6a919737
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 14 additions and 3 deletions

View File

@ -448,6 +448,7 @@ impl Model {
} }
/// Look through all graph's nodes in AST and set position where it is missing. /// Look through all graph's nodes in AST and set position where it is missing.
#[profile(Debug)]
fn initialize_nodes_positions(&self, default_gap_between_nodes: f32) { fn initialize_nodes_positions(&self, default_gap_between_nodes: f32) {
match self.controller.graph().nodes() { match self.controller.graph().nodes() {
Ok(nodes) => { Ok(nodes) => {
@ -562,6 +563,7 @@ struct ViewUpdate {
impl ViewUpdate { impl ViewUpdate {
/// Create ViewUpdate information from Graph Presenter's model. /// Create ViewUpdate information from Graph Presenter's model.
#[profile(Debug)]
fn new(model: &Model) -> FallibleResult<Self> { fn new(model: &Model) -> FallibleResult<Self> {
let state = model.state.clone_ref(); let state = model.state.clone_ref();
let nodes = model.controller.graph().nodes()?; let nodes = model.controller.graph().nodes()?;
@ -572,11 +574,13 @@ impl ViewUpdate {
} }
/// Remove nodes from the state and return node views to be removed. /// Remove nodes from the state and return node views to be removed.
#[profile(Debug)]
fn remove_nodes(&self) -> Vec<ViewNodeId> { fn remove_nodes(&self) -> Vec<ViewNodeId> {
self.state.update_from_controller().retain_nodes(&self.node_ids().collect()) self.state.update_from_controller().retain_nodes(&self.node_ids().collect())
} }
/// Returns number of nodes view should create. /// Returns number of nodes view should create.
#[profile(Debug)]
fn count_nodes_to_add(&self) -> usize { fn count_nodes_to_add(&self) -> usize {
self.node_ids().filter(|n| self.state.view_id_of_ast_node(*n).is_none()).count() self.node_ids().filter(|n| self.state.view_id_of_ast_node(*n).is_none()).count()
} }
@ -616,6 +620,7 @@ impl ViewUpdate {
/// input for nodes where position changed. /// input for nodes where position changed.
/// ///
/// The nodes not having views are also updated in the state. /// The nodes not having views are also updated in the state.
#[profile(Debug)]
fn set_node_positions(&self) -> Vec<(ViewNodeId, Vector2)> { fn set_node_positions(&self) -> Vec<(ViewNodeId, Vector2)> {
self.nodes self.nodes
.iter() .iter()
@ -629,6 +634,7 @@ impl ViewUpdate {
.collect() .collect()
} }
#[profile(Debug)]
fn set_node_visualizations(&self) -> Vec<(ViewNodeId, Option<visualization_view::Path>)> { fn set_node_visualizations(&self) -> Vec<(ViewNodeId, Option<visualization_view::Path>)> {
self.nodes self.nodes
.iter() .iter()
@ -640,11 +646,13 @@ impl ViewUpdate {
} }
/// Remove connections from the state and return views to be removed. /// Remove connections from the state and return views to be removed.
#[profile(Debug)]
fn remove_connections(&self) -> Vec<ViewConnection> { fn remove_connections(&self) -> Vec<ViewConnection> {
self.state.update_from_controller().retain_connections(&self.connections) self.state.update_from_controller().retain_connections(&self.connections)
} }
/// Add connections to the state and return endpoints of connections to be created in views. /// Add connections to the state and return endpoints of connections to be created in views.
#[profile(Debug)]
fn add_connections(&self) -> Vec<(EdgeEndpoint, EdgeEndpoint)> { fn add_connections(&self) -> Vec<(EdgeEndpoint, EdgeEndpoint)> {
let ast_conns = self.connections.iter(); let ast_conns = self.connections.iter();
ast_conns ast_conns
@ -654,6 +662,7 @@ impl ViewUpdate {
.collect() .collect()
} }
#[profile(Debug)]
fn node_ids(&self) -> impl Iterator<Item = AstNodeId> + '_ { fn node_ids(&self) -> impl Iterator<Item = AstNodeId> + '_ {
self.nodes.iter().map(controller::graph::Node::id) self.nodes.iter().map(controller::graph::Node::id)
} }
@ -699,10 +708,11 @@ impl Graph {
frp::extend! { network frp::extend! { network
update_view <- source::<()>(); update_view <- source::<()>();
update_view_once <- update_view.debounce();
// Position initialization should go before emitting `update_data` event. // Position initialization should go before emitting `update_data` event.
update_with_gap <- view.default_y_gap_between_nodes.sample(&update_view); update_with_gap <- view.default_y_gap_between_nodes.sample(&update_view_once);
eval update_with_gap ((gap) model.initialize_nodes_positions(*gap)); eval update_with_gap ((gap) model.initialize_nodes_positions(*gap));
update_data <- update_view.map(f_!([model] match ViewUpdate::new(&model) { update_data <- update_view_once.map(f_!([model] match ViewUpdate::new(&model) {
Ok(update) => Rc::new(update), Ok(update) => Rc::new(update),
Err(err) => { Err(err) => {
error!("Failed to update view: {err:?}"); error!("Failed to update view: {err:?}");

View File

@ -2029,6 +2029,7 @@ impl GraphEditorModel {
/// Warning! This function does not remove connected edges. It needs to be handled by the /// Warning! This function does not remove connected edges. It needs to be handled by the
/// implementation. /// implementation.
#[profile(Debug)]
fn remove_node(&self, node_id: impl Into<NodeId>) { fn remove_node(&self, node_id: impl Into<NodeId>) {
let node_id = node_id.into(); let node_id = node_id.into();
self.nodes.remove(&node_id); self.nodes.remove(&node_id);
@ -2276,6 +2277,7 @@ impl GraphEditorModel {
impl GraphEditorModel { impl GraphEditorModel {
#[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs. #[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs.
#[profile(Debug)]
pub fn set_node_position(&self, node_id: impl Into<NodeId>, position: Vector2) { pub fn set_node_position(&self, node_id: impl Into<NodeId>, position: Vector2) {
let node_id = node_id.into(); let node_id = node_id.into();
if let Some(node) = self.nodes.get_cloned_ref(&node_id) { if let Some(node) = self.nodes.get_cloned_ref(&node_id) {

View File

@ -407,7 +407,6 @@ fn on_frame_closure(
.then(move || on_after_animations.emit(time_info)) .then(move || on_after_animations.emit(time_info))
.then(move || on_before_layout.emit(time_info)) .then(move || on_before_layout.emit(time_info))
.then(move || on_before_rendering.emit(time_info)) .then(move || on_before_rendering.emit(time_info))
.then(move || drop(_profiler))
.schedule(); .schedule();
} }
} }