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

perf(*): more performant interpolation and lazy one-time binding #7700

Merged
merged 2 commits into from
Jul 15, 2014
Merged

perf(*): more performant interpolation and lazy one-time binding #7700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcf72ca
revert: feat(*): lazy one-time binding support
rodyhaddad Jun 2, 2014
86d55c1
perf(*): more performant interpolation and lazy one-time binding
rodyhaddad Jun 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,8 +1505,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
attrs[attrName], newIsolateScopeDirective.name);
};
lastValue = isolateScope[scopeName] = parentGet(scope);
isolateScope.$watch(function parentValueWatch() {
var parentValue = parentGet(scope);
var unwatch = scope.$watch($parse(attrs[attrName], function parentValueWatch(parentValue) {
if (!compare(parentValue, isolateScope[scopeName])) {
// we are out of sync and need to copy
if (!compare(parentValue, lastValue)) {
Expand All @@ -1517,9 +1516,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
parentSet(scope, parentValue = isolateScope[scopeName]);
}
}
parentValueWatch.$$unwatch = parentGet.$$unwatch;
return lastValue = parentValue;
}, null, parentGet.literal);
}), null, parentGet.literal);
isolateScope.$on('$destroy', unwatch);
break;

case '&':
Expand Down Expand Up @@ -1855,9 +1854,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
return function textInterpolateLinkFn(scope, node) {
var parent = node.parent(),
bindings = parent.data('$binding') || [];
// Need to interpolate again in case this is using one-time bindings in multiple clones
// of transcluded templates.
interpolateFn = $interpolate(text);
bindings.push(interpolateFn);
parent.data('$binding', bindings);
if (!hasCompileParent) safeAddClass(parent, 'ng-binding');
Expand Down
12 changes: 6 additions & 6 deletions src/ng/directive/ngBind.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,13 @@ var ngBindHtmlDirective = ['$sce', '$parse', function($sce, $parse) {
element.addClass('ng-binding').data('$binding', attr.ngBindHtml);

var parsed = $parse(attr.ngBindHtml);
function getStringValue() {
var value = parsed(scope);
getStringValue.$$unwatch = parsed.$$unwatch;
return (value || '').toString();
}
var changeDetector = $parse(attr.ngBindHtml, function getStringValue(value) {
return (value || '').toString();
});

scope.$watch(getStringValue, function ngBindHtmlWatchAction(value) {
scope.$watch(changeDetector, function ngBindHtmlWatchAction() {
// we re-evaluate the expr because we want a TrustedValueHolderType
// for $sce, not a string
element.html($sce.getTrustedHtml(parsed(scope)) || '');
});
};
Expand Down
77 changes: 29 additions & 48 deletions src/ng/interpolate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ function $InterpolateProvider() {
hasInterpolation = false,
hasText = false,
exp,
concat = [],
lastValuesCache = { values: {}, results: {}};
concat = [];

while(index < textLength) {
if ( ((startIndex = text.indexOf(startSymbol, index)) != -1) &&
Expand All @@ -205,7 +204,7 @@ function $InterpolateProvider() {
separators.push(text.substring(index, startIndex));
exp = text.substring(startIndex + startSymbolLength, endIndex);
expressions.push(exp);
parseFns.push($parse(exp));
parseFns.push($parse(exp, parseStringifyInterceptor));
index = endIndex + endSymbolLength;
hasInterpolation = true;
} else {
Expand Down Expand Up @@ -246,6 +245,7 @@ function $InterpolateProvider() {

var compute = function(values) {
for(var i = 0, ii = expressions.length; i < ii; i++) {
if (allOrNothing && isUndefined(values[i])) return;
concat[2*i] = separators[i];
concat[(2*i)+1] = values[i];
}
Expand All @@ -254,13 +254,9 @@ function $InterpolateProvider() {
};

var getValue = function (value) {
if (trustedContext) {
value = $sce.getTrusted(trustedContext, value);
} else {
value = $sce.valueOf(value);
}

return value;
return trustedContext ?
$sce.getTrusted(trustedContext, value) :
$sce.valueOf(value);
};

var stringify = function (value) {
Expand All @@ -284,64 +280,49 @@ function $InterpolateProvider() {
};

return extend(function interpolationFn(context) {
var scopeId = (context && 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 && context.$on) {
context.$on('$destroy', function() {
lastValuesCache.values[scopeId] = null;
lastValuesCache.results[scopeId] = null;
});
}
}


try {
interpolationFn.$$unwatch = true;
for (; i < ii; i++) {
val = getValue(parseFns[i](context));
if (allOrNothing && isUndefined(val)) {
interpolationFn.$$unwatch = undefined;
return;
}
val = stringify(val);
if (val !== lastValues[i]) {
inputsChanged = true;
}
values[i] = val;
interpolationFn.$$unwatch = interpolationFn.$$unwatch && parseFns[i].$$unwatch;
values[i] = parseFns[i](context);
}

if (inputsChanged) {
lastValuesCache.values[scopeId] = values;
lastValuesCache.results[scopeId] = lastResult = compute(values);
}
return 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
expressions: expressions,
$$watchDelegate: function (scope, listener, objectEquality, deregisterNotifier) {
var lastValue;
return scope.$watchGroup(parseFns, function interpolateFnWatcher(values, oldValues) {
var currValue = compute(values);
if (isFunction(listener)) {
listener.call(this, currValue, values !== oldValues ? lastValue : currValue, scope);
}
lastValue = currValue;
}, objectEquality, deregisterNotifier);
}
});
}

function parseStringifyInterceptor(value) {
try {
return stringify(getValue(value));
} catch(err) {
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,
err.toString());
$exceptionHandler(newErr);
}
}
}


Expand Down
114 changes: 67 additions & 47 deletions src/ng/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,68 +991,88 @@ function $ParseProvider() {
this.$get = ['$filter', '$sniffer', function($filter, $sniffer) {
$parseOptions.csp = $sniffer.csp;

return function(exp) {
var parsedExpression,
oneTime;
return function(exp, interceptorFn) {
var parsedExpression, oneTime,
cacheKey = (exp = trim(exp));

switch (typeof exp) {
case 'string':
if (cache.hasOwnProperty(cacheKey)) {
parsedExpression = cache[cacheKey];
} else {
if (exp.charAt(0) === ':' && exp.charAt(1) === ':') {
oneTime = true;
exp = exp.substring(2);
}

exp = trim(exp);

if (exp.charAt(0) === ':' && exp.charAt(1) === ':') {
oneTime = true;
exp = exp.substring(2);
}

if (cache.hasOwnProperty(exp)) {
return oneTime ? oneTimeWrapper(cache[exp]) : cache[exp];
}
var lexer = new Lexer($parseOptions);
var parser = new Parser(lexer, $filter, $parseOptions);
parsedExpression = parser.parse(exp);

var lexer = new Lexer($parseOptions);
var parser = new Parser(lexer, $filter, $parseOptions);
parsedExpression = parser.parse(exp);
if (parsedExpression.constant) parsedExpression.$$watchDelegate = constantWatch;
else if (oneTime) parsedExpression.$$watchDelegate = oneTimeWatch;

if (exp !== 'hasOwnProperty') {
// Only cache the value if it's not going to mess up the cache object
// This is more performant that using Object.prototype.hasOwnProperty.call
cache[exp] = parsedExpression;
if (cacheKey !== 'hasOwnProperty') {
// Only cache the value if it's not going to mess up the cache object
// This is more performant that using Object.prototype.hasOwnProperty.call
cache[cacheKey] = parsedExpression;
}
}

return oneTime || parsedExpression.constant ? oneTimeWrapper(parsedExpression) : parsedExpression;
return addInterceptor(parsedExpression, interceptorFn);

case 'function':
return exp;
return addInterceptor(exp, interceptorFn);

default:
return noop;
return addInterceptor(noop, interceptorFn);
}
};

function oneTimeWrapper(expression) {
var stable = false,
lastValue;
oneTimeParseFn.literal = expression.literal;
oneTimeParseFn.constant = expression.constant;
oneTimeParseFn.assign = expression.assign;
return oneTimeParseFn;

function oneTimeParseFn(self, locals) {
if (!stable) {
lastValue = expression.constant && lastValue ? lastValue : expression(self, locals);
oneTimeParseFn.$$unwatch = isDefined(lastValue);
if (oneTimeParseFn.$$unwatch && self && self.$$postDigestQueue) {
self.$$postDigestQueue.push(function () {
// create a copy if the value is defined and it is not a $sce value
if ((stable = isDefined(lastValue)) &&
(lastValue === null || !lastValue.$$unwrapTrustedValue)) {
lastValue = copy(lastValue, null);
}
});
function oneTimeWatch(scope, listener, objectEquality, deregisterNotifier, parsedExpression) {
var unwatch, lastValue;
return unwatch = scope.$watch(function oneTimeWatch(scope) {
return parsedExpression(scope);
}, function oneTimeListener(value, old, scope) {
lastValue = value;
if (isFunction(listener)) {
listener.apply(this, arguments);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

else {
  scope.$eval(listener)
}

???

if (isDefined(value)) {
scope.$$postDigest(function () {
if (isDefined(lastValue)) {
unwatch();
}
}
return lastValue;
});
}
}, objectEquality, deregisterNotifier);
}

function constantWatch(scope, listener, objectEquality, deregisterNotifier, parsedExpression) {
var unwatch;
return unwatch = scope.$watch(function constantWatch(scope) {
return parsedExpression(scope);
}, function constantListener(value, old, scope) {
if (isFunction(listener)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the listener can be a string (expression). you are not supporting case.

listener.apply(this, arguments);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you binding this here for any particular reason? the this at this point is an internal watch representation, is an api leak. it looks like you are going out of your way to preserve this api leak.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's change this to listener.apply(undefined, arguments)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also above in oneTimeWatch

}
unwatch();
}, objectEquality, deregisterNotifier);
}

function addInterceptor(parsedExpression, interceptorFn) {
if (isFunction(interceptorFn)) {
var fn = function interceptedExpression(scope, locals) {
var value = parsedExpression(scope, locals);
var result = interceptorFn(value, scope, locals);
// we only return the interceptor's result if the
// initial value is defined (for bind-once)
return isDefined(value) ? result : value;
};
fn.$$watchDelegate = parsedExpression.$$watchDelegate;
return fn;
} else {
return parsedExpression;
}
};
}
}];
}
Loading