From ec63b36994527d0969d8083a6c4be8cd325c7473 Mon Sep 17 00:00:00 2001
From: Marcos Casagrande <marcoscvp90@gmail.com>
Date: Thu, 17 Aug 2023 10:35:18 +0200
Subject: perf(ext/event): optimize Event constructor (#20181)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR optimizes `Event` constructor

- ~Added a fast path for empty `eventInitDict`~ Removed `EventInit`
dictionary converter
- Don't make `isTrusted` a
[LegacyUnforgeable](https://webidl.spec.whatwg.org/#LegacyUnforgeable)
property. Doing so makes it non-spec compliant but calling
`Object/Reflect.defineProperty` on the constructor is a big bottleneck.
Node did the same a few months ago
https://github.com/nodejs/node/pull/46974. In my opinion, the
performance gains are worth deviating from the spec for a
browser-related property.

**This PR**

```
cpu: 13th Gen Intel(R) Core(TM) i9-13900H
runtime: deno 1.36.1 (x86_64-unknown-linux-gnu)

benchmark                      time (avg)        iter/s             (min … max)       p75       p99      p995
------------------------------------------------------------------------------- -----------------------------
event constructor no init      36.69 ns/iter  27,257,504.6   (33.36 ns … 42.45 ns)  37.71 ns  39.61 ns  40.07 ns
event constructor               36.7 ns/iter  27,246,776.6   (33.35 ns … 56.03 ns)  37.73 ns  40.14 ns  41.74 ns
```

**main**

```
cpu: 13th Gen Intel(R) Core(TM) i9-13900H
runtime: deno 1.36.1 (x86_64-unknown-linux-gnu)

benchmark                      time (avg)        iter/s             (min … max)       p75       p99      p995
------------------------------------------------------------------------------- -----------------------------
event constructor no init     380.48 ns/iter   2,628,275.8 (366.66 ns … 399.39 ns) 384.58 ns 398.27 ns 399.39 ns
event constructor             480.33 ns/iter   2,081,882.6 (466.67 ns … 503.47 ns) 484.27 ns 501.28 ns 503.47 ns
```

```js
Deno.bench("event constructor no init", () => {
  const event = new Event("foo");
});

Deno.bench("event constructor", () => {
  const event = new Event("foo", { bubbles: true, cancelable: false });
});
```

towards https://github.com/denoland/deno/issues/20167
---
 ext/web/02_event.js | 78 +++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 47 deletions(-)

(limited to 'ext/web')

diff --git a/ext/web/02_event.js b/ext/web/02_event.js
index 859da2121..d59a897a6 100644
--- a/ext/web/02_event.js
+++ b/ext/web/02_event.js
@@ -122,20 +122,6 @@ const isTrusted = ObjectGetOwnPropertyDescriptor({
   },
 }, "isTrusted").get;
 
-webidl.converters.EventInit = webidl.createDictionaryConverter("EventInit", [{
-  key: "bubbles",
-  defaultValue: false,
-  converter: webidl.converters.boolean,
-}, {
-  key: "cancelable",
-  defaultValue: false,
-  converter: webidl.converters.boolean,
-}, {
-  key: "composed",
-  defaultValue: false,
-  converter: webidl.converters.boolean,
-}]);
-
 const _attributes = Symbol("[[attributes]]");
 const _canceledFlag = Symbol("[[canceledFlag]]");
 const _stopPropagationFlag = Symbol("[[stopPropagationFlag]]");
@@ -161,36 +147,7 @@ class Event {
     this[_isTrusted] = false;
     this[_path] = [];
 
-    if (!eventInitDict[_skipInternalInit]) {
-      webidl.requiredArguments(
-        arguments.length,
-        1,
-        "Failed to construct 'Event'",
-      );
-      type = webidl.converters.DOMString(
-        type,
-        "Failed to construct 'Event'",
-        "Argument 1",
-      );
-      const eventInit = webidl.converters.EventInit(
-        eventInitDict,
-        "Failed to construct 'Event'",
-        "Argument 2",
-      );
-      this[_attributes] = {
-        type,
-        ...eventInit,
-        currentTarget: null,
-        eventPhase: Event.NONE,
-        target: null,
-        timeStamp: DateNow(),
-      };
-      // [LegacyUnforgeable]
-      ReflectDefineProperty(this, "isTrusted", {
-        enumerable: true,
-        get: isTrusted,
-      });
-    } else {
+    if (eventInitDict?.[_skipInternalInit]) {
       this[_attributes] = {
         type,
         data: eventInitDict.data ?? null,
@@ -202,10 +159,30 @@ class Event {
         target: null,
         timeStamp: 0,
       };
-      // TODO(@littledivy): Not spec compliant but performance is hurt badly
-      // for users of `_skipInternalInit`.
-      this.isTrusted = false;
+      return;
     }
+
+    webidl.requiredArguments(
+      arguments.length,
+      1,
+      "Failed to construct 'Event'",
+    );
+    type = webidl.converters.DOMString(
+      type,
+      "Failed to construct 'Event'",
+      "Argument 1",
+    );
+
+    this[_attributes] = {
+      type,
+      bubbles: !!eventInitDict.bubbles,
+      cancelable: !!eventInitDict.cancelable,
+      composed: !!eventInitDict.composed,
+      currentTarget: null,
+      eventPhase: Event.NONE,
+      target: null,
+      timeStamp: DateNow(),
+    };
   }
 
   [SymbolFor("Deno.privateCustomInspect")](inspect) {
@@ -435,6 +412,13 @@ class Event {
   }
 }
 
+// Not spec compliant. The spec defines it as [LegacyUnforgeable]
+// but doing so has a big performance hit
+ReflectDefineProperty(Event.prototype, "isTrusted", {
+  enumerable: true,
+  get: isTrusted,
+});
+
 function defineEnumerableProps(
   Ctor,
   props,
-- 
cgit v1.2.3