From 580d7f6408b9ab2e482c5f45ed7b96ed7ea1ccad Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Sat, 19 Nov 2011 22:14:56 -0800 Subject: [PATCH] fix for js optimizer not noticing globals are modified --- tools/js-optimizer.js | 116 +++++++++++++++++------------- tools/test-js-optimizer-output.js | 4 +- tools/test-js-optimizer.js | 4 +- 3 files changed, 71 insertions(+), 53 deletions(-) diff --git a/tools/js-optimizer.js b/tools/js-optimizer.js index f8e65f554..aa4ba39bc 100644 --- a/tools/js-optimizer.js +++ b/tools/js-optimizer.js @@ -35,7 +35,7 @@ var FUNCTION = set('defun', 'function'); // it replaces the passed node in the tree. // @arg post: A callback to call after traversing all children. // @arg stack: If true, a stack will be implemented: If pre does not push on -// the stack, we push a null. We pop when we leave the node. The +// the stack, we push a 0. We pop when we leave the node. The // stack is passed as a third parameter to the callbacks. // @returns: If the root node was replaced, the new root node. If the traversal // was stopped, true. Otherwise undefined. @@ -46,7 +46,7 @@ function traverse(node, pre, post, stack) { if (stack) len = stack.length; var result = pre(node, type, stack); if (result == true) return true; - if (stack && len == stack.length) stack.push(null); + if (stack && len == stack.length) stack.push(0); } for (var i = 0; i < node.length; i++) { var subnode = node[i]; @@ -66,45 +66,8 @@ function traverse(node, pre, post, stack) { return result; } -function makeEmptyNode() { - return ['toplevel', []] -} - -// Passes - -// Undos closure's creation of global variables with values true, false, -// undefined, null. These cut down on size, but do not affect gzip size -// and make JS engine's lives slightly harder (?) -function unGlobalize(ast) { - assert(ast[0] == 'toplevel'); - var values = {}; - // Find global renamings of the relevant values - ast[1].forEach(function(node, i) { - if (node[0] != 'var') return; - node[1] = node[1].filter(function(varItem) { - var key = varItem[0]; - var value = varItem[1]; - if (!value) return true; - if (value[0] == 'name' && value[1] == 'null') { - values[key] = ['name','null']; - return false; - } else if (value[0] == 'unary-prefix' && value[1] == 'void' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0){ - values[key] = ['unary-prefix','void',['num',0]]; - return false; - } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0){ - values[key] = ['unary-prefix','!',['num',0]]; - return false; - } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 1){ - values[key] = ['unary-prefix','!',['num',1]]; - return false; - } - return true; - }); - if (node[1].length == 0) { - ast[1][i] = makeEmptyNode(); - } - }); - // Walk the ast, with an understanding of JS variable scope (i.e., function-level scoping) +// Walk the ast in a simple way, with an understanding of which JS variables are defined) +function traverseWithVariables(ast, callback) { traverse(ast, function(node, type, stack) { if (type in FUNCTION) { stack.push({ type: 'function', vars: node[2] }); @@ -121,21 +84,72 @@ function unGlobalize(ast) { var allVars = stack.map(function(item) { return item ? item.vars : [] }).reduce(concatenator, []); // FIXME dictionary for speed? traverse(node, function(node, type, stack) { // Be careful not to look into our inner functions. They have already been processed. - if (type == 'name' && sum(stack) == 1) { - var ident = node[1]; - if (ident in values && allVars.indexOf(ident) < 0) { - return copy(values[ident]); - } - } else if (type in FUNCTION) { - stack.push(1); - } else { - stack.push(0); - } + if (sum(stack) > 1) return; + if (type in FUNCTION) stack.push(1); + return callback(node, type, allVars); }, null, []); } }, []); } +function makeEmptyNode() { + return ['toplevel', []] +} + +// Passes + +// Undos closure's creation of global variables with values true, false, +// undefined, null. These cut down on size, but do not affect gzip size +// and make JS engine's lives slightly harder (?) +function unGlobalize(ast) { + assert(ast[0] == 'toplevel'); + var values = {}; + // Find global renamings of the relevant values + ast[1].forEach(function(node, i) { + if (node[0] != 'var') return; + node[1] = node[1].filter(function(varItem, j) { + var ident = varItem[0]; + var value = varItem[1]; + if (!value) return true; + var possible = false; + if (value[0] == 'name' && value[1] == 'null') { + possible = true; + } else if (value[0] == 'unary-prefix' && value[1] == 'void' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0) { + possible = true; + } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 0) { + possible = true; + } else if (value[0] == 'unary-prefix' && value[1] == '!' && value[2] && value[2].length == 2 && value[2][0] == 'num' && value[2][1] == 1) { + possible = true; + } + if (!possible) return true; + // Make sure there are no assignments to this variable. (This isn't fast, we traverse many times..) + ast[1][i][1][j] = makeEmptyNode(); + var assigned = false; + traverseWithVariables(ast, function(node, type, allVars) { + if (type == 'assign' && node[2][0] == 'name' && node[2][1] == ident) assigned = true; + }); + ast[1][i][1][j] = [ident, value]; + if (!assigned) { + values[ident] = value; + return false; + } + return true; + }); + + if (node[1].length == 0) { + ast[1][i] = makeEmptyNode(); + } + }); + traverseWithVariables(ast, function(node, type, allVars) { + if (type == 'name') { + var ident = node[1]; + if (ident in values && allVars.indexOf(ident) < 0) { + return copy(values[ident]); + } + } + }); +} + // Main var src = fs.readFileSync('/dev/stdin').toString(); diff --git a/tools/test-js-optimizer-output.js b/tools/test-js-optimizer-output.js index c50adde2a..a53ca461d 100644 --- a/tools/test-js-optimizer-output.js +++ b/tools/test-js-optimizer-output.js @@ -1,6 +1,6 @@ var c; var b = 5; - +var s3 = !0; function abc() { var cheez = void 0; var fleefl; @@ -11,6 +11,7 @@ function abc() { var waka3 = void 0, flake3 = 5, marfoosh3 = void 0; var waka4 = void 0, flake4 = void 0, marfoosh4 = 5; var test1 = !0, test2 = !1, test3 = null, test4 = z, test5 = b, test6 = void 0; + s3 = 9; } function xyz() { var x = 52; @@ -24,6 +25,7 @@ function xyz() { var i = x, j = null, k = 5, l = t, m = s2; var s2 = 8; }); + var patama = s3; } function xyz2(x) { var cheez = x; diff --git a/tools/test-js-optimizer.js b/tools/test-js-optimizer.js index 14306740a..f2674b316 100644 --- a/tools/test-js-optimizer.js +++ b/tools/test-js-optimizer.js @@ -1,6 +1,6 @@ var c; var b = 5, x = void 0, y = null, t = !0; -var s = !1, s2 = !1; +var s = !1, s2 = !1, s3 = !0; function abc() { var cheez = x; var fleefl; @@ -11,6 +11,7 @@ function abc() { var waka3 = x, flake3 = 5, marfoosh3 = x; var waka4 = x, flake4 = x, marfoosh4 = 5; var test1 = t, test2 = s, test3 = y, test4 = z, test5 = b, test6 = x; + s3 = 9; } function xyz() { var x = 52; @@ -24,6 +25,7 @@ function xyz() { var i = x, j = y, k = 5, l = t, m = s2; var s2 = 8; }; + var patama = s3; } function xyz2(x) { var cheez = x;