From edf364d215d203f2f0b3bdc56176c739876493a4 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sat, 30 Aug 2014 23:48:33 -0700 Subject: [PATCH 1/7] perf($parse): simplifying multi-statement execution --- src/ng/parse.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index b869f8666945..dd21ca5c4ee3 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -533,11 +533,8 @@ Parser.prototype = { ? statements[0] : function(self, locals) { var value; - for (var i = 0; i < statements.length; i++) { - var statement = statements[i]; - if (statement) { - value = statement(self, locals); - } + for (var i = 0, ii = statements.length; i < ii; i++) { + value = statements[i](self, locals); } return value; }; From dd078b7442fdda7fbcfdadaa7eab98910ebd791e Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sun, 31 Aug 2014 13:55:04 -0700 Subject: [PATCH 2/7] style($parse): adding function names for easier debugging --- src/ng/parse.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index dd21ca5c4ee3..c172f9e33623 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -531,7 +531,7 @@ Parser.prototype = { // TODO(size): maybe we should not support multiple statements? return (statements.length === 1) ? statements[0] - : function(self, locals) { + : function $parseStatements(self, locals) { var value; for (var i = 0, ii = statements.length; i < ii; i++) { value = statements[i](self, locals); @@ -598,7 +598,7 @@ Parser.prototype = { this.text.substring(0, token.index) + '] can not be assigned to', token); } right = this.ternary(); - return function(scope, locals) { + return function $parseAssignment(scope, locals) { return left.assign(scope, right(scope, locals), locals); }; } @@ -788,7 +788,7 @@ Parser.prototype = { } this.consume(']'); - return extend(function(self, locals) { + return extend(function $parseArrayLiteral(self, locals) { var array = []; for (var i = 0; i < elementFns.length; i++) { array.push(elementFns[i](self, locals)); @@ -821,7 +821,7 @@ Parser.prototype = { } this.consume('}'); - return extend(function(self, locals) { + return extend(function $parseObjectLiteral(self, locals) { var object = {}; for (var i = 0; i < keyValues.length; i++) { var keyValue = keyValues[i]; From 12eb24d6cea7aae53fc5a776f8d11a56cc9ad3b3 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sun, 31 Aug 2014 13:59:55 -0700 Subject: [PATCH 3/7] perf($parse): calculate array lengths once at start of loop --- src/ng/parse.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index c172f9e33623..0974d7073a1a 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -790,7 +790,7 @@ Parser.prototype = { return extend(function $parseArrayLiteral(self, locals) { var array = []; - for (var i = 0; i < elementFns.length; i++) { + for (var i = 0, ii = elementFns.length; i < ii; i++) { array.push(elementFns[i](self, locals)); } return array; @@ -823,7 +823,7 @@ Parser.prototype = { return extend(function $parseObjectLiteral(self, locals) { var object = {}; - for (var i = 0; i < keyValues.length; i++) { + for (var i = 0, ii = keyValues.length; i < ii; i++) { var keyValue = keyValues[i]; object[keyValue.key] = keyValue.value(self, locals); } From 48c6b11a9b9c8bae888aea1f2b2cb252125d9699 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Mon, 1 Sep 2014 19:48:57 -0700 Subject: [PATCH 4/7] style($parse): simplifying some while(true) loops --- src/ng/parse.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index 0974d7073a1a..c4424a8ecaa2 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -545,13 +545,10 @@ Parser.prototype = { filterChain: function() { var left = this.expression(); var token; - while (true) { - if ((token = this.expect('|'))) { - left = this.binaryFn(left, token.fn, this.filter()); - } else { - return left; - } + while ((token = this.expect('|'))) { + left = this.binaryFn(left, token.fn, this.filter()); } + return left; }, filter: function() { @@ -624,13 +621,10 @@ Parser.prototype = { logicalOR: function() { var left = this.logicalAND(); var token; - while (true) { - if ((token = this.expect('||'))) { - left = this.binaryFn(left, token.fn, this.logicalAND()); - } else { - return left; - } + while ((token = this.expect('||'))) { + left = this.binaryFn(left, token.fn, this.logicalAND()); } + return left; }, logicalAND: function() { From c074181ba81579e121629e201a7749425cbbd8cb Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Mon, 1 Sep 2014 20:42:35 -0700 Subject: [PATCH 5/7] perf($parse): removing references to Parser/Lexer from parsed expressions --- src/ng/parse.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index c4424a8ecaa2..bd0ad05b6bee 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -258,7 +258,7 @@ Lexer.prototype = { }, readIdent: function() { - var parser = this; + var parserText = this.text; var ident = ''; var start = this.index; @@ -317,13 +317,13 @@ Lexer.prototype = { token.fn = fn; token.constant = true; } else { - var getter = getterFn(ident, this.options, this.text); + var getter = getterFn(ident, this.options, parserText); // TODO(perf): consider exposing the getter reference token.fn = extend(function $parsePathGetter(self, locals) { return getter(self, locals); }, { assign: function(self, value) { - return setter(self, ident, value, parser.text); + return setter(self, ident, value, parserText); } }); } @@ -686,9 +686,9 @@ Parser.prototype = { }, fieldAccess: function(object) { - var parser = this; + var parserText = this.text; var field = this.expect().text; - var getter = getterFn(field, this.options, this.text); + var getter = getterFn(field, this.options, parserText); return extend(function $parseFieldAccess(scope, locals, self) { return getter(self || object(scope, locals)); @@ -696,13 +696,13 @@ Parser.prototype = { assign: function(scope, value, locals) { var o = object(scope, locals); if (!o) object.assign(scope, o = {}); - return setter(o, field, value, parser.text); + return setter(o, field, value, parserText); } }); }, objectIndex: function(obj) { - var parser = this; + var parserText = this.text; var indexFn = this.expression(); this.consume(']'); @@ -712,15 +712,15 @@ Parser.prototype = { i = indexFn(self, locals), v; - ensureSafeMemberName(i, parser.text); + ensureSafeMemberName(i, parserText); if (!o) return undefined; - v = ensureSafeObject(o[i], parser.text); + v = ensureSafeObject(o[i], parserText); return v; }, { assign: function(self, value, locals) { - var key = ensureSafeMemberName(indexFn(self, locals), parser.text); + var key = ensureSafeMemberName(indexFn(self, locals), parserText); // prevent overwriting of Function.constructor which would break ensureSafeObject check - var o = ensureSafeObject(obj(self, locals), parser.text); + var o = ensureSafeObject(obj(self, locals), parserText); if (!o) obj.assign(self, o = {}); return o[key] = value; } From eb47e910e06977bc2f6973b64cbe7d2741f4508c Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Tue, 2 Sep 2014 22:13:51 -0700 Subject: [PATCH 6/7] perf($parse): remove getterFn wrapper for internal use --- src/ng/parse.js | 68 ++++++++++++++++++++++++++------------------ test/ng/parseSpec.js | 2 +- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index bd0ad05b6bee..2e7de5ea3e08 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -80,12 +80,21 @@ function ensureSafeFunction(obj, fullExpression) { } } +//Keyword constants +var CONSTANTS = createMap(); +forEach({ + 'null':function(){return null;}, + 'true':function(){return true;}, + 'false':function(){return false;}, + 'undefined':function(){} +}, function(constFunc, name) { + constFunc.constant = constFunc.literal = constFunc.$$parseShared = true; + CONSTANTS[name] = constFunc; +}); + +//Operators - will be wrapped by binaryFn/unaryFn/assignment/filter var OPERATORS = extend(createMap(), { /* jshint bitwise : false */ - 'null':function(){return null;}, - 'true':function(){return true;}, - 'false':function(){return false;}, - undefined:noop, '+':function(self, locals, a,b){ a=a(self, locals); b=b(self, locals); if (isDefined(a)) { @@ -305,30 +314,11 @@ Lexer.prototype = { } } - - var token = { + this.tokens.push({ index: start, - text: ident - }; - - var fn = OPERATORS[ident]; - - if (fn) { - token.fn = fn; - token.constant = true; - } else { - var getter = getterFn(ident, this.options, parserText); - // TODO(perf): consider exposing the getter reference - token.fn = extend(function $parsePathGetter(self, locals) { - return getter(self, locals); - }, { - assign: function(self, value) { - return setter(self, ident, value, parserText); - } - }); - } - - this.tokens.push(token); + text: ident, + fn: CONSTANTS[ident] || getterFn(ident, this.options, parserText) + }); if (methodName) { this.tokens.push({ @@ -397,6 +387,7 @@ var Parser = function (lexer, $filter, options) { Parser.ZERO = extend(function () { return 0; }, { + $$parseShared: true, constant: true }); @@ -935,9 +926,14 @@ function getterFn(path, options, fullExp) { var evaledFnGetter = new Function('s', 'l', code); // s=scope, l=locals /* jshint +W054 */ evaledFnGetter.toString = valueFn(code); + evaledFnGetter.assign = function(self, value) { + return setter(self, path, value, path); + }; + fn = evaledFnGetter; } + fn.$$parseShared = true; getterFnCache[path] = fn; return fn; } @@ -1005,6 +1001,21 @@ function $ParseProvider() { this.$get = ['$filter', '$sniffer', function($filter, $sniffer) { $parseOptions.csp = $sniffer.csp; + function wrapSharedExpression(exp) { + var wrapped = exp; + + if (exp.$$parseShared) { + wrapped = function $parseWrapper(self, locals) { + return exp(self, locals); + }; + wrapped.literal = exp.literal; + wrapped.constant = exp.constant; + wrapped.assign = exp.assign; + } + + return wrapped; + } + return function $parse(exp, interceptorFn) { var parsedExpression, oneTime, cacheKey; @@ -1027,6 +1038,9 @@ function $ParseProvider() { if (parsedExpression.constant) { parsedExpression.$$watchDelegate = constantWatchDelegate; } else if (oneTime) { + //oneTime is not part of the exp passed to the Parser so we may have to + //wrap the parsedExpression before adding a $$watchDelegate + parsedExpression = wrapSharedExpression(parsedExpression); parsedExpression.$$watchDelegate = parsedExpression.literal ? oneTimeLiteralWatchDelegate : oneTimeWatchDelegate; } diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 265f12c67b54..31b7b18d4459 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -722,7 +722,7 @@ describe('parser', function() { scope.$eval('a.toString.constructor = 1', scope); }).toThrowMinErr( '$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' + - 'Expression: a.toString.constructor = 1'); + 'Expression: a.toString.constructor'); }); From 19a82b353381c2a7c825cd88f81a51899277414a Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 4 Sep 2014 22:11:30 -0700 Subject: [PATCH 7/7] test($parse): adding benchmark for execution of $parse()ed expressions --- benchmarks/parsed-expressions-bp/app.js | 81 +++++++++ benchmarks/parsed-expressions-bp/bp.conf.js | 11 ++ benchmarks/parsed-expressions-bp/main.html | 192 ++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100755 benchmarks/parsed-expressions-bp/app.js create mode 100755 benchmarks/parsed-expressions-bp/bp.conf.js create mode 100755 benchmarks/parsed-expressions-bp/main.html diff --git a/benchmarks/parsed-expressions-bp/app.js b/benchmarks/parsed-expressions-bp/app.js new file mode 100755 index 000000000000..0ebea963af73 --- /dev/null +++ b/benchmarks/parsed-expressions-bp/app.js @@ -0,0 +1,81 @@ +var app = angular.module('parsedExpressionBenchmark', []); + +app.config(function($compileProvider) { + if ($compileProvider.debugInfoEnabled) { + $compileProvider.debugInfoEnabled(false); + } +}); + +app.filter('noop', function() { + return function(input) { + return input; + }; +}); + +//Executes the specified expression as a watcher +app.directive('bmPeWatch', function() { + return { + restrict: 'A', + compile: function($element, $attrs) { + $element.text( $attrs.bmPeWatch ); + return function($scope, $element, $attrs) { + $scope.$watch($attrs.bmPeWatch); + }; + } + }; +}); + +//Executes the specified expression as a watcher +//Adds a simple wrapper method to allow use of $watch instead of $watchCollection +app.directive('bmPeWatchLiteral', function($parse) { + function retZero() { + return 0; + } + + return { + restrict: 'A', + compile: function($element, $attrs) { + $element.text( $attrs.bmPeWatchLiteral ); + return function($scope, $element, $attrs) { + $scope.$watch( $parse($attrs.bmPeWatchLiteral, retZero) ); + }; + } + }; +}); + +app.controller('DataController', function($scope, $rootScope) { + var totalRows = 2000; + + var data = $scope.data = []; + + $scope.func = function() {}; + + for (var i=0; i +
+
+

+ Tests the execution of $parse()ed expressions. Each test tries to isolate specific expression types. Expressions should (probably) not be constant so they get evaluated per digest. +

+ +
    +
  • + + +
    • + +
  • + + +
    • + +
  • + + +
    • + +
  • + + +
    • + +
  • + + +
    • + +
  • + + +
    • + +
  • + + +
    • + +
  • + + +
    • +
+ + + +
    +
  • + + + + + + + + + + + + +
    • + +
  • + + + + + + + + + + + + + +
    • + +
  • + + + + + + + + + + + + +
    • + +
  • + + + + + + + + + + + + +
    • + +
  • + + + + + + + + + + + + +
    • + +
  • + + + + + + + + + + + + + + +
    • + +
  • + + + + + + +
    • + +
  • + + + + + + + + + + + + +
    • + + +
+
+
+ \ No newline at end of file