[JSC] Optimize Object.assign with empty object

https://bugs.webkit.org/show_bug.cgi?id=256019 rdar://108585195 Reviewed by Alexey Shvayka. Actually, this is pretty common that Object.assign's source objects are empty. The reason is the pattern like this, function func(options = {}) { var options = Object.assign({ defaultOption1: true, defaultOption2: false }, options); ... } And if we call this func with no-options, then Object.assign will see empty options object. This patch adds a fast path for that in C++ and DFG / FTL JIT. 1. In DFG and FTL, we quickly check empty objects and skip the execution of Object.assign if source is empty. 2. In C++, we check property count before entering into putOwnDataPropertyBatching. 3. We also relax putOwnDataPropertyBatching. Transition possibility should be checked only when we cause the transition. Otherwise, we do not need to give up the optimization, like, just replacing properties. ToT Patched object-assign-transition 84.0831+-0.2116 ^ 70.7839+-0.2697 ^ definitely 1.1879x faster object-assign-replace 77.5201+-0.2244 ^ 72.5579+-0.1398 ^ definitely 1.0684x faster object-assign-empty 13.7165+-0.0783 ^ 7.0465+-0.0528 ^ definitely 1.9466x faster object-assign-multiple 132.1961+-0.1656 ^ 98.9260+-0.5316 ^ definitely 1.3363x faster * JSTests/microbenchmarks/object-assign-empty.js: Added. (test): * Source/JavaScriptCore/dfg/DFGOperations.cpp: (JSC::DFG::JSC_DEFINE_JIT_OPERATION): * Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp: * Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h: * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileObjectAssign): * Source/JavaScriptCore/runtime/JSObject.cpp: (JSC::JSObject::putOwnDataPropertyBatching): * Source/JavaScriptCore/runtime/ObjectConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/ObjectConstructorInlines.h: (JSC::objectAssignFast): * Source/JavaScriptCore/runtime/Structure.h: (JSC::Structure::propertyHashOffset): * Source/JavaScriptCore/runtime/StructureInlines.h: (JSC::Structure::canPerformFastPropertyEnumeration const): * Source/JavaScriptCore/runtime/StructureRareData.h: Canonical link: https://commits.webkit.org/263444@main
WebKit · Apr 27, 2023 · 8f50164 · 8f50164
1 parent 18d9fef
commit 8f50164
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 38 deletions.
diff --git a/JSTests/microbenchmarks/object-assign-empty.js b/JSTests/microbenchmarks/object-assign-empty.js
@@ -0,0 +1,10 @@
+function test(options)
+{
+    var options = Object.assign({ defaultParam: 32 }, options);
+    return options;
+}
+noInline(test);
+
+for (var i = 0; i < 1e6; ++i) {
+    test({});
+}
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -367,35 +367,9 @@ JSC_DEFINE_JIT_OPERATION(operationObjectAssignObject, void, (JSGlobalObject* glo
         // 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
-                PutPropertySlot putPropertySlot(target, true);
-                target->putOwnDataProperty(vm, properties[i].get(), values.at(i), putPropertySlot);
-            }
+        if (objectAssignFast(vm, target, source, properties, values))
             return;
-        }
     }
 
     scope.release();

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -15256,16 +15256,29 @@ void SpeculativeJIT::compileObjectAssign(Node* node)
     switch (node->child2().useKind()) {
     case ObjectUse: {
         SpeculateCellOperand source(this, node->child2());
+        GPRTemporary scratch1(this);
 
         GPRReg targetGPR = target.gpr();
         GPRReg sourceGPR = source.gpr();
+        GPRReg scratch1GPR = scratch1.gpr();
 
         speculateObject(node->child2(), sourceGPR);
 
         flushRegisters();
+
+        JumpList doneCases;
+        JumpList genericCases;
+
+        genericCases.append(branchIfNotType(sourceGPR, FinalObjectType));
+        genericCases.append(branchTest8(NonZero, Address(sourceGPR, JSObject::indexingTypeAndMiscOffset()), CCallHelpers::TrustedImm32(IndexingShapeMask)));
+        emitLoadStructure(vm(), sourceGPR, scratch1GPR);
+        doneCases.append(branchTest32(Zero, Address(scratch1GPR, Structure::propertyHashOffset())));
+
+        genericCases.link(this);
         callOperation(operationObjectAssignObject, LinkableConstant::globalObject(*this, node), targetGPR, sourceGPR);
         exceptionCheck();
 
+        doneCases.link(this);
         noResult(node);
         return;
     }

diff --git a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
@@ -158,6 +158,7 @@ namespace JSC { namespace FTL {
     macro(Structure_inlineCapacity, Structure::inlineCapacityOffset()) \
     macro(Structure_outOfLineTypeFlags, Structure::outOfLineTypeFlagsOffset()) \
     macro(Structure_previousOrRareData, Structure::previousOrRareDataOffset()) \
+    macro(Structure_propertyHash, Structure::propertyHashOffset()) \
     macro(Structure_prototype, Structure::prototypeOffset()) \
     macro(StructureRareData_cachedEnumerableStrings, StructureRareData::offsetOfCachedPropertyNames(CachedPropertyNamesKind::EnumerableStrings)) \
     macro(StructureRareData_cachedStrings, StructureRareData::offsetOfCachedPropertyNames(CachedPropertyNamesKind::Strings)) \

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -7821,9 +7821,32 @@ IGNORE_CLANG_WARNINGS_END
     {
         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
         switch (m_node->child2().useKind()) {
-        case ObjectUse:
-            vmCall(Void, operationObjectAssignObject, weakPointer(globalObject), lowCell(m_node->child1()), lowObject(m_node->child2()));
+        case ObjectUse: {
+            LBasicBlock checkStructure1 = m_out.newBlock();
+            LBasicBlock checkStructure2 = m_out.newBlock();
+            LBasicBlock genericCase = m_out.newBlock();
+            LBasicBlock continuation = m_out.newBlock();
+
+            LValue target = lowCell(m_node->child1());
+            LValue source = lowObject(m_node->child2());
+
+            m_out.branch(m_out.equal(m_out.load8ZeroExt32(source, m_heaps.JSCell_typeInfoType), m_out.constInt32(FinalObjectType)), unsure(checkStructure1), unsure(genericCase));
+
+            LBasicBlock lastNext = m_out.appendTo(checkStructure1, checkStructure2);
+            LValue indexingMode = m_out.load8ZeroExt32(source, m_heaps.JSCell_indexingTypeAndMisc);
+            m_out.branch(m_out.testIsZero32(indexingMode, m_out.constInt32(IndexingShapeMask)), unsure(checkStructure2), unsure(genericCase));
+
+            m_out.appendTo(checkStructure2, genericCase);
+            LValue structure = loadStructure(source);
+            m_out.branch(m_out.isZero32(m_out.load32(structure, m_heaps.Structure_propertyHash)), unsure(continuation), unsure(genericCase));
+
+            m_out.appendTo(genericCase, continuation);
+            vmCall(Void, operationObjectAssignObject, weakPointer(globalObject), target, source);
+            m_out.jump(continuation);
+
+            m_out.appendTo(continuation, lastNext);
             return;
+        }
         case UntypedUse:
             vmCall(Void, operationObjectAssignUntyped, weakPointer(globalObject), lowCell(m_node->child1()), lowJSValue(m_node->child2()));
             return;

diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -4095,7 +4095,7 @@ void JSObject::putOwnDataPropertyBatching(VM& vm, const RefPtr<UniquedStringImpl
 {
     unsigned i = 0;
     Structure* structure = this->structure();
-    if (!(structure->isDictionary() || (structure->transitionCountEstimate() + size) > Structure::s_maxTransitionLength || !structure->canPerformFastPropertyEnumeration() || (structure->transitionWatchpointSet().isBeingWatched() && structure->transitionWatchpointSet().isStillValid()))) {
+    if (!(structure->isDictionary() || (structure->transitionCountEstimate() + size) > Structure::s_maxTransitionLength || !structure->canPerformFastPropertyEnumeration())) {
         Vector<PropertyOffset, 16> offsets;
         offsets.reserveInitialCapacity(size);
 
@@ -4106,10 +4106,6 @@ void JSObject::putOwnDataPropertyBatching(VM& vm, const RefPtr<UniquedStringImpl
             if (Structure* newStructure = Structure::addPropertyTransitionToExistingStructure(structure, propertyName, 0, offset)) {
                 structure = newStructure;
                 offsets.uncheckedAppend(offset);
-                // If we detect that this structure requires transition watchpoint firing, then we need to stop this batching and rest of the values
-                // should be put via generic way.
-                if (UNLIKELY(structure->transitionWatchpointSet().isBeingWatched() && structure->transitionWatchpointSet().isStillValid()))
-                    break;
                 continue;
             }
 
@@ -4118,11 +4114,14 @@ void JSObject::putOwnDataPropertyBatching(VM& vm, const RefPtr<UniquedStringImpl
             if (offset != invalidOffset) {
                 structure->didReplaceProperty(offset);
                 offsets.uncheckedAppend(offset);
-                if (UNLIKELY(structure->transitionWatchpointSet().isBeingWatched() && structure->transitionWatchpointSet().isStillValid()))
-                    break;
                 continue;
             }
 
+            // If we detect that this structure requires transition watchpoint firing, then we need to stop this batching and rest of the values
+            // should be put via generic way.
+            if (UNLIKELY(structure->transitionWatchpointSet().isBeingWatched() && structure->transitionWatchpointSet().isStillValid()))
+                break;
+
             // It will go to the cacheable dictionary case. We stop the batching here and fall though to the generic case.
             // We break here before adding offset to offsets since this property itself should be put via generic path.
             if (UNLIKELY(structure->shouldDoCacheableDictionaryTransitionForAdd(PutPropertySlot::UnknownContext)))

diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
@@ -361,7 +361,10 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorAssign, (JSGlobalObject* globalObject,
                     return true;
                 });
             }
-            target->putOwnDataPropertyBatching(vm, properties.data(), values.data(), properties.size());
+
+            // Actually, assigning with empty object (option for example) is common. (`Object.assign(defaultOptions, passedOptions)` where `passedOptions` is empty object.)
+            if (properties.size())
+                target->putOwnDataPropertyBatching(vm, properties.data(), values.data(), properties.size());
             return JSValue::encode(target);
         }
     }

diff --git a/Source/JavaScriptCore/runtime/ObjectConstructorInlines.h b/Source/JavaScriptCore/runtime/ObjectConstructorInlines.h
@@ -63,7 +63,9 @@ ALWAYS_INLINE bool objectAssignFast(VM& vm, JSObject* target, JSObject* source,
     if (!canUseFastPath)
         return false;
 
-    target->putOwnDataPropertyBatching(vm, properties.data(), values.data(), properties.size());
+    // Actually, assigning with empty object (option for example) is common. (`Object.assign(defaultOptions, passedOptions)` where `passedOptions` is empty object.)
+    if (properties.size())
+        target->putOwnDataPropertyBatching(vm, properties.data(), values.data(), properties.size());
     return true;
 }
 

diff --git a/Source/JavaScriptCore/runtime/Structure.h b/Source/JavaScriptCore/runtime/Structure.h
@@ -714,6 +714,11 @@ class Structure : public JSCell {
         return OBJECT_OFFSETOF(Structure, m_bitField);
     }
 
+    static ptrdiff_t propertyHashOffset()
+    {
+        return OBJECT_OFFSETOF(Structure, m_propertyHash);
+    }
+
     static Structure* createStructure(VM&);
 
     bool transitionWatchpointSetHasBeenInvalidated() const

diff --git a/Source/JavaScriptCore/runtime/StructureInlines.h b/Source/JavaScriptCore/runtime/StructureInlines.h
@@ -784,6 +784,7 @@ ALWAYS_INLINE bool Structure::canPerformFastPropertyEnumeration() const
         return false;
     // FIXME: Indexed properties can be handled.
     // https://bugs.webkit.org/show_bug.cgi?id=185358
+
     if (hasIndexedProperties(indexingType()))
         return false;
     if (hasAnyKindOfGetterSetterProperties())

diff --git a/Source/JavaScriptCore/runtime/StructureRareData.h b/Source/JavaScriptCore/runtime/StructureRareData.h
@@ -123,6 +123,11 @@ class StructureRareData final : public JSCell {
         return OBJECT_OFFSETOF(StructureRareData, m_specialPropertyCache);
     }
 
+    static ptrdiff_t offsetOfPrevious()
+    {
+        return OBJECT_OFFSETOF(StructureRareData, m_previous);
+    }
+
     DECLARE_EXPORT_INFO;
 
     void finalizeUnconditionally(VM&, CollectionScope);