From 5f6bfe0a92a85aba32d3ed6d03c9c24cfe38fd1b Mon Sep 17 00:00:00 2001 From: "Evan W. Patton" Date: Wed, 18 Oct 2017 17:23:38 -0400 Subject: [PATCH] Fix performance when opening mutator with many entries This commit makes the following changes: 1. Improves the mutator code for use under the App Inventor blocks rendering algorithm which walks the blocks tree starting from a given node. Iterating over children resulted in an O(n^2) performance. 2. Prevents events from firing when the mutator is first opened, which prevents superfluous rerendering operations before any changes have been made by the user. 3. Defers resizing the mutator workspace on every block change to only occur once at the end of the current JavaScript execution context via setTimeout(). Fixes mit-cml/appinventor-sources#959 --- core/events.js | 18 ++++++++++++++---- core/mutator.js | 33 ++++++++++++++++++++++++++++----- core/workspace_svg.js | 19 +++++++++++-------- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/core/events.js b/core/events.js index 12766f220..b117b4eda 100644 --- a/core/events.js +++ b/core/events.js @@ -90,6 +90,13 @@ Blockly.Events.UI = 'ui'; */ Blockly.Events.FIRE_QUEUE_ = []; +/** + * Pending timeout, if any, for event firing. + * @type {?number} + * @private + */ +Blockly.Events.FIRE_TIMER_ = undefined; + /** * Create a custom event and fire it. * @param {!Blockly.Events.Abstract} event Custom data for event. @@ -98,11 +105,14 @@ Blockly.Events.fire = function(event) { if (!Blockly.Events.isEnabled()) { return; } - if (!Blockly.Events.FIRE_QUEUE_.length) { - // First event added; schedule a firing of the event queue. - setTimeout(Blockly.Events.fireNow_, 0); - } Blockly.Events.FIRE_QUEUE_.push(event); + if (!Blockly.Events.FIRE_TIMER_) { + // First event added; schedule a firing of the event queue. + Blockly.Events.FIRE_TIMER_ = setTimeout(function() { + Blockly.Events.FIRE_TIMER_ = undefined; + Blockly.Events.fireNow_(); + }); + } }; /** diff --git a/core/mutator.js b/core/mutator.js index 6881c5dce..f241b305f 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -171,6 +171,16 @@ Blockly.Mutator.prototype.updateEditable = function() { Blockly.Icon.prototype.updateEditable.call(this); }; +Blockly.Mutator.prototype.scheduleResizeBubble = function() { + if (!this.pendingBubbleResize_) { + var self = this; + this.pendingBubbleResize_ = setTimeout(function() { + self.workspace_ && self.resizeBubble_(); + self.pendingBubbleResize_ = undefined; + }); + } +} + /** * Callback function triggered when the bubble has resized. * Resize the workspace accordingly. @@ -224,6 +234,8 @@ Blockly.Mutator.prototype.setVisible = function(visible) { Blockly.Events.fire( new Blockly.Events.Ui(this.block_, 'mutatorOpen', !visible, visible)); if (visible) { + Blockly.Field.startCache(); + Blockly.Events.disable(); // Create the bubble. this.bubble_ = new Blockly.Bubble( /** @type {!Blockly.WorkspaceSvg} */ (this.block_.workspace), @@ -235,10 +247,7 @@ Blockly.Mutator.prototype.setVisible = function(visible) { } this.rootBlock_ = this.block_.decompose(this.workspace_); - var blocks = this.rootBlock_.getDescendants(); - for (var i = 0, child; child = blocks[i]; i++) { - child.render(); - } + this.renderWorkspace(); // The root block should not be dragable or deletable. this.rootBlock_.setMovable(false); this.rootBlock_.setDeletable(false); @@ -266,6 +275,8 @@ Blockly.Mutator.prototype.setVisible = function(visible) { // When the mutator's workspace changes, update the source block. this.workspace_.addChangeListener(this.workspaceChanged_.bind(this)); this.updateColour(); + Blockly.Events.enable(); + Blockly.Field.stopCache(); } else { // Dispose of the bubble. this.svgDialog_ = null; @@ -283,6 +294,16 @@ Blockly.Mutator.prototype.setVisible = function(visible) { } }; +/** + * Renders the mutator's workspace, starting from the root block. + */ +Blockly.Mutator.prototype.renderWorkspace = function() { + var blocks = this.rootBlock_.getDescendants(); + for (var i = 0, child; child = blocks[i]; i++) { + child.render(); + } +}; + /** * Update the source block when the mutator's blocks are changed. * Bump down any block that's too high. @@ -305,6 +326,7 @@ Blockly.Mutator.prototype.workspaceChanged_ = function() { // When the mutator's workspace changes, update the source block. if (this.rootBlock_.workspace == this.workspace_) { + Blockly.Field.startCache(); Blockly.Events.setGroup(true); var block = this.block_; var oldMutationDom = block.mutationToDom(); @@ -334,8 +356,9 @@ Blockly.Mutator.prototype.workspaceChanged_ = function() { if (block.rendered) { block.render(); } - this.resizeBubble_(); + this.scheduleResizeBubble(); Blockly.Events.setGroup(false); + Blockly.Field.stopCache(); } }; diff --git a/core/workspace_svg.js b/core/workspace_svg.js index 96cb15128..a5da8f59b 100644 --- a/core/workspace_svg.js +++ b/core/workspace_svg.js @@ -521,16 +521,19 @@ Blockly.WorkspaceSvg.prototype.updateScreenCalculations_ = function() { * @package */ Blockly.WorkspaceSvg.prototype.resizeContents = function() { - if (!this.resizesEnabled_ || !this.rendered) { + if (!this.resizesEnabled_ || !this.rendered || this.pendingResize_) { return; } - if (this.scrollbar) { - // TODO(picklesrus): Once rachel-fenichel's scrollbar refactoring - // is complete, call the method that only resizes scrollbar - // based on contents. - this.scrollbar.resize(); - } - this.updateInverseScreenCTM(); + this.pendingResize_ = setTimeout((function() { + if (this.scrollbar) { + // TODO(picklesrus): Once rachel-fenichel's scrollbar refactoring + // is complete, call the method that only resizes scrollbar + // based on contents. + this.scrollbar.resize(); + } + this.updateInverseScreenCTM(); + this.pendingResize_ = undefined; + }).bind(this)); }; /**