Merge pull request #2122 from LLK/revert-1930-runtime-script-cache

Revert "Cache hat block information for the runtime"

src/engine/blocks-runtime-cache.js

@@ -1,78 +0,0 @@
-/**
- * @fileoverview
- * The BlocksRuntimeCache caches data about the top block of scripts so that
- * Runtime can iterate a targeted opcode and iterate the returned set faster.
- * Many top blocks need to match fields as well as opcode, since that matching
- * compares strings in uppercase we can go ahead and uppercase the cached value
- * so we don't need to in the future.
- */
-
-/**
- * A set of cached data about the top block of a script.
- * @param {Blocks} container - Container holding the block and related data
- * @param {string} blockId - Id for whose block data is cached in this instance
- */
-class RuntimeScriptCache {
-    constructor (container, blockId) {
-        /**
-         * Container with block data for blockId.
-         * @type {Blocks}
-         */
-        this.container = container;
-
-        /**
-         * ID for block this instance caches.
-         * @type {string}
-         */
-        this.blockId = blockId;
-
-        const block = container.getBlock(blockId);
-        const fields = container.getFields(block);
-
-        /**
-         * Formatted fields or fields of input blocks ready for comparison in
-         * runtime.
-         *
-         * This is a clone of parts of the targeted blocks. Changes to these
-         * clones are limited to copies under RuntimeScriptCache and will not
-         * appear in the original blocks in their container. This copy is
-         * modified changing the case of strings to uppercase. These uppercase
-         * values will be compared later by the VM.
-         * @type {object}
-         */
-        this.fieldsOfInputs = Object.assign({}, fields);
-        if (Object.keys(fields).length === 0) {
-            const inputs = container.getInputs(block);
-            for (const input in inputs) {
-                if (!inputs.hasOwnProperty(input)) continue;
-                const id = inputs[input].block;
-                const inputBlock = container.getBlock(id);
-                const inputFields = container.getFields(inputBlock);
-                Object.assign(this.fieldsOfInputs, inputFields);
-            }
-        }
-        for (const key in this.fieldsOfInputs) {
-            const field = this.fieldsOfInputs[key] = Object.assign({}, this.fieldsOfInputs[key]);
-            if (field.value.toUpperCase) {
-                field.value = field.value.toUpperCase();
-            }
-        }
-    }
-}
-
-/**
- * Get an array of scripts from a block container prefiltered to match opcode.
- * @param {Blocks} container - Container of blocks
- * @param {string} opcode - Opcode to filter top blocks by
- */
-exports.getScripts = function () {
-    throw new Error('blocks.js has not initialized BlocksRuntimeCache');
-};
-
-/**
- * Exposed RuntimeScriptCache class used by integration in blocks.js.
- * @private
- */
-exports._RuntimeScriptCache = RuntimeScriptCache;
-
-require('./blocks');

src/engine/blocks.js

@@ -5,7 +5,6 @@ const MonitorRecord = require('./monitor-record');
 const Clone = require('../util/clone');
 const {Map} = require('immutable');
 const BlocksExecuteCache = require('./blocks-execute-cache');
-const BlocksRuntimeCache = require('./blocks-runtime-cache');
 const log = require('../util/log');
 const Variable = require('./variable');
 const getMonitorIdForBlockWithArgs = require('../util/get-monitor-id');
@@ -75,13 +74,7 @@ class Blocks {
              * actively monitored.
              * @type {Array<{blockId: string, target: Target}>}
              */
-            _monitored: null,
+            _monitored: null
-
-            /**
-             * A cache of hat opcodes to collection of theads to execute.
-             * @type {object.<string, object>}
-             */
-            scripts: {}
         };
 
         /**
@@ -516,7 +509,6 @@ class Blocks {
         this._cache.procedureDefinitions = {};
         this._cache._executeCached = {};
         this._cache._monitored = null;
-        this._cache.scripts = {};
     }
 
     /**
@@ -1223,35 +1215,4 @@ BlocksExecuteCache.getCached = function (blocks, blockId, CacheType) {
     return cached;
 };
 
-/**
- * Cache class constructor for runtime. Used to consider what threads should
- * start based on hat data.
- * @type {function}
- */
-const RuntimeScriptCache = BlocksRuntimeCache._RuntimeScriptCache;
-
-/**
- * Get an array of scripts from a block container prefiltered to match opcode.
- * @param {Blocks} blocks - Container of blocks
- * @param {string} opcode - Opcode to filter top blocks by
- * @returns {Array.<RuntimeScriptCache>} - Array of RuntimeScriptCache cache
- *   objects
- */
-BlocksRuntimeCache.getScripts = function (blocks, opcode) {
-    let scripts = blocks._cache.scripts[opcode];
-    if (!scripts) {
-        scripts = blocks._cache.scripts[opcode] = [];
-
-        const allScripts = blocks._scripts;
-        for (let i = 0; i < allScripts.length; i++) {
-            const topBlockId = allScripts[i];
-            const block = blocks.getBlock(topBlockId);
-            if (block.opcode === opcode) {
-                scripts.push(new RuntimeScriptCache(blocks, topBlockId));
-            }
-        }
-    }
-    return scripts;
-};
-
 module.exports = Blocks;

