Optimize: Replace O(N) blockList based lookups with O(1) blockIndex property

Author: Ashutoshx7

js/activity.js

@@ -7133,7 +7133,7 @@ class Activity {
                     pasteDy = 0;
                 const map = new Map();
                 for (let i = 0; i < blocksArray.length; i++) {
-                    const idx = this.blocks.blockList.indexOf(blocksArray[i]);
+                    const idx = blocksArray[i].blockIndex;
                     map.set(
                         idx,
                         blocksArray[i].connections.filter(blk => blk !== null)

js/block.js

@@ -274,6 +274,11 @@ class Block {
         this.name = protoblock.name;
         this.activity = this.blocks.activity;
 
+        // Cached index in blocks.blockList — set when block is added
+        // to blockList. Avoids O(N) blockList.indexOf() lookups that
+        // are scattered across the codebase (33+ call sites).
+        this.blockIndex = -1;
+
         this.collapsed = false; // Is this collapsible block collapsed?
         this.inCollapsed = false; // Is this block in a collapsed stack?
         this.trash = false; // Is this block in the trash?
@@ -1039,7 +1044,7 @@ class Block {
     generateArtwork(firstTime) {
         // Get the block labels from the protoblock.
         const that = this;
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
         let block_label = "";
 
         /**
@@ -1071,7 +1076,7 @@ class Block {
 
             const __callback = (that, firstTime) => {
                 that.activity.refreshCanvas();
-                const thisBlock = that.blocks.blockList.indexOf(that);
+                const thisBlock = that.blockIndex;
 
                 if (firstTime) {
                     that._loadEventHandlers();
@@ -1289,7 +1294,7 @@ class Block {
             artwork = artwork.replace("arg_label_" + i, this.protoblock.staticLabels[i]);
         }
 
-        that.blocks.blockArt[that.blocks.blockList.indexOf(that)] = artwork;
+        that.blocks.blockArt[that.blockIndex] = artwork;
 
         _blockMakeBitmap(artwork, __processBitmap, this);
     }
@@ -1300,7 +1305,7 @@ class Block {
      * @returns {void}
      */
     _finishImageLoad() {
-        // const thisBlock = this.blocks.blockList.indexOf(this);
+        // const thisBlock = this.blockIndex;
         let proto, obj, label, attr;
         // Value blocks get a modifiable text label.
         if (SPECIALINPUTS.includes(this.name)) {
@@ -1490,7 +1495,7 @@ class Block {
      * @returns {void}
      */
     _generateCollapseArtwork(postProcess) {
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
 
         /**
          * Run the postprocess function after the artwork is loaded.
@@ -1746,7 +1751,7 @@ class Block {
             that._ensureDecorationOnTop();
 
             // Save the collapsed block artwork for export.
-            that.blocks.blockCollapseArt[that.blocks.blockList.indexOf(that)] = that.collapseArtwork
+            that.blocks.blockCollapseArt[that.blockIndex] = that.collapseArtwork
                 .replace(/fill_color/g, PALETTEFILLCOLORS[that.protoblock.palette.name])
                 .replace(/stroke_color/g, PALETTESTROKECOLORS[that.protoblock.palette.name])
                 .replace("block_label", safeSVG(that.collapseText.text));
@@ -2076,7 +2081,7 @@ class Block {
      */
     getBlockId() {
         // Generate a UID based on the block index into the blockList.
-        const number = blockBlocks.blockList.indexOf(this);
+        const number = this.blockIndex;
         return "_" + number.toString();
     }
 
@@ -2099,7 +2104,7 @@ class Block {
      */
     loadThumbnail(imagePath) {
         // Load an image thumbnail onto block.
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
         const that = this;
 
         if (this.blocks.blockList[thisBlock].value === null && imagePath === null) {
@@ -2236,7 +2241,7 @@ class Block {
     collapseToggle() {
         // Find the blocks to collapse/expand inside of a collapable
         // block.
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
         this.blocks.findDragGroup(thisBlock);
 
         if (this.collapseBlockBitmap === null) {
@@ -2355,7 +2360,7 @@ class Block {
         const intervals = [];
         let i = 0;
 
-        let c = this.blocks.blockList.indexOf(this),
+        let c = this.blockIndex,
             lastIntervalBlock;
         while (c !== null) {
             lastIntervalBlock = c;
@@ -2868,7 +2873,7 @@ class Block {
      */
     _loadEventHandlers() {
         const that = this;
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
 
         this._calculateBlockHitArea();
 
@@ -2890,6 +2895,12 @@ class Block {
         let locked = false;
         let getInput = window.hasMouse;
 
+        // Cached drag state: computed once at mousedown, reused during pressmove.
+        // This avoids redundant O(N) findDragGroup and O(D) rest2 chain walks
+        // on every mouse move event (which fires 60+ times per second).
+        let _cachedDragGroup = null;
+        let _dragHasRest2 = false;
+
         /**
          * Handles the click event on the block container.
          * @param {Event} event - The click event.
@@ -3012,7 +3023,7 @@ class Block {
             that.blocks.mouseDownTime = new Date().getTime();
 
             that.blocks.longPressTimeout = setTimeout(() => {
-                that.blocks.activeBlock = that.blocks.blockList.indexOf(that);
+                that.blocks.activeBlock = that.blockIndex;
                 that._triggerLongPress = true;
                 that.blocks.triggerLongPress();
             }, LONGPRESSTIME);
@@ -3056,6 +3067,26 @@ class Block {
                 x: Math.round(that.container.x - that.original.x),
                 y: Math.round(that.container.y - that.original.y)
             };
+
+            // Pre-compute the drag group and rest2 check once at mousedown.
+            // The drag group doesn't change during a drag (blocks only
+            // connect/disconnect on mouseup), so caching it avoids
+            // redundant O(N) recursive traversal on every mouse move.
+            that.blocks.findDragGroup(thisBlock);
+            _cachedDragGroup = that.blocks.dragGroup.slice();
+
+            // Pre-compute whether the chain below contains a rest2 block.
+            // This avoids walking the entire connection chain on every
+            // mouse move event.
+            _dragHasRest2 = false;
+            let checkBlock = that.blocks.blockList[that.connections[1]];
+            while (checkBlock != null) {
+                if (checkBlock?.name === "rest2") {
+                    _dragHasRest2 = true;
+                    break;
+                }
+                checkBlock = checkBlock?.blocks.blockList[checkBlock.connections[1]];
+            }
         });
 
         /**
@@ -3079,14 +3110,11 @@ class Block {
                 return;
             }
 
-            // Do not allow a stack of blocks to be dragged if the stack contains a silence block.
-            let block = that.blocks.blockList[that.connections[1]];
-            while (block != null) {
-                if (block?.name === "rest2") {
-                    this.activity.errorMsg(_("Silence block cannot be removed."), block);
-                    return;
-                }
-                block = block?.blocks.blockList[block.connections[1]];
+            // Use the cached rest2 check from mousedown instead of
+            // walking the chain on every mouse move event.
+            if (_dragHasRest2) {
+                this.activity.errorMsg(_("Silence block cannot be removed."), that);
+                return;
             }
 
             if (window.hasMouse) {
@@ -3138,6 +3166,10 @@ class Block {
                 that.label.style.display = "none";
             }
 
+            // Defer checkBounds during the entire block movement to avoid
+            // N separate checkBounds calls (one per block in the drag group).
+            that.blocks._beginDeferCheckBounds();
+
             that.blocks.moveBlockRelative(thisBlock, dx, dy);
 
             // If we are over the trash, warn the user.
@@ -3157,17 +3189,19 @@ class Block {
                 that.container.setChildIndex(that.text, that.container.children.length - 1);
             }
 
-            // ...and move any connected blocks.
-            that.blocks.findDragGroup(thisBlock);
-            if (that.blocks.dragGroup.length > 0) {
-                for (let b = 0; b < that.blocks.dragGroup.length; b++) {
-                    const blk = that.blocks.dragGroup[b];
+            // Move connected blocks using the cached drag group from
+            // mousedown, avoiding redundant O(N) recursive traversal.
+            if (_cachedDragGroup !== null && _cachedDragGroup.length > 0) {
+                for (let b = 0; b < _cachedDragGroup.length; b++) {
+                    const blk = _cachedDragGroup[b];
                     if (b !== 0) {
                         that.blocks.moveBlockRelative(blk, dx, dy);
                     }
                 }
             }
 
+            that.blocks._endDeferCheckBounds();
+
             that.activity.refreshCanvas();
         });
 
@@ -3191,6 +3225,9 @@ class Block {
             }
             that.blocks.activeBlock = null;
 
+            // Clear cached drag state.
+            _cachedDragGroup = null;
+            _dragHasRest2 = false;
             moved = false;
         });
 
@@ -3213,6 +3250,9 @@ class Block {
             that.blocks.unhighlight(thisBlock, true);
             that.blocks.activeBlock = null;
 
+            // Clear cached drag state.
+            _cachedDragGroup = null;
+            _dragHasRest2 = false;
             moved = false;
         });
     }
@@ -3228,7 +3268,7 @@ class Block {
      * @returns {void}
      */
     _mouseoutCallback(event, moved, haveClick, hideDOM) {
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
         if (!this.activity.logo.runningLilypond) {
             document.body.style.cursor = "default";
         }
@@ -3270,7 +3310,7 @@ class Block {
                 // Just in case the blocks are not properly docked after
                 // the move (workaround for issue #38 -- Blocks fly
                 // apart). Still need to get to the root cause.
-                this.blocks.adjustDocks(this.blocks.blockList.indexOf(this), true);
+                this.blocks.adjustDocks(this.blockIndex, true);
             }
         } else if (SPECIALINPUTS.includes(this.name) || ["media", "loadFile"].includes(this.name)) {
             if (!haveClick) {
@@ -3328,7 +3368,7 @@ class Block {
         }
 
         // Numeric pie menus
-        const blk = this.blocks.blockList.indexOf(this);
+        const blk = this.blockIndex;
 
         if (this.blocks.octaveNumber(blk)) {
             return true;
@@ -3385,7 +3425,7 @@ class Block {
             return false;
         }
 
-        return this.blocks.blockList[cblk].connections[1] === this.blocks.blockList.indexOf(this);
+        return this.blocks.blockList[cblk].connections[1] === this.blockIndex;
     }
 
     /**
@@ -3407,7 +3447,7 @@ class Block {
             return false;
         }
 
-        return this.blocks.blockList[cblk].connections[2] === this.blocks.blockList.indexOf(this);
+        return this.blocks.blockList[cblk].connections[2] === this.blockIndex;
     }
 
     /**
@@ -3429,7 +3469,7 @@ class Block {
             return false;
         }
 
-        return this.blocks.blockList[cblk].connections[3] === this.blocks.blockList.indexOf(this);
+        return this.blocks.blockList[cblk].connections[3] === this.blockIndex;
     }
 
     /**
@@ -3976,7 +4016,7 @@ class Block {
         } else {
             // If the number block is connected to a pitch block, then
             // use the pie menu for octaves. Other special cases as well.
-            const blk = this.blocks.blockList.indexOf(this);
+            const blk = this.blockIndex;
             if (this.blocks.octaveNumber(blk)) {
                 piemenuNumber(this, [8, 7, 6, 5, 4, 3, 2, 1], this.value);
             } else if (this.blocks.noteValueNumber(blk, 2)) {
@@ -4124,7 +4164,7 @@ class Block {
             }
         }
 
-        // const blk = this.blocks.blockList.indexOf(this);
+        // const blk = this.blockIndex;
         if (!this._usePiemenu()) {
             let focused = false;
 
@@ -4257,7 +4297,7 @@ class Block {
     _checkWidgets(closeInput) {
         // Detect if label is changed, then reinit widget windows
         // if they are open.
-        const thisBlock = this.blocks.blockList.indexOf(this);
+        const thisBlock = this.blockIndex;
         const topBlock = this.blocks.findTopBlock(thisBlock);
         const widgetTitle = document.getElementsByClassName("wftTitle");
         let lockInit = false;
@@ -4436,7 +4476,7 @@ class Block {
             }
 
             if (isNaN(this.value)) {
-                const thisBlock = this.blocks.blockList.indexOf(this);
+                const thisBlock = this.blockIndex;
                 this.activity.errorMsg(newValue + ": " + _("Not a number"), thisBlock);
                 this.activity.refreshCanvas();
                 this.value = oldValue;
@@ -4447,15 +4487,15 @@ class Block {
                 this.blocks.blockList[cblk1].name === "pitch" &&
                 (this.value > 8 || this.value < 1)
             ) {
-                const thisBlock = this.blocks.blockList.indexOf(this);
+                const thisBlock = this.blockIndex;
                 this.activity.errorMsg(_("Octave value must be between 1 and 8."), thisBlock);
                 this.activity.refreshCanvas();
                 this.label.value = oldValue;
                 this.value = oldValue;
             }
 
             if (String(this.value).length > 10) {
-                const thisBlock = this.blocks.blockList.indexOf(this);
+                const thisBlock = this.blockIndex;
                 this.activity.errorMsg(_("Numbers can have at most 10 digits."), thisBlock);
                 this.activity.refreshCanvas();
                 this.label.value = oldValue;
@@ -4563,10 +4603,7 @@ class Block {
                     // Check to see which connection we are using in
                     // cblock.  We only do something if blk is attached to
                     // the name connection (1).
-                    if (
-                        cblock.connections[1] === this.blocks.blockList.indexOf(this) &&
-                        closeInput
-                    ) {
+                    if (cblock.connections[1] === this.blockIndex && closeInput) {
                         // If the label was the name of a storein, update the
                         // associated box this.blocks and the palette buttons.
                         if (this.value !== "box") {