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

Commit c6a6dd9

Browse files
jbedardjbedard
committed
WIP: perf($parse): only execute watched expressions when the inputs change
1 parent a1a9585 commit c6a6dd9

File tree

6 files changed

+293
-25
lines changed

6 files changed

+293
-25
lines changed

‎benchmarks/parsed-expressions-bp/main.html

+16
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@
3131
<label for="operators">Binary/Unary operators</label>
3232
</li>
3333

34+
<li>
35+
<input type="radio" ng-model="expressionType" value="shortCircuitingOperators" id="shortCircuitingOperators">
36+
<label for="shortCircuitingOperators">AND/OR short-circuiting operators</label>
37+
</li>
38+
3439
<li>
3540
<input type="radio" ng-model="expressionType" value="filters" id="filters">
3641
<label for="filters">Filters</label>
@@ -134,6 +139,17 @@
134139
<span bm-pe-watch="-rowIdx * 2 * rowIdx + rowIdx / rowIdx + 1"></span>
135140
</li>
136141

142+
<li ng-switch-when="shortCircuitingOperators" ng-repeat="(rowIdx, row) in ::data">
143+
<span bm-pe-watch="rowIdx && row.odd"></span>
144+
<span bm-pe-watch="row.odd && row.even"></span>
145+
<span bm-pe-watch="row.odd && !row.even"></span>
146+
<span bm-pe-watch="row.odd || row.even"></span>
147+
<span bm-pe-watch="row.odd || row.even || row.index"></span>
148+
<span bm-pe-watch="row.index === 1 || row.index === 2"></span>
149+
<span bm-pe-watch="row.num0 < row.num1 && row.num1 < row.num2"></span>
150+
<span bm-pe-watch="row.num0 < row.num1 || row.num1 < row.num2"></span>
151+
</li>
152+
137153
<li ng-switch-when="filters" ng-repeat="(rowIdx, row) in ::data">
138154
<span bm-pe-watch="rowIdx | noop"></span>
139155
<span bm-pe-watch="rowIdx | noop"></span>

‎src/ng/compile.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -1699,7 +1699,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
16991699
attrs[attrName], newIsolateScopeDirective.name);
17001700
};
17011701
lastValue = isolateBindingContext[scopeName] = parentGet(scope);
1702-
var unwatch = scope.$watch($parse(attrs[attrName], function parentValueWatch(parentValue) {
1702+
var parentValueWatch = function parentValueWatch(parentValue) {
17031703
if (!compare(parentValue, isolateBindingContext[scopeName])) {
17041704
// we are out of sync and need to copy
17051705
if (!compare(parentValue, lastValue)) {
@@ -1711,7 +1711,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
17111711
}
17121712
}
17131713
return lastValue = parentValue;
1714-
}), null, parentGet.literal);
1714+
};
1715+
parentValueWatch.externalInput = true;
1716+
var unwatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal);
17151717
isolateScope.$on('$destroy', unwatch);
17161718
break;
17171719

‎src/ng/parse.js

+116-21
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,10 @@ Lexer.prototype = {
377377
};
378378

379379

380+
function isConstant(exp) {
381+
return exp.constant;
382+
}
383+
380384
/**
381385
* @constructor
382386
*/
@@ -494,23 +498,25 @@ Parser.prototype = {
494498
return extend(function(self, locals) {
495499
return fn(self, locals, right);
496500
}, {
497-
constant:right.constant
501+
constant:right.constant,
502+
inputs: [right]
498503
});
499504
},
500505

501506
ternaryFn: function(left, middle, right){
502507
return extend(function(self, locals){
503508
return left(self, locals) ? middle(self, locals) : right(self, locals);
504509
}, {
505-
constant: left.constant && middle.constant && right.constant
510+
constant:left.constant && middle.constant && right.constant
506511
});
507512
},
508513

509-
binaryFn: function(left, fn, right) {
514+
binaryFn: function(left, fn, right, isBranching) {
510515
return extend(function(self, locals) {
511516
return fn(self, locals, left, right);
512517
}, {
513-
constant:left.constant && right.constant
518+
constant: left.constant && right.constant,
519+
inputs: !isBranching && [left, right]
514520
});
515521
},
516522

@@ -558,7 +564,9 @@ Parser.prototype = {
558564
}
559565
}
560566

561-
return function $parseFilter(self, locals) {
567+
var inputs = [inputFn].concat(argsFn || []);
568+
569+
return extend(function $parseFilter(self, locals) {
562570
var input = inputFn(self, locals);
563571
if (args) {
564572
args[0] = input;
@@ -572,7 +580,10 @@ Parser.prototype = {
572580
}
573581

574582
return fn(input);
575-
};
583+
}, {
584+
constant: !fn.externalInput && inputs.every(isConstant),
585+
inputs: !fn.externalInput && inputs
586+
});
576587
},
577588