src/engine/runtime.js

@@ -4,7 +4,6 @@ const escapeHtml = require('escape-html');
 
 const ArgumentType = require('../extension-support/argument-type');
 const Blocks = require('./blocks');
-const BlocksRuntimeCache = require('./blocks-runtime-cache');
 const BlockType = require('../extension-support/block-type');
 const Profiler = require('./profiler');
 const Sequencer = require('./sequencer');
@@ -1435,11 +1434,16 @@ class Runtime extends EventEmitter {
      * @return {!Thread} The newly created thread.
      */
     _pushThread (id, target, opts) {
+        opts = Object.assign({
+            stackClick: false,
+            updateMonitor: false
+        }, opts);
+
         const thread = new Thread(id);
         thread.target = target;
-        thread.stackClick = Boolean(opts && opts.stackClick);
+        thread.stackClick = opts.stackClick;
-        thread.updateMonitor = Boolean(opts && opts.updateMonitor);
+        thread.updateMonitor = opts.updateMonitor;
-        thread.blockContainer = thread.updateMonitor ?
+        thread.blockContainer = opts.updateMonitor ?
             this.monitorBlocks :
             target.blocks;
 
@@ -1582,20 +1586,6 @@ class Runtime extends EventEmitter {
         }
     }
 
-    allScriptsByOpcodeDo (opcode, f, optTarget) {
-        let targets = this.executableTargets;
-        if (optTarget) {
-            targets = [optTarget];
-        }
-        for (let t = targets.length - 1; t >= 0; t--) {
-            const target = targets[t];
-            const scripts = BlocksRuntimeCache.getScripts(target.blocks, opcode);
-            for (let j = 0; j < scripts.length; j++) {
-                f(scripts[j], target);
-            }
-        }
-    }
-
     /**
      * Start all relevant hats.
      * @param {!string} requestedHatOpcode Opcode of hats to start.
@@ -1620,52 +1610,71 @@ class Runtime extends EventEmitter {
         }
 
         // Consider all scripts, looking for hats with opcode `requestedHatOpcode`.
-        this.allScriptsByOpcodeDo(requestedHatOpcode, (script, target) => {
+        this.allScriptsDo((topBlockId, target) => {
-            const {
+            const blocks = target.blocks;
-                blockId: topBlockId,
+            const block = blocks.getBlock(topBlockId);
-                fieldsOfInputs: hatFields
+            const potentialHatOpcode = block.opcode;
-            } = script;
+            if (potentialHatOpcode !== requestedHatOpcode) {
+                // Not the right hat.
+                return;
+            }
 
             // Match any requested fields.
             // For example: ensures that broadcasts match.
             // This needs to happen before the block is evaluated
             // (i.e., before the predicate can be run) because "broadcast and wait"
             // needs to have a precise collection of started threads.
+            let hatFields = blocks.getFields(block);
+
+            // If no fields are present, check inputs (horizontal blocks)
+            if (Object.keys(hatFields).length === 0) {
+                hatFields = {}; // don't overwrite the block's actual fields list
+                const hatInputs = blocks.getInputs(block);
+                for (const input in hatInputs) {
+                    if (!hatInputs.hasOwnProperty(input)) continue;
+                    const id = hatInputs[input].block;
+                    const inpBlock = blocks.getBlock(id);
+                    const fields = blocks.getFields(inpBlock);
+                    Object.assign(hatFields, fields);
+                }
+            }
+
+            if (optMatchFields) {
                 for (const matchField in optMatchFields) {
-                if (hatFields[matchField].value !== optMatchFields[matchField]) {
+                    if (hatFields[matchField].value.toUpperCase() !==
+                        optMatchFields[matchField]) {
                         // Field mismatch.
                         return;
                     }
                 }
+            }
 
             if (hatMeta.restartExistingThreads) {
                 // If `restartExistingThreads` is true, we should stop
                 // any existing threads starting with the top block.
-                for (let i = 0; i < this.threads.length; i++) {
+                for (let i = 0; i < instance.threads.length; i++) {
-                    if (this.threads[i].target === target &&
+                    if (instance.threads[i].topBlock === topBlockId &&
-                        this.threads[i].topBlock === topBlockId &&
+                        !instance.threads[i].stackClick && // stack click threads and hat threads can coexist
-                        // stack click threads and hat threads can coexist
+                        instance.threads[i].target === target) {
-                        !this.threads[i].stackClick) {
+                        newThreads.push(instance._restartThread(instance.threads[i]));
-                        newThreads.push(this._restartThread(this.threads[i]));
                         return;
                     }
                 }
             } else {
                 // If `restartExistingThreads` is false, we should
                 // give up if any threads with the top block are running.
-                for (let j = 0; j < this.threads.length; j++) {
+                for (let j = 0; j < instance.threads.length; j++) {
-                    if (this.threads[j].target === target &&
+                    if (instance.threads[j].topBlock === topBlockId &&
-                        this.threads[j].topBlock === topBlockId &&
+                        instance.threads[j].target === target &&
-                        // stack click threads and hat threads can coexist
+                        !instance.threads[j].stackClick && // stack click threads and hat threads can coexist
-                        !this.threads[j].stackClick &&
+                        instance.threads[j].status !== Thread.STATUS_DONE) {
-                        this.threads[j].status !== Thread.STATUS_DONE) {
                         // Some thread is already running.
                         return;
                     }
                 }
             }
             // Start the thread with this top block.
-            newThreads.push(this._pushThread(topBlockId, target));
+            newThreads.push(instance._pushThread(topBlockId, target));
         }, optTarget);
         // For compatibility with Scratch 2, edge triggered hats need to be processed before
         // threads are stepped. See ScratchRuntime.as for original implementation

test/integration/hat-threads-run-every-frame.js

@@ -64,52 +64,6 @@ test('edge activated hat thread runs once every frame', t => {
     });
 });
 
-/**
- * When a hat is added it should run in the next frame. Any block related
- * caching should be reset.
- */
-test('edge activated hat thread runs after being added to previously executed target', t => {
-    const vm = new VirtualMachine();
-    vm.attachStorage(makeTestStorage());
-
-    // Start VM, load project, and run
-    t.doesNotThrow(() => {
-        // Note: don't run vm.start(), we handle calling _step() manually in this test
-        vm.runtime.currentStepTime = Runtime.THREAD_STEP_INTERVAL;
-        vm.clear();
-        vm.setCompatibilityMode(false);
-        vm.setTurboMode(false);
-
-        vm.loadProject(project).then(() => {
-            t.equal(vm.runtime.threads.length, 0);
-
-            vm.runtime._step();
-            let threads = vm.runtime._lastStepDoneThreads;
-            t.equal(vm.runtime.threads.length, 0);
-            t.equal(threads.length, 1);
-            checkIsHatThread(t, vm, threads[0]);
-            t.assert(threads[0].status === Thread.STATUS_DONE);
-
-            // Add a second hat that should create a second thread
-            const hatBlock = threads[0].target.blocks.getBlock(threads[0].topBlock);
-            threads[0].target.blocks.createBlock(Object.assign(
-                {}, hatBlock, {id: 'hatblock2', next: null}
-            ));
-
-            // Check that the hat thread is added again when another step is taken
-            vm.runtime._step();
-            threads = vm.runtime._lastStepDoneThreads;
-            t.equal(vm.runtime.threads.length, 0);
-            t.equal(threads.length, 2);
-            checkIsHatThread(t, vm, threads[0]);
-            checkIsHatThread(t, vm, threads[1]);
-            t.assert(threads[0].status === Thread.STATUS_DONE);
-            t.assert(threads[1].status === Thread.STATUS_DONE);
-            t.end();
-        });
-    });
-});
-
 /**
  * If the hat doesn't finish evaluating within one frame, it shouldn't be added again
  * on the next frame. (We skip execution by setting the step time to 0)