From 6b7582f1c70ce4f90a0ca5eb986d96771414d86b Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Wed, 24 Jan 2018 14:34:02 -0500 Subject: [PATCH 1/5] Report block result to stackFrame.justReported - Add stackFrame.justReported Report the block result to a known key `justReported` instead of a dynamic key on `reported`. Assuming blocks with a promised value are relatively infrequent the most common recursive input flow immediately reads the value "just" reported. In the assumed uncommon case of a promised thread status, empty the already argValues assigned values onto the currentStackFrame's reported member. In the next execute call on this stackFrame, values assigned to reported are read back off onto argValues, and execute will returned to the assumed common case. This is a safe assumption since a thread in the promise state will not exit that state until the next frame when javascript has a chance to call the resolve handle, setting the thread's state back to another executable state. Using direct assignment to `justReported` saves building an object dynamically. Instead of always building `reported` and `argValues` only `argValues` is built until a promised state is reached. This also gives a known time when `reported` is used, allowing cleanup of a stackFrame's reported to only happen when it was used to persist already reported values. --- src/engine/execute.js | 34 +++++++++++++++++++++++++++------- src/engine/thread.js | 8 ++++---- test/unit/engine_thread.js | 4 ++-- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/engine/execute.js b/src/engine/execute.js index 13742ec7..4dd2abe9 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -204,6 +204,16 @@ const execute = function (sequencer, thread) { // Actually execute the block. execute(sequencer, thread); if (thread.status === Thread.STATUS_PROMISE_WAIT) { + for (const _inputName in inputs) { + if (_inputName === inputName) break; + if (!inputs.hasOwnProperty(_inputName)) continue; + if (_inputName === 'custom_block') continue; + if (_inputName === 'BROADCAST_INPUT') { + currentStackFrame.reported[_inputName] = argValues[_inputName].name; + } else { + currentStackFrame.reported[_inputName] = argValues[_inputName]; + } + } return; } @@ -212,7 +222,23 @@ const execute = function (sequencer, thread) { currentStackFrame.waitingReporter = null; thread.popStack(); } - const inputValue = currentStackFrame.reported[inputName]; + 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]; + } if (inputName === 'BROADCAST_INPUT') { const broadcastInput = inputs[inputName]; // Check if something is plugged into the broadcast block, or @@ -244,12 +270,6 @@ const execute = function (sequencer, thread) { 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; blockUtility.thread = thread; diff --git a/src/engine/thread.js b/src/engine/thread.js index def1fc1e..cfde43a3 100644 --- a/src/engine/thread.js +++ b/src/engine/thread.js @@ -124,7 +124,8 @@ 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. - reported: {}, // Collects reported input values. + justReported: null, // Reported value from just executed block. + reported: {}, // Persists reported inputs during async block. waitingReporter: null, // Name of waiting reporter. params: {}, // Procedure parameters. executionContext: {} // A context passed to block implementations. @@ -209,9 +210,8 @@ class Thread { */ pushReportedValue (value) { const parentStackFrame = this.peekParentStackFrame(); - if (parentStackFrame) { - const waitingReporter = parentStackFrame.waitingReporter; - parentStackFrame.reported[waitingReporter] = value; + if (parentStackFrame !== null) { + parentStackFrame.justReported = value; } } diff --git a/test/unit/engine_thread.js b/test/unit/engine_thread.js index 15fab7c0..7667a1b2 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().reported.null, 'value'); - + t.strictEquals(th.peekParentStackFrame().justReported, 'value'); + t.end(); }); From ab8d8e5f344bc1a81194ff3a2042e2a5ae3b2b10 Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Wed, 24 Jan 2018 14:44:07 -0500 Subject: [PATCH 2/5] Add optional recursiveCall argument to execute When execute calls itself to step into the next stack level, pass RECURSIVE to the recursiveCall level. This argument provides opportunity to recursed calls in execute to reduce the time needed for some checks. - Reduce time for checking around setting thread.requestScriptGlowInFrame - Call thread.pushReportedValue at end of execute when in a recursive call A boolean check is faster than a type lookup, command blocks will only be executed in non-recursive calls. This saves a minor amount of time for any result reporting blocks like blocks that get variables or add two inputs together. If the block is not returning a promise and execute is a recursed call, that implies that the block is neither a hat or that the thread is not at the top. That reduces handleReport to pushReportedValue. Using the recursiveCall argument the final block calling handleReport can shortcut the extra work in handleReport and reduce it to immediately calling pushReportedValue. --- src/engine/execute.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/engine/execute.js b/src/engine/execute.js index 4dd2abe9..919dcf66 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -98,12 +98,20 @@ 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) { +const execute = function (sequencer, thread, recursiveCall) { const runtime = sequencer.runtime; const target = thread.target; @@ -202,7 +210,7 @@ const execute = function (sequencer, thread) { // Save name of input for `Thread.pushReportedValue`. currentStackFrame.waitingReporter = inputName; // Actually execute the block. - execute(sequencer, thread); + execute(sequencer, thread, RECURSIVE); if (thread.status === Thread.STATUS_PROMISE_WAIT) { for (const _inputName in inputs) { if (_inputName === inputName) break; @@ -291,7 +299,7 @@ const execute = function (sequencer, thread) { runtime.profiler.records.push(runtime.profiler.STOP, performance.now()); } - if (typeof primitiveReportedValue === 'undefined') { + if (recursiveCall !== RECURSIVE && typeof primitiveReportedValue === 'undefined') { // No value reported - potentially a command block. // Edge-activated hats don't request a glow; all commands do. thread.requestScriptGlowInFrame = true; @@ -338,7 +346,11 @@ const execute = function (sequencer, thread) { thread.popStack(); }); } else if (thread.status === Thread.STATUS_RUNNING) { - handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat); + if (recursiveCall === RECURSIVE) { + thread.pushReportedValue(primitiveReportedValue); + } else { + handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat); + } } }; From 4286e3e9824f38fb7771f38b0dcc89820a8a5208 Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Wed, 24 Jan 2018 14:56:02 -0500 Subject: [PATCH 3/5] Add Thread.blockContainer Store the thread's blocks at blockContainer letting execute quickly determine the block source. Monitor threads are a monitor thread. They do not become a target thread suddenly. --- src/engine/execute.js | 7 +------ src/engine/runtime.js | 4 ++++ src/engine/thread.js | 6 ++++++ test/unit/engine_sequencer.js | 3 ++- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/engine/execute.js b/src/engine/execute.js index 919dcf66..6017ac46 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -126,12 +126,7 @@ const execute = function (sequencer, thread, recursiveCall) { const currentBlockId = thread.peekStack(); const currentStackFrame = thread.peekStackFrame(); - let blockContainer; - if (thread.updateMonitor) { - blockContainer = runtime.monitorBlocks; - } else { - blockContainer = target.blocks; - } + let blockContainer = thread.blockContainer; let block = blockContainer.getBlock(currentBlockId); if (typeof block === 'undefined') { blockContainer = runtime.flyoutBlocks; diff --git a/src/engine/runtime.js b/src/engine/runtime.js index bf7969a0..ea86a953 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -861,6 +861,9 @@ 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); @@ -890,6 +893,7 @@ 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/thread.js b/src/engine/thread.js index cfde43a3..a618b460 100644 --- a/src/engine/thread.js +++ b/src/engine/thread.js @@ -42,6 +42,12 @@ 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} diff --git a/test/unit/engine_sequencer.js b/test/unit/engine_sequencer.js index 751e9bb6..55faeb69 100644 --- a/test/unit/engine_sequencer.js +++ b/test/unit/engine_sequencer.js @@ -91,7 +91,8 @@ 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; From 41d6a8f925e44c1dd533d61c8ff74a827b9eeacc Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Wed, 24 Jan 2018 15:03:57 -0500 Subject: [PATCH 4/5] Check if a thread's target is null in sequencer Before calling execute, if a thread's target is null, retire that thread. This saves repeatedly checking if the thread's target is null in recursive calls where even if scratch-gui or blocks, or some other related library set the target to null, that will not happen during block execution. It will happen at some time outside of the sequencer letting the sequencer check once instead of execute checking at every recursive level. --- src/engine/execute.js | 8 -------- src/engine/sequencer.js | 6 +++++- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/engine/execute.js b/src/engine/execute.js index 6017ac46..762df5b9 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -113,14 +113,6 @@ const RECURSIVE = true; */ const execute = function (sequencer, thread, recursiveCall) { 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(); diff --git a/src/engine/sequencer.js b/src/engine/sequencer.js index 2db1c6ed..6a8c3dd6 100644 --- a/src/engine/sequencer.js +++ b/src/engine/sequencer.js @@ -197,7 +197,11 @@ class Sequencer { this.runtime.profiler.records.push( this.runtime.profiler.START, executeProfilerId, null, performance.now()); } - execute(this, thread); + if (thread.target === null) { + this.retireThread(thread); + } else { + execute(this, thread); + } if (this.runtime.profiler !== null) { // this.runtime.profiler.stop(); this.runtime.profiler.records.push(this.runtime.profiler.STOP, performance.now()); From 10acf3c4f53e755ae905d8e59c0d66dcb64c8631 Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Wed, 24 Jan 2018 15:21:13 -0500 Subject: [PATCH 5/5] Add BlocksExecuteCache Add a Blocks cache available only to execute. This cache lets execute get a blocks inputs, fields, opcode, and mutation in one request and an object execute can further modify to store derivative values it use this and every later execute iteration to perform its duties quicker. --- src/engine/blocks-execute-cache.js | 19 +++++ src/engine/blocks.js | 40 ++++++++++- src/engine/execute.js | 108 ++++++++++++++++++++--------- 3 files changed, 133 insertions(+), 34 deletions(-) create mode 100644 src/engine/blocks-execute-cache.js diff --git a/src/engine/blocks-execute-cache.js b/src/engine/blocks-execute-cache.js new file mode 100644 index 00000000..ffa8708b --- /dev/null +++ b/src/engine/blocks-execute-cache.js @@ -0,0 +1,19 @@ +/** + * @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 b889f201..8854feb2 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -4,6 +4,7 @@ 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 @@ -47,7 +48,14 @@ class Blocks { * Cache procedure definitions by block id * @type {object.} */ - procedureDefinitions: {} + procedureDefinitions: {}, + + /** + * A cache for execute to use and store on. Only available to + * execute. + * @type {object.} + */ + _executeCached: {} }; } @@ -335,6 +343,7 @@ class Blocks { this._cache.inputs = {}; this._cache.procedureParamNames = {}; this._cache.procedureDefinitions = {}; + this._cache._executeCached = {}; } /** @@ -411,7 +420,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) { @@ -697,4 +706,31 @@ 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 762df5b9..85af1b82 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -1,4 +1,5 @@ const BlockUtility = require('./block-utility'); +const BlocksExecuteCache = require('./blocks-execute-cache'); const log = require('../util/log'); const Thread = require('./thread'); const {Map} = require('immutable'); @@ -131,33 +132,83 @@ const execute = function (sequencer, thread, recursiveCall) { } } - 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); + const blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId); + if (blockCached._initialized !== true) { + const {opcode, fields, inputs} = blockCached; + // 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'; - if (!opcode) { - log.warn(`Could not get opcode for block: ${currentBlockId}`); - return; + 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; } + 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 (typeof blockFunction === 'undefined') { - if (isHat) { - // Skip through the block (hat with no predicate). + if (!blockCached._definedBlockFunction) { + if (!opcode) { + log.warn(`Could not get opcode for block: ${currentBlockId}`); return; } - const keys = Object.keys(fields); - if (keys.length === 1 && Object.keys(inputs).length === 0) { + + if (recursiveCall === RECURSIVE && blockCached._isShadowBlock) { // One field and no inputs - treat as arg. - handleReport(fields[keys[0]].value, sequencer, thread, currentBlockId, opcode, isHat); + thread.pushReportedValue(blockCached._shadowValue); + thread.status = Thread.STATUS_RUNNING; + } else if (isHat) { + // Skip through the block (hat with no predicate). + return; } else { log.warn(`Could not get implementation for opcode: ${opcode}`); } @@ -169,24 +220,22 @@ const execute = function (sequencer, thread, recursiveCall) { const argValues = {}; // Add all fields on this block to the argValues. - 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 { + 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) { 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? @@ -201,8 +250,6 @@ const execute = function (sequencer, thread, recursiveCall) { if (thread.status === Thread.STATUS_PROMISE_WAIT) { for (const _inputName in inputs) { if (_inputName === inputName) break; - if (!inputs.hasOwnProperty(_inputName)) continue; - if (_inputName === 'custom_block') continue; if (_inputName === 'BROADCAST_INPUT') { currentStackFrame.reported[_inputName] = argValues[_inputName].name; } else { @@ -260,10 +307,7 @@ const execute = function (sequencer, thread, recursiveCall) { } // Add any mutation to args (e.g., for procedures). - const mutation = blockContainer.getMutation(block); - if (mutation !== null) { - argValues.mutation = mutation; - } + argValues.mutation = mutation; let primitiveReportedValue = null; blockUtility.sequencer = sequencer;