Skip to content

Commit

Permalink
[JSC] Use cloning for Object.assign with empty object
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=267239
rdar://120667395

Reviewed by Justin Michaud.

This patch implements fast cloning path for `Object.assign({}, source)` case.
When conditions are met, we clone Butterfly in source, set it to the empty target object,
and use the source's Structure for target. This completely avoids iterating all object properties.

* JSTests/stress/object-assign-clone.js: Added.
(shouldBe):
(shouldBeArray):
(shouldBe.Reflect.getOwnPropertyDescriptor):
(shouldBe.set get let):
* Source/JavaScriptCore/dfg/DFGOperations.cpp:
(JSC::DFG::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/runtime/JSObject.h:
* Source/JavaScriptCore/runtime/JSObjectInlines.h:
(JSC::JSObject::fastForEachPropertyWithSideEffectFreeFunctor): Deleted.
* Source/JavaScriptCore/runtime/ObjectConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
(JSC::toPropertyDescriptor):
(JSC::defineProperties):
* Source/JavaScriptCore/runtime/ObjectConstructorInlines.h:
(JSC::objectCloneFast):
(JSC::objectAssignFast):
* Source/JavaScriptCore/runtime/Structure.cpp:
(JSC::Structure::Structure):
(JSC::Structure::nonPropertyTransitionSlow):
* Source/JavaScriptCore/runtime/Structure.h:
(JSC::Structure::seenProperties const):
(JSC::Structure::bitFieldFlagsCantBeChangedWithoutTransition):
(JSC::Structure::transitionPropertyAttributes const):
(JSC::Structure::setTransitionPropertyAttributes):
* Source/JavaScriptCore/runtime/StructureInlines.h:
(JSC::Structure::add):
(JSC::Structure::attributeChange):
(JSC::Structure::addOrReplacePropertyWithoutTransition):

Canonical link: https://commits.webkit.org/272794@main
  • Loading branch information
@Constellation
Constellation committed Jan 9, 2024
1 parent d34db4f commit 11aee46
Show file tree
Hide file tree
Showing 9 changed files with 306 additions and 83 deletions.
61 changes: 61 additions & 0 deletions JSTests/stress/object-assign-clone.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function shouldBeArray(actual, expected) {
shouldBe(actual.length, expected.length);
for (var i = 0; i < expected.length; ++i) {
try {
shouldBe(actual[i], expected[i]);
} catch(e) {
print(JSON.stringify(actual));
throw e;
}
}
}

{
let source = {};
Reflect.defineProperty(source, "hello", {
enumerable: false,
configurable: true,
writable: true,
value: 42,
});
let result = Object.assign({}, source);
shouldBe(Reflect.getOwnPropertyDescriptor(result, "hello"), undefined);
}
{
let source = {};
Reflect.defineProperty(source, "hello", {
enumerable: true,
configurable: false,
writable: true,
value: 42,
});
let result = Object.assign({}, source);
shouldBe(JSON.stringify(Reflect.getOwnPropertyDescriptor(result, "hello")), `{"value":42,"writable":true,"enumerable":true,"configurable":true}`);
}
{
let source = {};
Reflect.defineProperty(source, "hello", {
enumerable: true,
configurable: true,
writable: false,
value: 42,
});
let result = Object.assign({}, source);
shouldBe(JSON.stringify(Reflect.getOwnPropertyDescriptor(result, "hello")), `{"value":42,"writable":true,"enumerable":true,"configurable":true}`);
}
{
let source = {};
Reflect.defineProperty(source, "hello", {
enumerable: true,
configurable: true,
get() { return 42; },
set() { }
});
let result = Object.assign({}, source);
shouldBe(JSON.stringify(Reflect.getOwnPropertyDescriptor(result, "hello")), `{"value":42,"writable":true,"enumerable":true,"configurable":true}`);
}
11 changes: 4 additions & 7 deletions Source/JavaScriptCore/dfg/DFGOperations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,8 @@ JSC_DEFINE_JIT_OPERATION(operationObjectAssignObject, void, (JSGlobalObject* glo
CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
auto scope = DECLARE_THROW_SCOPE(vm);
bool targetCanPerformFastPut = jsDynamicCast<JSFinalObject*>(target) && target->canPerformFastPutInlineExcludingProto() && target->isStructureExtensible();

if (targetCanPerformFastPut) {
if (auto* targetObject = jsDynamicCast<JSFinalObject*>(target); targetObject && targetObject->canPerformFastPutInlineExcludingProto() && targetObject->isStructureExtensible()) {
Vector<RefPtr<UniquedStringImpl>, 8> properties;
MarkedArgumentBuffer values;
if (!source->staticPropertiesReified()) {
Expand All @@ -311,7 +310,7 @@ JSC_DEFINE_JIT_OPERATION(operationObjectAssignObject, void, (JSGlobalObject* glo
// https://bugs.webkit.org/show_bug.cgi?id=187837

// Do not clear since Vector::clear shrinks the backing store.
bool objectAssignFastSucceeded = objectAssignFast(globalObject, target, source, properties, values);
bool objectAssignFastSucceeded = objectAssignFast(globalObject, targetObject, source, properties, values);
RETURN_IF_EXCEPTION(scope, void());
if (objectAssignFastSucceeded)
return;
Expand All @@ -328,23 +327,21 @@ JSC_DEFINE_JIT_OPERATION(operationObjectAssignUntyped, void, (JSGlobalObject* gl
JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
auto scope = DECLARE_THROW_SCOPE(vm);

bool targetCanPerformFastPut = jsDynamicCast<JSFinalObject*>(target) && target->canPerformFastPutInlineExcludingProto() && target->isStructureExtensible();

JSValue sourceValue = JSValue::decode(encodedSource);
if (sourceValue.isUndefinedOrNull())
return;
JSObject* source = sourceValue.toObject(globalObject);
RETURN_IF_EXCEPTION(scope, void());

if (targetCanPerformFastPut) {
if (auto* targetObject = jsDynamicCast<JSFinalObject*>(target); targetObject && targetObject->canPerformFastPutInlineExcludingProto() && targetObject->isStructureExtensible()) {
if (!source->staticPropertiesReified()) {
source->reifyAllStaticProperties(globalObject);
RETURN_IF_EXCEPTION(scope, void());
}

Vector<RefPtr<UniquedStringImpl>, 8> properties;
MarkedArgumentBuffer values;
bool objectAssignFastSucceeded = objectAssignFast(globalObject, target, source, properties, values);
bool objectAssignFastSucceeded = objectAssignFast(globalObject, targetObject, source, properties, values);
RETURN_IF_EXCEPTION(scope, void());
if (objectAssignFastSucceeded)
return;
Expand Down
3 changes: 0 additions & 3 deletions Source/JavaScriptCore/runtime/JSObject.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1054,9 +1054,6 @@ class JSObject : public JSCell {

DECLARE_EXPORT_INFO;

template<typename Functor>
bool fastForEachPropertyWithSideEffectFreeFunctor(VM&, const Functor&);

bool getOwnNonIndexPropertySlot(VM&, Structure*, PropertyName, PropertySlot&);
bool getNonIndexPropertySlot(JSGlobalObject*, PropertyName, PropertySlot&);

Expand Down
15 changes: 0 additions & 15 deletions Source/JavaScriptCore/runtime/JSObjectInlines.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,21 +864,6 @@ inline void JSObject::setPrivateBrand(JSGlobalObject* globalObject, JSValue bran
this->setStructure(vm, newStructure);
}

template<typename Functor>
bool JSObject::fastForEachPropertyWithSideEffectFreeFunctor(VM& vm, const Functor& functor)
{
if (hasNonReifiedStaticProperties())
return false;

Structure* structure = this->structure();

if (!structure->canPerformFastPropertyEnumerationCommon())
return false;

structure->forEachProperty(vm, functor);
return true;
}

// Function forEachOwnIndexedProperty should only used in the fast path
// for copying own non-GetterSetter indexed properties.
template<JSObject::SortMode mode, typename Functor>
Expand Down
117 changes: 68 additions & 49 deletions Source/JavaScriptCore/runtime/ObjectConstructor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorAssign, (JSGlobalObject* globalObject,

// FIXME: Extend this for non JSFinalObject. For example, we would like to use this fast path for function objects too.
// https://bugs.webkit.org/show_bug.cgi?id=185358
bool targetCanPerformFastPut = jsDynamicCast<JSFinalObject*>(target) && target->canPerformFastPutInlineExcludingProto() && target->isStructureExtensible();
JSFinalObject* targetObject = jsDynamicCast<JSFinalObject*>(target);
bool targetCanPerformFastPut = targetObject && targetObject->canPerformFastPutInlineExcludingProto() && targetObject->isStructureExtensible();
unsigned argsCount = callFrame->argumentCount();

// argsCount == 2 case does not need to use arguments' batching.
Expand Down Expand Up @@ -387,7 +388,7 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorAssign, (JSGlobalObject* globalObject,
RETURN_IF_EXCEPTION(scope, { });
}

bool objectAssignFastSucceeded = objectAssignFast(globalObject, target, source, properties, values);
bool objectAssignFastSucceeded = objectAssignFast(globalObject, targetObject, source, properties, values);
RETURN_IF_EXCEPTION(scope, { });
if (objectAssignFastSucceeded)
continue;
Expand Down Expand Up @@ -420,19 +421,23 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorEntries, (JSGlobalObject* globalObject
Vector<RefPtr<UniquedStringImpl>, 8> properties;
MarkedArgumentBuffer values;
bool canUseFastPath = false;
if (!target->canHaveExistingOwnIndexedProperties()) {
canUseFastPath = target->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;
if (!target->canHaveExistingOwnIndexedProperties() && !target->hasNonReifiedStaticProperties()) {
Structure* targetStructure = target->structure();
if (targetStructure->canPerformFastPropertyEnumerationCommon()) {
canUseFastPath = true;
targetStructure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;

if (entry.key()->isSymbol())
return true;
if (entry.key()->isSymbol())
return true;

properties.append(entry.key());
values.appendWithCrashOnOverflow(target->getDirect(entry.offset()));
properties.append(entry.key());
values.appendWithCrashOnOverflow(target->getDirect(entry.offset()));

return true;
});
return true;
});
}
}

if (canUseFastPath) {
Expand Down Expand Up @@ -568,17 +573,21 @@ JSC_DEFINE_HOST_FUNCTION(objectConstructorValues, (JSGlobalObject* globalObject,
{
MarkedArgumentBuffer namedPropertyValues;
bool canUseFastPath = false;
if (!target->canHaveExistingOwnIndexedGetterSetterProperties()) {
canUseFastPath = target->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;
if (!target->canHaveExistingOwnIndexedGetterSetterProperties() && !target->hasNonReifiedStaticProperties()) {
Structure* targetStructure = target->structure();
if (targetStructure->canPerformFastPropertyEnumerationCommon()) {
canUseFastPath = true;
targetStructure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;

if (entry.key()->isSymbol())
return true;
if (entry.key()->isSymbol())
return true;

namedPropertyValues.appendWithCrashOnOverflow(target->getDirect(entry.offset()));
return true;
});
namedPropertyValues.appendWithCrashOnOverflow(target->getDirect(entry.offset()));
return true;
});
}
}

if (canUseFastPath) {
Expand Down Expand Up @@ -668,24 +677,27 @@ inline bool toPropertyDescriptor(JSGlobalObject* globalObject, JSValue in, Prope


bool canUseFastPath = false;
if (globalObject->propertyDescriptorFastPathWatchpointSet().isStillValid() && globalObject->objectPrototypeChainIsSane() && description->inherits<JSFinalObject>() && description->getPrototypeDirect() == globalObject->objectPrototype() && description->structure()->canPerformFastPropertyEnumeration()) {
canUseFastPath = description->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
PropertyName propertyName(entry.key());
if (propertyName == vm.propertyNames->enumerable)
enumerable = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->configurable)
configurable = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->value)
value = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->writable)
writable = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->get)
get = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->set)
set = description->getDirect(entry.offset());
return true;
});
if (canUseFastPath) {
if (globalObject->propertyDescriptorFastPathWatchpointSet().isStillValid() && globalObject->objectPrototypeChainIsSane() && description->inherits<JSFinalObject>() && description->getPrototypeDirect() == globalObject->objectPrototype() && !description->hasNonReifiedStaticProperties()) {
Structure* descriptionStructure = description->structure();
if (descriptionStructure->canPerformFastPropertyEnumeration()) {
canUseFastPath = true;
descriptionStructure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
PropertyName propertyName(entry.key());
if (propertyName == vm.propertyNames->enumerable)
enumerable = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->configurable)
configurable = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->value)
value = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->writable)
writable = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->get)
get = description->getDirect(entry.offset());
else if (propertyName == vm.propertyNames->set)
set = description->getDirect(entry.offset());
return true;
});

if (enumerable)
desc.setEnumerable(enumerable.toBoolean(globalObject));
if (configurable)
Expand Down Expand Up @@ -850,19 +862,26 @@ static JSValue defineProperties(JSGlobalObject* globalObject, JSObject* object,

Vector<RefPtr<UniquedStringImpl>, 8> propertyNames;
MarkedArgumentBuffer values;
bool canUseFastPath = !hasIndexedProperties(properties->indexingType()) && properties->fastForEachPropertyWithSideEffectFreeFunctor(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;
bool canUseFastPath = false;
if (!hasIndexedProperties(properties->indexingType())) {
Structure* propertiesStructure = properties->structure();
if (!properties->hasNonReifiedStaticProperties() && propertiesStructure->canPerformFastPropertyEnumerationCommon()) {
canUseFastPath = true;
propertiesStructure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;

PropertyName propertyName(entry.key());
if (propertyName.isPrivateName())
return true;
PropertyName propertyName(entry.key());
if (propertyName.isPrivateName())
return true;

propertyNames.append(entry.key());
values.appendWithCrashOnOverflow(properties->getDirect(entry.offset()));
propertyNames.append(entry.key());
values.appendWithCrashOnOverflow(properties->getDirect(entry.offset()));

return true;
});
return true;
});
}
}
if (UNLIKELY(!canUseFastPath))
RELEASE_AND_RETURN(scope, definePropertiesSlow(globalObject, object, properties));

Expand Down
Loading

0 comments on commit 11aee46

Please sign in to comment.