AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[builtins] DeleteProperty: Handle last-added fast properties

Browse Source 
In general, deleting a property from a fast-properties object
requires transitioning the object to dictionary mode. However,
when the most-recently-added property is deleted, we can simply
roll back the last map transition that the object went through.

This is a performance experiment: it should make things faster,
but if it turns out to have more negative than positive impact,
we will have to revert it.

TBR=bmeurer@chromium.org (just adding a comment)

Review-Url: https://codereview.chromium.org/2830093002
Cr-Commit-Position: refs/heads/master@{#44799}
This commit is contained in:
jkummerow 2017-04-24 06:27:41 -07:00 committed by Commit bot
parent 7079d41a81
commit 98acfb36e1
7 changed files with 119 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
src/builtins/builtins-internal-gen.cc
View File

@ -169,6 +169,85 @@ class DeletePropertyBaseAssembler : public CodeStubAssembler {
explicit DeletePropertyBaseAssembler(compiler::CodeAssemblerState* state)
: CodeStubAssembler(state) {}
void DeleteFastProperty(Node* receiver, Node* receiver_map, Node* properties,
Node* name, Label* dont_delete, Label* not_found,
Label* slow) {
// This builtin implements a special case for fast property deletion:
// when the last property in an object is deleted, then instead of
// normalizing the properties, we can undo the last map transition,
// with a few prerequisites:
// (1) The current map must not be marked stable. Otherwise there could
// be optimized code that depends on the assumption that no object that
// reached this map transitions away from it (without triggering the
// "deoptimize dependent code" mechanism).
Node* bitfield3 = LoadMapBitField3(receiver_map);
GotoIfNot(IsSetWord32<Map::IsUnstable>(bitfield3), slow);
// (2) The property to be deleted must be the last property.
Node* descriptors = LoadMapDescriptors(receiver_map);
Node* nof = DecodeWord32<Map::NumberOfOwnDescriptorsBits>(bitfield3);
GotoIf(Word32Equal(nof, Int32Constant(0)), not_found);
Node* descriptor_number = Int32Sub(nof, Int32Constant(1));
Node* key_index = DescriptorArrayToKeyIndex(descriptor_number);
Node* actual_key = LoadFixedArrayElement(descriptors, key_index);
// TODO(jkummerow): We could implement full descriptor search in order
// to avoid the runtime call for deleting nonexistent properties, but
// that's probably a rare case.
GotoIf(WordNotEqual(actual_key, name), slow);
// (3) The property to be deleted must be deletable.
Node* details =
LoadDetailsByKeyIndex<DescriptorArray>(descriptors, key_index);
GotoIf(IsSetWord32(details, PropertyDetails::kAttributesDontDeleteMask),
dont_delete);
// (4) The map must have a back pointer.
Node* backpointer =
LoadObjectField(receiver_map, Map::kConstructorOrBackPointerOffset);
GotoIfNot(IsMap(backpointer), slow);
// (5) The last transition must have been caused by adding a property
// (and not any kind of special transition).
Node* previous_nof = DecodeWord32<Map::NumberOfOwnDescriptorsBits>(
LoadMapBitField3(backpointer));
GotoIfNot(Word32Equal(previous_nof, descriptor_number), slow);
// Preconditions successful, perform the map rollback!
// Zap the property to avoid keeping objects alive.
// Zapping is not necessary for properties stored in the descriptor array.
Label zapping_done(this);
GotoIf(Word32NotEqual(DecodeWord32<PropertyDetails::LocationField>(details),
Int32Constant(kField)),
&zapping_done);
Node* field_index =
DecodeWordFromWord32<PropertyDetails::FieldIndexField>(details);
Node* inobject_properties = LoadMapInobjectProperties(receiver_map);
Label inobject(this), backing_store(this);
// We don't need to special-case inobject slack tracking here (by using
// the one_pointer_filler_map as filler), because it'll trim objects to
// the size of the largest known map anyway, so rolled-back properties
// can be zapped with |undefined|.
Node* filler = UndefinedConstant();
DCHECK(Heap::RootIsImmortalImmovable(Heap::kUndefinedValueRootIndex));
Branch(UintPtrLessThan(field_index, inobject_properties), &inobject,
&backing_store);
BIND(&inobject);
{
Node* field_offset =
IntPtrMul(IntPtrSub(LoadMapInstanceSize(receiver_map),
IntPtrSub(inobject_properties, field_index)),
IntPtrConstant(kPointerSize));
StoreObjectFieldNoWriteBarrier(receiver, field_offset, filler);
Goto(&zapping_done);
}
BIND(&backing_store);
{
Node* backing_store_index = IntPtrSub(field_index, inobject_properties);
StoreFixedArrayElement(properties, backing_store_index, filler,
SKIP_WRITE_BARRIER);
Goto(&zapping_done);
}
BIND(&zapping_done);
StoreMap(receiver, backpointer);
Return(TrueConstant());
}
void DeleteDictionaryProperty(Node* receiver, Node* properties, Node* name,
Node* context, Label* dont_delete,
Label* notfound) {
@ -250,8 +329,8 @@ TF_BUILTIN(DeleteProperty, DeletePropertyBaseAssembler) {
Node* properties_map = LoadMap(properties);
GotoIf(WordEqual(properties_map, LoadRoot(Heap::kHashTableMapRootIndex)),
&dictionary);
// TODO(jkummerow): Implement support for fast properties?
Goto(&slow);
DeleteFastProperty(receiver, receiver_map, properties, unique, &dont_delete,
&if_notfound, &slow);
BIND(&dictionary);
{

9
src/compiler/js-native-context-specialization.cc
View File

@ -2310,6 +2310,15 @@ Node* JSNativeContextSpecialization::BuildCheckMaps(
Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore(
Handle<Map> map, Node* properties, Node* effect, Node* control) {
// TODO(bmeurer/jkummerow): Property deletions can undo map transitions
// while keeping the backing store around, meaning that even though the
// map might believe that objects have no unused property fields, there
// might actually be some. It would be nice to not create a new backing
// store in that case (i.e. when properties->length() >= new_length).
// However, introducing branches and Phi nodes here would make it more
// difficult for escape analysis to get rid of the backing stores used
// for intermediate states of chains of property additions. That makes
// it unclear what the best approach is here.
DCHECK_EQ(0, map->unused_property_fields());
// Compute the length of the old {properties} and the new properties.
int length = map->NextFreePropertyIndex() - map->GetInObjectProperties();

29
src/ic/accessor-assembler.cc
View File

@ -984,15 +984,7 @@ void AccessorAssembler::HandleStoreFieldAndReturn(Node* handler_word,
BIND(&if_out_of_object);
{
if (transition_to_field) {
Label storage_extended(this);
GotoIfNot(IsSetWord<StoreHandler::ExtendStorageBits>(handler_word),
&storage_extended);
Comment("[ Extend storage");
ExtendPropertiesBackingStore(holder);
Comment("] Extend storage");
Goto(&storage_extended);
BIND(&storage_extended);
ExtendPropertiesBackingStore(holder, handler_word);
}
StoreNamedField(handler_word, holder, false, representation, prepared_value,
@ -1053,7 +1045,12 @@ Node* AccessorAssembler::PrepareValueForStore(Node* handler_word, Node* holder,
return value;
}
void AccessorAssembler::ExtendPropertiesBackingStore(Node* object) {
void AccessorAssembler::ExtendPropertiesBackingStore(Node* object,
Node* handler_word) {
Label done(this);
GotoIfNot(IsSetWord<StoreHandler::ExtendStorageBits>(handler_word), &done);
Comment("[ Extend storage");
ParameterMode mode = OptimalParameterMode();
Node* properties = LoadProperties(object);
@ -1061,6 +1058,14 @@ void AccessorAssembler::ExtendPropertiesBackingStore(Node* object) {
? LoadAndUntagFixedArrayBaseLength(properties)
: LoadFixedArrayBaseLength(properties);
// Previous property deletion could have left behind unused backing store
// capacity even for a map that think it doesn't have any unused fields.
// Perform a bounds check to see if we actually have to grow the array.
Node* offset = DecodeWord<StoreHandler::FieldOffsetBits>(handler_word);
Node* size = ElementOffsetFromIndex(length, FAST_ELEMENTS, mode,
FixedArray::kHeaderSize);
GotoIf(UintPtrLessThan(offset, size), &done);
Node* delta = IntPtrOrSmiConstant(JSObject::kFieldsAdded, mode);
Node* new_capacity = IntPtrOrSmiAdd(length, delta, mode);
@ -1088,6 +1093,10 @@ void AccessorAssembler::ExtendPropertiesBackingStore(Node* object) {
SKIP_WRITE_BARRIER, mode);
StoreObjectField(object, JSObject::kPropertiesOffset, new_properties);
Comment("] Extend storage");
Goto(&done);
BIND(&done);
}
void AccessorAssembler::StoreNamedField(Node* handler_word, Node* object,

2
src/ic/accessor-assembler.h
View File

@ -193,7 +193,7 @@ class AccessorAssembler : public CodeStubAssembler {
Node* value, Label* bailout);
// Extends properties backing store by JSObject::kFieldsAdded elements.
void ExtendPropertiesBackingStore(Node* object);
void ExtendPropertiesBackingStore(Node* object, Node* handler_word);
void StoreNamedField(Node* handler_word, Node* object, bool is_inobject,
Representation representation, Node* value,

9
src/objects.cc
View File

@ -3458,11 +3458,15 @@ void MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
}
PropertyDetails details = new_map->GetLastDescriptorDetails();
int target_index = details.field_index() - new_map->GetInObjectProperties();
bool have_space = old_map->unused_property_fields() > 0 ||
(details.location() == kField && target_index >= 0 &&
object->properties()->length() > target_index);
// Either new_map adds an kDescriptor property, or a kField property for
// which there is still space, and which does not require a mutable double
// box (an out-of-object double).
if (details.location() == kDescriptor ||
(old_map->unused_property_fields() > 0 &&
(have_space &&
((FLAG_unbox_double_fields && object->properties()->length() == 0) ||
!details.representation().IsDouble()))) {
object->synchronized_set_map(*new_map);
@ -3471,7 +3475,7 @@ void MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
// If there is still space in the object, we need to allocate a mutable
// double box.
if (old_map->unused_property_fields() > 0) {
if (have_space) {
FieldIndex index =
FieldIndex::ForDescriptor(*new_map, new_map->LastAdded());
DCHECK(details.representation().IsDouble());
@ -3498,7 +3502,6 @@ void MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
}
DCHECK_EQ(kField, details.location());
DCHECK_EQ(kData, details.kind());
int target_index = details.field_index() - new_map->GetInObjectProperties();
DCHECK(target_index >= 0); // Must be a backing store index.
new_storage->set(target_index, *value);

2
test/mjsunit/dictionary-properties.js
View File

@ -11,6 +11,7 @@ function SlowObject() {
this.foo = 1;
this.bar = 2;
this.qux = 3;
this.z = 4;
delete this.qux;
assertFalse(%HasFastProperties(this));
}
@ -38,6 +39,7 @@ function SlowPrototype() {
}
SlowPrototype.prototype.bar = 2;
SlowPrototype.prototype.baz = 3;
SlowPrototype.prototype.z = 4;
delete SlowPrototype.prototype.baz;
assertFalse(%HasFastProperties(SlowPrototype.prototype));
var slow_proto = new SlowPrototype;

1
test/mjsunit/regress/regress-252797.js
View File

@ -45,6 +45,7 @@ assertFalse(%HasFastProperties(holder));
// Create a receiver into dictionary mode.
var receiver = Object.create(holder, {
killMe: {value: 0, configurable: true},
keepMe: {value: 0, configurable: true}
});
delete receiver.killMe;
assertFalse(%HasFastProperties(receiver));