Skip to content

Commit

Permalink
perf(common): avoid excessive DOM mutation in NgClass (#48433)
Browse files Browse the repository at this point in the history
This commit represents rewrite of the NgClass directive to address
severe performance problem (excessive DOM mutation). The modified
algorithm removes all the known performance clifs and has number of
desirable properties:
- it is shorter and (arguably) easier to follow;
- drops usage of existing differs thus limiting dependencies on other
part of the code without increasing size of the directive;
- doesn't degrade any other performance metrics.

Fixes #25518

PR Close #48433
  • Loading branch information
pkozlowski-opensource authored and AndrewKushnir committed Jan 13, 2023
1 parent 3f440d9 commit 032b2bd
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 83 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/aio-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"aio-local": {
"uncompressed": {
"runtime": 4325,
"main": 457970,
"main": 457343,
"polyfills": 33952,
"styles": 74752,
"light-theme": 88046,
Expand Down
182 changes: 101 additions & 81 deletions packages/common/src/directives/ng_class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,30 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Directive, DoCheck, ElementRef, Input, IterableChanges, IterableDiffer, IterableDiffers, KeyValueChanges, KeyValueDiffer, KeyValueDiffers, Renderer2, ɵisListLikeIterable as isListLikeIterable, ɵstringify as stringify} from '@angular/core';
import {Directive, DoCheck, ElementRef, Input, IterableDiffers, KeyValueDiffers, Renderer2, ɵstringify as stringify} from '@angular/core';

type NgClassSupportedTypes = string[]|Set<string>|{[klass: string]: any}|null|undefined;

const WS_REGEXP = /\s+/;

const EMPTY_ARRAY: string[] = [];

/**
* Represents internal object used to track state of each CSS class. There are 3 different (boolean)
* flags that, combined together, indicate state of a given CSS class:
* - enabled: indicates if a class should be present in the DOM (true) or not (false);
* - changed: tracks if a class was toggled (added or removed) during the custom dirty-checking
* process; changed classes must be synchronized with the DOM;
* - touched: tracks if a class is present in the current object bound to the class / ngClass input;
* classes that are not present any more can be removed from the internal data structures;
*/
interface CssClassState {
// PERF: could use a bit mask to represent state as all fields are boolean flags
enabled: boolean;
changed: boolean;
touched: boolean;
}

/**
* @ngModule CommonModule
*
Expand Down Expand Up @@ -42,116 +62,116 @@ type NgClassSupportedTypes = string[]|Set<string>|{[klass: string]: any}|null|un
standalone: true,
})
export class NgClass implements DoCheck {
private _iterableDiffer: IterableDiffer<string>|null = null;
private _keyValueDiffer: KeyValueDiffer<string, any>|null = null;
private _initialClasses: string[] = [];
private _rawClass: NgClassSupportedTypes = null;
private initialClasses = EMPTY_ARRAY;
private rawClass: NgClassSupportedTypes;

private stateMap = new Map<string, CssClassState>();

constructor(
// leaving references to differs in place since flex layout is extending NgClass...
private _iterableDiffers: IterableDiffers, private _keyValueDiffers: KeyValueDiffers,
private _ngEl: ElementRef, private _renderer: Renderer2) {}


@Input('class')
set klass(value: string) {
this._removeClasses(this._initialClasses);
this._initialClasses = typeof value === 'string' ? value.split(/\s+/) : [];
this._applyClasses(this._initialClasses);
this._applyClasses(this._rawClass);
this.initialClasses = value != null ? value.trim().split(WS_REGEXP) : EMPTY_ARRAY;
}

@Input('ngClass')
set ngClass(value: string|string[]|Set<string>|{[klass: string]: any}|null|undefined) {
this._removeClasses(this._rawClass);
this._applyClasses(this._initialClasses);

this._iterableDiffer = null;
this._keyValueDiffer = null;

this._rawClass = typeof value === 'string' ? value.split(/\s+/) : value;

if (this._rawClass) {
if (isListLikeIterable(this._rawClass)) {
this._iterableDiffer = this._iterableDiffers.find(this._rawClass).create();
} else {
this._keyValueDiffer = this._keyValueDiffers.find(this._rawClass).create();
}
}
this.rawClass = typeof value === 'string' ? value.trim().split(WS_REGEXP) : value;
}

ngDoCheck() {
if (this._iterableDiffer) {
const iterableChanges = this._iterableDiffer.diff(this._rawClass as string[]);
if (iterableChanges) {
this._applyIterableChanges(iterableChanges);
}
} else if (this._keyValueDiffer) {
const keyValueChanges = this._keyValueDiffer.diff(this._rawClass as {[k: string]: any});
if (keyValueChanges) {
this._applyKeyValueChanges(keyValueChanges);
}
/*
The NgClass directive uses the custom change detection algorithm for its inputs. The custom
algorithm is necessary since inputs are represented as complex object or arrays that need to be
deeply-compared.
This algorithm is perf-sensitive since NgClass is used very frequently and its poor performance
might negatively impact runtime performance of the entire change detection cycle. The design of
this algorithm is making sure that:
- there is no unnecessary DOM manipulation (CSS classes are added / removed from the DOM only when
needed), even if references to bound objects change;
- there is no memory allocation if nothing changes (even relatively modest memory allocation
during the change detection cycle can result in GC pauses for some of the CD cycles).
The algorithm works by iterating over the set of bound classes, staring with [class] binding and
then going over [ngClass] binding. For each CSS class name:
- check if it was seen before (this information is tracked in the state map) and if its value
changed;
- mark it as "touched" - names that are not marked are not present in the latest set of binding
and we can remove such class name from the internal data structures;
After iteration over all the CSS class names we've got data structure with all the information
necessary to synchronize changes to the DOM - it is enough to iterate over the state map, flush
changes to the DOM and reset internal data structures so those are ready for the next change
detection cycle.
*/
ngDoCheck(): void {
// classes from the [class] binding
for (const klass of this.initialClasses) {
this._updateState(klass, true);
}
}

private _applyKeyValueChanges(changes: KeyValueChanges<string, any>): void {
changes.forEachAddedItem((record) => this._toggleClass(record.key, record.currentValue));
changes.forEachChangedItem((record) => this._toggleClass(record.key, record.currentValue));
changes.forEachRemovedItem((record) => {
if (record.previousValue) {
this._toggleClass(record.key, false);
// classes from the [ngClass] binding
const rawClass = this.rawClass;
if (Array.isArray(rawClass) || rawClass instanceof Set) {
for (const klass of rawClass) {
this._updateState(klass, true);
}
});
}

private _applyIterableChanges(changes: IterableChanges<string>): void {
changes.forEachAddedItem((record) => {
if (typeof record.item === 'string') {
this._toggleClass(record.item, true);
} else {
throw new Error(`NgClass can only toggle CSS classes expressed as strings, got ${
stringify(record.item)}`);
} else if (rawClass != null) {
for (const klass of Object.keys(rawClass)) {
this._updateState(klass, Boolean(rawClass[klass]));
}
});
}

changes.forEachRemovedItem((record) => this._toggleClass(record.item, false));
this._applyStateDiff();
}

/**
* Applies a collection of CSS classes to the DOM element.
*
* For argument of type Set and Array CSS class names contained in those collections are always
* added.
* For argument of type Map CSS class name in the map's key is toggled based on the value (added
* for truthy and removed for falsy).
*/
private _applyClasses(rawClassVal: NgClassSupportedTypes) {
if (rawClassVal) {
if (Array.isArray(rawClassVal) || rawClassVal instanceof Set) {
(<any>rawClassVal).forEach((klass: string) => this._toggleClass(klass, true));
} else {
Object.keys(rawClassVal).forEach(klass => this._toggleClass(klass, !!rawClassVal[klass]));
private _updateState(klass: string, nextEnabled: boolean) {
const state = this.stateMap.get(klass);
if (state !== undefined) {
if (state.enabled !== nextEnabled) {
state.changed = true;
state.enabled = nextEnabled;
}
state.touched = true;
} else {
this.stateMap.set(klass, {enabled: nextEnabled, changed: true, touched: true});
}
}

/**
* Removes a collection of CSS classes from the DOM element. This is mostly useful for cleanup
* purposes.
*/
private _removeClasses(rawClassVal: NgClassSupportedTypes) {
if (rawClassVal) {
if (Array.isArray(rawClassVal) || rawClassVal instanceof Set) {
(<any>rawClassVal).forEach((klass: string) => this._toggleClass(klass, false));
} else {
Object.keys(rawClassVal).forEach(klass => this._toggleClass(klass, false));
private _applyStateDiff() {
for (const stateEntry of this.stateMap) {
const klass = stateEntry[0];
const state = stateEntry[1];

if (state.changed) {
this._toggleClass(klass, state.enabled);
state.changed = false;
} else if (!state.touched) {
// A class that was previously active got removed from the new collection of classes -
// remove from the DOM as well.
if (state.enabled) {
this._toggleClass(klass, false);
}
this.stateMap.delete(klass);
}

state.touched = false;
}
}

private _toggleClass(klass: string, enabled: boolean): void {
if (ngDevMode) {
if (typeof klass !== 'string') {
throw new Error(
`NgClass can only toggle CSS classes expressed as strings, got ${stringify(klass)}`);
}
}
klass = klass.trim();
if (klass) {
klass.split(/\s+/g).forEach(klass => {
if (klass.length > 0) {
klass.split(WS_REGEXP).forEach(klass => {
if (enabled) {
this._renderer.addClass(this._ngEl.nativeElement, klass);
} else {
Expand Down
66 changes: 66 additions & 0 deletions packages/common/test/directives/ng_class_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ describe('binding to CSS class list', () => {

getComponent().strExpr = 'bar';
detectChangesAndExpectClassName(`bar small`);

getComponent().strExpr = undefined;
detectChangesAndExpectClassName(`small`);
}));

it('should co-operate with the class attribute and binding to it', waitForAsync(() => {
Expand Down Expand Up @@ -409,6 +412,48 @@ describe('binding to CSS class list', () => {
detectChangesAndExpectClassName('color-red');
});

it('should not write to the native node when values are the same (obj reference change)',
() => {
fixture = createTestComponent(`<div [ngClass]="objExpr"></div>`);
detectChangesAndExpectClassName('foo');

// Overwrite CSS classes so that we can check if ngClass performed DOM manipulation to
// update it
fixture.debugElement.children[0].nativeElement.className = '';
// Assert that the DOM node still has the same value after change detection
detectChangesAndExpectClassName('');

// change the object reference (without changing values)
fixture.componentInstance.objExp = {...fixture.componentInstance.objExp};
detectChangesAndExpectClassName('');
});

it('should not write to the native node when values are the same (array reference change)',
() => {
fixture = createTestComponent(`<div [ngClass]="arrExpr"></div>`);
detectChangesAndExpectClassName('foo');

// Overwrite CSS classes so that we can check if ngClass performed DOM manipulation to
// update it
fixture.debugElement.children[0].nativeElement.className = '';
// Assert that the DOM node still has the same value after change detection
detectChangesAndExpectClassName('');

// change the object reference (without changing values)
fixture.componentInstance.arrExpr = [...fixture.componentInstance.arrExpr];
detectChangesAndExpectClassName('');
});

it('should not add css class when bound initial class is removed by ngClass binding', () => {
fixture = createTestComponent(`<div [class]="'bar'" [ngClass]="objExpr"></div>`);
detectChangesAndExpectClassName('foo');
});

it('should not add css class when static initial class is removed by ngClass binding', () => {
fixture = createTestComponent(`<div class="bar" [ngClass]="objExpr"></div>`);
detectChangesAndExpectClassName('foo');
});

it('should allow classes with trailing and leading spaces in [ngClass]', () => {
@Component({
template: `
Expand All @@ -430,6 +475,27 @@ describe('binding to CSS class list', () => {
expect(trailing.className).toBe('foo');
});

it('should mix class and ngClass bindings with the same value', () => {
@Component({
selector: 'test-component',
imports: [NgClass],
template: `<div class="{{'option-' + level}}" [ngClass]="'option-' + level"></div>`,
standalone: true,
})
class TestComponent {
level = 1;
}

const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();

expect(fixture.nativeElement.firstChild.className).toBe('option-1');

fixture.componentInstance.level = 5;
fixture.detectChanges();
expect(fixture.nativeElement.firstChild.className).toBe('option-5');
});

it('should be available as a standalone directive', () => {
@Component({
selector: 'test-component',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1764,7 +1764,7 @@
"name": "stringify"
},
{
"name": "stringify10"
"name": "stringify11"
},
{
"name": "stringifyCSSSelector"
Expand Down

0 comments on commit 032b2bd

Please sign in to comment.