From 2a506cc904acfe784925baa6d8c10d518510a35c Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Wed, 16 Aug 2023 18:56:10 +0200 Subject: [PATCH 1/4] perf(ext/event): optimize Event constructor --- cli/tests/unit/event_test.ts | 15 +------ ext/web/02_event.js | 84 ++++++++++++++++++++++-------------- tools/wpt/expectation.json | 4 +- 3 files changed, 54 insertions(+), 49 deletions(-) diff --git a/cli/tests/unit/event_test.ts b/cli/tests/unit/event_test.ts index d81023da1359ba..88ff803af7df85 100644 --- 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 index 859da2121787c1..1a97dfd93b99d5 100644 --- 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 index 326b7f55d3b9fe..c3deef9bd12048 100644 --- 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, From 2931d84baf655a17bce3b788bc542cb22a88a8a2 Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Wed, 16 Aug 2023 19:52:11 +0200 Subject: [PATCH 2/4] disable isTrusted test --- tools/wpt/expectation.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json index c3deef9bd12048..e4fc2f490d01db 100644 --- a/tools/wpt/expectation.json +++ b/tools/wpt/expectation.json @@ -2257,6 +2257,7 @@ }, "idlharness-shadowrealm.window.html": false, "idlharness.any.worker.html": [ + "Event interface object length", "Event interface: attribute srcElement", "Event interface: operation composedPath()", "Event interface: constant NONE on interface object", From 0d932088826f51a062e9c98b6bdcc34c1b64b700 Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Wed, 16 Aug 2023 20:08:01 +0200 Subject: [PATCH 3/4] reenable test --- ext/web/02_event.js | 2 +- tools/wpt/expectation.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/web/02_event.js b/ext/web/02_event.js index 1a97dfd93b99d5..c6b576ec7231d6 100644 --- 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 = undefined) { // TODO(lucacasonato): remove when this interface is spec aligned this[SymbolToStringTag] = "Event"; this[_canceledFlag] = false; diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json index e4fc2f490d01db..c3deef9bd12048 100644 --- a/tools/wpt/expectation.json +++ b/tools/wpt/expectation.json @@ -2257,7 +2257,6 @@ }, "idlharness-shadowrealm.window.html": false, "idlharness.any.worker.html": [ - "Event interface object length", "Event interface: attribute srcElement", "Event interface: operation composedPath()", "Event interface: constant NONE on interface object", From 2224dc2d277002ae56582c1350b7b3a444e49f63 Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Wed, 16 Aug 2023 20:48:35 +0200 Subject: [PATCH 4/4] remove EventInit dictionary converter --- ext/web/02_event.js | 42 ++++-------------------------------------- 1 file changed, 4 insertions(+), 38 deletions(-) diff --git a/ext/web/02_event.js b/ext/web/02_event.js index c6b576ec7231d6..d59a897a62672c 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]]"); @@ -150,7 +136,7 @@ const _path = Symbol("[[path]]"); const _skipInternalInit = Symbol("[[skipSlowInit]]"); class Event { - constructor(type, eventInitDict = undefined) { + constructor(type, eventInitDict = {}) { // TODO(lucacasonato): remove when this interface is spec aligned this[SymbolToStringTag] = "Event"; this[_canceledFlag] = false; @@ -162,7 +148,6 @@ class Event { this[_path] = []; if (eventInitDict?.[_skipInternalInit]) { - eventInitDict = eventInitDict ?? {}; this[_attributes] = { type, data: eventInitDict.data ?? null, @@ -188,30 +173,11 @@ class Event { "Argument 1", ); - if (!eventInitDict) { - // fast path - this[_attributes] = { - type, - currentTarget: null, - eventPhase: Event.NONE, - target: null, - timeStamp: DateNow(), - // inline default eventInit - bubbles: false, - cancelable: false, - composed: false, - }; - return; - } - - const eventInit = webidl.converters.EventInit( - eventInitDict, - "Failed to construct 'Event'", - "Argument 2", - ); this[_attributes] = { type, - ...eventInit, + bubbles: !!eventInitDict.bubbles, + cancelable: !!eventInitDict.cancelable, + composed: !!eventInitDict.composed, currentTarget: null, eventPhase: Event.NONE, target: null,