perf(platform-browser): only append style element on creation (#52237)

alan-agius4 · pkozlowski-opensource · commit c9cde3ab1036 · 2023-10-17T11:18:02.000+02:00
Prior to this change the style element was appended to host element multiple times. Whilst the actual element was not added multiple to the DOM multiple times. This causes a performance regression and it caused repainting. This can be observed in the below benchmark. ```js (() => { const time = (name, fn) => { const t = performance.now(); fn(); console.log(name, performance.now() - t); } const s = document.createElement("style"); s.textContent = "@layer x{} @font-face { font-family: foo; }"; time("append and enable", () => { document.head.append(s); s.disabled = false; }); time("compute body color", () => { getComputedStyle(document.body).color; }); time("compute body layout", () => { document.body.offsetTop; }); time("append and disable", () => { document.head.append(s); s.disabled = false; }); time("compute body color", () => { getComputedStyle(document.body).color; }); time("compute body layout", () => { document.body.offsetTop; }); })(); ``` Output ``` append and enable 0.20000000298023224 compute body color 0.7999999970197678 compute body layout 2.899999998509884 append and disable 0.10000000149011612 compute body color 0.7000000029802322 compute body layout 2.2999999970197678 ``` When commenting the 2nd `document.head.append(s);`, the results are slightly different and we can see that calling `getComputedStyle` does not incur any performance impact this is a result of no repainting. ``` append and enable 0.10000000149011612 compute body color 0.7999999970197678 compute body layout 3.1999999955296516 append and disable 0.10000000149011612 compute body color 0 compute body layout 0 ``` Pantheon benchmarks: http://docs/spreadsheets/d/1iLRLGCmVYZHuVRdI7dO_WM7wnQ1DvkS-tJzi-0-u1KY?resourcekey=0-kwtrf0nbAhcPqAGdqbdz4g#gid=0 PR Close #52237
diff --git a/packages/platform-browser/src/dom/shared_styles_host.ts b/packages/platform-browser/src/dom/shared_styles_host.ts
@@ -139,6 +139,7 @@ export class SharedStylesHost implements OnDestroy {
     if (existingStyleElement) {
       return existingStyleElement;
     }
+
     const styleNodesInDOM = this.styleNodesInDOM;
     const styleEl = styleNodesInDOM?.get(style);
     if (styleEl?.parentNode === host) {
@@ -166,6 +167,8 @@ export class SharedStylesHost implements OnDestroy {
         styleEl.setAttribute(APP_ID_ATTRIBUTE_NAME, this.appId);
       }
 
+      host.appendChild(styleEl);
+
       return styleEl;
     }
   }
@@ -175,8 +178,6 @@ export class SharedStylesHost implements OnDestroy {
     const styleResult = styleRef.get(style)!;  // This will always be defined in `changeUsageCount`
     const styleEl = this.getStyleElement(host, style, styleResult.elements);
 
-    host.appendChild(styleEl);
-
     if (styleResult.usage === 0) {
       disableStylesheet(styleEl);
     } else {

Original file line number	Diff line number	Diff line change
`@@ -139,6 +139,7 @@ export class SharedStylesHost implements OnDestroy {`
`139`	`139`	`if (existingStyleElement) {`
`140`	`140`	`return existingStyleElement;`
`141`	`141`	`}`
	`142`	`+`
`142`	`143`	`const styleNodesInDOM = this.styleNodesInDOM;`
`143`	`144`	`const styleEl = styleNodesInDOM?.get(style);`
`144`	`145`	`if (styleEl?.parentNode === host) {`
`@@ -166,6 +167,8 @@ export class SharedStylesHost implements OnDestroy {`
`166`	`167`	`styleEl.setAttribute(APP_ID_ATTRIBUTE_NAME, this.appId);`
`167`	`168`	`}`
`168`	`169`
	`170`	`+ host.appendChild(styleEl);`
	`171`	`+`
`169`	`172`	`return styleEl;`
`170`	`173`	`}`
`171`	`174`	`}`
`@@ -175,8 +178,6 @@ export class SharedStylesHost implements OnDestroy {`
`175`	`178`	const styleResult = styleRef.get(style)!; // This will always be defined in `changeUsageCount`
`176`	`179`	`const styleEl = this.getStyleElement(host, style, styleResult.elements);`
`177`	`180`
`178`		`- host.appendChild(styleEl);`
`179`		`-`
`180`	`181`	`if (styleResult.usage === 0) {`
`181`	`182`	`disableStylesheet(styleEl);`
`182`	`183`	`} else {`