diff options
author | Marcos Casagrande <marcoscvp90@gmail.com> | 2023-08-17 10:35:18 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-17 10:35:18 +0200 |
commit | ec63b36994527d0969d8083a6c4be8cd325c7473 (patch) | |
tree | 50560eb78791e401b79061decf9e98d20b3e6ee8 /ext/web/02_event.js | |
parent | 1b0e394d48324b5e8416f9a563eb2f35719f0906 (diff) |
perf(ext/event): optimize Event constructor (#20181)
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
Diffstat (limited to 'ext/web/02_event.js')
-rw-r--r-- | ext/web/02_event.js | 78 |
1 files changed, 31 insertions, 47 deletions
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, |