Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Suppressing spurious type updates. #1445

Merged
merged 8 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@
load.][1413]
- [Fixed a case where IDE could lose connection to the backend after some
time.][1428]
- [Improved performance of the graph editor, particularly when opening a project
for a first time.][1445]

#### EnsoGL (rendering engine)

Expand Down Expand Up @@ -164,6 +166,7 @@ you can find their release notes
[1428]: https://github.com/enso-org/ide/pull/1428
[1438]: https://github.com/enso-org/ide/pull/1438
[1367]: https://github.com/enso-org/ide/pull/1367
[1445]: https://github.com/enso-org/ide/pull/1445

<br/>

Expand Down
41 changes: 29 additions & 12 deletions src/rust/ide/src/ide/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ struct Model {
node_views : RefCell<BiMap<ast::Id,graph_editor::NodeId>>,
node_view_by_expression : RefCell<HashMap<ast::Id,graph_editor::NodeId>>,
expression_views : RefCell<HashMap<graph_editor::NodeId,graph_editor::component::node::Expression>>,
expression_types : SharedHashMap<ExpressionId,Option<graph_editor::Type>>,
connection_views : RefCell<BiMap<controller::graph::Connection,graph_editor::EdgeId>>,
code_view : CloneRefCell<ensogl_text::Text>,
visualizations : SharedHashMap<graph_editor::NodeId,VisualizationId>,
Expand Down Expand Up @@ -427,13 +428,15 @@ impl Model {
let node_view_by_expression = default();
let connection_views = default();
let expression_views = default();
let expression_types = default();
let code_view = default();
let visualizations = default();
let error_visualizations = default();
let searcher = default();
let this = Model
{view,graph,text,searcher,node_views,expression_views,connection_views,code_view,logger
,visualization,visualizations,error_visualizations,project,node_view_by_expression};
{view,graph,text,searcher,node_views,expression_views,expression_types,connection_views
,code_view,logger,visualization,visualizations,error_visualizations,project
,node_view_by_expression};

this.init_project_name();
this.load_visualizations();
Expand Down Expand Up @@ -623,7 +626,9 @@ impl Model {
input_span_tree : trees.inputs,
output_span_tree : trees.outputs.unwrap_or_else(default)
};
if !self.expression_views.borrow().get(&id).contains(&&code_and_trees) {
let expression_changed =
!self.expression_views.borrow().get(&id).contains(&&code_and_trees);
if expression_changed {
for sub_expression in node.info.ast().iter_recursive() {
if let Some(expr_id) = sub_expression.id {
self.node_view_by_expression.borrow_mut().insert(expr_id,id);
Expand All @@ -636,7 +641,7 @@ impl Model {
// Set initially available type information on ports (identifiable expression's sub-parts).
for expression_part in node.info.expression().iter_recursive() {
if let Some(id) = expression_part.id {
self.refresh_computed_info(id);
self.refresh_computed_info(id,expression_changed);
}
}
}
Expand All @@ -645,7 +650,7 @@ impl Model {
fn refresh_computed_infos(&self, expressions_to_refresh:&[ExpressionId]) -> FallibleResult {
debug!(self.logger, "Refreshing type information for IDs: {expressions_to_refresh:?}.");
for id in expressions_to_refresh {
self.refresh_computed_info(*id)
self.refresh_computed_info(*id,false)
}
Ok(())
}
Expand All @@ -654,12 +659,12 @@ impl Model {
/// graph editor view.
///
/// The computed value information includes the expression type and the target method pointer.
fn refresh_computed_info(&self, id:ExpressionId) {
fn refresh_computed_info(&self, id:ExpressionId, force_type_info_refresh:bool) {
let info = self.lookup_computed_info(&id);
let info = info.as_ref();
let typename = info.and_then(|info| info.typename.clone().map(graph_editor::Type));
if let Some(node_id) = self.node_view_by_expression.borrow().get(&id).cloned() {
self.set_type(node_id,id,typename);
self.set_type(node_id,id,typename, force_type_info_refresh);
let method_pointer = info.and_then(|info| {
info.method_call.and_then(|entry_id| {
let opt_method = self.project.suggestion_db().lookup_method_ptr(entry_id).ok();
Expand All @@ -677,9 +682,20 @@ impl Model {
}

/// Set given type (or lack of such) on the given sub-expression.
fn set_type(&self, node_id:graph_editor::NodeId, id:ExpressionId, typename:Option<graph_editor::Type>) {
let event = (node_id,id,typename);
self.view.graph().frp.input.set_expression_usage_type.emit(&event);
fn set_type
( &self
, node_id : graph_editor::NodeId
, id : ExpressionId
, typename : Option<graph_editor::Type>
, force_refresh : bool
) {
// We suppress spurious type information updates here, as they were causing performance
// issues. See: https://github.com/enso-org/ide/issues/952
let previous_type_opt = self.expression_types.insert(id,typename.clone());
if force_refresh || previous_type_opt.as_ref() != Some(&typename) {
let event = (node_id,id,typename);
self.view.graph().frp.input.set_expression_usage_type.emit(&event);
}
}

/// Set given method pointer (or lack of such) on the given sub-expression.
Expand Down Expand Up @@ -851,7 +867,7 @@ impl Model {
use controller::graph::executed::Notification;
use controller::graph::Notification::*;

debug!(self.logger, "Received notification {notification:?}");
debug!(self.logger, "Received graph notification {notification:?}");
let result = match notification {
Some(Notification::Graph(Invalidate)) => self.on_graph_invalidated(),
Some(Notification::Graph(PortsUpdate)) => self.on_graph_expression_update(),
Expand All @@ -874,7 +890,7 @@ impl Model {
pub fn handle_text_notification(&self, notification:Option<controller::text::Notification>) {
use controller::text::Notification;

debug!(self.logger, "Received notification {notification:?}");
debug!(self.logger, "Received text notification {notification:?}");
let result = match notification {
Some(Notification::Invalidate) => self.on_text_invalidated(),
other => {
Expand All @@ -892,6 +908,7 @@ impl Model {
pub fn handle_searcher_notification(&self, notification:controller::searcher::Notification) {
use controller::searcher::Notification;
use controller::searcher::UserAction;
debug!(self.logger, "Received searcher notification {notification:?}");
match notification {
Notification::NewActionList => with(self.searcher.borrow(), |searcher| {
if let Some(searcher) = &*searcher {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ ensogl::define_endpoints! {
set_hover (bool),
set_connected (bool,Option<Type>),
set_parent_connected (bool),
set_definition_type (Option<Type>),
set_definition_type (Option<Type>),
set_usage_type (Option<Type>),
}

Expand Down
4 changes: 0 additions & 4 deletions src/rust/ide/view/graph-editor/src/component/type_coloring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,8 @@ use std::hash::Hasher;
/// parametrization, other mechanisms should be used. For example, `Point Float` and `Point Number`
/// should have similar colors, completely distinct from their parameter types.
pub fn compute(tp:&Type, styles:&StyleWatch) -> color::Lcha {
// FIXME: Left for performance debug purposes. See: https://github.com/enso-org/ide/issues/952
// println!("Type coloring for: '{}",tp.as_str());
let types_path = theme::code::types::overriden::HERE.path();
let type_path = types_path.into_subs(tp.as_str().split('.'));
// FIXME: Left for performance debug purposes. See: https://github.com/enso-org/ide/issues/952
// println!("Path: {:?}",type_path);
let hue = styles.get(type_path.sub("hue")).number_or_else(|| auto_hue(tp,styles));
let lightness = styles.get(type_path.sub("lightness")).number_or_else(||
styles.get_number_or(theme::code::types::lightness,0.85));
Expand Down
2 changes: 0 additions & 2 deletions src/rust/ide/view/graph-editor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3043,5 +3043,3 @@ impl display::Object for GraphEditor {
self.model.display_object()
}
}