mirror of
https://github.com/denoland/deno.git
synced 2024-11-28 16:20:57 -05:00
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
This commit is contained in:
parent
ff2439bf77
commit
3552ba378b
3 changed files with 34 additions and 63 deletions
|
@ -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]> = [
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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,
|
||||
|
|
Loading…
Reference in a new issue