WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
248754
[JSC] Improve Object.defineProperties & Object.defineProperty perf via fast iteration in ToPropertyDescriptor
https://bugs.webkit.org/show_bug.cgi?id=248754
Summary
[JSC] Improve Object.defineProperties & Object.defineProperty perf via fast i...
Jarred Sumner
Reported
2022-12-05 01:21:19 PST
A diff like this in ObjectConstructor::toPropertyDescriptor improves Object.defineProperties perf 15% - 70% ``` if (canPerformFastPropertyEnumerationForObjectAssign(description->structure())) { description->structure()->forEachProperty(vm, [&] (const PropertyTableEntry& entry) -> bool { if (entry.attributes() & PropertyAttribute::DontEnum) return true; PropertyName propertyName(entry.key()); if (propertyName == propertyNames->enumerable) { desc.setEnumerable(description->getDirect(entry.offset()).toBoolean(globalObject)); RETURN_IF_EXCEPTION(scope, false); } else if (propertyName == propertyNames->configurable) { desc.setConfigurable(description->getDirect(entry.offset()).toBoolean(globalObject)); RETURN_IF_EXCEPTION(scope, false); } else if (propertyName == propertyNames->value) { desc.setValue(description->getDirect(entry.offset())); RETURN_IF_EXCEPTION(scope, false); } else if (propertyName == propertyNames->writable) { desc.setWritable(description->getDirect(entry.offset()).toBoolean(globalObject)); RETURN_IF_EXCEPTION(scope, false); } else if (propertyName == propertyNames->get) { JSValue getter = description->getDirect(entry.offset()); RETURN_IF_EXCEPTION(scope, false); if (!getter.isUndefinedOrNull() && !getter.isCallable()) { throwTypeError(globalObject, scope, "Getter must be a function."_s); return false; } desc.setGetter(getter); } else if (propertyName == propertyNames->set) { JSValue setter = description->getDirect(entry.offset()); RETURN_IF_EXCEPTION(scope, false); if (!setter.isUndefinedOrNull() && !setter.isCallable()) { throwTypeError(globalObject, scope, "Setter must be a function."_s); return false; } desc.setSetter(setter); } return true; }); RETURN_IF_EXCEPTION(scope, false); } else { ``` This diff isn't 100% correct but fast iteration through each of the leaf property descriptor objects when possible should improve perf Here is a microbenchmark: ``` import { bench, run } from "../node_modules/mitata/src/cli.mjs"; const properties = { closed: { get() { return this._writableState ? this._writableState.closed : false; }, }, destroyed: { get() { return this._writableState ? this._writableState.destroyed : false; }, set(value) { if (this._writableState) { this._writableState.destroyed = value; } }, }, writable: { get() { const w = this._writableState; return ( !!w && w.writable !== false && !w.destroyed && !w.errored && !w.ending && !w.ended ); }, set(val) { if (this._writableState) { this._writableState.writable = !!val; } }, }, writableFinished: { get() { return this._writableState ? this._writableState.finished : false; }, }, writableObjectMode: { get() { return this._writableState ? this._writableState.objectMode : false; }, }, writableBuffer: { get() { return this._writableState && this._writableState.getBuffer(); }, }, writableEnded: { get() { return this._writableState ? this._writableState.ending : false; }, }, writableNeedDrain: { get() { const wState = this._writableState; if (!wState) return false; return !wState.destroyed && !wState.ending && wState.needDrain; }, }, writableHighWaterMark: { get() { return this._writableState && this._writableState.highWaterMark; }, }, writableCorked: { get() { return this._writableState ? this._writableState.corked : 0; }, }, writableLength: { get() { return this._writableState && this._writableState.length; }, }, errored: { enumerable: false, get() { return this._writableState ? this._writableState.errored : null; }, }, writableAborted: { enumerable: false, get: function () { return !!( this._writableState.writable !== false && (this._writableState.destroyed || this._writableState.errored) && !this._writableState.finished ); }, }, }; var count = 10_000; bench("Object.defineProperty x " + count, () => { const prop = { enumerable: false, get: function () { return !!( this._writableState.writable !== false && (this._writableState.destroyed || this._writableState.errored) && !this._writableState.finished ); }, }; for (let i = 0; i < count; i++) { function Hey() { return this; } Object.defineProperty(Hey.prototype, "writableAborted", prop); } }); bench("Object.defineProperties x " + count, () => { for (let i = 0; i < count; i++) { function Hey() { return this; } Object.defineProperties(Hey.prototype, properties); } }); bench("(all the keys) Object.defineProperties x " + count, () => { var first; { function Hey() { return this; } Object.defineProperties(Hey.prototype, properties); first = Object.getOwnPropertyDescriptors(Hey.prototype); } for (let i = 0; i < count; i++) { function Hey() { return this; } Object.defineProperties(Hey.prototype, first); } }); await run(); ```
Attachments
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2022-12-07 19:39:20 PST
<
rdar://problem/103101281
>
Yusuke Suzuki
Comment 2
2023-12-12 19:16:24 PST
Pull request:
https://github.com/WebKit/WebKit/pull/21717
EWS
Comment 3
2023-12-12 23:46:46 PST
Committed
271972@main
(2432909f3aec): <
https://commits.webkit.org/271972@main
> Reviewed commits have been landed. Closing PR #21717 and removing active labels.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug