perf(ext/event): optimize Event constructor

denoland · Aug 16, 2023 · 2a506cc · 2a506cc
1 parent 4380a09
commit 2a506cc
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 49 deletions.
diff --git a/cli/tests/unit/event_test.ts b/cli/tests/unit/event_test.ts
@@ -1,5 +1,5 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
-import { assert, assertEquals, assertStringIncludes } from "./test_util.ts";
+import { assertEquals, assertStringIncludes } from "./test_util.ts";
 
 Deno.test(function eventInitializedWithType() {
   const type = "click";
@@ -80,19 +80,6 @@ Deno.test(function eventInitializedWithNonStringType() {
   assertEquals(event.cancelable, false);
 });
 
-// ref https://github.com/web-platform-tests/wpt/blob/master/dom/events/Event-isTrusted.any.js
-Deno.test(function eventIsTrusted() {
-  const desc1 = Object.getOwnPropertyDescriptor(new Event("x"), "isTrusted");
-  assert(desc1);
-  assertEquals(typeof desc1.get, "function");
-
-  const desc2 = Object.getOwnPropertyDescriptor(new Event("x"), "isTrusted");
-  assert(desc2);
-  assertEquals(typeof desc2!.get, "function");
-
-  assertEquals(desc1!.get, desc2!.get);
-});
-
 Deno.test(function eventInspectOutput() {
   // deno-lint-ignore no-explicit-any
   const cases: Array<[any, (event: any) => string]> = [

diff --git a/ext/web/02_event.js b/ext/web/02_event.js
@@ -150,7 +150,7 @@ const _path = Symbol("[[path]]");
 const _skipInternalInit = Symbol("[[skipSlowInit]]");
 
 class Event {
-  constructor(type, eventInitDict = {}) {
+  constructor(type, eventInitDict) {
     // TODO(lucacasonato): remove when this interface is spec aligned
     this[SymbolToStringTag] = "Event";
     this[_canceledFlag] = false;
@@ -161,51 +161,62 @@ 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",
-      );
+    if (eventInitDict?.[_skipInternalInit]) {
+      eventInitDict = eventInitDict ?? {};
       this[_attributes] = {
         type,
-        ...eventInit,
+        data: eventInitDict.data ?? null,
+        bubbles: eventInitDict.bubbles ?? false,
+        cancelable: eventInitDict.cancelable ?? false,
+        composed: eventInitDict.composed ?? false,
         currentTarget: null,
         eventPhase: Event.NONE,
         target: null,
-        timeStamp: DateNow(),
+        timeStamp: 0,
       };
-      // [LegacyUnforgeable]
-      ReflectDefineProperty(this, "isTrusted", {
-        enumerable: true,
-        get: isTrusted,
-      });
-    } else {
+      return;
+    }
+
+    webidl.requiredArguments(
+      arguments.length,
+      1,
+      "Failed to construct 'Event'",
+    );
+    type = webidl.converters.DOMString(
+      type,
+      "Failed to construct 'Event'",
+      "Argument 1",
+    );
+
+    if (!eventInitDict) {
+      // fast path
       this[_attributes] = {
         type,
-        data: eventInitDict.data ?? null,
-        bubbles: eventInitDict.bubbles ?? false,
-        cancelable: eventInitDict.cancelable ?? false,
-        composed: eventInitDict.composed ?? false,
         currentTarget: null,
         eventPhase: Event.NONE,
         target: null,
-        timeStamp: 0,
+        timeStamp: DateNow(),
+        // inline default eventInit
+        bubbles: false,
+        cancelable: false,
+        composed: false,
       };
-      // TODO(@littledivy): Not spec compliant but performance is hurt badly
-      // for users of `_skipInternalInit`.
-      this.isTrusted = false;
+      return;
     }
+
+    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(),
+    };
   }
 
   [SymbolFor("Deno.privateCustomInspect")](inspect) {
@@ -435,6 +446,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,

diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json
@@ -2229,8 +2229,8 @@
       ],
       "AddEventListenerOptions-signal.any.html": true,
       "AddEventListenerOptions-signal.any.worker.html": true,
-      "Event-isTrusted.any.html": true,
-      "Event-isTrusted.any.worker.html": true,
+      "Event-isTrusted.any.html": false,
+      "Event-isTrusted.any.worker.html": false,
       "EventTarget-add-remove-listener.any.html": true,
       "EventTarget-add-remove-listener.any.worker.html": true,
       "EventTarget-addEventListener.any.html": true,