Skip to content

Commit

Permalink
perf(platform-browser): disable styles of removed components instead …
Browse files Browse the repository at this point in the history
…of removing

This commit changes the behaviour of `REMOVE_STYLES_ON_COMPONENT_DESTROY` in 2 main ways:
- `style` nodes are disabled instead of removed from DOM. This causes the same runtime behaviour but avoids recomputations when the stylesheet is re-added when the component is re-created. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/css_style_sheet.h;l=266;drc=31fb07c05718d671d96c227855bfe97af9e3fb20
- `style` nodes of components with `ViewEncapsulation.Emulated` are neither removed nor disabled. This is because such styles from such components cannot leak and effect other components.

NB: This changes is being done following some performance bottlenecks observed in Phanteon and their own recommendations.

Context:
http://chat/room/AAAAxKxTk40/jaP6Lj6fhmQ/jaP6Lj6fhmQ
https://crbug.com/1444522
http://b/289992821
  • Loading branch information
alan-agius4 committed Sep 18, 2023
1 parent fb3e6d6 commit e3d9560
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 113 deletions.
36 changes: 16 additions & 20 deletions packages/platform-browser/src/dom/dom_renderer.ts
Expand Up @@ -36,13 +36,13 @@ const REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT = true;

/**
* A [DI token](guide/glossary#di-token "DI token definition") that indicates whether styles
* of destroyed components should be removed from DOM.
* of destroyed components with `ViewEncapsulation.None` should be disabled.
*
* By default, the value is set to `true`.
* @publicApi
*/
export const REMOVE_STYLES_ON_COMPONENT_DESTROY =
new InjectionToken<boolean>('RemoveStylesOnCompDestroy', {
new InjectionToken<boolean>(ngDevMode ? 'RemoveStylesOnCompDestroy' : '', {
providedIn: 'root',
factory: () => REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT,
});
Expand Down Expand Up @@ -70,7 +70,7 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
private readonly eventManager: EventManager,
private readonly sharedStylesHost: SharedStylesHost,
@Inject(APP_ID) private readonly appId: string,
@Inject(REMOVE_STYLES_ON_COMPONENT_DESTROY) private removeStylesOnCompDestroy: boolean,
@Inject(REMOVE_STYLES_ON_COMPONENT_DESTROY) private disableStylesOnCompDestroy: boolean,
@Inject(DOCUMENT) private readonly doc: Document,
@Inject(PLATFORM_ID) readonly platformId: Object,
readonly ngZone: NgZone,
Expand Down Expand Up @@ -112,22 +112,20 @@ export class DomRendererFactory2 implements RendererFactory2, OnDestroy {
const ngZone = this.ngZone;
const eventManager = this.eventManager;
const sharedStylesHost = this.sharedStylesHost;
const removeStylesOnCompDestroy = this.removeStylesOnCompDestroy;
const platformIsServer = this.platformIsServer;

switch (type.encapsulation) {
case ViewEncapsulation.Emulated:
renderer = new EmulatedEncapsulationDomRenderer2(
eventManager, sharedStylesHost, type, this.appId, removeStylesOnCompDestroy, doc,
ngZone, platformIsServer);
eventManager, sharedStylesHost, type, this.appId, doc, ngZone, platformIsServer);
break;
case ViewEncapsulation.ShadowDom:
return new ShadowDomRenderer(
eventManager, sharedStylesHost, element, type, doc, ngZone, this.nonce,
platformIsServer);
default:
renderer = new NoneEncapsulationDomRenderer(
eventManager, sharedStylesHost, type, removeStylesOnCompDestroy, doc, ngZone,
eventManager, sharedStylesHost, type, this.disableStylesOnCompDestroy, doc, ngZone,
platformIsServer);
break;
}
Expand Down Expand Up @@ -395,32 +393,29 @@ class ShadowDomRenderer extends DefaultDomRenderer2 {
}

class NoneEncapsulationDomRenderer extends DefaultDomRenderer2 {
private readonly styles: string[];
protected styles: string[];

constructor(
eventManager: EventManager,
private readonly sharedStylesHost: SharedStylesHost,
component: RendererType2,
private removeStylesOnCompDestroy: boolean,
private disableStylesOnCompDestroy: boolean,
doc: Document,
ngZone: NgZone,
platformIsServer: boolean,
compId?: string,
) {
super(eventManager, doc, ngZone, platformIsServer);
this.styles = compId ? shimStylesContent(compId, component.styles) : component.styles;
this.styles = component.styles;
}

applyStyles(): void {
this.sharedStylesHost.addStyles(this.styles);
}

override destroy(): void {
if (!this.removeStylesOnCompDestroy) {
return;
if (this.disableStylesOnCompDestroy) {
this.sharedStylesHost.disableStyles(this.styles);
}

this.sharedStylesHost.removeStyles(this.styles);
}
}

Expand All @@ -430,12 +425,11 @@ class EmulatedEncapsulationDomRenderer2 extends NoneEncapsulationDomRenderer {

constructor(
eventManager: EventManager, sharedStylesHost: SharedStylesHost, component: RendererType2,
appId: string, removeStylesOnCompDestroy: boolean, doc: Document, ngZone: NgZone,
platformIsServer: boolean) {
appId: string, doc: Document, ngZone: NgZone, platformIsServer: boolean) {
super(eventManager, sharedStylesHost, component, false, doc, ngZone, platformIsServer);
const compId = appId + '-' + component.id;
super(
eventManager, sharedStylesHost, component, removeStylesOnCompDestroy, doc, ngZone,
platformIsServer, compId);

this.styles = shimStylesContent(compId, component.styles);
this.contentAttr = shimContentAttribute(compId);
this.hostAttr = shimHostAttribute(compId);
}
Expand All @@ -450,4 +444,6 @@ class EmulatedEncapsulationDomRenderer2 extends NoneEncapsulationDomRenderer {
super.setAttribute(el, this.contentAttr, '');
return el;
}

override destroy(): void {}
}
30 changes: 18 additions & 12 deletions packages/platform-browser/src/dom/shared_styles_host.ts
Expand Up @@ -44,16 +44,22 @@ export class SharedStylesHost implements OnDestroy {
}
}

removeStyles(styles: string[]): void {
disableStyles(styles: string[]): void {
for (const style of styles) {
const usageCount = this.changeUsageCount(style, -1);

if (usageCount <= 0) {
this.onStyleRemoved(style);
this.visitStyleElement(style, (node) => {
node.disabled = true;
});
}
}
}

visitStyleElement(style: string, callback: (node: HTMLStyleElement) => void): void {
this.styleRef.get(style)?.elements?.forEach(callback);
}

ngOnDestroy(): void {
const styleNodesInDOM = this.styleNodesInDOM;
if (styleNodesInDOM) {
Expand All @@ -62,7 +68,10 @@ export class SharedStylesHost implements OnDestroy {
}

for (const style of this.getAllStyles()) {
this.onStyleRemoved(style);
this.visitStyleElement(style, (node) => {
node.remove();
});
this.styleRef.delete(style);
}

this.resetHostNodes();
Expand Down Expand Up @@ -90,12 +99,6 @@ export class SharedStylesHost implements OnDestroy {
}
}

private onStyleRemoved(style: string): void {
const styleRef = this.styleRef;
styleRef.get(style)?.elements?.forEach((node) => node.remove());
styleRef.delete(style);
}

private collectServerRenderedStyles(): Map<string, HTMLStyleElement>|null {
const styles = this.doc.head?.querySelectorAll<HTMLStyleElement>(
`style[${APP_ID_ATTRIBUTE_NAME}="${this.appId}"]`);
Expand All @@ -119,13 +122,14 @@ export class SharedStylesHost implements OnDestroy {
const map = this.styleRef;
if (map.has(style)) {
const styleRefValue = map.get(style)!;
styleRefValue.usage += delta;
styleRefValue.usage = Math.max(0, styleRefValue.usage + delta);

return styleRefValue.usage;
}

map.set(style, {usage: delta, elements: []});
return delta;
const usage = Math.max(0, delta);
map.set(style, {usage, elements: []});
return usage;
}

private getStyleElement(host: Node, style: string): HTMLStyleElement {
Expand Down Expand Up @@ -163,6 +167,8 @@ export class SharedStylesHost implements OnDestroy {
private addStyleToHost(host: Node, style: string): void {
const styleEl = this.getStyleElement(host, style);

// Enable the stylesheet
styleEl.disabled = false;
host.appendChild(styleEl);

const styleRef = this.styleRef;
Expand Down

0 comments on commit e3d9560

Please sign in to comment.