Benchmark + optimize (ampproject#1096)

* Create benchmarks for createElement * Create IS_SERVER for optimizing path. * Optimize the paths and measure results along the way * rm comments * unecessary type * whoops extra file * Some more early exits, only supply value when true. * remove minification for server-lib * be smarter (attach to process.env for var replacment) * undo unrelated changes * add comment for context on lazy css
knownotunknown · Sep 28, 2021 · b9ffedd · b9ffedd
1 parent cc71f9c
commit b9ffedd
Show file tree

Hide file tree

Showing 10 changed files with 85 additions and 122 deletions.
diff --git a/benchmark.mjs b/benchmark.mjs
@@ -0,0 +1,15 @@
+import Benchmark from 'benchmark';
+
+import { createDocument } from './dist/server-lib.mjs';
+const doc = createDocument();
+
+var suite = new Benchmark.Suite();
+suite
+  .add('createElement', function () {
+    doc.createElement('test-elem');
+  })
+  .on('complete', function () {
+    const results = Array.from(this);
+    console.log(results.join('\n'));
+  })
+  .run();
diff --git a/config/rollup.main-thread.js b/config/rollup.main-thread.js
@@ -1,7 +1,6 @@
 import compiler from '@ampproject/rollup-plugin-closure-compiler';
 import { terser } from 'rollup-plugin-terser';
-import replace from '@rollup/plugin-replace';
-import { babelPlugin, removeDebugCommandExecutors, removeWorkerWhitespace } from './rollup.plugins.js';
+import { babelPlugin, removeDebugCommandExecutors, removeWorkerWhitespace, replacePlugin } from './rollup.plugins.js';
 
 const ESModules = [
   {
@@ -14,13 +13,7 @@ const ESModules = [
     plugins: [
       removeWorkerWhitespace(),
       removeDebugCommandExecutors(),
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-          IS_AMP: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: false,
         allowConsole: false,
@@ -38,13 +31,7 @@ const ESModules = [
     },
     plugins: [
       removeWorkerWhitespace(),
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-          IS_AMP: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: true,
@@ -61,13 +48,7 @@ const ESModules = [
     plugins: [
       removeWorkerWhitespace(),
       removeDebugCommandExecutors(),
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-          IS_AMP: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ amp: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: false,
@@ -85,13 +66,7 @@ const ESModules = [
     },
     plugins: [
       removeWorkerWhitespace(),
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-          IS_AMP: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true, amp: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: true,
@@ -112,13 +87,7 @@ const IIFEModules = [
     plugins: [
       removeWorkerWhitespace(),
       removeDebugCommandExecutors(),
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-          IS_AMP: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: true,
         allowConsole: false,
@@ -137,13 +106,7 @@ const IIFEModules = [
     },
     plugins: [
       removeWorkerWhitespace(),
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-          IS_AMP: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true, amp: true }),
       babelPlugin({
         transpileToES5: true,
         allowConsole: true,

diff --git a/config/rollup.plugins.js b/config/rollup.plugins.js
@@ -1,4 +1,5 @@
 import babel from '@rollup/plugin-babel';
+import replace from '@rollup/plugin-replace';
 import MagicString from 'magic-string';
 import fs from 'fs';
 import * as path from 'path';
@@ -18,6 +19,9 @@ export function babelPlugin({ transpileToES5, allowConsole = false, allowPostMes
   return babel({
     babelHelpers: 'bundled',
     exclude: 'node_modules/**',
+    assumptions: {
+      setPublicClassFields: true,
+    },
     presets: [
       [
         '@babel/env',
@@ -139,3 +143,14 @@ export function removeWorkerWhitespace() {
     },
   };
 }
+
+export function replacePlugin({ debug = false, server = false, amp = false } = {}) {
+  return replace({
+    values: {
+      WORKER_DOM_DEBUG: debug,
+      IS_AMP: amp,
+      'process.env.SERVER': server,
+    },
+    preventAssignment: true,
+  });
+}
diff --git a/config/rollup.worker-thread.js b/config/rollup.worker-thread.js
@@ -1,7 +1,6 @@
 import compiler from '@ampproject/rollup-plugin-closure-compiler';
 import { terser } from 'rollup-plugin-terser';
-import replace from '@rollup/plugin-replace';
-import { babelPlugin } from './rollup.plugins.js';
+import { babelPlugin, replacePlugin } from './rollup.plugins.js';
 
 // Compile plugins should always be added at the end of the plugin list.
 const compilePlugins = [
@@ -24,17 +23,11 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ server: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: false,
       }),
-      ...compilePlugins,
     ],
   },
   {
@@ -46,12 +39,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: false,
         allowConsole: false,
@@ -68,12 +56,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: true,
@@ -89,12 +72,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: false,
         allowConsole: false,
@@ -111,6 +89,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: true,
@@ -126,12 +105,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({}),
       babelPlugin({
         transpileToES5: true,
         allowConsole: false,
@@ -148,12 +122,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: true,
         allowConsole: true,
@@ -169,12 +138,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: false,
         allowConsole: true,
@@ -191,12 +155,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: false,
         allowConsole: true,
@@ -212,12 +171,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: true,
         allowConsole: false,
@@ -234,12 +188,7 @@ const ESModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: true,
         allowConsole: true,
@@ -258,12 +207,7 @@ const IIFEModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: false,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin(),
       babelPlugin({
         transpileToES5: true,
         allowConsole: false,
@@ -280,12 +224,7 @@ const IIFEModules = [
       sourcemap: true,
     },
     plugins: [
-      replace({
-        values: {
-          WORKER_DOM_DEBUG: true,
-        },
-        preventAssignment: true,
-      }),
+      replacePlugin({ debug: true }),
       babelPlugin({
         transpileToES5: true,
         allowConsole: true,

diff --git a/package.json b/package.json
@@ -51,6 +51,7 @@
     "ava": "3.15.0",
     "babel-plugin-minify-replace": "0.5.0",
     "babel-plugin-transform-remove-console": "6.9.4",
+    "benchmark": "2.1.4",
     "cross-env": "7.0.3",
     "esm": "3.2.25",
     "husky": "7.0.2",

diff --git a/src/worker-thread/MutationTransfer.ts b/src/worker-thread/MutationTransfer.ts
@@ -13,6 +13,10 @@ let pendingMutations: Array<number> = [];
 
 // TODO(choumx): Change `mutation` to Array<Uint16> to prevent casting errors e.g. integer underflow, precision loss.
 export function transfer(document: Document | DocumentStub, mutation: Array<number>): void {
+  if (process.env.SERVER) {
+    return;
+  }
+
   if (phase > Phase.Initializing && document[TransferrableKeys.allowTransfer]) {
     pending = true;
     pendingMutations = pendingMutations.concat(mutation);

diff --git a/src/worker-thread/dom/Element.ts b/src/worker-thread/dom/Element.ts
@@ -76,8 +76,8 @@ export class Element extends ParentNode {
 
   public localName: NodeName;
   public attributes: Attr[] = [];
-  public style: CSSStyleDeclaration = new CSSStyleDeclaration(this);
   public namespaceURI: NamespaceURI;
+  private style_: CSSStyleDeclaration | undefined;
 
   /**
    * Element "kind" dictates certain behaviors e.g. start/end tag semantics.
@@ -100,6 +100,15 @@ export class Element extends ParentNode {
     ];
   }
 
+  // We lazily instantiate the CSSStyleDeclaration to keep element creation cheap
+  // See https://github.com/ampproject/worker-dom/pull/1096 for context.
+  get style(): CSSStyleDeclaration {
+    if (!this.style_) {
+      this.style_ = new CSSStyleDeclaration(this);
+    }
+    return this.style_;
+  }
+
   // Unimplemented properties
   // Element.clientHeight – https://developer.mozilla.org/en-US/docs/Web/API/Element/clientHeight
   // Element.clientLeft – https://developer.mozilla.org/en-US/docs/Web/API/Element/clientLeft

diff --git a/src/worker-thread/dom/Node.ts b/src/worker-thread/dom/Node.ts
@@ -51,10 +51,12 @@ export abstract class Node {
   constructor(nodeType: NodeType, nodeName: NodeName, ownerDocument: Node | null, overrideIndex?: number) {
     this.nodeType = nodeType;
     this.nodeName = nodeName;
-
     this.ownerDocument = ownerDocument || this;
-    this[TransferrableKeys.scopingRoot] = this;
+    if (process.env.SERVER) {
+      return;
+    }
 
+    this[TransferrableKeys.scopingRoot] = this;
     this[TransferrableKeys.index] = overrideIndex ? storeOverrideNodeMapping(this, overrideIndex) : storeNodeMapping(this);
     this[TransferrableKeys.transferredFormat] = [this[TransferrableKeys.index]];
   }