Skip to content

Commit 331e0f9

Browse files
gkalpakellimist
gkalpak
authored and
ellimist
committed
perf(ngClass): avoid unnecessary .data() accesses, deep-watching and copies
Includes the following commits (see angular#15246 for details): - **perf(ngClass): only access the element's `data` once** - **refactor(ngClass): simplify conditions** - **refactor(ngClass): move helper functions outside the closure** - **refactor(ngClass): exit `arrayDifference()` early if an input is empty** - **perf(ngClass): avoid deep-watching (if possible) and unnecessary copies** The cases that should benefit most are: 1. When using large objects as values (e.g.: `{loaded: $ctrl.data}`). 2. When using objects/arrays and there are frequent changes. 3. When there are many `$index` changes (e.g. addition/deletion/reordering in large `ngRepeat`s). The differences in operations per digest include: 1. `Regular expression (when not changed)` **Before:** `equals()` **After:** `toClassString()` 2. `Regular expression (when changed)` **Before:** `copy()` + 2 x `arrayClasses()` + `shallowCopy()` **After:** 2 x `split()` 3. `One-time expression (when not changed)` **Before:** `equals()` **After:** `toFlatValue()` + `equals()`* 4. `One-time expression (when changed)` **Before:** `copy()` + 2 x `arrayClasses()` + `shallowCopy()` **After:** `copy()`* + `toClassString()`* + 2 x `split()` 5. `$index modulo changed` **Before:** `arrayClasses()` **After:** - (*): on flatter structure In large based on angular#14404. Kudos to @drpicox for the initial idea and a big part of the implementation. Closes angular#14404 Closes angular#15246
1 parent 4bf1313 commit 331e0f9

File tree

2 files changed

+173
-92
lines changed

2 files changed

+173
-92
lines changed

‎src/ng/directive/ngClass.js

+133-77
Original file line numberDiff line numberDiff line change
@@ -8,122 +8,178 @@
88

99
function classDirective(name, selector) {
1010
name = 'ngClass' + name;
11+
var indexWatchExpression;
1112

12-
return [function() {
13+
return ['$parse', function($parse) {
1314
return {
1415
restrict: 'AC',
1516
link: function(scope, element, attr) {
16-
var oldVal;
17+
var expression = attr[name].trim();
18+
var isOneTime = (expression.charAt(0) === ':') && (expression.charAt(1) === ':');
19+
20+
var watchInterceptor = isOneTime ? toFlatValue : toClassString;
21+
var watchExpression = $parse(expression, watchInterceptor);
22+
var watchAction = isOneTime ? ngClassOneTimeWatchAction : ngClassWatchAction;
23+
24+
var classCounts = element.data('$classCounts');
25+
var oldModulo = true;
26+
var oldClassString;
27+
28+
if (!classCounts) {
29+
// Use createMap() to prevent class assumptions involving property
30+
// names in Object.prototype
31+
classCounts = createMap();
32+
element.data('$classCounts', classCounts);
33+
}
1734

1835
if (name !== 'ngClass') {
19-
scope.$watch('$index', function($index, old$index) {
20-
/* eslint-disable no-bitwise */
21-
var mod = $index & 1;
22-
if (mod !== (old$index & 1)) {
23-
var classes = arrayClasses(oldVal);
24-
if (mod === selector) {
25-
addClasses(classes);
26-
} else {
27-
removeClasses(classes);
28-
}
29-
}
30-
/* eslint-enable */
31-
});
36+
if (!indexWatchExpression) {
37+
indexWatchExpression = $parse('$index', function moduloTwo($index) {
38+
// eslint-disable-next-line no-bitwise
39+
return $index & 1;
40+
});
41+
}
42+
43+
scope.$watch(indexWatchExpression, ngClassIndexWatchAction);
3244
}
3345

34-
scope.$watch(attr[name], ngClassWatchAction, true);
46+
scope.$watch(watchExpression, watchAction, isOneTime);
3547

36-
function addClasses(classes) {
37-
var newClasses = digestClassCounts(classes, 1);
38-
attr.$addClass(newClasses);
48+
function addClasses(classString) {
49+
classString = digestClassCounts(split(classString), 1);
50+
attr.$addClass(classString);
3951
}
4052

41-
function removeClasses(classes) {
42-
var newClasses = digestClassCounts(classes, -1);
43-
attr.$removeClass(newClasses);
53+
function removeClasses(classString) {
54+
classString = digestClassCounts(split(classString), -1);
55+
attr.$removeClass(classString);
4456
}
4557

46-
function digestClassCounts(classes, count) {
47-
// Use createMap() to prevent class assumptions involving property
48-
// names in Object.prototype
49-
var classCounts = element.data('$classCounts') || createMap();
58+
function updateClasses(oldClassString, newClassString) {
59+
var oldClassArray = split(oldClassString);
60+
var newClassArray = split(newClassString);
61+
62+
var toRemoveArray = arrayDifference(oldClassArray, newClassArray);
63+
var toAddArray = arrayDifference(newClassArray, oldClassArray);
64+
65+
var toRemoveString = digestClassCounts(toRemoveArray, -1);
66+
var toAddString = digestClassCounts(toAddArray, 1);
67+
68+
attr.$addClass(toAddString);
69+
attr.$removeClass(toRemoveString);
70+
}
71+
72+
function digestClassCounts(classArray, count) {
5073
var classesToUpdate = [];
51-
forEach(classes, function(className) {
74+
75+
forEach(classArray, function(className) {
5276
if (count > 0 || classCounts[className]) {
5377
classCounts[className] = (classCounts[className] || 0) + count;
5478
if (classCounts[className] === +(count > 0)) {
5579
classesToUpdate.push(className);
5680
}
5781
}
5882
});
59-
element.data('$classCounts', classCounts);
83+
6084
return classesToUpdate.join(' ');
6185
}
6286

63-
function updateClasses(oldClasses, newClasses) {
64-
var toAdd = arrayDifference(newClasses, oldClasses);
65-
var toRemove = arrayDifference(oldClasses, newClasses);
66-
toAdd = digestClassCounts(toAdd, 1);
67-
toRemove = digestClassCounts(toRemove, -1);
87+
function ngClassIndexWatchAction(newModulo) {
88+
// This watch-action should run before the `ngClass[OneTime]WatchAction()`, thus it
89+
// adds/removes `oldClassString`. If the `ngClass` expression has changed as well, the
90+
// `ngClass[OneTime]WatchAction()` will update the classes.
91+
if (newModulo === selector) {
92+
addClasses(oldClassString);
93+
} else {
94+
removeClasses(oldClassString);
95+
}
6896

69-
attr.$addClass(toAdd);
70-
attr.$removeClass(toRemove);
97+
oldModulo = newModulo;
7198
}
7299

73-
function ngClassWatchAction(newVal) {
74-
// eslint-disable-next-line no-bitwise
75-
if (selector === true || (scope.$index & 1) === selector) {
76-
var newClasses = arrayClasses(newVal || []);
77-
if (!oldVal) {
78-
addClasses(newClasses);
79-
} else if (!equals(newVal,oldVal)) {
80-
var oldClasses = arrayClasses(oldVal);
81-
updateClasses(oldClasses, newClasses);
82-
}
100+
function ngClassOneTimeWatchAction(newClassValue) {
101+
var newClassString = toClassString(newClassValue);
102+
103+
if (newClassString !== oldClassString) {
104+
ngClassWatchAction(newClassString);
83105
}
84-
if (isArray(newVal)) {
85-
oldVal = newVal.map(function(v) { return shallowCopy(v); });
86-
} else {
87-
oldVal = shallowCopy(newVal);
106+
}
107+
108+
function ngClassWatchAction(newClassString) {
109+
if (oldModulo === selector) {
110+
updateClasses(oldClassString, newClassString);
88111
}
112+
113+
oldClassString = newClassString;
89114
}
90115
}
91116
};
117+
}];
92118

93-
function arrayDifference(tokens1, tokens2) {
94-
var values = [];
119+
// Helpers
120+
function arrayDifference(tokens1, tokens2) {
121+
if (!tokens1 || !tokens1.length) return [];
122+
if (!tokens2 || !tokens2.length) return tokens1;
95123

96-
outer:
97-
for (var i = 0; i < tokens1.length; i++) {
98-
var token = tokens1[i];
99-
for (var j = 0; j < tokens2.length; j++) {
100-
if (token === tokens2[j]) continue outer;
101-
}
102-
values.push(token);
124+
var values = [];
125+
126+
outer:
127+
for (var i = 0; i < tokens1.length; i++) {
128+
var token = tokens1[i];
129+
for (var j = 0; j < tokens2.length; j++) {
130+
if (token === tokens2[j]) continue outer;
103131
}
104-
return values;
132+
values.push(token);
105133
}
106134

107-
function arrayClasses(classVal) {
108-
var classes = [];
109-
if (isArray(classVal)) {
110-
forEach(classVal, function(v) {
111-
classes = classes.concat(arrayClasses(v));
112-
});
113-
return classes;
114-
} else if (isString(classVal)) {
115-
return classVal.split(' ');
116-
} else if (isObject(classVal)) {
117-
forEach(classVal, function(v, k) {
118-
if (v) {
119-
classes = classes.concat(k.split(' '));
120-
}
121-
});
122-
return classes;
135+
return values;
136+
}
137+
138+
function split(classString) {
139+
return classString && classString.split(' ');
140+
}
141+
142+
function toClassString(classValue) {
143+
var classString = classValue;
144+
145+
if (isArray(classValue)) {
146+
classString = classValue.map(toClassString).join(' ');
147+
} else if (isObject(classValue)) {
148+
classString = Object.keys(classValue).
149+
filter(function(key) { return classValue[key]; }).
150+
join(' ');
151+
}
152+
153+
return classString;
154+
}
155+
156+
function toFlatValue(classValue) {
157+
var flatValue = classValue;
158+
159+
if (isArray(classValue)) {
160+
flatValue = classValue.map(toFlatValue);
161+
} else if (isObject(classValue)) {
162+
var hasUndefined = false;
163+
164+
flatValue = Object.keys(classValue).filter(function(key) {
165+
var value = classValue[key];
166+
167+
if (!hasUndefined && isUndefined(value)) {
168+
hasUndefined = true;
169+
}
170+
171+
return value;
172+
});
173+
174+
if (hasUndefined) {
175+
// Prevent the `oneTimeLiteralWatchInterceptor` from unregistering
176+
// the watcher, by including at least one `undefined` value.
177+
flatValue.push(undefined);
123178
}
124-
return classVal;
125179
}
126-
}];
180+
181+
return flatValue;
182+
}
127183
}
128184

129185
/**

‎test/ng/directive/ngClassSpec.js

+40-15
Original file line numberDiff line numberDiff line change
@@ -568,46 +568,71 @@ describe('ngClass', function() {
568568
);
569569

570570
describe('large objects', function() {
571+
var getProp;
572+
var veryLargeObj;
571573

572-
var verylargeobject, getProp;
573574
beforeEach(function() {
574575
getProp = jasmine.createSpy('getProp');
575-
verylargeobject = {};
576-
Object.defineProperty(verylargeobject, 'prop', {
576+
veryLargeObj = {};
577+
578+
Object.defineProperty(veryLargeObj, 'prop', {
577579
get: getProp,
578580
enumerable: true
579581
});
580582
});
581583

582-
it('should not be copied if static', inject(function($rootScope, $compile) {
583-
element = $compile('<div ng-class="{foo: verylargeobject}"></div>')($rootScope);
584-
$rootScope.verylargeobject = verylargeobject;
584+
it('should not be copied when using an expression', inject(function($compile, $rootScope) {
585+
element = $compile('<div ng-class="fooClass"></div>')($rootScope);
586+
$rootScope.fooClass = {foo: veryLargeObj};
585587
$rootScope.$digest();
586588

589+
expect(element).toHaveClass('foo');
587590
expect(getProp).not.toHaveBeenCalled();
588591
}));
589592

590-
it('should not be copied if dynamic', inject(function($rootScope, $compile) {
591-
$rootScope.fooClass = {foo: verylargeobject};
592-
element = $compile('<div ng-class="fooClass"></div>')($rootScope);
593+
it('should not be copied when using a literal', inject(function($compile, $rootScope) {
594+
element = $compile('<div ng-class="{foo: veryLargeObj}"></div>')($rootScope);
595+
$rootScope.veryLargeObj = veryLargeObj;
593596
$rootScope.$digest();
594597

598+
expect(element).toHaveClass('foo');
595599
expect(getProp).not.toHaveBeenCalled();
596600
}));
597601

598-
it('should not be copied if inside an array', inject(function($rootScope, $compile) {
599-
element = $compile('<div ng-class="[{foo: verylargeobject}]"></div>')($rootScope);
600-
$rootScope.verylargeobject = verylargeobject;
602+
it('should not be copied when inside an array', inject(function($compile, $rootScope) {
603+
element = $compile('<div ng-class="[{foo: veryLargeObj}]"></div>')($rootScope);
604+
$rootScope.veryLargeObj = veryLargeObj;
601605
$rootScope.$digest();
602606

607+
expect(element).toHaveClass('foo');
603608
expect(getProp).not.toHaveBeenCalled();
604609
}));
605610

606-
it('should not be copied when one-time binding', inject(function($rootScope, $compile) {
607-
element = $compile('<div ng-class="::{foo: verylargeobject}"></div>')($rootScope);
608-
$rootScope.verylargeobject = verylargeobject;
611+
it('should not be copied when using one-time binding', inject(function($compile, $rootScope) {
612+
element = $compile('<div ng-class="::{foo: veryLargeObj, bar: bar}"></div>')($rootScope);
613+
$rootScope.veryLargeObj = veryLargeObj;
609614
$rootScope.$digest();
610615

616+
expect(element).toHaveClass('foo');
617+
expect(element).not.toHaveClass('bar');
618+
expect(getProp).not.toHaveBeenCalled();
619+
620+
$rootScope.$apply('veryLargeObj.bar = "bar"');
621+
622+
expect(element).toHaveClass('foo');
623+
expect(element).not.toHaveClass('bar');
624+
expect(getProp).not.toHaveBeenCalled();
625+
626+
$rootScope.$apply('bar = "bar"');
627+
628+
expect(element).toHaveClass('foo');
629+
expect(element).toHaveClass('bar');
630+
expect(getProp).not.toHaveBeenCalled();
631+
632+
$rootScope.$apply('veryLargeObj.bar = "qux"');
633+
634+
expect(element).toHaveClass('foo');
635+
expect(element).toHaveClass('bar');
611636
expect(getProp).not.toHaveBeenCalled();
612637
}));
613638
});

0 commit comments

Comments
 (0)