Optimization: Remove empty FallthroughState states. (issue 7541050)

[usrb...@yahoo.com]

Reviewers: arv-chromium, slightlylate, peterhal,

Message:
Another one that I've let sit too long.

This patch doesn't make as big a difference now that we use 'return' to
handle generator close.

#--cut--
dl_check() {
local f=$(basename $1)
test -e $f || curl -sO $1
openssl sha1 < $f | grep -q "= $2" && return 0
echo sha1 mismatch!
return 1
}

git checkout -b issue7541050-statemachine-opt

cat > gen-empty-states.js <<"END"
function* G(n1, n2) {
for (var i = n1; i < n2; i++) {
yield i;
if (i % 42 === 0)
break;
}
}
END

## before
./traceur --out gen-empty-states.out1.js -- gen-empty-states.js

dl_check https://codereview.appspot.com/download/issue7541050_1.diff \
526ec7a4f0dcfd71 && git apply issue7541050_1.diff && make test

## after
./traceur --out gen-empty-states.out2.js -- gen-empty-states.js

## the empty state is gone.
diff -u gen-empty-states.out1.js gen-empty-states.out2.js
#--cut--



https://codereview.appspot.com/7541050/diff/1/src/codegeneration/generator/GeneratorTransformer.js
File src/codegeneration/generator/GeneratorTransformer.js (right):

https://codereview.appspot.com/7541050/diff/1/src/codegeneration/generator/GeneratorTransformer.js#newcode203
src/codegeneration/generator/GeneratorTransformer.js:203:
this.removeEmptyStates(machine.states),
State machine manipulation functions like this should probably all be
moved to a utility library eventually.

Description:
Optimization: Remove empty FallthroughState states.

BUG=None
TEST=None


Please review this at https://codereview.appspot.com/7541050/

Affected files:
M src/codegeneration/generator/CPSTransformer.js
M src/codegeneration/generator/GeneratorTransformer.js


Index: src/codegeneration/generator/CPSTransformer.js
diff --git a/src/codegeneration/generator/CPSTransformer.js
b/src/codegeneration/generator/CPSTransformer.js
index
02bcf163b79e549923e332ef50ef73f45f0f832e..39a01e39dc74f06d82538a22a6bd5f227cf2255a
100644
--- a/src/codegeneration/generator/CPSTransformer.js
+++ b/src/codegeneration/generator/CPSTransformer.js
@@ -415,6 +415,30 @@ export class CPSTransformer extends
ParseTreeTransformer {

/**
* @param {Array.<State>} oldStates
+ * @return {Array.<State>} An array with empty states removed.
+ */
+ removeEmptyStates(oldStates) {
+ var emptyStates = [], newStates = [];
+ // Remove empty FallThroughState states.
+ for (var i = 0; i < oldStates.length; i++) {
+ if (oldStates[i] instanceof FallThroughState &&
+ oldStates[i].statements.length === 0) {
+ emptyStates.push(oldStates[i]);
+ continue;
+ }
+ newStates.push(oldStates[i]);
+ }
+ // Fix up dangling state transitions.
+ for (i = 0; i < newStates.length; i++) {
+ newStates[i] = emptyStates.reduce((state, {id, fallThroughState}) =>
{
+ return state.replaceState(id, fallThroughState);
+ }, newStates[i]);
+ }
+ return newStates;
+ }
+
+ /**
+ * @param {Array.<State>} oldStates
* @param {number} oldState
* @param {number} newState
* @param {Array.<State>} newStates
Index: src/codegeneration/generator/GeneratorTransformer.js
diff --git a/src/codegeneration/generator/GeneratorTransformer.js
b/src/codegeneration/generator/GeneratorTransformer.js
index
b1cf29786742c4219c1c3c2961530d10f8b60d2b..ca52b81b7672f0eecde1ed82c09b0fd53b6e4e79
100644
--- a/src/codegeneration/generator/GeneratorTransformer.js
+++ b/src/codegeneration/generator/GeneratorTransformer.js
@@ -198,6 +198,10 @@ export class GeneratorTransformer extends
CPSTransformer {
return tree;
}
var machine = transformedTree;
+ machine = new StateMachine(machine.startState,
+ machine.fallThroughState,
+ this.removeEmptyStates(machine.states),
+ machine.exceptionBlocks);

var statements = [];

[a...@chromium.org]

LGTM

I changed the continue to if/else before committing.

Committed as aa3ae4930eeb9c46445924393d771c573ac79058


https://codereview.appspot.com/7541050/diff/1/src/codegeneration/generator/CPSTransformer.js
File src/codegeneration/generator/CPSTransformer.js (right):

https://codereview.appspot.com/7541050/diff/1/src/codegeneration/generator/CPSTransformer.js#newcode427
src/codegeneration/generator/CPSTransformer.js:427: continue;
You can do if/else instead here

https://codereview.appspot.com/7541050/

[usrb...@yahoo.com]

https://codereview.appspot.com/7541050/diff/1/src/codegeneration/generator/CPSTransformer.js
File src/codegeneration/generator/CPSTransformer.js (right):

https://codereview.appspot.com/7541050/diff/1/src/codegeneration/generator/CPSTransformer.js#newcode432
src/codegeneration/generator/CPSTransformer.js:432: for (i = 0; i <
newStates.length; i++) {
Oops, just realized this for loop is O(n^2). I'll fix this in a
follow-up.

https://codereview.appspot.com/7541050/