248754 2022-12-05 01:21:19 -0800 [JSC] Improve Object.defineProperties & Object.defineProperty perf via fast iteration in ToPropertyDescriptor 2023-12-12 23:47:08 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jarred ysuzuki mark.lam webkit-bug-importer ysuzuki oldest_to_newest 1916645 0 jarred 2022-12-05 01:21:19 -0800 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(); ``` 1917456 1 webkit-bug-importer 2022-12-07 19:39:20 -0800 <rdar://problem/103101281> 1999171 2 ysuzuki 2023-12-12 19:16:24 -0800 Pull request: https://github.com/WebKit/WebKit/pull/21717 1999206 3 ews-feeder 2023-12-12 23:46:46 -0800 Committed 271972@main (2432909f3aec): <https://commits.webkit.org/271972@main> Reviewed commits have been landed. Closing PR #21717 and removing active labels.