578589
expression: function() {
@@ -589,9 +600,11 @@ Parser.prototype = {
589600
this.text.substring(0, token.index) + '] can not be assigned to', token);
590601
}
591602
right = this.ternary();
592-
return function $parseAssignment(scope, locals) {
603+
return extend(function $parseAssignment(scope, locals) {
593604
return left.assign(scope, right(scope, locals), locals);
594-
};
605+
}, {
606+
inputs: [left, right]
607+
});
595608
}
596609
return left;
597610
},
@@ -616,7 +629,7 @@ Parser.prototype = {
616629
var left = this.logicalAND();
617630
var token;
618631
while ((token = this.expect('||'))) {
619-
left = this.binaryFn(left, token.fn, this.logicalAND());
632+
left = this.binaryFn(left, token.fn, this.logicalAND(), true);
620633
}
621634
return left;
622635
},
@@ -625,7 +638,7 @@ Parser.prototype = {
625638
var left = this.equality();
626639
var token;
627640
if ((token = this.expect('&&'))) {
628-
left = this.binaryFn(left, token.fn, this.logicalAND());
641+
left = this.binaryFn(left, token.fn, this.logicalAND(), true);
629642
}
630643
return left;
631644
},
@@ -760,7 +773,6 @@ Parser.prototype = {
760773
// This is used with json array declaration
761774
arrayDeclaration: function () {
762775
var elementFns = [];
763-
var allConstant = true;
764776
if (this.peekToken().text !== ']') {
765777
do {
766778
if (this.peek(']')) {
@@ -769,9 +781,6 @@ Parser.prototype = {
769781
}
770782
var elementFn = this.expression();
771783
elementFns.push(elementFn);
772-
if (!elementFn.constant) {
773-
allConstant = false;
774-
}
775784
} while (this.expect(','));
776785
}
777786
this.consume(']');
@@ -784,13 +793,13 @@ Parser.prototype = {
784793
return array;
785794
}, {
786795
literal: true,
787-
constant: allConstant
796+
constant: elementFns.every(isConstant),
797+
inputs: elementFns
788798
});
789799
},
790800

791801
object: function () {
792802
var keys = [], values = [];
793-
var allConstant = true;
794803
if (this.peekToken().text !== '}') {
795804
do {
796805
if (this.peek('}')) {
@@ -802,9 +811,6 @@ Parser.prototype = {
802811
this.consume(':');
803812
var value = this.expression();
804813
values.push(value);
805-
if (!value.constant) {
806-
allConstant = false;
807-
}
808814
} while (this.expect(','));
809815
}
810816
this.consume('}');
@@ -817,7 +823,8 @@ Parser.prototype = {
817823
return object;
818824
}, {
819825
literal: true,
820-
constant: allConstant
826+
constant: values.every(isConstant),
827+
inputs: values
821828
});
822829
}
823830
};
@@ -1045,6 +1052,9 @@ function $ParseProvider() {
10451052
parsedExpression.$$watchDelegate = parsedExpression.literal ?
10461053
oneTimeLiteralWatchDelegate : oneTimeWatchDelegate;
10471054
}
1055+
else if (parsedExpression.inputs) {
1056+
parsedExpression.$$watchDelegate = inputsWatchDelegate;
1057+
}
10481058

10491059
cache[cacheKey] = parsedExpression;
10501060
}
@@ -1058,6 +1068,81 @@ function $ParseProvider() {
10581068
}
10591069
};
10601070

