From 546cb429d9cea25a9bdadbb87dfd401366b0b908 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 18 Apr 2014 16:20:02 -0700 Subject: [PATCH] perf($interpolate): speed up interpolation by recreating watchGroup approach This change undoes the use of watchGroup by code that uses $interpolate, by moving the optimizations into the $interpolate itself. While this is not ideal, it means that we are making the existing api faster rather than require people to use $interpolate differently in order to benefit from the speed improvements. --- src/ng/compile.js | 21 +++----- src/ng/directive/select.js | 14 +++-- src/ng/interpolate.js | 105 +++++++++++++++++++++++-------------- test/ng/interpolateSpec.js | 70 ++++++++++++------------- 4 files changed, 115 insertions(+), 95 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 782d0734d411..4256adcaabc7 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1804,9 +1804,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings = parent.data('$binding') || []; bindings.push(interpolateFn); safeAddClass(parent.data('$binding', bindings), 'ng-binding'); - scope.$watchGroup(interpolateFn.expressions, - function textInterpolationWatchAction(newValues) { - node[0].nodeValue = interpolateFn.compute(newValues); + scope.$watch(interpolateFn, function interpolateFnWatchAction(value) { + node[0].nodeValue = value; }); }) }); @@ -1848,8 +1847,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return { pre: function attrInterpolatePreLinkFn(scope, element, attr) { var $$observers = (attr.$$observers || (attr.$$observers = {})); - var interpolationResult; - var lastInterpolationResult; if (EVENT_HANDLER_ATTR_REGEXP.test(name)) { throw $compileMinErr('nodomevents', @@ -1868,25 +1865,21 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // initialize attr object so that it's ready in case we need the value for isolate // scope initialization, otherwise the value would not be available from isolate // directive's linking fn during linking phase - attr[name] = interpolationResult = interpolateFn(scope); + attr[name] = interpolateFn(scope); ($$observers[name] || ($$observers[name] = [])).$$inter = true; (attr.$$observers && attr.$$observers[name].$$scope || scope). - $watchGroup(interpolateFn.expressions, - function interpolationWatchAction(newValues) { - - lastInterpolationResult = interpolationResult; - interpolationResult = interpolateFn.compute(newValues); + $watch(interpolateFn, function interpolateFnWatchAction(newValue, oldValue) { //special case for class attribute addition + removal //so that class changes can tap into the animation //hooks provided by the $animate service. Be sure to //skip animations when the first digest occurs (when //both the new and the old values are the same) since //the CSS classes are the non-interpolated values - if(name === 'class' && interpolationResult != lastInterpolationResult) { - attr.$updateClass(interpolationResult, lastInterpolationResult); + if(name === 'class' && newValue != oldValue) { + attr.$updateClass(newValue, oldValue); } else { - attr.$set(name, interpolationResult); + attr.$set(name, newValue); } }); } diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 958e6b3dfbb2..95fe5ad7d088 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -609,8 +609,6 @@ var optionDirective = ['$interpolate', function($interpolate) { parent = element.parent(), selectCtrl = parent.data(selectCtrlName) || parent.parent().data(selectCtrlName); // in case we are in optgroup - var newString; - var oldString; if (selectCtrl && selectCtrl.databound) { // For some reason Opera defaults to true and if not overridden this messes up the repeater. @@ -621,12 +619,12 @@ var optionDirective = ['$interpolate', function($interpolate) { } if (interpolateFn) { - scope.$watchGroup(interpolateFn.expressions, function interpolateWatchAction(newVals, oldVals) { - oldString = newString; - newString = interpolateFn.compute(newVals); - attr.$set('value', newString); - if (oldString) selectCtrl.removeOption(oldString); - selectCtrl.addOption(newString); + scope.$watch(interpolateFn, function interpolateWatchAction(newVal, oldVal) { + attr.$set('value', newVal); + if (oldVal !== newVal) { + selectCtrl.removeOption(oldVal); + } + selectCtrl.addOption(newVal); }); } else { selectCtrl.addOption(attr.value); diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index 87ad2de7303a..9ac68e8b7727 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -114,15 +114,10 @@ function $InterpolateProvider() { * result through {@link ng.$sce#getTrusted $sce.getTrusted(interpolatedResult, * trustedContext)} before returning it. Refer to the {@link ng.$sce $sce} service that * provides Strict Contextual Escaping for details. - * @returns {Object} An object describing the interpolation template string. + * @returns {function(context)} an interpolation function which is used to compute the + * interpolated string. The function has these parameters: * - * The properties of the returned object include: - * - * - `template` — `{string}` — original interpolation template string. - * - `separators` — `{Array.}` — array of separators extracted from the template. - * - `expressions` — `{Array.}` — array of expressions extracted from the template. - * - `compute` — {function(Array)()} — function that when called with an array of values will - * compute the result of interpolation for the given interpolation template and values. + * - `context`: evaluation context for all expressions embedded in the interpolated text */ function $interpolate(text, mustHaveExpression, trustedContext) { var startIndex, @@ -130,12 +125,13 @@ function $InterpolateProvider() { index = 0, separators = [], expressions = [], + parseFns = [], textLength = text.length, hasInterpolation = false, hasText = false, - fn, exp, - concat = []; + concat = [], + lastValuesCache = { values: {}, results: {}}; while(index < textLength) { if ( ((startIndex = text.indexOf(startSymbol, index)) != -1) && @@ -144,6 +140,7 @@ function $InterpolateProvider() { separators.push(text.substring(index, startIndex)); exp = text.substring(startIndex + startSymbolLength, endIndex); expressions.push(exp); + parseFns.push($parse(exp)); index = endIndex + endSymbolLength; hasInterpolation = true; } else { @@ -176,31 +173,16 @@ function $InterpolateProvider() { if (!mustHaveExpression || hasInterpolation) { concat.length = separators.length + expressions.length; - return extend(function interpolationFn(scope) { - var values = []; - forEach(interpolationFn.expressions, function(expression) { - values.push(scope.$eval(expression)); - }); - return interpolationFn.compute(values); - }, { - exp: text, //deprecated - template: text, - separators: separators, - expressions: expressions, - compute: function(values) { - for(var i = 0, ii = expressions.length; i < ii; i++) { - concat[2*i] = separators[i]; - concat[(2*i)+1] = stringify(values[i]); - } - concat[2*ii] = separators[ii]; - return concat.join(''); + var compute = function(values) { + for(var i = 0, ii = expressions.length; i < ii; i++) { + concat[2*i] = separators[i]; + concat[(2*i)+1] = values[i]; } - }); - } - - function stringify(value) { - try { + concat[2*ii] = separators[ii]; + return concat.join(''); + }; + var stringify = function (value) { if (trustedContext) { value = $sce.getTrusted(trustedContext, value); } else { @@ -214,12 +196,59 @@ function $InterpolateProvider() { } return value; + }; - } catch(err) { - var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, - err.toString()); - $exceptionHandler(newErr); - } + return extend(function interpolationFn(context) { + var scopeId = context.$id || 'notAScope'; + var lastValues = lastValuesCache.values[scopeId]; + var lastResult = lastValuesCache.results[scopeId]; + var i = 0; + var ii = expressions.length; + var values = new Array(ii); + var val; + var inputsChanged = lastResult === undefined ? true: false; + + + // if we haven't seen this context before, initialize the cache and try to setup + // a cleanup routine that purges the cache when the scope goes away. + if (!lastValues) { + lastValues = []; + inputsChanged = true; + if (context.$on) { + context.$on('$destroy', function() { + lastValuesCache.values[scopeId] = null; + lastValuesCache.results[scopeId] = null; + }); + } + } + + + try { + for (; i < ii; i++) { + val = stringify(parseFns[i](context)); + if (val !== lastValues[i]) { + inputsChanged = true; + } + values[i] = val; + } + + if (inputsChanged) { + lastValuesCache.values[scopeId] = values; + lastValuesCache.results[scopeId] = lastResult = compute(values); + } + } catch(err) { + var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, + err.toString()); + $exceptionHandler(newErr); + } + + return lastResult; + }, { + // all of these properties are undocumented for now + exp: text, //just for compatibility with regular watchers created via $watch + separators: separators, + expressions: expressions + }); } } diff --git a/test/ng/interpolateSpec.js b/test/ng/interpolateSpec.js index 1fff54916e9d..864b4b87d06c 100644 --- a/test/ng/interpolateSpec.js +++ b/test/ng/interpolateSpec.js @@ -3,16 +3,14 @@ describe('$interpolate', function() { it('should return the interpolation object when there are no bindings and textOnly is undefined', - inject(function($interpolate, $rootScope) { + inject(function($interpolate) { var interpolateFn = $interpolate('some text'); expect(interpolateFn.exp).toBe('some text'); - expect(interpolateFn.template).toBe('some text'); expect(interpolateFn.separators).toEqual(['some text']); expect(interpolateFn.expressions).toEqual([]); expect(interpolateFn({})).toBe('some text'); - expect(interpolateFn.compute([])).toBe('some text'); })); @@ -22,15 +20,15 @@ describe('$interpolate', function() { })); it('should suppress falsy objects', inject(function($interpolate) { - expect($interpolate('{{undefined}}').compute([undefined])).toEqual(''); - expect($interpolate('{{null}}').compute([null])).toEqual(''); - expect($interpolate('{{a.b}}').compute([undefined])).toEqual(''); + expect($interpolate('{{undefined}}')({})).toEqual(''); + expect($interpolate('{{null}}')({})).toEqual(''); + expect($interpolate('{{a.b}}')({})).toEqual(''); })); it('should jsonify objects', inject(function($interpolate) { - expect($interpolate('{{ {} }}').compute([{}])).toEqual('{}'); - expect($interpolate('{{ true }}').compute([true])).toEqual('true'); - expect($interpolate('{{ false }}').compute([false])).toEqual('false'); + expect($interpolate('{{ {} }}')({})).toEqual('{}'); + expect($interpolate('{{ true }}')({})).toEqual('true'); + expect($interpolate('{{ false }}')({})).toEqual('false'); })); @@ -38,7 +36,6 @@ describe('$interpolate', function() { var interpolateFn = $interpolate('Hello {{name}}!'); expect(interpolateFn.exp).toBe('Hello {{name}}!'); - expect(interpolateFn.template).toBe('Hello {{name}}!'); expect(interpolateFn.separators).toEqual(['Hello ', '!']); expect(interpolateFn.expressions).toEqual(['name']); @@ -46,12 +43,11 @@ describe('$interpolate', function() { scope.name = 'Bubu'; expect(interpolateFn(scope)).toBe('Hello Bubu!'); - expect(interpolateFn.compute(['Bubu'])).toBe('Hello Bubu!'); })); it('should ignore undefined model', inject(function($interpolate) { - expect($interpolate("Hello {{'World'}}{{foo}}").compute(['World'])).toBe('Hello World'); + expect($interpolate("Hello {{'World'}}{{foo}}")({})).toBe('Hello World'); })); @@ -67,28 +63,32 @@ describe('$interpolate', function() { inject(['$sce', function($sce) { sce = $sce; }]); }); - it('should NOT interpolate non-trusted expressions', inject(function($interpolate) { - var foo = "foo"; + it('should NOT interpolate non-trusted expressions', inject(function($interpolate, $rootScope) { + var scope = $rootScope.$new(); + scope.foo = "foo"; + expect(function() { - $interpolate('{{foo}}', true, sce.CSS).compute([foo]); + $interpolate('{{foo}}', true, sce.CSS)(scope); }).toThrowMinErr('$interpolate', 'interr'); })); - it('should NOT interpolate mistyped expressions', inject(function($interpolate) { - var foo = sce.trustAsCss("foo"); + it('should NOT interpolate mistyped expressions', inject(function($interpolate, $rootScope) { + var scope = $rootScope.$new(); + scope.foo = sce.trustAsCss("foo"); + expect(function() { - $interpolate('{{foo}}', true, sce.HTML).compute([foo]); + $interpolate('{{foo}}', true, sce.HTML)(scope); }).toThrowMinErr('$interpolate', 'interr'); })); it('should interpolate trusted expressions in a regular context', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true).compute([foo])).toEqual('foo'); + expect($interpolate('{{foo}}', true)({foo: foo})).toBe('foo'); })); it('should interpolate trusted expressions in a specific trustedContext', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true, sce.CSS).compute([foo])).toEqual('foo'); + expect($interpolate('{{foo}}', true, sce.CSS)({foo: foo})).toBe('foo'); })); // The concatenation of trusted values does not necessarily result in a trusted value. (For @@ -98,7 +98,7 @@ describe('$interpolate', function() { var foo = sce.trustAsCss("foo"); var bar = sce.trustAsCss("bar"); expect(function() { - return $interpolate('{{foo}}{{bar}}', true, sce.CSS).compute([foo, bar]); + return $interpolate('{{foo}}{{bar}}', true, sce.CSS)({foo: foo, bar: bar}); }).toThrowMinErr( "$interpolate", "noconcat", "Error while interpolating: {{foo}}{{bar}}\n" + "Strict Contextual Escaping disallows interpolations that concatenate multiple " + @@ -117,8 +117,8 @@ describe('$interpolate', function() { it('should not get confused with same markers', inject(function($interpolate) { expect($interpolate('---').separators).toEqual(['---']); expect($interpolate('---').expressions).toEqual([]); - expect($interpolate('----').compute([])).toEqual(''); - expect($interpolate('--1--').compute([1])).toEqual('1'); + expect($interpolate('----')({})).toEqual(''); + expect($interpolate('--1--')({})).toEqual('1'); })); }); @@ -138,7 +138,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['a', 'C']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('a123C'); + expect(interpolateFn({b: 123})).toEqual('a123C'); })); it('should Parse Ending Binding', inject(function($interpolate) { @@ -146,7 +146,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['a', '']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('a123'); + expect(interpolateFn({b: 123})).toEqual('a123'); })); it('should Parse Begging Binding', inject(function($interpolate) { @@ -154,7 +154,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', 'c']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('123c'); + expect(interpolateFn({b: 123})).toEqual('123c'); })); it('should Parse Loan Binding', inject(function($interpolate) { @@ -162,7 +162,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', '']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('123'); + expect(interpolateFn({b: 123})).toEqual('123'); })); it('should Parse Two Bindings', inject(function($interpolate) { @@ -170,7 +170,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', '', '']); expect(expressions).toEqual(['b', 'c']); - expect(interpolateFn.compute([111, 222])).toEqual('111222'); + expect(interpolateFn({b: 111, c: 222})).toEqual('111222'); })); it('should Parse Two Bindings With Text In Middle', inject(function($interpolate) { @@ -178,7 +178,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', 'x', '']); expect(expressions).toEqual(['b', 'c']); - expect(interpolateFn.compute([111, 222])).toEqual('111x222'); + expect(interpolateFn({b: 111, c: 222})).toEqual('111x222'); })); it('should Parse Multiline', inject(function($interpolate) { @@ -186,7 +186,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['"X\nY', 'C\nD"']); expect(expressions).toEqual(['A\n+B']); - expect(interpolateFn.compute([123])).toEqual('"X\nY123C\nD"'); + expect(interpolateFn({'A': 'aa', 'B': 'bb'})).toEqual('"X\nYaabbC\nD"'); })); }); @@ -215,9 +215,9 @@ describe('$interpolate', function() { })); it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate) { - expect($interpolate('some/{{id}}').compute([undefined])).toEqual('some/'); - expect($interpolate('some/{{id}}').compute([1])).toEqual('some/1'); - expect($interpolate('{{foo}}{{bar}}').compute([1, 2])).toEqual('12'); + expect($interpolate('some/{{id}}')({})).toEqual('some/'); + expect($interpolate('some/{{id}}')({id: 1})).toEqual('some/1'); + expect($interpolate('{{foo}}{{bar}}')({foo: 1, bar: 2})).toEqual('12'); })); }); @@ -249,8 +249,8 @@ describe('$interpolate', function() { inject(function($interpolate) { expect($interpolate('---').separators).toEqual(['---']); expect($interpolate('---').expressions).toEqual([]); - expect($interpolate('----').compute([])).toEqual(''); - expect($interpolate('--1--').compute([1])).toEqual('1'); + expect($interpolate('----')({})).toEqual(''); + expect($interpolate('--1--')({})).toEqual('1'); }); }); });