Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
perf($interpolate): speed up interpolation by recreating watchGroup a…
Browse files Browse the repository at this point in the history 
…pproach

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.
  • Loading branch information
@IgorMinar
IgorMinar committed Apr 21, 2014
1 parent 1db3b8c commit 546cb42
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 95 deletions.
21 changes: 7 additions & 14 deletions src/ng/compile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
})
});
Expand Down Expand Up @@ -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',
Expand All @@ -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);
}
});
}
Expand Down
14 changes: 6 additions & 8 deletions src/ng/directive/select.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down
105 changes: 67 additions & 38 deletions src/ng/interpolate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,24 @@ 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.<string>}` — array of separators extracted from the template.
* - `expressions` — `{Array.<string>}` — 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,
endIndex,
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) &&
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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() {

This comment has been minimized.

Sign in to view

Copy link
@jbedard

jbedard Jun 26, 2014

Contributor

@IgorMinar

This watcher seems very expensive. I'm seeing the watcher+closure consume 13k each which adds up quickly when ng-repeated.

Is that worth it? Or is there another way? I tried putting the cache into the scope by interpolation text, instead of putting the cache into the interpolation by scope id. This removes the use of the scope listener and (in my specific test) reduced load time (10%) and memory usage (30%). But doing this means the cache lives with the scope, so if the interpolation function gets GCed but the scope is not then the cache will live on, and I'm not sure that is acceptable? See 155367f

Edit: this listener also means the following code (from nodeLinkFn) basically leaks the listeners...

    // If the attribute has been provided then we trigger an interpolation to ensure
    // the value is there for use in the link fn
    isolateScope[scopeName] = $interpolate(attrs[attrName])(scope);

This comment has been minimized.

Sign in to view

Copy link
@IgorMinar

IgorMinar Jun 26, 2014

Author Contributor

we refactored this code already and there is another refactoring on the way: #7700

This comment has been minimized.

Sign in to view

Copy link
@jbedard

jbedard Jun 26, 2014

Contributor

#7700 looks great, I'll keep an eye on that, thanks.
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
});
}
}

Expand Down
Loading

0 comments on commit 546cb42

Please sign in to comment.