Expand JSObject::defineOwnIndexedProperty() fast path for existing properties
Created attachment 401736 [details] Patch
(In reply to Alexey Shvayka from comment #1) > Created attachment 401736 [details] > Patch Warmed-up runs, --outer 128: r262942 patch array-redefine-index-reverse 57.2178+-0.6638 ^ 3.3607+-0.1611 ^ definitely 17.0256x faster array-redefine-index 86.8468+-1.0661 ^ 64.3230+-0.7674 ^ definitely 1.3502x faster <geometric> 70.4131+-0.5940 ^ 14.6275+-0.2617 ^ definitely 4.8137x faster
Comment on attachment 401736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401736&action=review > JSTests/stress/define-own-indexed-property-fast-path.js:11 > + for (const key of Object.keys(descriptor).sort()) Sorting can be removed when https://bugs.webkit.org/show_bug.cgi?id=142933 is fixed.
Comment on attachment 401736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401736&action=review r=me with comments about TypedArrays. > Source/JavaScriptCore/runtime/JSObject.cpp:2626 > + static const PropertyDescriptor emptyAttributesDescriptor(jsUndefined(), static_cast<unsigned>(PropertyAttribute::None)); We should not use static here. Creating PropertyDescriptor is not costly. And let's put `ASSERT(emptyAttributesDescriptor.attributes() == PropertyAttribute::None)`. > Source/JavaScriptCore/runtime/JSObject.cpp:2635 > +#if ASSERT_ENABLED > + if (canGetIndexQuickly(index)) { > + PropertyDescriptor currentDescriptor; > + ASSERT(getOwnPropertyDescriptor(globalObject, Identifier::from(vm, index), currentDescriptor)); > + scope.assertNoException(); > + ASSERT(currentDescriptor.attributes() == emptyAttributesDescriptor.attributes()); > + } > +#endif I think this is not correct for typed-arrays. Can you add tests and fix this? While canDoFastPutDirectIndex can prevent using putDirectIndex for typed-arrays, but this is executed for typed-arrays too.
Created attachment 401918 [details] Patch Drop `static`, add PropertyAttribute::None ASSERT, fix canGetIndexQuickly() ASSERT.
(In reply to Yusuke Suzuki from comment #4) Thank you for review! > I think this is not correct for typed-arrays. Can you add tests and fix this? > While canDoFastPutDirectIndex can prevent using putDirectIndex for > typed-arrays, but this is executed for typed-arrays too. JSGenericTypedArrayView<Adaptor>::defineOwnProperty() appears to be called instead of JSObject::defineOwnIndexedProperty(), that is why no tests are failing. I've added canDoFastPutDirectIndex() check to the assert nonetheless. [[DefineOwnProperty]] on typed arrays is covered by JSTests/stress/typedarray-configure-index.js, as well as by test262 suite: https://test262.report/browse/built-ins/TypedArrayConstructors/internals/DefineOwnProperty.
Committed r263070: <https://trac.webkit.org/changeset/263070> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401918 [details].
<rdar://problem/64387661>