[JSC] Make Object.values faster

https://bugs.webkit.org/show_bug.cgi?id=248749
rdar://102966742

Reviewed by Justin Michaud.

This patch adds JSObject::fastForEachPropertyWithSideEffectFreeFunctor, and apply this to Object.values
to accelerate performance. It makes Object.values 3x faster.

                               ToT                     Patched

    object-values        17.2415+-0.0379     ^      5.6210+-0.0133        ^ definitely 3.0673x faster

* Source/JavaScriptCore/dfg/DFGOperations.cpp:
(JSC::DFG::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/runtime/JSONObject.cpp:
(JSC::Stringifier::Holder::appendNextProperty):
(JSC::FastStringifier::append):
* Source/JavaScriptCore/runtime/JSObject.h:
* Source/JavaScriptCore/runtime/JSObjectInlines.h:
(JSC::JSObject::fastForEachPropertyWithSideEffectFreeFunctor):
* Source/JavaScriptCore/runtime/ObjectConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/ObjectConstructorInlines.h:
(JSC::objectAssignFast):
(JSC::canPerformFastPropertyEnumerationForObjectAssign): Deleted.
(JSC::canPerformFastPropertyEnumerationForJSONStringify): Deleted.
(JSC::canPerformFastPropertyEnumerationForObjectEntries): Deleted.
* Source/JavaScriptCore/runtime/Structure.h:
* Source/JavaScriptCore/runtime/StructureInlines.h:
(JSC::Structure::canPerformFastPropertyEnumeration const):

Canonical link: https://commits.webkit.org/257382@main

Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -331,37 +331,37 @@ JSC_DEFINE_JIT_OPERATION(operationObjectAssignObject, void, (JSGlobalObject* glo
             RETURN_IF_EXCEPTION(scope, void());
         }
 
-        if (canPerformFastPropertyEnumerationForObjectAssign(source->structure())) {
-            // |source| Structure does not have any getters. And target can perform fast put.
-            // So enumerating properties and putting properties are non observable.
-
-            // FIXME: It doesn't seem like we should have to do this in two phases, but
-            // we're running into crashes where it appears that source is transitioning
-            // under us, and even ends up in a state where it has a null butterfly. My
-            // leading hypothesis here is that we fire some value replacement watchpoint
-            // that ends up transitioning the structure underneath us.
-            // https://bugs.webkit.org/show_bug.cgi?id=187837
-
-            // FIXME: This fast path is very similar to ObjectConstructor' one. But extracting it to a function caused performance
-            // regression in object-assign-replace. Since the code is small and fast path, we keep both.
-
-            // Do not clear since Vector::clear shrinks the backing store.
-            properties.resize(0);
-            values.clear();
-            source->structure()->forEachProperty(vm, [&] (const PropertyTableEntry& entry) -> bool {
-                if (entry.attributes() & PropertyAttribute::DontEnum)
-                    return true;
-
-                PropertyName propertyName(entry.key());
-                if (propertyName.isPrivateName())
-                    return true;
-
-                properties.append(entry.key());
-                values.appendWithCrashOnOverflow(source->getDirect(entry.offset()));
+        // |source| Structure does not have any getters. And target can perform fast put.
+        // So enumerating properties and putting properties are non observable.
+
+        // FIXME: It doesn't seem like we should have to do this in two phases, but
+        // we're running into crashes where it appears that source is transitioning
+        // under us, and even ends up in a state where it has a null butterfly. My
+        // leading hypothesis here is that we fire some value replacement watchpoint
+        // that ends up transitioning the structure underneath us.
+        // https://bugs.webkit.org/show_bug.cgi?id=187837
+
+        // FIXME: This fast path is very similar to ObjectConstructor' one. But extracting it to a function caused performance
+        // regression in object-assign-replace. Since the code is small and fast path, we keep both.
+
+        // Do not clear since Vector::clear shrinks the backing store.
+        properties.resize(0);
+        values.clear();
+        bool canUseFastPath = source->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
+            if (entry.attributes() & PropertyAttribute::DontEnum)
+                return true;
 
+            PropertyName propertyName(entry.key());
+            if (propertyName.isPrivateName())
                 return true;
-            });
 
+            properties.append(entry.key());
+            values.appendWithCrashOnOverflow(source->getDirect(entry.offset()));
+
+            return true;
+        });
+
+        if (canUseFastPath) {
             for (size_t i = 0; i < properties.size(); ++i) {
                 // FIXME: We could put properties in a batching manner to accelerate Object.assign more.
                 // https://bugs.webkit.org/show_bug.cgi?id=185358
@@ -397,12 +397,10 @@ JSC_DEFINE_JIT_OPERATION(operationObjectAssignUntyped, void, (JSGlobalObject* gl
             RETURN_IF_EXCEPTION(scope, void());
         }
 
-        if (canPerformFastPropertyEnumerationForObjectAssign(source->structure())) {
-            Vector<RefPtr<UniquedStringImpl>, 8> properties;
-            MarkedArgumentBuffer values;
-            objectAssignFast(vm, target, source, properties, values);
+        Vector<RefPtr<UniquedStringImpl>, 8> properties;
+        MarkedArgumentBuffer values;
+        if (objectAssignFast(vm, target, source, properties, values))
             return;
-        }
     }
 
     scope.release();

Source/JavaScriptCore/runtime/JSONObject.cpp

@@ -520,7 +520,7 @@ bool Stringifier::Holder::appendNextProperty(Stringifier& stringifier, StringBui
             if (stringifier.m_usingArrayReplacer) {
                 m_propertyNames = stringifier.m_arrayReplacerPropertyNames.data();
                 m_size = m_propertyNames->propertyNameVector().size();
-            } else if (m_structure && m_object->structureID() == m_structure->id() && canPerformFastPropertyEnumerationForJSONStringify(m_structure)) {
+            } else if (m_structure && m_object->structureID() == m_structure->id() && m_structure->canPerformFastPropertyEnumeration()) {
                 m_structure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
                     if (entry.attributes() & PropertyAttribute::DontEnum)
                         return true;
@@ -1046,7 +1046,7 @@ void FastStringifier::append(JSValue value)
             return;
         }
         m_buffer[m_length++] = '{';
-        if (UNLIKELY(!canPerformFastPropertyEnumerationForJSONStringify(&structure))) {
+        if (UNLIKELY(!structure.canPerformFastPropertyEnumeration())) {
             recordFastPropertyEnumerationFailure(object);
             return;
         }

Source/JavaScriptCore/runtime/JSObject.h

@@ -995,6 +995,9 @@ class JSObject : public JSCell {
 
     DECLARE_EXPORT_INFO;
 
+    template<typename Functor>
+    bool fastForEachPropertyWithSideEffectFreeFunctor(VM&, const Functor&);
+
 protected:
     void finishCreation(VM& vm)
     {

Source/JavaScriptCore/runtime/JSObjectInlines.h

@@ -807,4 +807,19 @@ inline void JSObject::setPrivateBrand(JSGlobalObject* globalObject, JSValue bran
     this->setStructure(vm, newStructure);
 }
 
+template<typename Functor>
+bool JSObject::fastForEachPropertyWithSideEffectFreeFunctor(VM& vm, const Functor& functor)
+{
+    if (!staticPropertiesReified())
+        return false;
+
+    Structure* structure = this->structure();
+
+    if (!structure->canPerformFastPropertyEnumeration())
+        return false;
+
+    structure->forEachProperty(vm, functor);
+    return true;
+}
+
 } // namespace JSC

Source/JavaScriptCore/runtime/ObjectConstructor.cpp

@@ -341,10 +341,8 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorAssign, (JSGlobalObject* globalObject,
                 RETURN_IF_EXCEPTION(scope, { });
             }
 
-            if (canPerformFastPropertyEnumerationForObjectAssign(source->structure())) {
-                objectAssignFast(vm, target, source, properties, values);
+            if (objectAssignFast(vm, target, source, properties, values))
                 continue;
-            }
         }
 
         targetCanPerformFastPut = false;
@@ -370,10 +368,10 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorEntries, (JSGlobalObject* globalObject
         RETURN_IF_EXCEPTION(scope, { });
     }
 
-    if (canPerformFastPropertyEnumerationForObjectEntries(target->structure())) {
+    {
         Vector<RefPtr<UniquedStringImpl>, 8> properties;
         MarkedArgumentBuffer values;
-        target->structure()->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
+        bool canUseFastPath = target->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
             if (entry.attributes() & PropertyAttribute::DontEnum)
                 return true;
 
@@ -386,68 +384,70 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorEntries, (JSGlobalObject* globalObject
             return true;
         });
 
-        Structure* arrayStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
-        JSArray* entries = JSArray::tryCreate(vm, arrayStructure, properties.size());
-        if (UNLIKELY(!entries)) {
-            throwOutOfMemoryError(globalObject, scope);
-            return { };
-        }
-
-        Structure* targetStructure = target->structure();
-        JSImmutableButterfly* cachedButterfly = nullptr;
-        if (LIKELY(!globalObject->isHavingABadTime())) {
-            auto* butterfly = targetStructure->cachedPropertyNames(CachedPropertyNamesKind::Keys);
-            if (butterfly) {
-                ASSERT(butterfly->length() == properties.size());
-                if (butterfly->length() == properties.size())
-                    cachedButterfly = butterfly;
+        if (canUseFastPath) {
+            Structure* arrayStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
+            JSArray* entries = JSArray::tryCreate(vm, arrayStructure, properties.size());
+            if (UNLIKELY(!entries)) {
+                throwOutOfMemoryError(globalObject, scope);
+                return { };
             }
-        }
 
-        if (!cachedButterfly && properties.size() < MIN_SPARSE_ARRAY_INDEX && !globalObject->isHavingABadTime() && targetStructure->canCacheOwnPropertyNames()) {
-            auto* canSentinel = targetStructure->cachedPropertyNamesIgnoringSentinel(CachedPropertyNamesKind::Keys);
-            if (canSentinel == StructureRareData::cachedPropertyNamesSentinel()) {
-                size_t numProperties = properties.size();
-                auto* newButterfly = JSImmutableButterfly::create(vm, CopyOnWriteArrayWithContiguous, numProperties);
-                for (size_t i = 0; i < numProperties; i++) {
-                    const auto& identifier = properties[i];
-                    newButterfly->setIndex(vm, i, jsOwnedString(vm, identifier.get()));
+            Structure* targetStructure = target->structure();
+            JSImmutableButterfly* cachedButterfly = nullptr;
+            if (LIKELY(!globalObject->isHavingABadTime())) {
+                auto* butterfly = targetStructure->cachedPropertyNames(CachedPropertyNamesKind::Keys);
+                if (butterfly) {
+                    ASSERT(butterfly->length() == properties.size());
+                    if (butterfly->length() == properties.size())
+                        cachedButterfly = butterfly;
                 }
+            }
 
-                targetStructure->setCachedPropertyNames(vm, CachedPropertyNamesKind::Keys, newButterfly);
-                cachedButterfly = newButterfly;
-            } else
-                targetStructure->setCachedPropertyNames(vm, CachedPropertyNamesKind::Keys, StructureRareData::cachedPropertyNamesSentinel());
-        }
+            if (!cachedButterfly && properties.size() < MIN_SPARSE_ARRAY_INDEX && !globalObject->isHavingABadTime() && targetStructure->canCacheOwnPropertyNames()) {
+                auto* canSentinel = targetStructure->cachedPropertyNamesIgnoringSentinel(CachedPropertyNamesKind::Keys);
+                if (canSentinel == StructureRareData::cachedPropertyNamesSentinel()) {
+                    size_t numProperties = properties.size();
+                    auto* newButterfly = JSImmutableButterfly::create(vm, CopyOnWriteArrayWithContiguous, numProperties);
+                    for (size_t i = 0; i < numProperties; i++) {
+                        const auto& identifier = properties[i];
+                        newButterfly->setIndex(vm, i, jsOwnedString(vm, identifier.get()));
+                    }
 
-        for (size_t i = 0; i < properties.size(); ++i) {
-            JSString* key = nullptr;
-            if (cachedButterfly) {
-                auto* cachedKey = asString(cachedButterfly->get(i));
-                if (cachedKey->tryGetValueImpl() == properties[i].get())
-                    key = cachedKey;
+                    targetStructure->setCachedPropertyNames(vm, CachedPropertyNamesKind::Keys, newButterfly);
+                    cachedButterfly = newButterfly;
+                } else
+                    targetStructure->setCachedPropertyNames(vm, CachedPropertyNamesKind::Keys, StructureRareData::cachedPropertyNamesSentinel());
             }
 
-            if (!key)
-                key = jsOwnedString(vm, properties[i].get());
+            for (size_t i = 0; i < properties.size(); ++i) {
+                JSString* key = nullptr;
+                if (cachedButterfly) {
+                    auto* cachedKey = asString(cachedButterfly->get(i));
+                    if (cachedKey->tryGetValueImpl() == properties[i].get())
+                        key = cachedKey;
+                }
 
-            JSArray* entry = nullptr;
-            {
-                ObjectInitializationScope initializationScope(vm);
-                if (LIKELY(entry = JSArray::tryCreateUninitializedRestricted(initializationScope, nullptr, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous), 2))) {
-                    entry->initializeIndex(initializationScope, 0, key);
-                    entry->initializeIndex(initializationScope, 1, values.at(i));
+                if (!key)
+                    key = jsOwnedString(vm, properties[i].get());
+
+                JSArray* entry = nullptr;
+                {
+                    ObjectInitializationScope initializationScope(vm);
+                    if (LIKELY(entry = JSArray::tryCreateUninitializedRestricted(initializationScope, nullptr, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous), 2))) {
+                        entry->initializeIndex(initializationScope, 0, key);
+                        entry->initializeIndex(initializationScope, 1, values.at(i));
+                    }
                 }
+                if (UNLIKELY(!entry)) {
+                    throwOutOfMemoryError(globalObject, scope);
+                    return { };
+                }
+                entries->putDirectIndex(globalObject, i, entry);
+                RETURN_IF_EXCEPTION(scope, { });
             }
-            if (UNLIKELY(!entry)) {
-                throwOutOfMemoryError(globalObject, scope);
-                return { };
-            }
-            entries->putDirectIndex(globalObject, i, entry);
-            RETURN_IF_EXCEPTION(scope, { });
-        }
 
-        return JSValue::encode(entries);
+            return JSValue::encode(entries);
+        }
     }
 
     JSArray* entries = constructEmptyArray(globalObject, nullptr);
@@ -509,6 +509,40 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorValues, (JSGlobalObject* globalObject,
     JSObject* target = targetValue.toObject(globalObject);
     RETURN_IF_EXCEPTION(scope, { });
 
+    if (!target->staticPropertiesReified()) {
+        target->reifyAllStaticProperties(globalObject);
+        RETURN_IF_EXCEPTION(scope, { });
+    }
+
+    {
+        MarkedArgumentBuffer values;
+        bool canUseFastPath = target->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
+            if (entry.attributes() & PropertyAttribute::DontEnum)
+                return true;
+
+            if (entry.key()->isSymbol())
+                return true;
+
+            values.appendWithCrashOnOverflow(target->getDirect(entry.offset()));
+            return true;
+        });
+
+        if (canUseFastPath) {
+            Structure* arrayStructure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
+            {
+                ObjectInitializationScope initializationScope(vm);
+                JSArray* result = nullptr;
+                if (LIKELY(result = JSArray::tryCreateUninitializedRestricted(initializationScope, nullptr, arrayStructure, values.size()))) {
+                    for (unsigned i = 0; i < values.size(); ++i)
+                        result->initializeIndex(initializationScope, i, values.at(i));
+                    return JSValue::encode(result);
+                }
+            }
+            throwOutOfMemoryError(globalObject, scope);
+            return { };
+        }
+    }
+
     JSArray* values = constructEmptyArray(globalObject, nullptr);
     RETURN_IF_EXCEPTION(scope, { });
 

Source/JavaScriptCore/runtime/ObjectConstructorInlines.h

@@ -29,41 +29,7 @@
 
 namespace JSC {
 
-ALWAYS_INLINE bool canPerformFastPropertyEnumerationForObjectAssign(Structure* structure)
-{
-    if (structure->typeInfo().overridesGetOwnPropertySlot())
-        return false;
-    if (structure->typeInfo().overridesAnyFormOfGetOwnPropertyNames())
-        return false;
-    // FIXME: Indexed properties can be handled.
-    // https://bugs.webkit.org/show_bug.cgi?id=185358
-    if (hasIndexedProperties(structure->indexingType()))
-        return false;
-    if (structure->hasGetterSetterProperties())
-        return false;
-    if (structure->hasReadOnlyOrGetterSetterPropertiesExcludingProto())
-        return false;
-    if (structure->hasCustomGetterSetterProperties())
-        return false;
-    if (structure->isUncacheableDictionary())
-        return false;
-    // Cannot perform fast [[Put]] to |target| if the property names of the |source| contain "__proto__".
-    if (structure->hasUnderscoreProtoPropertyExcludingOriginalProto())
-        return false;
-    return true;
-}
-
-ALWAYS_INLINE bool canPerformFastPropertyEnumerationForJSONStringify(Structure* structure)
-{
-    return canPerformFastPropertyEnumerationForObjectAssign(structure);
-}
-
-ALWAYS_INLINE bool canPerformFastPropertyEnumerationForObjectEntries(Structure* structure)
-{
-    return canPerformFastPropertyEnumerationForObjectAssign(structure);
-}
-
-ALWAYS_INLINE void objectAssignFast(VM& vm, JSObject* target, JSObject* source, Vector<RefPtr<UniquedStringImpl>, 8>& properties, MarkedArgumentBuffer& values)
+ALWAYS_INLINE bool objectAssignFast(VM& vm, JSObject* target, JSObject* source, Vector<RefPtr<UniquedStringImpl>, 8>& properties, MarkedArgumentBuffer& values)
 {
     // |source| Structure does not have any getters. And target can perform fast put.
     // So enumerating properties and putting properties are non observable.
@@ -81,7 +47,7 @@ ALWAYS_INLINE void objectAssignFast(VM& vm, JSObject* target, JSObject* source,
     // Do not clear since Vector::clear shrinks the backing store.
     properties.resize(0);
     values.clear();
-    source->structure()->forEachProperty(vm, [&] (const PropertyTableEntry& entry) -> bool {
+    bool canUseFastPath = source->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
         if (entry.attributes() & PropertyAttribute::DontEnum)
             return true;
 
@@ -94,13 +60,16 @@ ALWAYS_INLINE void objectAssignFast(VM& vm, JSObject* target, JSObject* source,
 
         return true;
     });
+    if (!canUseFastPath)
+        return false;
 
     for (size_t i = 0; i < properties.size(); ++i) {
         // FIXME: We could put properties in a batching manner to accelerate Object.assign more.
         // https://bugs.webkit.org/show_bug.cgi?id=185358
         PutPropertySlot putPropertySlot(target, true);
         target->putOwnDataProperty(vm, properties[i].get(), values.at(i), putPropertySlot);
     }
+    return true;
 }
 
 

Source/JavaScriptCore/runtime/Structure.h

@@ -572,6 +572,8 @@ class Structure : public JSCell {
     PropertyOffset get(VM&, PropertyName);
     PropertyOffset get(VM&, PropertyName, unsigned& attributes);
 
+    bool canPerformFastPropertyEnumeration() const;
+
     // This is a somewhat internalish method. It will call your functor while possibly holding the
     // Structure's lock. There is no guarantee whether the lock is held or not in any particular
     // call. So, you have to assume the worst. Also, the functor returns true if it wishes for you