Skip to content

Commit 48bfe19

Browse files
ConstellationConstellation
committed
Avoid "push" with multiple arguments due to performance issue.
Since "push" with multiple arguments is not supported (at least SpiderMonkey's IonMonkey), it hurts performance.
1 parent 9497c8e commit 48bfe19

File tree

4 files changed

+128
-29
lines changed

4 files changed

+128
-29
lines changed

‎escodegen.js

+48-24
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,8 @@
793793
}
794794

795795
if (arrow) {
796-
result.push(space, '=>');
796+
result.push(space);
797+
result.push('=>');
797798
}
798799

799800
if (node.expression) {
@@ -818,7 +819,8 @@
818819
withIndent(function () {
819820
if (stmt.left.type === Syntax.VariableDeclaration) {
820821
withIndent(function () {
821-
result.push(stmt.left.kind + noEmptySpace(), generateStatement(stmt.left.declarations[0], {
822+
result.push(stmt.left.kind + noEmptySpace());
823+
result.push(generateStatement(stmt.left.declarations[0], {
822824
allowIn: false
823825
}));
824826
});
@@ -971,7 +973,8 @@
971973
if (expr.operator === '/' && fragment.toString().charAt(0) === '/' ||
972974
expr.operator.slice(-1) === '<' && fragment.toString().slice(0, 3) === '!--') {
973975
// If '/' concats with '/' or `<` concats with `!--`, it is interpreted as comment start
974-
result.push(noEmptySpace(), fragment);
976+
result.push(noEmptySpace());
977+
result.push(fragment);
975978
} else {
976979
result = join(result, fragment);
977980
}
@@ -1053,11 +1056,13 @@
10531056
})];
10541057

10551058
if (expr.computed) {
1056-
result.push('[', generateExpression(expr.property, {
1059+
result.push('[');
1060+
result.push(generateExpression(expr.property, {
10571061
precedence: Precedence.Sequence,
10581062
allowIn: true,
10591063
allowCall: allowCall
1060-
}), ']');
1064+
}));
1065+
result.push(']');
10611066
} else {
10621067
if (expr.object.type === Syntax.Literal && typeof expr.object.value === 'number') {
10631068
fragment = toSourceNodeWhenNeeded(result).toString();
@@ -1076,7 +1081,8 @@
10761081
result.push('.');
10771082
}
10781083
}
1079-
result.push('.', generateIdentifier(expr.property));
1084+
result.push('.');
1085+
result.push(generateIdentifier(expr.property));
10801086
}
10811087

10821088
result = parenthesize(result, Precedence.Member, precedence);
@@ -1106,7 +1112,8 @@
11061112

11071113
if (((leftCharCode === 0x2B /* + */ || leftCharCode === 0x2D /* - */) && leftCharCode === rightCharCode) ||
11081114
(esutils.code.isIdentifierPart(leftCharCode) && esutils.code.isIdentifierPart(rightCharCode))) {
1109-
result.push(noEmptySpace(), fragment);
1115+
result.push(noEmptySpace());
1116+
result.push(fragment);
11101117
} else {
11111118
result.push(fragment);
11121119
}
@@ -1196,7 +1203,8 @@
11961203
result.push(',');
11971204
}
11981205
} else {
1199-
result.push(multiline ? indent : '', generateExpression(expr.elements[i], {
1206+
result.push(multiline ? indent : '');
1207+
result.push(generateExpression(expr.elements[i], {
12001208
precedence: Precedence.Assignment,
12011209
allowIn: true,
12021210
allowCall: true
@@ -1210,7 +1218,8 @@
12101218
if (multiline && !endsWithLineTerminator(toSourceNodeWhenNeeded(result).toString())) {
12111219
result.push(newline);
12121220
}
1213-
result.push(multiline ? base : '', ']');
1221+
result.push(multiline ? base : '');
1222+
result.push(']');
12141223
break;
12151224

12161225
case Syntax.Property:
@@ -1240,7 +1249,8 @@
12401249
precedence: Precedence.Sequence,
12411250
allowIn: true,
12421251
allowCall: true
1243-
}), generateFunctionBody(expr.value));
1252+
}));
1253+
result.push(generateFunctionBody(expr.value));
12441254
} else {
12451255
result = [
12461256
generateExpression(expr.key, {
@@ -1296,7 +1306,8 @@
12961306
if (multiline) {
12971307
result.push(',' + newline);
12981308
for (i = 1, len = expr.properties.length; i < len; ++i) {
1299-
result.push(indent, generateExpression(expr.properties[i], {
1309+
result.push(indent);
1310+
result.push(generateExpression(expr.properties[i], {
13001311
precedence: Precedence.Sequence,
13011312
allowIn: true,
13021313
allowCall: true,
@@ -1312,7 +1323,8 @@
13121323
if (!endsWithLineTerminator(toSourceNodeWhenNeeded(result).toString())) {
13131324
result.push(newline);
13141325
}
1315-
result.push(base, '}');
1326+
result.push(base);
1327+
result.push('}');
13161328
break;
13171329

13181330
case Syntax.ObjectPattern:
@@ -1340,7 +1352,8 @@
13401352

13411353
withIndent(function (indent) {
13421354
for (i = 0, len = expr.properties.length; i < len; ++i) {
1343-
result.push(multiline ? indent : '', generateExpression(expr.properties[i], {
1355+
result.push(multiline ? indent : '');
1356+
result.push(generateExpression(expr.properties[i], {
13441357
precedence: Precedence.Sequence,
13451358
allowIn: true,
13461359
allowCall: true
@@ -1354,7 +1367,8 @@
13541367
if (multiline && !endsWithLineTerminator(toSourceNodeWhenNeeded(result).toString())) {
13551368
result.push(newline);
13561369
}
1357-
result.push(multiline ? base : '', '}');
1370+
result.push(multiline ? base : '');
1371+
result.push('}');
13581372
break;
13591373

13601374
case Syntax.ThisExpression:
@@ -1679,7 +1693,8 @@
16791693
// };
16801694
if (stmt.declarations.length === 1 && stmt.declarations[0].init &&
16811695
stmt.declarations[0].init.type === Syntax.FunctionExpression) {
1682-
result.push(noEmptySpace(), generateStatement(stmt.declarations[0], {
1696+
result.push(noEmptySpace());
1697+
result.push(generateStatement(stmt.declarations[0], {
16831698
allowIn: allowIn
16841699
}));
16851700
} else {
@@ -1689,23 +1704,27 @@
16891704
withIndent(function () {
16901705
node = stmt.declarations[0];
16911706
if (extra.comment && node.leadingComments) {
1692-
result.push('\n', addIndent(generateStatement(node, {
1707+
result.push('\n');
1708+
result.push(addIndent(generateStatement(node, {
16931709
allowIn: allowIn
16941710
})));
16951711
} else {
1696-
result.push(noEmptySpace(), generateStatement(node, {
1712+
result.push(noEmptySpace());
1713+
result.push(generateStatement(node, {
16971714
allowIn: allowIn
16981715
}));
16991716
}
17001717

17011718
for (i = 1, len = stmt.declarations.length; i < len; ++i) {
17021719
node = stmt.declarations[i];
17031720
if (extra.comment && node.leadingComments) {
1704-
result.push(',' + newline, addIndent(generateStatement(node, {
1721+
result.push(',' + newline);
1722+
result.push(addIndent(generateStatement(node, {
17051723
allowIn: allowIn
17061724
})));
17071725
} else {
1708-
result.push(',' + space, generateStatement(node, {
1726+
result.push(',' + space);
1727+
result.push(generateStatement(node, {
17091728
allowIn: allowIn
17101729
}));
17111730
}
@@ -1867,28 +1886,33 @@
18671886
precedence: Precedence.Sequence,
18681887
allowIn: false,
18691888
allowCall: true
1870-
}), ';');
1889+
}));
1890+
result.push(';');
18711891
}
18721892
} else {
18731893
result.push(';');
18741894
}
18751895

18761896
if (stmt.test) {
1877-
result.push(space, generateExpression(stmt.test, {
1897+
result.push(space);
1898+
result.push(generateExpression(stmt.test, {
18781899
precedence: Precedence.Sequence,
18791900
allowIn: true,
18801901
allowCall: true
1881-
}), ';');
1902+
}));
1903+
result.push(';');
18821904
} else {
18831905
result.push(';');
18841906
}
18851907

18861908
if (stmt.update) {
1887-
result.push(space, generateExpression(stmt.update, {
1909+
result.push(space);
1910+
result.push(generateExpression(stmt.update, {
18881911
precedence: Precedence.Sequence,
18891912
allowIn: true,
18901913
allowCall: true
1891-
}), ')');
1914+
}));
1915+
result.push(')');
18921916
} else {
18931917
result.push(')');
18941918
}

‎gulpfile.js

+23-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
notice, this list of conditions and the following disclaimer in the
1111
documentation and/or other materials provided with the distribution.
1212
13-
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
13+
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 'AS IS'
1414
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
1515
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
1616
ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
@@ -22,11 +22,12 @@
2222
THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
2323
*/
2424

25-
'use strict'
25+
'use strict';
2626

2727
var gulp = require('gulp');
2828
var mocha = require('gulp-mocha');
2929
var jshint = require('gulp-jshint');
30+
var eslint = require('gulp-eslint');
3031

3132
var TEST = [ 'test/*.js' ];
3233

@@ -35,12 +36,31 @@ var LINT = [
3536
'escodegen.js'
3637
];
3738

39+
var ESLINT_OPTION = {
40+
'rulesdir': 'tools/rules/',
41+
'rules': {
42+
'push-with-multiple-arguments': 2,
43+
'quotes': 0,
44+
'eqeqeq': 0,
45+
'no-use-before-define': 0,
46+
'no-shadow': 0
47+
},
48+
'env': {
49+
'node': true
50+
}
51+
};
52+
3853
gulp.task('test', function () {
3954
return gulp.src(TEST).pipe(mocha({ reporter: 'spec' }));
4055
});
4156

4257
gulp.task('lint', function () {
43-
return gulp.src(LINT).pipe(jshint('.jshintrc'));
58+
return gulp.src(LINT)
59+
.pipe(jshint('.jshintrc'))
60+
.pipe(jshint.reporter(require('jshint-stylish')))
61+
.pipe(jshint.reporter('fail'))
62+
.pipe(eslint(ESLINT_OPTION))
63+
.pipe(eslint.formatEach('compact', process.stderr));
4464
});
4565

4666
gulp.task('travis', [ 'lint', 'test' ]);

‎package.json

+4-2
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@
3737
"bower": "*",
3838
"semver": "*",
3939
"chai": "~1.7.2",
40-
"gulp-jshint": "~1.3.4",
4140
"gulp": "~3.5.0",
42-
"gulp-mocha": "~0.4.1"
41+
"gulp-mocha": "~0.4.1",
42+
"gulp-eslint": "~0.1.2",
43+
"jshint-stylish": "~0.1.5",
44+
"gulp-jshint": "~1.4.0"
4345
},
4446
"licenses": [
4547
{

‎tools/rules/push-with-multiple-arguments.js

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
Copyright (C) 2014 Yusuke Suzuki <[email protected]>
3+
4+
Redistribution and use in source and binary forms, with or without
5+
modification, are permitted provided that the following conditions are met:
6+
7+
* Redistributions of source code must retain the above copyright
8+
notice, this list of conditions and the following disclaimer.
9+
* Redistributions in binary form must reproduce the above copyright
10+
notice, this list of conditions and the following disclaimer in the
11+
documentation and/or other materials provided with the distribution.
12+
13+
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
14+
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
15+
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
16+
ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
17+
DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
18+
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
19+
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
20+
ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
21+
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
22+
THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
23+
*/
24+
25+
'use strict'
26+
27+
module.exports = function (context) {
28+
var MESSAGE = '"push" with multiple arguments hurts performance since optimizing compiler would not support it';
29+
function checkPush(node) {
30+
var member;
31+
if (node.callee.type !== 'MemberExpression') {
32+
return;
33+
}
34+
member = node.callee;
35+
if (member.computed) {
36+
return;
37+
}
38+
if (member.property.type !== 'Identifier') {
39+
return;
40+
}
41+
if (member.property.name !== 'push') {
42+
return;
43+
}
44+
// Ensure arguments length is 1.
45+
if (node.arguments.length !== 1) {
46+
context.report(node, MESSAGE);
47+
}
48+
}
49+
50+
return {
51+
'CallExpression': checkPush
52+
};
53+
};

0 commit comments

Comments
 (0)