Reviewers: danno,
Message:
Committed patchset #1 manually as r17607 (presubmit successful).
Description:
Reland [Object.observe] Don't force normalization of elements for observed
objects
Original Issue: https://codereview.chromium.org/29353003/
TBR=danno
BUG=v8:2946
Committed: https://code.google.com/p/v8/source/detail?r=17607
Please review this at https://codereview.chromium.org/66933003/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+30, -49 lines):
M src/arm/ic-arm.cc
M src/builtins.cc
M src/ia32/ic-ia32.cc
M src/mips/ic-mips.cc
M src/objects-debug.cc
M src/objects-inl.h
M src/objects.h
M src/objects.cc
M src/x64/ic-x64.cc
Index: src/arm/ic-arm.cc
diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc
index
025a590f0cba202b9636b88d03626deea2e371b6..a3b2a6e2a19842e2c59584a70029a29c675a1360
100644
--- a/src/arm/ic-arm.cc
+++ b/src/arm/ic-arm.cc
@@ -1432,10 +1432,10 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler*
masm,
__ JumpIfSmi(receiver, &slow);
// Get the map of the object.
__ ldr(receiver_map, FieldMemOperand(receiver, HeapObject::kMapOffset));
- // Check that the receiver does not require access checks. We need
- // to do this because this generic stub does not perform map checks.
+ // Check that the receiver does not require access checks and is not
observed.
+ // The generic stub does not perform map checks or handle observed
objects.
__ ldrb(ip, FieldMemOperand(receiver_map, Map::kBitFieldOffset));
- __ tst(ip, Operand(1 << Map::kIsAccessCheckNeeded));
+ __ tst(ip, Operand(1 << Map::kIsAccessCheckNeeded | 1 <<
Map::kIsObserved));
__ b(ne, &slow);
// Check if the object is a JS array or not.
__ ldrb(r4, FieldMemOperand(receiver_map, Map::kInstanceTypeOffset));
Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index
758967ec08d875ce5ccab732d4d120366e6a5079..40772724266dc0277a59da827c77d2dbaa76aefa
100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -311,6 +311,7 @@ static inline MaybeObject*
EnsureJSArrayWithWritableFastElements(
Heap* heap, Object* receiver, Arguments* args, int first_added_arg) {
if (!receiver->IsJSArray()) return NULL;
JSArray* array = JSArray::cast(receiver);
+ if (array->map()->is_observed()) return NULL;
HeapObject* elms = array->elements();
Map* map = elms->map();
if (map == heap->fixed_array_map()) {
Index: src/ia32/ic-ia32.cc
diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc
index
0b7c4a828bce881c03ed05d44cfbf3943e103164..dab9dd7a447ae089d87419557feba98cce0c4e43
100644
--- a/src/ia32/ic-ia32.cc
+++ b/src/ia32/ic-ia32.cc
@@ -874,10 +874,10 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler*
masm,
__ JumpIfSmi(edx, &slow);
// Get the map from the receiver.
__ mov(edi, FieldOperand(edx, HeapObject::kMapOffset));
- // Check that the receiver does not require access checks. We need
- // to do this because this generic stub does not perform map checks.
+ // Check that the receiver does not require access checks and is not
observed.
+ // The generic stub does not perform map checks or handle observed
objects.
__ test_b(FieldOperand(edi, Map::kBitFieldOffset),
- 1 << Map::kIsAccessCheckNeeded);
+ 1 << Map::kIsAccessCheckNeeded | 1 << Map::kIsObserved);
__ j(not_zero, &slow);
// Check that the key is a smi.
__ JumpIfNotSmi(ecx, &slow);
Index: src/mips/ic-mips.cc
diff --git a/src/mips/ic-mips.cc b/src/mips/ic-mips.cc
index
0fe044a6c5a02dfc66fa51b02748672122a61eba..c7e1a2ada602392c952118d8f36162a37e67682b
100644
--- a/src/mips/ic-mips.cc
+++ b/src/mips/ic-mips.cc
@@ -1354,10 +1354,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler*
masm,
__ JumpIfSmi(receiver, &slow);
// Get the map of the object.
__ lw(receiver_map, FieldMemOperand(receiver, HeapObject::kMapOffset));
- // Check that the receiver does not require access checks. We need
- // to do this because this generic stub does not perform map checks.
+ // Check that the receiver does not require access checks and is not
observed.
+ // The generic stub does not perform map checks or handle observed
objects.
__ lbu(t0, FieldMemOperand(receiver_map, Map::kBitFieldOffset));
- __ And(t0, t0, Operand(1 << Map::kIsAccessCheckNeeded));
+ __ And(t0, t0, Operand(1 << Map::kIsAccessCheckNeeded |
+ 1 << Map::kIsObserved));
__ Branch(&slow, ne, t0, Operand(zero_reg));
// Check if the object is a JS array or not.
__ lbu(t0, FieldMemOperand(receiver_map, Map::kInstanceTypeOffset));
Index: src/objects-debug.cc
diff --git a/src/objects-debug.cc b/src/objects-debug.cc
index
6ab2ddffe2a9e0ea3772163e20e87bd269f1a7e0..80610c6a8b5de272099591d1bff2b688be818d5d
100644
--- a/src/objects-debug.cc
+++ b/src/objects-debug.cc
@@ -366,9 +366,6 @@ void Map::MapVerify() {
SLOW_ASSERT(transitions()->IsSortedNoDuplicates());
SLOW_ASSERT(transitions()->IsConsistentWithBackPointers(this));
}
- ASSERT(!is_observed() || instance_type() < FIRST_JS_OBJECT_TYPE ||
- instance_type() > LAST_JS_OBJECT_TYPE ||
- has_slow_elements_kind() || has_external_array_elements());
}
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
bef807eaf688cd14d0278f34d3a79277dcebdb5a..d28fad3df22c464d76b898be40fd969eb5b29a56
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -3649,16 +3649,13 @@ bool Map::owns_descriptors() {
}
-void Map::set_is_observed(bool is_observed) {
- ASSERT(instance_type() < FIRST_JS_OBJECT_TYPE ||
- instance_type() > LAST_JS_OBJECT_TYPE ||
- has_slow_elements_kind() || has_external_array_elements());
- set_bit_field3(IsObserved::update(bit_field3(), is_observed));
+void Map::set_has_instance_call_handler() {
+ set_bit_field3(HasInstanceCallHandler::update(bit_field3(), true));
}
-bool Map::is_observed() {
- return IsObserved::decode(bit_field3());
+bool Map::has_instance_call_handler() {
+ return HasInstanceCallHandler::decode(bit_field3());
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
441c25e70c649aecb7b54f49d5670c1daf136569..d9a48a8a0c083a5066aabcc20879dc0f6e96858a
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -5614,12 +5614,6 @@ void JSObject::SetObserved(Handle<JSObject> object) {
if (object->map()->is_observed())
return;
- if (!object->HasExternalArrayElements()) {
- // Go to dictionary mode, so that we don't skip map checks.
- NormalizeElements(object);
- ASSERT(!object->HasFastElements());
- }
-
LookupResult result(isolate);
object->map()->LookupTransition(*object,
isolate->heap()->observed_symbol(),
@@ -5633,7 +5627,7 @@ void JSObject::SetObserved(Handle<JSObject> object) {
new_map = Map::CopyForObserved(handle(object->map()));
} else {
new_map = Map::Copy(handle(object->map()));
- new_map->set_is_observed(true);
+ new_map->set_is_observed();
}
object->set_map(*new_map);
}
@@ -6971,7 +6965,7 @@ Handle<Map> Map::CopyForObserved(Handle<Map> map) {
map->set_transitions(*transitions);
- new_map->set_is_observed(true);
+ new_map->set_is_observed();
if (map->owns_descriptors()) {
new_map->InitializeDescriptors(map->instance_descriptors());
@@ -11226,7 +11220,6 @@ MaybeObject*
JSObject::SetFastElementsCapacityAndLength(
Heap* heap = GetHeap();
// We should never end in here with a pixel or external array.
ASSERT(!HasExternalArrayElements());
- ASSERT(!map()->is_observed());
// Allocate a new fast elements backing store.
FixedArray* new_elements;
@@ -11311,7 +11304,6 @@ MaybeObject*
JSObject::SetFastDoubleElementsCapacityAndLength(
Heap* heap = GetHeap();
// We should never end in here with a pixel or external array.
ASSERT(!HasExternalArrayElements());
- ASSERT(!map()->is_observed());
FixedArrayBase* elems;
{ MaybeObject* maybe_obj =
@@ -11460,10 +11452,6 @@ MaybeObject* JSArray::SetElementsLength(Object*
len) {
if (!new_length_handle->ToArrayIndex(&new_length))
return Failure::InternalError();
- // Observed arrays should always be in dictionary mode;
- // if they were in fast mode, the below is slower than necessary
- // as it iterates over the array backing store multiple times.
- ASSERT(self->HasDictionaryElements());
static const PropertyAttributes kNoAttrFilter = NONE;
int num_elements = self->NumberOfLocalElements(kNoAttrFilter);
if (num_elements > 0) {
@@ -11474,6 +11462,8 @@ MaybeObject* JSArray::SetElementsLength(Object*
len) {
}
} else {
// For sparse arrays, only iterate over existing elements.
+ // TODO(rafaelw): For fast, sparse arrays, we can avoid iterating
over
+ // the to-be-removed indices twice.
Handle<FixedArray> keys =
isolate->factory()->NewFixedArray(num_elements);
self->GetLocalElementKeys(*keys, kNoAttrFilter);
while (num_elements-- > 0) {
@@ -12872,7 +12862,6 @@ MaybeObject*
JSObject::UpdateAllocationSite(ElementsKind to_kind) {
MaybeObject* JSObject::TransitionElementsKind(ElementsKind to_kind) {
- ASSERT(!map()->is_observed());
ElementsKind from_kind = map()->elements_kind();
if (IsFastHoleyElementsKind(from_kind)) {
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
3105579e28f4e3aee5b4b0235247c8a9af11ac61..c4d82bc9bb40e35c219d84dad8d9efe3530047e6
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -5664,7 +5664,7 @@ class Map: public HeapObject {
class FunctionWithPrototype: public BitField<bool, 23, 1> {};
class DictionaryMap: public BitField<bool, 24, 1> {};
class OwnsDescriptors: public BitField<bool, 25, 1> {};
- class IsObserved: public BitField<bool, 26, 1> {};
+ class HasInstanceCallHandler: public BitField<bool, 26, 1> {};
class Deprecated: public BitField<bool, 27, 1> {};
class IsFrozen: public BitField<bool, 28, 1> {};
class IsUnstable: public BitField<bool, 29, 1> {};
@@ -5727,12 +5727,12 @@ class Map: public HeapObject {
}
// Tells whether the instance has a call-as-function handler.
- inline void set_has_instance_call_handler() {
- set_bit_field(bit_field() | (1 << kHasInstanceCallHandler));
+ inline void set_is_observed() {
+ set_bit_field(bit_field() | (1 << kIsObserved));
}
- inline bool has_instance_call_handler() {
- return ((1 << kHasInstanceCallHandler) & bit_field()) != 0;
+ inline bool is_observed() {
+ return ((1 << kIsObserved) & bit_field()) != 0;
}
inline void set_is_extensible(bool value);
@@ -5741,10 +5741,6 @@ class Map: public HeapObject {
inline void set_elements_kind(ElementsKind elements_kind) {
ASSERT(elements_kind < kElementsKindCount);
ASSERT(kElementsKindCount <= (1 << kElementsKindBitCount));
- ASSERT(!is_observed() ||
- elements_kind == DICTIONARY_ELEMENTS ||
- elements_kind == NON_STRICT_ARGUMENTS_ELEMENTS ||
- IsExternalArrayElementsKind(elements_kind));
set_bit_field2((bit_field2() & ~kElementsKindMask) |
(elements_kind << kElementsKindShift));
ASSERT(this->elements_kind() == elements_kind);
@@ -5997,8 +5993,8 @@ class Map: public HeapObject {
inline bool owns_descriptors();
inline void set_owns_descriptors(bool is_shared);
- inline bool is_observed();
- inline void set_is_observed(bool is_observed);
+ inline bool has_instance_call_handler();
+ inline void set_has_instance_call_handler();
inline void freeze();
inline bool is_frozen();
inline void mark_unstable();
@@ -6257,7 +6253,7 @@ class Map: public HeapObject {
static const int kHasNamedInterceptor = 3;
static const int kHasIndexedInterceptor = 4;
static const int kIsUndetectable = 5;
- static const int kHasInstanceCallHandler = 6;
+ static const int kIsObserved = 6;
static const int kIsAccessCheckNeeded = 7;
// Bit positions for bit field 2
Index: src/x64/ic-x64.cc
diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc
index
721ae1d982aa9279dff7845b326da554459de690..fe8734caf47988adfa4c2ddb1c10c4b44e1af6b9
100644
--- a/src/x64/ic-x64.cc
+++ b/src/x64/ic-x64.cc
@@ -749,10 +749,10 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler*
masm,
__ JumpIfSmi(rdx, &slow_with_tagged_index);
// Get the map from the receiver.
__ movq(r9, FieldOperand(rdx, HeapObject::kMapOffset));
- // Check that the receiver does not require access checks. We need
- // to do this because this generic stub does not perform map checks.
+ // Check that the receiver does not require access checks and is not
observed.
+ // The generic stub does not perform map checks or handle observed
objects.
__ testb(FieldOperand(r9, Map::kBitFieldOffset),
- Immediate(1 << Map::kIsAccessCheckNeeded));
+ Immediate(1 << Map::kIsAccessCheckNeeded | 1 <<
Map::kIsObserved));
__ j(not_zero, &slow_with_tagged_index);
// Check that the key is a smi.
__ JumpIfNotSmi(rcx, &slow_with_tagged_index);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