1071+
function collectExpressionInputs(inputs, list) {
1072+
for (var i = 0, ii = inputs.length; i < ii; i++) {
1073+
var input = inputs[i];
1074+
if (!input.constant) {
1075+
if (input.inputs) {
1076+
collectExpressionInputs(input.inputs, list);
1077+
}
1078+
else if (-1 === list.indexOf(input)) {
1079+
list.push(input);
1080+
}
1081+
}
1082+
}
1083+
1084+
return list;
1085+
}
1086+
1087+
function simpleEquals(o1, o2) {
1088+
if (o1 == null || o2 == null) return o1 === o2; // null/undefined
1089+
1090+
if (typeof o1 === "object") {
1091+
//The same object is not supported because it may have been mutated
1092+
if (o1 === o2) return false;
1093+
1094+
if (typeof o2 !== "object") return false;
1095+
1096+
//Convert to primitive if possible
1097+
o1 = o1.valueOf();
1098+
o2 = o2.valueOf();
1099+
1100+
//If the type became a non-object then we can use the primitive check below
1101+
if (typeof o1 === "object") return false;
1102+
}
1103+
1104+
//Primitive or NaN
1105+
return o1 === o2 || (o1 !== o1 && o2 !== o2);
1106+
}
1107+
1108+
function inputsWatchDelegate(scope, listener, objectEquality, parsedExpression) {
1109+
var inputExpressions = parsedExpression.$$inputs ||
1110+
(parsedExpression.$$inputs = collectExpressionInputs(parsedExpression.inputs, []));
1111+
1112+
var inputs = [simpleEquals/*=something that will never equal an evaluated input*/];
1113+
var lastResult;
1114+
1115+
if (1 === inputExpressions.length) {
1116+
inputs = inputs[0];
1117+
inputExpressions = inputExpressions[0];
1118+
return scope.$watch(function expressionInputWatch(scope) {
1119+
var newVal = inputExpressions(scope);
1120+
if (!simpleEquals(newVal, inputs)) {
1121+
lastResult = parsedExpression(scope);
1122+
inputs = newVal;
1123+
}
1124+
return lastResult;
1125+
}, listener, objectEquality);
1126+
}
1127+
1128+
return scope.$watch(function expressionInputsWatch(scope) {
1129+
var changed = false;
1130+
1131+
for (var i=0, ii=inputExpressions.length; i<ii; i++) {
1132+
var valI = inputExpressions[i](scope);
1133+
if (changed || (changed = !simpleEquals(valI, inputs[i]))) {
1134+
inputs[i] = valI;
1135+
}
1136+
}
1137+
1138+
if (changed) {
1139+
lastResult = parsedExpression(scope);
1140+
}
1141+
1142+
return lastResult;
1143+
}, listener, objectEquality);
1144+
}
1145+
10611146
function oneTimeWatchDelegate(scope, listener, objectEquality, parsedExpression) {
10621147
var unwatch, lastValue;
10631148
return unwatch = scope.$watch(function oneTimeWatch(scope) {
@@ -1123,7 +1208,17 @@ function $ParseProvider() {
11231208
// initial value is defined (for bind-once)
11241209
return isDefined(value) ? result : value;
11251210
};
1126-
fn.$$watchDelegate = parsedExpression.$$watchDelegate;
1211+
1212+
//Propogate $$watchDelegates other then inputsWatchDelegate
1213+
if (parsedExpression.$$watchDelegate && parsedExpression.$$watchDelegate !== inputsWatchDelegate) {
1214+
fn.$$watchDelegate = parsedExpression.$$watchDelegate;
1215+
}
1216+
//Treat the interceptorFn similar to filters - it is assumed to be a pure function unless flagged
1217+
else if (!interceptorFn.externalInput) {
1218+
fn.$$watchDelegate = inputsWatchDelegate;
1219+
fn.inputs = [parsedExpression];
1220+
}
1221+
11271222
return fn;
11281223
}
11291224
}];

‎src/ng/rootScope.js

+2
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,8 @@ function $RootScopeProvider(){
515515
* de-registration function is executed, the internal watch operation is terminated.
516516
*/
517517
$watchCollection: function(obj, listener) {
518+
$watchCollectionInterceptor.externalInput = true;
519+
518520
var self = this;
519521
// the current value, updated on each dirty-check run
520522
var newValue;

‎test/ng/directive/ngRepeatSpec.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -867,12 +867,15 @@ describe('ngRepeat', function() {
867867
// This creates one item, but it has no parent so we can't get to it
868868
$rootScope.items = [1, 2];
869869
$rootScope.$apply();
870+
expect(logs).toContain(1);
871+
expect(logs).toContain(2);
872+
logs.length = 0;
870873

871874
// This cleans up to prevent memory leak
872875
$rootScope.items = [];
873876
$rootScope.$apply();
874877
expect(angular.mock.dump(element)).toBe('<!-- ngRepeat: i in items -->');
875-
expect(logs).toEqual([1, 2, 1, 2]);
878+
expect(logs.length).toBe(0);
876879
}));
877880

878881

@@ -894,12 +897,15 @@ describe('ngRepeat', function() {
894897
// This creates one item, but it has no parent so we can't get to it
895898
$rootScope.items = [1, 2];
896899
$rootScope.$apply();
900+
expect(logs).toContain(1);
901+
expect(logs).toContain(2);
902+
logs.length = 0;
897903

898904
// This cleans up to prevent memory leak
899905
$rootScope.items = [];
900906
$rootScope.$apply();
901907
expect(sortedHtml(element)).toBe('<span>-</span><!-- ngRepeat: i in items --><span>-</span>');
902-
expect(logs).toEqual([1, 2, 1, 2]);
908+
expect(logs.length).toBe(0);
903909
}));
904910

905911

0 commit comments

Comments
 (0)