Changeset 263070 in webkit

Timestamp:

Jun 15, 2020, 6:23:44 PM (5 years ago)

Author:

Alexey Shvayka

Message:

Expand JSObject::defineOwnIndexedProperty() fast path for existing properties
​https://bugs.webkit.org/show_bug.cgi?id=213133

Reviewed by Yusuke Suzuki.

JSTests:

• microbenchmarks/array-redefine-index-and-reverse.js: Added.
• microbenchmarks/array-redefine-index.js: Added.
• stress/define-own-indexed-property-fast-path.js: Added.

Source/JavaScriptCore:

This patch expands fast path of JSObject::defineOwnIndexedProperty() to cover existing properties
if given data descriptor has no falsy attributes, preventing the object from entering SparseMode.
The optimization is possible due to this invariant: indexed properties of non-SparseMode objects
have attributes of PropertyAttribute::None (except for typed arrays; added assert covers it).

PropertyDescriptor::attributesOverridingCurrent() with PropertyAttribute::None descriptor
is used to support partial descriptors like {value: 1, writable: true}.

This change advances Object.defineProperty microbenchmark by 35%; array read/write benchmark
following property redefinition is progressed by a factor of 16 due to avoiding SparseMode.

• runtime/JSObject.cpp:

(JSC::JSObject::defineOwnIndexedProperty):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/microbenchmarks/array-redefine-index-and-reverse.js (added)
• JSTests/microbenchmarks/array-redefine-index.js (added)
• JSTests/stress/define-own-indexed-property-fast-path.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObject.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2020-06-15  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        Expand JSObject::defineOwnIndexedProperty() fast path for existing properties
+        https://bugs.webkit.org/show_bug.cgi?id=213133
+
+        Reviewed by Yusuke Suzuki.
+
+        * microbenchmarks/array-redefine-index-and-reverse.js: Added.
+        * microbenchmarks/array-redefine-index.js: Added.
+        * stress/define-own-indexed-property-fast-path.js: Added.
+
 2020-06-15  Caio Lima  <ticaiolima@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-06-15  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        Expand JSObject::defineOwnIndexedProperty() fast path for existing properties
+        https://bugs.webkit.org/show_bug.cgi?id=213133
+
+        Reviewed by Yusuke Suzuki.
+
+        This patch expands fast path of JSObject::defineOwnIndexedProperty() to cover existing properties
+        if given data descriptor has no falsy attributes, preventing the object from entering SparseMode.
+        The optimization is possible due to this invariant: indexed properties of non-SparseMode objects
+        have attributes of PropertyAttribute::None (except for typed arrays; added assert covers it).
+
+        PropertyDescriptor::attributesOverridingCurrent() with PropertyAttribute::None descriptor
+        is used to support partial descriptors like {value: 1, writable: true}.
+
+        This change advances Object.defineProperty microbenchmark by 35%; array read/write benchmark
+        following property redefinition is progressed by a factor of 16 due to avoiding SparseMode.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::defineOwnIndexedProperty):
+
 2020-06-15  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -2612 +2612 @@
 }
 
-// Defined in ES5.1 8.12.9
+// https://tc39.es/ecma262/#sec-ordinarydefineownproperty
 bool JSObject::defineOwnIndexedProperty(JSGlobalObject* globalObject, unsigned index, const PropertyDescriptor& descriptor, bool throwException)
 {
@@ -2623 +2623 @@
 
     if (!inSparseIndexingMode()) {
+        const PropertyDescriptor emptyAttributesDescriptor(jsUndefined(), static_cast<unsigned>(PropertyAttribute::None));
+        ASSERT(emptyAttributesDescriptor.attributes() == static_cast<unsigned>(PropertyAttribute::None));
+
+#if ASSERT_ENABLED
+        if (canGetIndexQuickly(index) && canDoFastPutDirectIndex(vm, this)) {
+            PropertyDescriptor currentDescriptor;
+            ASSERT(getOwnPropertyDescriptor(globalObject, Identifier::from(vm, index), currentDescriptor));
+            scope.assertNoException();
+            ASSERT(currentDescriptor.attributes() == emptyAttributesDescriptor.attributes());
+        }
+#endif
         // Fast case: we're putting a regular property to a regular array
-        // FIXME: this will pessimistically assume that if attributes are missing then they'll default to false
-        // however if the property currently exists missing attributes will override from their current 'true'
-        // state (i.e. defineOwnProperty could be used to set a value without needing to entering 'SparseMode').
-        if (!descriptor.attributes() && descriptor.value() && canDoFastPutDirectIndex(vm, this)) {
+        if (descriptor.value()
+            && (!descriptor.attributes() || (canGetIndexQuickly(index) && !descriptor.attributesOverridingCurrent(emptyAttributesDescriptor)))
+            && canDoFastPutDirectIndex(vm, this)) {
             ASSERT(!descriptor.isAccessorDescriptor());
             RELEASE_AND_RETURN(scope, putDirectIndex(globalObject, index, descriptor.value(), 0, throwException ? PutDirectIndexShouldThrow : PutDirectIndexShouldNotThrow));