From fffe9a69cd225926a788a95a213ef071c2431551 Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Thu, 5 Dec 2013 18:08:52 -0500 Subject: [PATCH] perf($parse) use a faster path when the number of path parts is low Use a faster path when the number of path tokens is low (ie the common case). This results in a better than 19x improvement in the time spent in $parse and produces output that is about the same speed in chrome and substantially faster in firefox. http://jsperf.com/angularjs-parse-getter/6 --- src/ng/parse.js | 60 +++++++++++++++++++++++++++++++------------- test/ng/parseSpec.js | 1 + 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index 8b3f145a8211..50462e4313df 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -891,19 +891,19 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { ? function cspSafeGetter(scope, locals) { var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope; - if (pathVal === null || pathVal === undefined) return pathVal; + if (pathVal == null) return pathVal; pathVal = pathVal[key0]; - if (!key1 || pathVal === null || pathVal === undefined) return pathVal; + if (!key1 || pathVal == null) return pathVal; pathVal = pathVal[key1]; - if (!key2 || pathVal === null || pathVal === undefined) return pathVal; + if (!key2 || pathVal == null) return pathVal; pathVal = pathVal[key2]; - if (!key3 || pathVal === null || pathVal === undefined) return pathVal; + if (!key3 || pathVal == null) return pathVal; pathVal = pathVal[key3]; - if (!key4 || pathVal === null || pathVal === undefined) return pathVal; + if (!key4 || pathVal == null) return pathVal; pathVal = pathVal[key4]; return pathVal; @@ -912,7 +912,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope, promise; - if (pathVal === null || pathVal === undefined) return pathVal; + if (pathVal == null) return pathVal; pathVal = pathVal[key0]; if (pathVal && pathVal.then) { @@ -924,7 +924,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } pathVal = pathVal.$$v; } - if (!key1 || pathVal === null || pathVal === undefined) return pathVal; + if (!key1 || pathVal == null) return pathVal; pathVal = pathVal[key1]; if (pathVal && pathVal.then) { @@ -936,7 +936,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } pathVal = pathVal.$$v; } - if (!key2 || pathVal === null || pathVal === undefined) return pathVal; + if (!key2 || pathVal == null) return pathVal; pathVal = pathVal[key2]; if (pathVal && pathVal.then) { @@ -948,7 +948,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } pathVal = pathVal.$$v; } - if (!key3 || pathVal === null || pathVal === undefined) return pathVal; + if (!key3 || pathVal == null) return pathVal; pathVal = pathVal[key3]; if (pathVal && pathVal.then) { @@ -960,7 +960,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } pathVal = pathVal.$$v; } - if (!key4 || pathVal === null || pathVal === undefined) return pathVal; + if (!key4 || pathVal == null) return pathVal; pathVal = pathVal[key4]; if (pathVal && pathVal.then) { @@ -976,6 +976,27 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { }; } +function simpleGetterFn1(key0, fullExp) { + ensureSafeMemberName(key0, fullExp); + + return function simpleGetterFn1(scope, locals) { + if (scope == null) return scope; + return ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0]; + }; +} + +function simpleGetterFn2(key0, key1, fullExp) { + ensureSafeMemberName(key0, fullExp); + ensureSafeMemberName(key1, fullExp); + + return function simpleGetterFn2(scope, locals) { + if (scope == null) return scope; + scope = ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0]; + + return scope == null ? scope : scope[key1]; + }; +} + function getterFn(path, options, fullExp) { // Check whether the cache has this getter already. // We can use hasOwnProperty directly on the cache because we ensure, @@ -988,7 +1009,13 @@ function getterFn(path, options, fullExp) { pathKeysLength = pathKeys.length, fn; - if (options.csp) { + // When we have only 1 or 2 tokens, use optimized special case closures. + // http://jsperf.com/angularjs-parse-getter/6 + if (!options.unwrapPromises && pathKeysLength === 1) { + fn = simpleGetterFn1(pathKeys[0], fullExp); + } else if (!options.unwrapPromises && pathKeysLength === 2) { + fn = simpleGetterFn2(pathKeys[0], pathKeys[1], fullExp); + } else if (options.csp) { if (pathKeysLength < 6) { fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp, options); @@ -1006,11 +1033,10 @@ function getterFn(path, options, fullExp) { }; } } else { - var code = 'var l, fn, p;\n'; + var code = 'var p;\n'; forEach(pathKeys, function(key, index) { ensureSafeMemberName(key, fullExp); - code += 'if(s === null || s === undefined) return s;\n' + - 'l=s;\n' + + code += 'if(s == null) return s;\n' + 's='+ (index // we simply dereference 's' on any .dot notation ? 's' @@ -1033,10 +1059,10 @@ function getterFn(path, options, fullExp) { /* jshint -W054 */ var evaledFnGetter = new Function('s', 'k', 'pw', code); // s=scope, k=locals, pw=promiseWarning /* jshint +W054 */ - evaledFnGetter.toString = function() { return code; }; - fn = function(scope, locals) { + evaledFnGetter.toString = valueFn(code); + fn = options.unwrapPromises ? function(scope, locals) { return evaledFnGetter(scope, locals, promiseWarning); - }; + } : evaledFnGetter; } // Only cache the value if it's not going to mess up the cache object diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 2147c4b0b0d1..5e985f1f1ece 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -906,6 +906,7 @@ describe('parser', function() { expect($parse('a.b')({a: {b: 0}}, {a: {b:1}})).toEqual(1); expect($parse('a.b')({a: null}, {a: {b:1}})).toEqual(1); expect($parse('a.b')({a: {b: 0}}, {a: null})).toEqual(undefined); + expect($parse('a.b.c')({a: null}, {a: {b: {c: 1}}})).toEqual(1); })); });