Commit 6b89c694 authored by verwaest's avatar verwaest Committed by Commit bot

[builtins] Add an initial fast-path to Object.assign.

In the case of a simple fast-mode receiver without fancy properties, we
can just walk over the descriptor array to find all its initial property
names. As long as the map stays the same, we can also use that
descriptor array to figure out how to handle the properties.

This speeds up
https://github.com/kpdecker/six-speed/tree/master/tests/object-assign by
~2x.

BUG=

Review URL: https://codereview.chromium.org/1688953004

Cr-Commit-Position: refs/heads/master@{#33895}
parent a2935d63
...@@ -1578,6 +1578,75 @@ BUILTIN(ArrayIsArray) { ...@@ -1578,6 +1578,75 @@ BUILTIN(ArrayIsArray) {
return *isolate->factory()->ToBoolean(result.FromJust()); return *isolate->factory()->ToBoolean(result.FromJust());
} }
namespace {
MUST_USE_RESULT Maybe<bool> FastAssign(Handle<JSReceiver> to,
Handle<Object> next_source) {
// Non-empty strings are the only non-JSReceivers that need to be handled
// explicitly by Object.assign.
if (!next_source->IsJSReceiver()) {
return Just(!next_source->IsString() ||
String::cast(*next_source)->length() == 0);
}
Isolate* isolate = to->GetIsolate();
Handle<Map> map(JSReceiver::cast(*next_source)->map(), isolate);
if (!map->IsJSObjectMap()) return Just(false);
if (!map->OnlyHasSimpleProperties()) return Just(false);
Handle<JSObject> from = Handle<JSObject>::cast(next_source);
if (from->elements() != isolate->heap()->empty_fixed_array()) {
return Just(false);
}
Handle<DescriptorArray> descriptors(map->instance_descriptors(), isolate);
int length = map->NumberOfOwnDescriptors();
for (int i = 0; i < length; i++) {
Handle<Name> next_key(descriptors->GetKey(i), isolate);
Handle<Object> prop_value;
// Directly decode from the descriptor array if |from| did not change shape.
if (from->map() == *map) {
PropertyDetails details = descriptors->GetDetails(i);
if (!details.IsEnumerable()) continue;
if (details.kind() == kData) {
if (details.location() == kDescriptor) {
prop_value = handle(descriptors->GetValue(i), isolate);
} else {
Representation representation = details.representation();
FieldIndex index = FieldIndex::ForDescriptor(*map, i);
prop_value = JSObject::FastPropertyAt(from, representation, index);
}
} else {
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, prop_value, Object::GetProperty(from, next_key, STRICT),
Nothing<bool>());
}
} else {
// If the map did change, do a slower lookup. We are still guaranteed that
// the object has a simple shape, and that the key is a name.
LookupIterator it(from, next_key, LookupIterator::OWN_SKIP_INTERCEPTOR);
if (!it.IsFound()) continue;
DCHECK(it.state() == LookupIterator::DATA ||
it.state() == LookupIterator::ACCESSOR);
if (!it.IsEnumerable()) continue;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, prop_value,
Object::GetProperty(&it, STRICT),
Nothing<bool>());
}
Handle<Object> status;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, status,
Object::SetProperty(to, next_key, prop_value, STRICT,
Object::CERTAINLY_NOT_STORE_FROM_KEYED),
Nothing<bool>());
}
return Just(true);
}
} // namespace
// ES6 19.1.2.1 Object.assign // ES6 19.1.2.1 Object.assign
BUILTIN(ObjectAssign) { BUILTIN(ObjectAssign) {
...@@ -1595,10 +1664,13 @@ BUILTIN(ObjectAssign) { ...@@ -1595,10 +1664,13 @@ BUILTIN(ObjectAssign) {
// 4. For each element nextSource of sources, in ascending index order, // 4. For each element nextSource of sources, in ascending index order,
for (int i = 2; i < args.length(); ++i) { for (int i = 2; i < args.length(); ++i) {
Handle<Object> next_source = args.at<Object>(i); Handle<Object> next_source = args.at<Object>(i);
Maybe<bool> fast_assign = FastAssign(to, next_source);
if (fast_assign.IsNothing()) return isolate->heap()->exception();
if (fast_assign.FromJust()) continue;
// 4a. If nextSource is undefined or null, let keys be an empty List. // 4a. If nextSource is undefined or null, let keys be an empty List.
if (next_source->IsUndefined() || next_source->IsNull()) continue;
// 4b. Else, // 4b. Else,
// 4b i. Let from be ToObject(nextSource). // 4b i. Let from be ToObject(nextSource).
// Only non-empty strings and JSReceivers have enumerable properties.
Handle<JSReceiver> from = Handle<JSReceiver> from =
Object::ToObject(isolate, next_source).ToHandleChecked(); Object::ToObject(isolate, next_source).ToHandleChecked();
// 4b ii. Let keys be ? from.[[OwnPropertyKeys]](). // 4b ii. Let keys be ? from.[[OwnPropertyKeys]]().
......
...@@ -238,6 +238,7 @@ class LookupIterator final BASE_EMBEDDED { ...@@ -238,6 +238,7 @@ class LookupIterator final BASE_EMBEDDED {
} }
bool IsConfigurable() const { return property_details().IsConfigurable(); } bool IsConfigurable() const { return property_details().IsConfigurable(); }
bool IsReadOnly() const { return property_details().IsReadOnly(); } bool IsReadOnly() const { return property_details().IsReadOnly(); }
bool IsEnumerable() const { return property_details().IsEnumerable(); }
Representation representation() const { Representation representation() const {
return property_details().representation(); return property_details().representation();
} }
......
...@@ -250,7 +250,7 @@ Handle<Object> CallSite::GetMethodName() { ...@@ -250,7 +250,7 @@ Handle<Object> CallSite::GetMethodName() {
if (!current->IsJSObject()) break; if (!current->IsJSObject()) break;
Handle<JSObject> current_obj = Handle<JSObject>::cast(current); Handle<JSObject> current_obj = Handle<JSObject>::cast(current);
if (current_obj->IsAccessCheckNeeded()) break; if (current_obj->IsAccessCheckNeeded()) break;
Handle<FixedArray> keys = JSObject::GetEnumPropertyKeys(current_obj, false); Handle<FixedArray> keys = JSObject::GetEnumPropertyKeys(current_obj);
for (int i = 0; i < keys->length(); i++) { for (int i = 0; i < keys->length(); i++) {
HandleScope inner_scope(isolate_); HandleScope inner_scope(isolate_);
if (!keys->get(i)->IsName()) continue; if (!keys->get(i)->IsName()) continue;
......
...@@ -8539,13 +8539,22 @@ static Handle<FixedArray> ReduceFixedArrayTo( ...@@ -8539,13 +8539,22 @@ static Handle<FixedArray> ReduceFixedArrayTo(
return new_array; return new_array;
} }
bool Map::OnlyHasSimpleProperties() {
// Wrapped string elements aren't explicitly stored in the elements backing
// store, but are loaded indirectly from the underlying string.
return !IsStringWrapperElementsKind(elements_kind()) &&
!is_access_check_needed() && !has_named_interceptor() &&
!has_indexed_interceptor() && !has_hidden_prototype() &&
!is_dictionary_map();
}
namespace { namespace {
Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate, Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
Handle<JSObject> object, Handle<JSObject> object) {
bool cache_enum_length) {
Handle<Map> map(object->map()); Handle<Map> map(object->map());
bool cache_enum_length = map->OnlyHasSimpleProperties();
Handle<DescriptorArray> descs = Handle<DescriptorArray> descs =
Handle<DescriptorArray>(map->instance_descriptors(), isolate); Handle<DescriptorArray>(map->instance_descriptors(), isolate);
int own_property_count = map->EnumLength(); int own_property_count = map->EnumLength();
...@@ -8618,12 +8627,10 @@ Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate, ...@@ -8618,12 +8627,10 @@ Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
} // namespace } // namespace
Handle<FixedArray> JSObject::GetEnumPropertyKeys(Handle<JSObject> object) {
Handle<FixedArray> JSObject::GetEnumPropertyKeys(Handle<JSObject> object,
bool cache_enum_length) {
Isolate* isolate = object->GetIsolate(); Isolate* isolate = object->GetIsolate();
if (object->HasFastProperties()) { if (object->HasFastProperties()) {
return GetFastEnumPropertyKeys(isolate, object, cache_enum_length); return GetFastEnumPropertyKeys(isolate, object);
} else if (object->IsJSGlobalObject()) { } else if (object->IsJSGlobalObject()) {
Handle<GlobalDictionary> dictionary(object->global_dictionary()); Handle<GlobalDictionary> dictionary(object->global_dictionary());
int length = dictionary->NumberOfEnumElements(); int length = dictionary->NumberOfEnumElements();
...@@ -8727,26 +8734,7 @@ static Maybe<bool> GetKeysFromJSObject(Isolate* isolate, ...@@ -8727,26 +8734,7 @@ static Maybe<bool> GetKeysFromJSObject(Isolate* isolate,
MAYBE_RETURN(success, Nothing<bool>()); MAYBE_RETURN(success, Nothing<bool>());
if (*filter == ENUMERABLE_STRINGS) { if (*filter == ENUMERABLE_STRINGS) {
// We can cache the computed property keys if access checks are Handle<FixedArray> enum_keys = JSObject::GetEnumPropertyKeys(object);
// not needed and no interceptors are involved.
//
// We do not use the cache if the object has elements and
// therefore it does not make sense to cache the property names
// for arguments objects. Arguments objects will always have
// elements.
// Wrapped strings have elements, but don't have an elements
// array or dictionary. So the fast inline test for whether to
// use the cache says yes, so we should not create a cache.
Handle<JSFunction> arguments_function(
JSFunction::cast(isolate->sloppy_arguments_map()->GetConstructor()));
bool cache_enum_length =
((object->map()->GetConstructor() != *arguments_function) &&
!object->IsJSValue() && !object->IsAccessCheckNeeded() &&
!object->HasNamedInterceptor() && !object->HasIndexedInterceptor() &&
!object->map()->has_hidden_prototype());
// Compute the property keys and cache them if possible.
Handle<FixedArray> enum_keys =
JSObject::GetEnumPropertyKeys(object, cache_enum_length);
accumulator->AddKeys(enum_keys, DO_NOT_CONVERT); accumulator->AddKeys(enum_keys, DO_NOT_CONVERT);
} else { } else {
object->CollectOwnPropertyNames(accumulator, *filter); object->CollectOwnPropertyNames(accumulator, *filter);
......
...@@ -2297,8 +2297,7 @@ class JSObject: public JSReceiver { ...@@ -2297,8 +2297,7 @@ class JSObject: public JSReceiver {
KeyAccumulator* keys, KeyAccumulator* keys,
PropertyFilter filter); PropertyFilter filter);
static Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object, static Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object);
bool cache_result);
// Returns a new map with all transitions dropped from the object's current // Returns a new map with all transitions dropped from the object's current
// map and the ElementsKind set. // map and the ElementsKind set.
...@@ -5847,6 +5846,10 @@ class Map: public HeapObject { ...@@ -5847,6 +5846,10 @@ class Map: public HeapObject {
inline Cell* RetrieveDescriptorsPointer(); inline Cell* RetrieveDescriptorsPointer();
// Checks whether all properties are stored either in the map or on the object
// (inobject, properties, or elements backing store), requiring no special
// checks.
bool OnlyHasSimpleProperties();
inline int EnumLength(); inline int EnumLength();
inline void SetEnumLength(int length); inline void SetEnumLength(int length);
......
...@@ -330,6 +330,7 @@ class PropertyDetails BASE_EMBEDDED { ...@@ -330,6 +330,7 @@ class PropertyDetails BASE_EMBEDDED {
bool IsReadOnly() const { return (attributes() & READ_ONLY) != 0; } bool IsReadOnly() const { return (attributes() & READ_ONLY) != 0; }
bool IsConfigurable() const { return (attributes() & DONT_DELETE) == 0; } bool IsConfigurable() const { return (attributes() & DONT_DELETE) == 0; }
bool IsDontEnum() const { return (attributes() & DONT_ENUM) != 0; } bool IsDontEnum() const { return (attributes() & DONT_ENUM) != 0; }
bool IsEnumerable() const { return !IsDontEnum(); }
PropertyCellType cell_type() const { PropertyCellType cell_type() const {
return PropertyCellTypeField::decode(value_); return PropertyCellTypeField::decode(value_);
} }
......
...@@ -138,3 +138,36 @@ assertSame(Object.assign(o, {}), o); ...@@ -138,3 +138,36 @@ assertSame(Object.assign(o, {}), o);
assertThrows(function() { return Object.assign(target, source); }, ErrorB); assertThrows(function() { return Object.assign(target, source); }, ErrorB);
assertEquals(log, "b"); assertEquals(log, "b");
})(); })();
(function add_to_source() {
var target = {set k1(v) { source.k3 = 100; }};
var source = {k1:10};
Object.defineProperty(source, "k2",
{value: 20, enumerable: false, configurable: true});
Object.assign(target, source);
assertEquals(undefined, target.k2);
assertEquals(undefined, target.k3);
})();
(function reconfigure_enumerable_source() {
var target = {set k1(v) {
Object.defineProperty(source, "k2", {value: 20, enumerable: true});
}};
var source = {k1:10};
Object.defineProperty(source, "k2",
{value: 20, enumerable: false, configurable: true});
Object.assign(target, source);
assertEquals(20, target.k2);
})();
(function propagate_assign_failure() {
var target = {set k1(v) { throw "fail" }};
var source = {k1:10};
assertThrows(()=>Object.assign(target, source));
})();
(function propagate_read_failure() {
var target = {};
var source = {get k1() { throw "fail" }};
assertThrows(()=>Object.assign(target, source));
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment