Changeset 273766 in webkit

Timestamp:

Mar 2, 2021 3:33:43 PM (17 months ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Optimize getEnumerableLength
​https://bugs.webkit.org/show_bug.cgi?id=222539

Reviewed by Alexey Shvayka.

Now getEnumerableLength is only overridden by JSProxy. And this is called in the critical path of propertyNameEnumerator.
We should not use indirect call for getEnumerableLength. We remove indirect functions for getEnumerableLength. For JSProxy,
any results is OK since anyway JSProxy does not utilize the result of this function since it cannot use fast index enumerator.

We also avoid calling holesMustForwardToPrototype in getEnumerableLength when it is meaningless. For example,
if the object is ALL_BLANK_INDEXING_TYPES, then regardless of the condition of holesMustForwardToPrototype, the result of
getEnumerableLength is zero.

• runtime/ClassInfo.h:
• runtime/JSCast.h:
• runtime/JSCell.cpp:

(JSC::JSCell::getEnumerableLength): Deleted.

• runtime/JSCell.h:
• runtime/JSObject.cpp:

(JSC::JSObject::getEnumerableLength):

• runtime/JSObject.h:
• runtime/JSPropertyNameEnumerator.h:

(JSC::propertyNameEnumerator):

• runtime/JSProxy.cpp:

(JSC::JSProxy::getEnumerableLength): Deleted.

• runtime/JSProxy.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ClassInfo.h (modified) (2 diffs)
• runtime/JSCast.h (modified) (1 diff)
• runtime/JSCell.cpp (modified) (1 diff)
• runtime/JSCell.h (modified) (1 diff)
• runtime/JSObject.cpp (modified) (3 diffs)
• runtime/JSObject.h (modified) (1 diff)
• runtime/JSPropertyNameEnumerator.h (modified) (1 diff)
• runtime/JSProxy.cpp (modified) (1 diff)
• runtime/JSProxy.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-03-02  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Optimize getEnumerableLength
+        https://bugs.webkit.org/show_bug.cgi?id=222539
+
+        Reviewed by Alexey Shvayka.
+
+        Now getEnumerableLength is only overridden by JSProxy. And this is called in the critical path of propertyNameEnumerator.
+        We should not use indirect call for getEnumerableLength. We remove indirect functions for getEnumerableLength. For JSProxy,
+        any results is OK since anyway JSProxy does not utilize the result of this function since it cannot use fast index enumerator.
+
+        We also avoid calling holesMustForwardToPrototype in getEnumerableLength when it is meaningless. For example,
+        if the object is ALL_BLANK_INDEXING_TYPES, then regardless of the condition of holesMustForwardToPrototype, the result of
+        getEnumerableLength is zero.
+
+        * runtime/ClassInfo.h:
+        * runtime/JSCast.h:
+        * runtime/JSCell.cpp:
+        (JSC::JSCell::getEnumerableLength): Deleted.
+        * runtime/JSCell.h:
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::getEnumerableLength):
+        * runtime/JSObject.h:
+        * runtime/JSPropertyNameEnumerator.h:
+        (JSC::propertyNameEnumerator):
+        * runtime/JSProxy.cpp:
+        (JSC::JSProxy::getEnumerableLength): Deleted.
+        * runtime/JSProxy.h:
+
 2021-03-02  BJ Burg  <bburg@apple.com>
 

trunk/Source/JavaScriptCore/runtime/ClassInfo.h

@@ -81 +81 @@
     GetOwnPropertyNamesFunctionPtr METHOD_TABLE_ENTRY(getOwnPropertyNames);
     GetOwnPropertyNamesFunctionPtr METHOD_TABLE_ENTRY(getOwnSpecialPropertyNames);
-
-    using GetEnumerableLengthFunctionPtr = uint32_t (*)(JSGlobalObject*, JSObject*);
-    GetEnumerableLengthFunctionPtr METHOD_TABLE_ENTRY(getEnumerableLength);
 
     using ClassNameFunctionPtr = String (*)(const JSObject*, VM&);
@@ -173 +170 @@
         &ClassName::getOwnPropertyNames, \
         &ClassName::getOwnSpecialPropertyNames, \
-        &ClassName::getEnumerableLength, \
         &ClassName::className, \
         &ClassName::toStringName, \

trunk/Source/JavaScriptCore/runtime/JSCast.h

@@ -80 +80 @@
     macro(JSArrayBufferView, FirstTypedArrayType, LastTypedArrayType) \
     macro(JSPromise, JSType::JSPromiseType, JSType::JSPromiseType) \
+    macro(JSProxy, JSType::PureForwardingProxyType, JSType::ImpureProxyType) \
     macro(JSSet, JSType::JSSetType, JSType::JSSetType) \
     macro(JSMap, JSType::JSMapType, JSType::JSMapType) \

trunk/Source/JavaScriptCore/runtime/JSCell.cpp

@@ -246 +246 @@
 }
 
-uint32_t JSCell::getEnumerableLength(JSGlobalObject*, JSObject*)
-{
-    RELEASE_ASSERT_NOT_REACHED();
-    return 0;
-}
-
 bool JSCell::preventExtensions(JSObject*, JSGlobalObject*)
 {

trunk/Source/JavaScriptCore/runtime/JSCell.h

@@ -249 +249 @@
     static NO_RETURN_DUE_TO_CRASH void getOwnSpecialPropertyNames(JSObject*, JSGlobalObject*, PropertyNameArray&, DontEnumPropertiesMode);
 
-    static uint32_t getEnumerableLength(JSGlobalObject*, JSObject*);
     static NO_RETURN_DUE_TO_CRASH bool preventExtensions(JSObject*, JSGlobalObject*);
     static NO_RETURN_DUE_TO_CRASH bool isExtensible(JSObject*, JSGlobalObject*);

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -3755 +3755 @@
 }
 
-uint32_t JSObject::getEnumerableLength(JSGlobalObject* globalObject, JSObject* object)
+uint32_t JSObject::getEnumerableLength(JSGlobalObject* globalObject)
 {
     VM& vm = globalObject->vm();
-    Structure* structure = object->structure(vm);
-    if (structure->holesMustForwardToPrototype(vm, object))
-        return 0;
+    JSObject* object = this;
+
     switch (object->indexingType()) {
     case ALL_BLANK_INDEXING_TYPES:
     case ALL_UNDECIDED_INDEXING_TYPES:
+        // Regardless of holesMustForwardToPrototype condition, it returns zero.
         return 0;
         
@@ -3769 +3769 @@
     case ALL_CONTIGUOUS_INDEXING_TYPES: {
         Butterfly* butterfly = object->butterfly();
-        unsigned usedLength = butterfly->publicLength();
-        for (unsigned i = 0; i < usedLength; ++i) {
+        unsigned enumerableLength = butterfly->publicLength();
+        if (!enumerableLength)
+            return 0;
+        if (object->structure(vm)->holesMustForwardToPrototype(vm, object))
+            return 0;
+        for (unsigned i = 0; i < enumerableLength; ++i) {
             if (!butterfly->contiguous().at(object, i))
                 return 0;
         }
-        return usedLength;
+        return enumerableLength;
     }
         
     case ALL_DOUBLE_INDEXING_TYPES: {
         Butterfly* butterfly = object->butterfly();
-        unsigned usedLength = butterfly->publicLength();
-        for (unsigned i = 0; i < usedLength; ++i) {
+        unsigned enumerableLength = butterfly->publicLength();
+        if (!enumerableLength)
+            return 0;
+        if (object->structure(vm)->holesMustForwardToPrototype(vm, object))
+            return 0;
+        for (unsigned i = 0; i < enumerableLength; ++i) {
             double value = butterfly->contiguousDouble().at(object, i);
             if (value != value)
                 return 0;
         }
-        return usedLength;
+        return enumerableLength;
     }
         
@@ -3793 +3801 @@
             return 0;
         
-        unsigned usedVectorLength = std::min(storage->length(), storage->vectorLength());
-        for (unsigned i = 0; i < usedVectorLength; ++i) {
+        unsigned enumerableLength = std::min(storage->length(), storage->vectorLength());
+        if (!enumerableLength)
+            return 0;
+        if (object->structure(vm)->holesMustForwardToPrototype(vm, object))
+            return 0;
+        for (unsigned i = 0; i < enumerableLength; ++i) {
             if (!storage->m_vector[i])
                 return 0;
         }
-        return usedVectorLength;
+        return enumerableLength;
     }
         

trunk/Source/JavaScriptCore/runtime/JSObject.h

@@ -686 +686 @@
     void getNonReifiedStaticPropertyNames(VM&, PropertyNameArray&, DontEnumPropertiesMode);
 
-    JS_EXPORT_PRIVATE static uint32_t getEnumerableLength(JSGlobalObject*, JSObject*);
+    JS_EXPORT_PRIVATE uint32_t getEnumerableLength(JSGlobalObject*);
 
     JS_EXPORT_PRIVATE JSValue toPrimitive(JSGlobalObject*, PreferredPrimitiveType = NoPreference) const;

trunk/Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.h

@@ -109 +109 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    uint32_t indexedLength = base->methodTable(vm)->getEnumerableLength(globalObject, base);
+    uint32_t indexedLength = base->getEnumerableLength(globalObject);
 
     JSPropertyNameEnumerator* enumerator = nullptr;

trunk/Source/JavaScriptCore/runtime/JSProxy.cpp

@@ -119 +119 @@
 }
 
-uint32_t JSProxy::getEnumerableLength(JSGlobalObject* globalObject, JSObject* object)
-{
-    JSProxy* thisObject = jsCast<JSProxy*>(object);
-    return thisObject->target()->methodTable(globalObject->vm())->getEnumerableLength(globalObject, thisObject->target());
-}
-
 void JSProxy::getOwnPropertyNames(JSObject* object, JSGlobalObject* globalObject, PropertyNameArray& propertyNames, DontEnumPropertiesMode mode)
 {

trunk/Source/JavaScriptCore/runtime/JSProxy.h

@@ -97 +97 @@
     JS_EXPORT_PRIVATE static bool deletePropertyByIndex(JSCell*, JSGlobalObject*, unsigned);
     JS_EXPORT_PRIVATE static void getOwnPropertyNames(JSObject*, JSGlobalObject*, PropertyNameArray&, DontEnumPropertiesMode);
-    JS_EXPORT_PRIVATE static uint32_t getEnumerableLength(JSGlobalObject*, JSObject*);
     JS_EXPORT_PRIVATE static bool defineOwnProperty(JSObject*, JSGlobalObject*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
     JS_EXPORT_PRIVATE static bool setPrototype(JSObject*, JSGlobalObject*, JSValue, bool shouldThrowIfCantSet);