Switch interpreter state stack code to use reversed order #15645
Conversation
| this.interpreter.stateStack[0].done); | ||
| !this.interpreter.stateStack[this.interpreter.stateStack.length - 1] || | ||
| (this.interpreter.stateStack[this.interpreter.stateStack.length - 1].node.type === 'Program' && | ||
| this.interpreter.stateStack[this.interpreter.stateStack.length - 1].done); |
There was a problem hiding this comment.
Let's rewrite this so we can extract
const topFrame = this.interpreter.stateStack[this.interpreter.stateStack.length - 1];
for readability and DRY.
| @@ -530,7 +530,7 @@ JSInterpreter.prototype.executeInterpreter = function (firstStep, runUntilCallba | |||
| } | |||
| this.executionError = safeStepInterpreter(this); | |||
| if (!this.executionError && this.interpreter.stateStack.length) { | |||
| var state = this.interpreter.stateStack[0], nodeType = state.node.type; | |||
| var state = this.interpreter.stateStack[this.interpreter.stateStack.length - 1], nodeType = state.node.type; | |||
There was a problem hiding this comment.
Whoops - I don't think we're enforcing this with the linter yet, but prefer one initialized declaration per statement/line. I'd also make these const if we can, especially since one is derived from the other.
| // Store that we've seen a call expression at this depth in callExpressionSeenAtDepth: | ||
| this.callExpressionSeenAtDepth[stackDepth] = true; | ||
| } | ||
|
|
||
| if (this.paused) { | ||
| // Store the first call expression stack depth seen while in this step operation: | ||
| if (inUserCode && this.interpreter.stateStack[0].node.type === "CallExpression") { | ||
| if (inUserCode && this.interpreter.stateStack[this.interpreter.stateStack.length - 1].node.type === "CallExpression") { |
There was a problem hiding this comment.
This and line 562 above would be a good place to extract topFrame - or even to extract the whole expression, something like inUserCallExpression.
| if (this.interpreter.stateStack[0]) { | ||
| var node = this.interpreter.stateStack[0].node; | ||
| if (this.interpreter.stateStack[this.interpreter.stateStack.length - 1]) { | ||
| var node = this.interpreter.stateStack[this.interpreter.stateStack.length - 1].node; |
| @@ -828,7 +828,7 @@ JSInterpreter.prototype.getNearestUserCodeLine = function () { | |||
| } | |||
| var userCodeRow = -1; | |||
| for (var i = 0; i < this.interpreter.stateStack.length; i++) { | |||
| var node = this.interpreter.stateStack[i].node; | |||
| var node = this.interpreter.stateStack[this.interpreter.stateStack.length - 1 - i].node; | |||
There was a problem hiding this comment.
Why this instead of for (let i = this.interpreter.stateStack.length - 1; i >= 0; i--)?
|
|
||
| if (node.type === 'ForStatement') { | ||
| var mode = interpreter.stateStack[0].mode || 0, subNode; | ||
| var mode = interpreter.stateStack[interpreter.stateStack.length - 1].mode || 0, subNode; |
|
My approach with these interpreter upgrades is to change as little code as possible when actually fixing the interpreter stuff so that the commits/PRs are easier to read if they get looked at again down the road. Then in separate PRs I would work on actual code cleanup that doesn't do any sort of behavior change or bug fixing. |
pcardune
force-pushed
the
pcardune-reverse-state-stack
branch
from
55706d0
to
b7f07a6
Compare
pcardune
force-pushed
the
pcardune-reverse-state-stack
branch
from
b7f07a6
to
0b9b6b3
Compare
In the most recent commit we are upgrading the interpreter too, the stack has been reversed so that things are pushed and popped from the end of the array instead of from the beginning, which apparently lead to a 10% performance boost. Now that performance boost is ours!