From 107adad647e63be9de90c8d8864d34609ef0701f Mon Sep 17 00:00:00 2001 From: kchadha Date: Fri, 2 Feb 2018 12:42:09 -0500 Subject: [PATCH] Revert "Push reported" --- src/engine/blocks-execute-cache.js | 19 ---- src/engine/blocks.js | 40 +------ src/engine/execute.js | 175 +++++++++-------------------- src/engine/runtime.js | 4 - src/engine/sequencer.js | 6 +- src/engine/thread.js | 14 +-- test/unit/engine_sequencer.js | 3 +- test/unit/engine_thread.js | 4 +- 8 files changed, 66 insertions(+), 199 deletions(-) delete mode 100644 src/engine/blocks-execute-cache.js diff --git a/src/engine/blocks-execute-cache.js b/src/engine/blocks-execute-cache.js deleted file mode 100644 index ffa8708b2..000000000 --- a/src/engine/blocks-execute-cache.js +++ /dev/null @@ -1,19 +0,0 @@ -/** - * @fileoverview - * Access point for private method shared between blocks.js and execute.js for - * caching execute information. - */ - -/** - * A private method shared with execute to build an object containing the block - * information execute needs and that is reset when other cached Blocks info is - * reset. - * @param {Blocks} blocks Blocks containing the expected blockId - * @param {string} blockId blockId for the desired execute cache - */ -exports.getCached = function () { - throw new Error('blocks.js has not initialized BlocksExecuteCache'); -}; - -// Call after the default throwing getCached is assigned for Blocks to replace. -require('./blocks'); diff --git a/src/engine/blocks.js b/src/engine/blocks.js index c16da7114..25fb0c179 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -4,7 +4,6 @@ const xmlEscape = require('../util/xml-escape'); const MonitorRecord = require('./monitor-record'); const Clone = require('../util/clone'); const {Map} = require('immutable'); -const BlocksExecuteCache = require('./blocks-execute-cache'); /** * @fileoverview @@ -48,14 +47,7 @@ class Blocks { * Cache procedure definitions by block id * @type {object.} */ - procedureDefinitions: {}, - - /** - * A cache for execute to use and store on. Only available to - * execute. - * @type {object.} - */ - _executeCached: {} + procedureDefinitions: {} }; } @@ -351,7 +343,6 @@ class Blocks { this._cache.inputs = {}; this._cache.procedureParamNames = {}; this._cache.procedureDefinitions = {}; - this._cache._executeCached = {}; } /** @@ -428,7 +419,7 @@ class Blocks { const isSpriteSpecific = optRuntime.monitorBlockInfo.hasOwnProperty(block.opcode) && optRuntime.monitorBlockInfo[block.opcode].isSpriteSpecific; block.targetId = isSpriteSpecific ? optRuntime.getEditingTarget().id : null; - + if (wasMonitored && !block.isMonitored) { optRuntime.requestRemoveMonitor(block.id); } else if (!wasMonitored && block.isMonitored) { @@ -737,31 +728,4 @@ class Blocks { } } -/** - * A private method shared with execute to build an object containing the block - * information execute needs and that is reset when other cached Blocks info is - * reset. - * @param {Blocks} blocks Blocks containing the expected blockId - * @param {string} blockId blockId for the desired execute cache - * @return {object} execute cache object - */ -BlocksExecuteCache.getCached = function (blocks, blockId) { - const block = blocks.getBlock(blockId); - if (typeof block === 'undefined') return null; - let cached = blocks._cache._executeCached[blockId]; - if (typeof cached !== 'undefined') { - return cached; - } - - cached = { - _initialized: false, - opcode: blocks.getOpcode(block), - fields: blocks.getFields(block), - inputs: blocks.getInputs(block), - mutation: blocks.getMutation(block) - }; - blocks._cache._executeCached[blockId] = cached; - return cached; -}; - module.exports = Blocks; diff --git a/src/engine/execute.js b/src/engine/execute.js index 85af1b821..13742ec76 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -1,5 +1,4 @@ const BlockUtility = require('./block-utility'); -const BlocksExecuteCache = require('./blocks-execute-cache'); const log = require('../util/log'); const Thread = require('./thread'); const {Map} = require('immutable'); @@ -99,27 +98,32 @@ const handleReport = function ( } }; -/** - * A convenienve constant to hide that the recursiveCall argument to execute is - * a boolean trap. - * @const {boolean} - */ -const RECURSIVE = true; - /** * Execute a block. * @param {!Sequencer} sequencer Which sequencer is executing. * @param {!Thread} thread Thread which to read and execute. - * @param {boolean} recursiveCall is execute called from another execute call? */ -const execute = function (sequencer, thread, recursiveCall) { +const execute = function (sequencer, thread) { const runtime = sequencer.runtime; + const target = thread.target; + + // Stop if block or target no longer exists. + if (target === null) { + // No block found: stop the thread; script no longer exists. + sequencer.retireThread(thread); + return; + } // Current block to execute is the one on the top of the stack. const currentBlockId = thread.peekStack(); const currentStackFrame = thread.peekStackFrame(); - let blockContainer = thread.blockContainer; + let blockContainer; + if (thread.updateMonitor) { + blockContainer = runtime.monitorBlocks; + } else { + blockContainer = target.blocks; + } let block = blockContainer.getBlock(currentBlockId); if (typeof block === 'undefined') { blockContainer = runtime.flyoutBlocks; @@ -132,83 +136,33 @@ const execute = function (sequencer, thread, recursiveCall) { } } - const blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId); - if (blockCached._initialized !== true) { - const {opcode, fields, inputs} = blockCached; + const opcode = blockContainer.getOpcode(block); + const fields = blockContainer.getFields(block); + const inputs = blockContainer.getInputs(block); + const blockFunction = runtime.getOpcodeFunction(opcode); + const isHat = runtime.getIsHat(opcode); - // Assign opcode isHat and blockFunction data to avoid dynamic lookups. - blockCached._isHat = runtime.getIsHat(opcode); - blockCached._blockFunction = runtime.getOpcodeFunction(opcode); - blockCached._definedBlockFunction = typeof blockCached._blockFunction !== 'undefined'; - const fieldKeys = Object.keys(fields); - - // Store the current shadow value if there is a shadow value. - blockCached._isShadowBlock = fieldKeys.length === 1 && Object.keys(inputs).length === 0; - blockCached._shadowValue = fieldKeys.length === 1 && fields[fieldKeys[0]].value; - - // Store a fields copy. If fields is a VARIABLE, LIST, or - // BROADCAST_OPTION, store the created values so fields assignment to - // argValues does not iterate over fields. - blockCached._fields = Object.assign({}, blockCached.fields); - blockCached._isFieldVariable = fieldKeys.length === 1 && fieldKeys.includes('VARIABLE'); - blockCached._fieldVariable = blockCached._isFieldVariable ? - { - id: fields.VARIABLE.id, - name: fields.VARIABLE.value - } : - null; - blockCached._isFieldList = fieldKeys.length === 1 && fieldKeys.includes('LIST'); - blockCached._fieldList = blockCached._isFieldList ? - { - id: fields.LIST.id, - name: fields.LIST.value - } : - null; - blockCached._isFieldBroadcastOption = fieldKeys.length === 1 && fieldKeys.includes('BROADCAST_OPTION'); - blockCached._fieldBroadcastOption = blockCached._isFieldBroadcastOption ? - { - id: fields.BROADCAST_OPTION.id, - name: fields.BROADCAST_OPTION.value - } : - null; - blockCached._isFieldKnown = blockCached._isFieldVariable || - blockCached._isFieldList || blockCached._isFieldBroadcastOption; - - // Store a modified inputs. This assures the keys are its own properties - // and that custom_block will not be evaluated. - blockCached._inputs = Object.assign({}, blockCached.inputs); - delete blockCached._inputs.custom_block; - - blockCached._initialized = true; + if (!opcode) { + log.warn(`Could not get opcode for block: ${currentBlockId}`); + return; } - const opcode = blockCached.opcode; - const fields = blockCached._fields; - const inputs = blockCached._inputs; - const mutation = blockCached.mutation; - const blockFunction = blockCached._blockFunction; - const isHat = blockCached._isHat; - // Hats and single-field shadows are implemented slightly differently // from regular blocks. // For hats: if they have an associated block function, // it's treated as a predicate; if not, execution will proceed as a no-op. // For single-field shadows: If the block has a single field, and no inputs, // immediately return the value of the field. - if (!blockCached._definedBlockFunction) { - if (!opcode) { - log.warn(`Could not get opcode for block: ${currentBlockId}`); - return; - } - - if (recursiveCall === RECURSIVE && blockCached._isShadowBlock) { - // One field and no inputs - treat as arg. - thread.pushReportedValue(blockCached._shadowValue); - thread.status = Thread.STATUS_RUNNING; - } else if (isHat) { + if (typeof blockFunction === 'undefined') { + if (isHat) { // Skip through the block (hat with no predicate). return; + } + const keys = Object.keys(fields); + if (keys.length === 1 && Object.keys(inputs).length === 0) { + // One field and no inputs - treat as arg. + handleReport(fields[keys[0]].value, sequencer, thread, currentBlockId, opcode, isHat); } else { log.warn(`Could not get implementation for opcode: ${opcode}`); } @@ -220,22 +174,24 @@ const execute = function (sequencer, thread, recursiveCall) { const argValues = {}; // Add all fields on this block to the argValues. - if (blockCached._isFieldKnown) { - if (blockCached._isFieldVariable) { - argValues.VARIABLE = blockCached._fieldVariable; - } else if (blockCached._isFieldList) { - argValues.LIST = blockCached._fieldList; - } else if (blockCached._isFieldBroadcastOption) { - argValues.BROADCAST_OPTION = blockCached._fieldBroadcastOption; - } - } else { - for (const fieldName in fields) { + for (const fieldName in fields) { + if (!fields.hasOwnProperty(fieldName)) continue; + if (fieldName === 'VARIABLE' || fieldName === 'LIST' || + fieldName === 'BROADCAST_OPTION') { + argValues[fieldName] = { + id: fields[fieldName].id, + name: fields[fieldName].value + }; + } else { argValues[fieldName] = fields[fieldName].value; } } // Recursively evaluate input blocks. for (const inputName in inputs) { + if (!inputs.hasOwnProperty(inputName)) continue; + // Do not evaluate the internal custom command block within definition + if (inputName === 'custom_block') continue; const input = inputs[inputName]; const inputBlockId = input.block; // Is there no value for this input waiting in the stack frame? @@ -246,16 +202,8 @@ const execute = function (sequencer, thread, recursiveCall) { // Save name of input for `Thread.pushReportedValue`. currentStackFrame.waitingReporter = inputName; // Actually execute the block. - execute(sequencer, thread, RECURSIVE); + execute(sequencer, thread); if (thread.status === Thread.STATUS_PROMISE_WAIT) { - for (const _inputName in inputs) { - if (_inputName === inputName) break; - if (_inputName === 'BROADCAST_INPUT') { - currentStackFrame.reported[_inputName] = argValues[_inputName].name; - } else { - currentStackFrame.reported[_inputName] = argValues[_inputName]; - } - } return; } @@ -264,23 +212,7 @@ const execute = function (sequencer, thread, recursiveCall) { currentStackFrame.waitingReporter = null; thread.popStack(); } - let inputValue; - if ( - currentStackFrame.waitingReporter === null - ) { - inputValue = currentStackFrame.justReported; - } else if (currentStackFrame.waitingReporter === inputName) { - inputValue = currentStackFrame.justReported; - currentStackFrame.waitingReporter = null; - // If we've gotten this far, all of the input blocks are evaluated, - // and `argValues` is fully populated. So, execute the block - // primitive. First, clear `currentStackFrame.reported`, so any - // subsequent execution (e.g., on return from a branch) gets fresh - // inputs. - currentStackFrame.reported = {}; - } else if (typeof currentStackFrame.reported[inputName] !== 'undefined') { - inputValue = currentStackFrame.reported[inputName]; - } + const inputValue = currentStackFrame.reported[inputName]; if (inputName === 'BROADCAST_INPUT') { const broadcastInput = inputs[inputName]; // Check if something is plugged into the broadcast block, or @@ -307,7 +239,16 @@ const execute = function (sequencer, thread, recursiveCall) { } // Add any mutation to args (e.g., for procedures). - argValues.mutation = mutation; + const mutation = blockContainer.getMutation(block); + if (mutation !== null) { + argValues.mutation = mutation; + } + + // If we've gotten this far, all of the input blocks are evaluated, + // and `argValues` is fully populated. So, execute the block primitive. + // First, clear `currentStackFrame.reported`, so any subsequent execution + // (e.g., on return from a branch) gets fresh inputs. + currentStackFrame.reported = {}; let primitiveReportedValue = null; blockUtility.sequencer = sequencer; @@ -330,7 +271,7 @@ const execute = function (sequencer, thread, recursiveCall) { runtime.profiler.records.push(runtime.profiler.STOP, performance.now()); } - if (recursiveCall !== RECURSIVE && typeof primitiveReportedValue === 'undefined') { + if (typeof primitiveReportedValue === 'undefined') { // No value reported - potentially a command block. // Edge-activated hats don't request a glow; all commands do. thread.requestScriptGlowInFrame = true; @@ -377,11 +318,7 @@ const execute = function (sequencer, thread, recursiveCall) { thread.popStack(); }); } else if (thread.status === Thread.STATUS_RUNNING) { - if (recursiveCall === RECURSIVE) { - thread.pushReportedValue(primitiveReportedValue); - } else { - handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat); - } + handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat); } }; diff --git a/src/engine/runtime.js b/src/engine/runtime.js index ea86a9536..bf7969a0c 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -861,9 +861,6 @@ class Runtime extends EventEmitter { thread.target = target; thread.stackClick = opts.stackClick; thread.updateMonitor = opts.updateMonitor; - thread.blockContainer = opts.updateMonitor ? - this.monitorBlocks : - target.blocks; thread.pushStack(id); this.threads.push(thread); @@ -893,7 +890,6 @@ class Runtime extends EventEmitter { newThread.target = thread.target; newThread.stackClick = thread.stackClick; newThread.updateMonitor = thread.updateMonitor; - newThread.blockContainer = thread.blockContainer; newThread.pushStack(thread.topBlock); const i = this.threads.indexOf(thread); if (i > -1) { diff --git a/src/engine/sequencer.js b/src/engine/sequencer.js index 6a8c3dd6d..2db1c6ed8 100644 --- a/src/engine/sequencer.js +++ b/src/engine/sequencer.js @@ -197,11 +197,7 @@ class Sequencer { this.runtime.profiler.records.push( this.runtime.profiler.START, executeProfilerId, null, performance.now()); } - if (thread.target === null) { - this.retireThread(thread); - } else { - execute(this, thread); - } + execute(this, thread); if (this.runtime.profiler !== null) { // this.runtime.profiler.stop(); this.runtime.profiler.records.push(this.runtime.profiler.STOP, performance.now()); diff --git a/src/engine/thread.js b/src/engine/thread.js index a618b460b..def1fc1ef 100644 --- a/src/engine/thread.js +++ b/src/engine/thread.js @@ -42,12 +42,6 @@ class Thread { */ this.target = null; - /** - * The Blocks this thread will execute. - * @type {Blocks} - */ - this.blockContainer = null; - /** * Whether the thread requests its script to glow during this frame. * @type {boolean} @@ -130,8 +124,7 @@ class Thread { this.stackFrames.push({ isLoop: false, // Whether this level of the stack is a loop. warpMode: warpMode, // Whether this level is in warp mode. - justReported: null, // Reported value from just executed block. - reported: {}, // Persists reported inputs during async block. + reported: {}, // Collects reported input values. waitingReporter: null, // Name of waiting reporter. params: {}, // Procedure parameters. executionContext: {} // A context passed to block implementations. @@ -216,8 +209,9 @@ class Thread { */ pushReportedValue (value) { const parentStackFrame = this.peekParentStackFrame(); - if (parentStackFrame !== null) { - parentStackFrame.justReported = value; + if (parentStackFrame) { + const waitingReporter = parentStackFrame.waitingReporter; + parentStackFrame.reported[waitingReporter] = value; } } diff --git a/test/unit/engine_sequencer.js b/test/unit/engine_sequencer.js index 55faeb69e..751e9bb64 100644 --- a/test/unit/engine_sequencer.js +++ b/test/unit/engine_sequencer.js @@ -91,8 +91,7 @@ const generateThread = function (runtime) { rt.blocks.createBlock(generateBlock(next)); th.pushStack(next); th.target = rt; - th.blockContainer = rt.blocks; - + runtime.threads.push(th); return th; diff --git a/test/unit/engine_thread.js b/test/unit/engine_thread.js index 7667a1b2e..15fab7c02 100644 --- a/test/unit/engine_thread.js +++ b/test/unit/engine_thread.js @@ -89,8 +89,8 @@ test('pushReportedValue', t => { th.pushStack('arbitraryString'); th.pushStack('secondString'); th.pushReportedValue('value'); - t.strictEquals(th.peekParentStackFrame().justReported, 'value'); - + t.strictEquals(th.peekParentStackFrame().reported.null, 'value'); + t.end(); });