Revision: 12230
Author: [email protected]
Date: Mon Jul 30 06:09:09 2012
Log: Limit initial size of hidden properties and store identity hashes
inline.
BUG=v8:2211
TEST=test-heap/Regress2211
Review URL: https://chromiumcodereview.appspot.com/10827040
http://code.google.com/p/v8/source/detail?r=12230
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/factory.cc
/branches/bleeding_edge/src/factory.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/test/cctest/test-dictionary.cc
/branches/bleeding_edge/test/cctest/test-heap-profiler.cc
/branches/bleeding_edge/test/cctest/test-heap.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Mon Jul 23 07:22:46 2012
+++ /branches/bleeding_edge/src/api.cc Mon Jul 30 06:09:09 2012
@@ -3303,9 +3303,10 @@
i::HandleScope scope(isolate);
i::Handle<i::JSObject> self = Utils::OpenHandle(this);
i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
+ i::Handle<i::String> key_symbol = FACTORY->LookupSymbol(key_obj);
i::Handle<i::Object> value_obj = Utils::OpenHandle(*value);
i::Handle<i::Object> result =
- i::JSObject::SetHiddenProperty(self, key_obj, value_obj);
+ i::JSObject::SetHiddenProperty(self, key_symbol, value_obj);
return *result == *self;
}
@@ -3317,7 +3318,8 @@
ENTER_V8(isolate);
i::Handle<i::JSObject> self = Utils::OpenHandle(this);
i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
- i::Handle<i::Object> result(self->GetHiddenProperty(*key_obj));
+ i::Handle<i::String> key_symbol = FACTORY->LookupSymbol(key_obj);
+ i::Handle<i::Object> result(self->GetHiddenProperty(*key_symbol));
if (result->IsUndefined()) return v8::Local<v8::Value>();
return Utils::ToLocal(result);
}
@@ -3330,7 +3332,8 @@
i::HandleScope scope(isolate);
i::Handle<i::JSObject> self = Utils::OpenHandle(this);
i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
- self->DeleteHiddenProperty(*key_obj);
+ i::Handle<i::String> key_symbol = FACTORY->LookupSymbol(key_obj);
+ self->DeleteHiddenProperty(*key_symbol);
return true;
}
=======================================
--- /branches/bleeding_edge/src/factory.cc Wed Jul 25 04:12:29 2012
+++ /branches/bleeding_edge/src/factory.cc Mon Jul 30 06:09:09 2012
@@ -1011,7 +1011,7 @@
}
-void Factory::SetIdentityHash(Handle<JSObject> object, Object* hash) {
+void Factory::SetIdentityHash(Handle<JSObject> object, Smi* hash) {
CALL_HEAP_FUNCTION_VOID(
isolate(),
object->SetIdentityHash(hash, ALLOW_CREATION));
=======================================
--- /branches/bleeding_edge/src/factory.h Wed Jul 25 04:12:29 2012
+++ /branches/bleeding_edge/src/factory.h Mon Jul 30 06:09:09 2012
@@ -297,7 +297,7 @@
void BecomeJSObject(Handle<JSReceiver> object);
void BecomeJSFunction(Handle<JSReceiver> object);
- void SetIdentityHash(Handle<JSObject> object, Object* hash);
+ void SetIdentityHash(Handle<JSObject> object, Smi* hash);
Handle<JSFunction> NewFunction(Handle<String> name,
Handle<Object> prototype);
=======================================
--- /branches/bleeding_edge/src/objects.cc Fri Jul 27 10:03:12 2012
+++ /branches/bleeding_edge/src/objects.cc Mon Jul 30 06:09:09 2012
@@ -2761,7 +2761,7 @@
Object* hash;
if (maybe_hash->To<Object>(&hash) && hash->IsSmi()) {
Handle<JSObject> new_self(JSObject::cast(*self));
- isolate->factory()->SetIdentityHash(new_self, hash);
+ isolate->factory()->SetIdentityHash(new_self, Smi::cast(hash));
}
}
@@ -3505,7 +3505,7 @@
}
-MaybeObject* JSObject::SetIdentityHash(Object* hash, CreationFlag flag) {
+MaybeObject* JSObject::SetIdentityHash(Smi* hash, CreationFlag flag) {
MaybeObject* maybe = SetHiddenProperty(GetHeap()->identity_hash_symbol(),
hash);
if (maybe->IsFailure()) return maybe;
@@ -3551,6 +3551,7 @@
Object* JSObject::GetHiddenProperty(String* key) {
+ ASSERT(key->IsSymbol());
if (IsJSGlobalProxy()) {
// For a proxy, use the prototype as target object.
Object* proxy_parent = GetPrototype();
@@ -3560,22 +3561,32 @@
return JSObject::cast(proxy_parent)->GetHiddenProperty(key);
}
ASSERT(!IsJSGlobalProxy());
- MaybeObject* hidden_lookup = GetHiddenPropertiesDictionary(false);
+ MaybeObject* hidden_lookup =
+ GetHiddenPropertiesHashTable(ONLY_RETURN_INLINE_VALUE);
ASSERT(!hidden_lookup->IsFailure()); // No failure when passing false
as arg.
- if (hidden_lookup->ToObjectUnchecked()->IsUndefined()) {
- return GetHeap()->undefined_value();
- }
- StringDictionary* dictionary =
- StringDictionary::cast(hidden_lookup->ToObjectUnchecked());
- int entry = dictionary->FindEntry(key);
- if (entry == StringDictionary::kNotFound) return
GetHeap()->undefined_value();
- return dictionary->ValueAt(entry);
+ Object* inline_value = hidden_lookup->ToObjectUnchecked();
+
+ if (inline_value->IsSmi()) {
+ // Handle inline-stored identity hash.
+ if (key == GetHeap()->identity_hash_symbol()) {
+ return inline_value;
+ } else {
+ return GetHeap()->undefined_value();
+ }
+ }
+
+ if (inline_value->IsUndefined()) return GetHeap()->undefined_value();
+
+ ObjectHashTable* hashtable = ObjectHashTable::cast(inline_value);
+ Object* entry = hashtable->Lookup(key);
+ if (entry->IsTheHole()) return GetHeap()->undefined_value();
+ return entry;
}
Handle<Object> JSObject::SetHiddenProperty(Handle<JSObject> obj,
- Handle<String> key,
- Handle<Object> value) {
+ Handle<String> key,
+ Handle<Object> value) {
CALL_HEAP_FUNCTION(obj->GetIsolate(),
obj->SetHiddenProperty(*key, *value),
Object);
@@ -3583,6 +3594,7 @@
MaybeObject* JSObject::SetHiddenProperty(String* key, Object* value) {
+ ASSERT(key->IsSymbol());
if (IsJSGlobalProxy()) {
// For a proxy, use the prototype as target object.
Object* proxy_parent = GetPrototype();
@@ -3592,27 +3604,31 @@
return JSObject::cast(proxy_parent)->SetHiddenProperty(key, value);
}
ASSERT(!IsJSGlobalProxy());
- MaybeObject* hidden_lookup = GetHiddenPropertiesDictionary(true);
- StringDictionary* dictionary;
- if (!hidden_lookup->To<StringDictionary>(&dictionary)) return
hidden_lookup;
-
- // If it was found, check if the key is already in the dictionary.
- int entry = dictionary->FindEntry(key);
- if (entry != StringDictionary::kNotFound) {
- // If key was found, just update the value.
- dictionary->ValueAtPut(entry, value);
- return this;
- }
- // Key was not already in the dictionary, so add the entry.
- MaybeObject* insert_result = dictionary->Add(key,
- value,
- PropertyDetails(NONE,
NORMAL));
- StringDictionary* new_dict;
- if (!insert_result->To<StringDictionary>(&new_dict)) return
insert_result;
- if (new_dict != dictionary) {
+
+ // If there is no backing store yet, store the identity hash inline.
+ MaybeObject* hidden_lookup =
+ GetHiddenPropertiesHashTable(ONLY_RETURN_INLINE_VALUE);
+ ASSERT(!hidden_lookup->IsFailure());
+ Object* inline_value = hidden_lookup->ToObjectUnchecked();
+
+ if (value->IsSmi() &&
+ key == GetHeap()->identity_hash_symbol() &&
+ (inline_value->IsUndefined() || inline_value->IsSmi())) {
+ return SetHiddenPropertiesHashTable(value);
+ }
+
+ hidden_lookup = GetHiddenPropertiesHashTable(CREATE_NEW_IF_ABSENT);
+ ObjectHashTable* hashtable;
+ if (!hidden_lookup->To(&hashtable)) return hidden_lookup;
+
+ // If it was found, check if the key is already in the dictionary.
+ MaybeObject* insert_result = hashtable->Put(key, value);
+ ObjectHashTable* new_table;
+ if (!insert_result->To(&new_table)) return insert_result;
+ if (new_table != hashtable) {
// If adding the key expanded the dictionary (i.e., Add returned a new
// dictionary), store it back to the object.
- MaybeObject* store_result = SetHiddenPropertiesDictionary(new_dict);
+ MaybeObject* store_result = SetHiddenPropertiesHashTable(new_table);
if (store_result->IsFailure()) return store_result;
}
// Return this to mark success.
@@ -3621,6 +3637,7 @@
void JSObject::DeleteHiddenProperty(String* key) {
+ ASSERT(key->IsSymbol());
if (IsJSGlobalProxy()) {
// For a proxy, use the prototype as target object.
Object* proxy_parent = GetPrototype();
@@ -3630,18 +3647,18 @@
JSObject::cast(proxy_parent)->DeleteHiddenProperty(key);
return;
}
- MaybeObject* hidden_lookup = GetHiddenPropertiesDictionary(false);
+ MaybeObject* hidden_lookup =
+ GetHiddenPropertiesHashTable(ONLY_RETURN_INLINE_VALUE);
ASSERT(!hidden_lookup->IsFailure()); // No failure when passing false
as arg.
if (hidden_lookup->ToObjectUnchecked()->IsUndefined()) return;
- StringDictionary* dictionary =
- StringDictionary::cast(hidden_lookup->ToObjectUnchecked());
- int entry = dictionary->FindEntry(key);
- if (entry == StringDictionary::kNotFound) {
- // Key wasn't in dictionary. Deletion is a success.
- return;
- }
- // Key was in the dictionary. Remove it.
- dictionary->DeleteProperty(entry, JSReceiver::FORCE_DELETION);
+ // We never delete (inline-stored) identity hashes.
+ ASSERT(!hidden_lookup->ToObjectUnchecked()->IsSmi());
+
+ ObjectHashTable* hashtable =
+ ObjectHashTable::cast(hidden_lookup->ToObjectUnchecked());
+ MaybeObject* delete_result = hashtable->Put(key,
GetHeap()->the_hole_value());
+ USE(delete_result);
+ ASSERT(!delete_result->IsFailure()); // Delete does not cause GC.
}
@@ -3652,8 +3669,10 @@
}
-MaybeObject* JSObject::GetHiddenPropertiesDictionary(bool
create_if_absent) {
+MaybeObject* JSObject::GetHiddenPropertiesHashTable(
+ InitializeHiddenProperties init_option) {
ASSERT(!IsJSGlobalProxy());
+ Object* inline_value;
if (HasFastProperties()) {
// If the object has fast properties, check whether the first slot
// in the descriptor array matches the hidden symbol. Since the
@@ -3663,43 +3682,60 @@
if ((descriptors->number_of_descriptors() > 0) &&
(descriptors->GetKey(0) == GetHeap()->hidden_symbol())) {
ASSERT(descriptors->GetType(0) == FIELD);
- Object* hidden_store =
- this->FastPropertyAt(descriptors->GetFieldIndex(0));
- return StringDictionary::cast(hidden_store);
+ inline_value = this->FastPropertyAt(descriptors->GetFieldIndex(0));
+ } else {
+ inline_value = GetHeap()->undefined_value();
}
} else {
PropertyAttributes attributes;
// You can't install a getter on a property indexed by the hidden
symbol,
// so we can be sure that GetLocalPropertyPostInterceptor returns a
real
// object.
- Object* lookup =
+ inline_value =
GetLocalPropertyPostInterceptor(this,
GetHeap()->hidden_symbol(),
&attributes)->ToObjectUnchecked();
- if (!lookup->IsUndefined()) {
- return StringDictionary::cast(lookup);
- }
- }
- if (!create_if_absent) return GetHeap()->undefined_value();
- const int kInitialSize = 5;
- MaybeObject* dict_alloc = StringDictionary::Allocate(kInitialSize);
- StringDictionary* dictionary;
- if (!dict_alloc->To<StringDictionary>(&dictionary)) return dict_alloc;
+ }
+
+ if (init_option == ONLY_RETURN_INLINE_VALUE ||
+ inline_value->IsHashTable()) {
+ return inline_value;
+ }
+
+ ObjectHashTable* hashtable;
+ static const int kInitialCapacity = 4;
+ MaybeObject* maybe_obj =
+ ObjectHashTable::Allocate(kInitialCapacity,
+
ObjectHashTable::USE_CUSTOM_MINIMUM_CAPACITY);
+ if (!maybe_obj->To<ObjectHashTable>(&hashtable)) return maybe_obj;
+
+ if (inline_value->IsSmi()) {
+ // We were storing the identity hash inline and now allocated an actual
+ // dictionary. Put the identity hash into the new dictionary.
+ MaybeObject* insert_result =
+ hashtable->Put(GetHeap()->identity_hash_symbol(), inline_value);
+ ObjectHashTable* new_table;
+ if (!insert_result->To(&new_table)) return insert_result;
+ // We expect no resizing for the first insert.
+ ASSERT_EQ(hashtable, new_table);
+ }
+
MaybeObject* store_result =
SetPropertyPostInterceptor(GetHeap()->hidden_symbol(),
- dictionary,
+ hashtable,
DONT_ENUM,
kNonStrictMode,
OMIT_EXTENSIBILITY_CHECK);
if (store_result->IsFailure()) return store_result;
- return dictionary;
+ return hashtable;
}
-MaybeObject* JSObject::SetHiddenPropertiesDictionary(
- StringDictionary* dictionary) {
+MaybeObject* JSObject::SetHiddenPropertiesHashTable(Object* value) {
ASSERT(!IsJSGlobalProxy());
- ASSERT(HasHiddenProperties());
+ // We can store the identity hash inline iff there is no backing store
+ // for hidden properties yet.
+ ASSERT(HasHiddenProperties() != value->IsSmi());
if (HasFastProperties()) {
// If the object has fast properties, check whether the first slot
// in the descriptor array matches the hidden symbol. Since the
@@ -3709,13 +3745,13 @@
if ((descriptors->number_of_descriptors() > 0) &&
(descriptors->GetKey(0) == GetHeap()->hidden_symbol())) {
ASSERT(descriptors->GetType(0) == FIELD);
- this->FastPropertyAtPut(descriptors->GetFieldIndex(0), dictionary);
+ this->FastPropertyAtPut(descriptors->GetFieldIndex(0), value);
return this;
}
}
MaybeObject* store_result =
SetPropertyPostInterceptor(GetHeap()->hidden_symbol(),
- dictionary,
+ value,
DONT_ENUM,
kNonStrictMode,
OMIT_EXTENSIBILITY_CHECK);
@@ -11046,8 +11082,12 @@
template<typename Shape, typename Key>
MaybeObject* HashTable<Shape, Key>::Allocate(int at_least_space_for,
+ MinimumCapacity
capacity_option,
PretenureFlag pretenure) {
- int capacity = ComputeCapacity(at_least_space_for);
+ ASSERT(!capacity_option || IS_POWER_OF_TWO(at_least_space_for));
+ int capacity = (capacity_option == USE_CUSTOM_MINIMUM_CAPACITY)
+ ? at_least_space_for
+ : ComputeCapacity(at_least_space_for);
if (capacity > HashTable::kMaxCapacity) {
return Failure::OutOfMemoryException();
}
@@ -11156,7 +11196,9 @@
(capacity > kMinCapacityForPretenure)
&& !GetHeap()->InNewSpace(this);
Object* obj;
{ MaybeObject* maybe_obj =
- Allocate(nof * 2, pretenure ? TENURED : NOT_TENURED);
+ Allocate(nof * 2,
+ USE_DEFAULT_MINIMUM_CAPACITY,
+ pretenure ? TENURED : NOT_TENURED);
if (!maybe_obj->ToObject(&obj)) return maybe_obj;
}
@@ -11185,7 +11227,9 @@
!GetHeap()->InNewSpace(this);
Object* obj;
{ MaybeObject* maybe_obj =
- Allocate(at_least_room_for, pretenure ? TENURED : NOT_TENURED);
+ Allocate(at_least_room_for,
+ USE_DEFAULT_MINIMUM_CAPACITY,
+ pretenure ? TENURED : NOT_TENURED);
if (!maybe_obj->ToObject(&obj)) return maybe_obj;
}
=======================================
--- /branches/bleeding_edge/src/objects.h Thu Jul 26 01:27:20 2012
+++ /branches/bleeding_edge/src/objects.h Mon Jul 30 06:09:09 2012
@@ -1731,7 +1731,7 @@
static int GetIdentityHash(Handle<JSObject> obj);
MUST_USE_RESULT MaybeObject* GetIdentityHash(CreationFlag flag);
- MUST_USE_RESULT MaybeObject* SetIdentityHash(Object* hash, CreationFlag
flag);
+ MUST_USE_RESULT MaybeObject* SetIdentityHash(Smi* hash, CreationFlag
flag);
static Handle<Object> DeleteProperty(Handle<JSObject> obj,
Handle<String> name);
@@ -2235,16 +2235,22 @@
Object* setter,
PropertyAttributes attributes);
- // Returns the hidden properties backing store object, currently
- // a StringDictionary, stored on this object.
- // If no hidden properties object has been put on this object,
- // return undefined, unless create_if_absent is true, in which case
- // a new dictionary is created, added to this object, and returned.
- MUST_USE_RESULT MaybeObject* GetHiddenPropertiesDictionary(
- bool create_if_absent);
- // Updates the existing hidden properties dictionary.
- MUST_USE_RESULT MaybeObject* SetHiddenPropertiesDictionary(
- StringDictionary* dictionary);
+
+ enum InitializeHiddenProperties {
+ CREATE_NEW_IF_ABSENT,
+ ONLY_RETURN_INLINE_VALUE
+ };
+
+ // If create_if_absent is true, return the hash table backing store
+ // for hidden properties. If there is no backing store, allocate one.
+ // If create_if_absent is false, return the hash table backing store
+ // or the inline stored identity hash, whatever is found.
+ MUST_USE_RESULT MaybeObject* GetHiddenPropertiesHashTable(
+ InitializeHiddenProperties init_option);
+ // Set the hidden property backing store to either a hash table or
+ // the inline-stored identity hash.
+ MUST_USE_RESULT MaybeObject* SetHiddenPropertiesHashTable(
+ Object* value);
DISALLOW_IMPLICIT_CONSTRUCTORS(JSObject);
};
@@ -2747,6 +2753,11 @@
template<typename Shape, typename Key>
class HashTable: public FixedArray {
public:
+ enum MinimumCapacity {
+ USE_DEFAULT_MINIMUM_CAPACITY,
+ USE_CUSTOM_MINIMUM_CAPACITY
+ };
+
// Wrapper methods
inline uint32_t Hash(Key key) {
if (Shape::UsesSeed) {
@@ -2799,6 +2810,7 @@
// Returns a new HashTable object. Might return Failure.
MUST_USE_RESULT static MaybeObject* Allocate(
int at_least_space_for,
+ MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY,
PretenureFlag pretenure = NOT_TENURED);
// Computes the required capacity for a table holding the given
=======================================
--- /branches/bleeding_edge/test/cctest/test-dictionary.cc Mon Jun 25
06:57:52 2012
+++ /branches/bleeding_edge/test/cctest/test-dictionary.cc Mon Jul 30
06:09:09 2012
@@ -105,6 +105,12 @@
LocalContext context;
Handle<ObjectHashSet> table = FACTORY->NewObjectHashSet(1);
Handle<JSObject> key = FACTORY->NewJSArray(0);
+ v8::Handle<v8::Object> key_obj = v8::Utils::ToLocal(key);
+
+ // Force allocation of hash table backing store for hidden properties.
+ key_obj->SetHiddenValue(v8_str("key 1"), v8_str("val 1"));
+ key_obj->SetHiddenValue(v8_str("key 2"), v8_str("val 2"));
+ key_obj->SetHiddenValue(v8_str("key 3"), v8_str("val 3"));
// Simulate a full heap so that generating an identity hash code
// in subsequent calls will request GC.
@@ -128,6 +134,12 @@
LocalContext context;
Handle<ObjectHashTable> table = FACTORY->NewObjectHashTable(1);
Handle<JSObject> key = FACTORY->NewJSArray(0);
+ v8::Handle<v8::Object> key_obj = v8::Utils::ToLocal(key);
+
+ // Force allocation of hash table backing store for hidden properties.
+ key_obj->SetHiddenValue(v8_str("key 1"), v8_str("val 1"));
+ key_obj->SetHiddenValue(v8_str("key 2"), v8_str("val 2"));
+ key_obj->SetHiddenValue(v8_str("key 3"), v8_str("val 3"));
// Simulate a full heap so that generating an identity hash code
// in subsequent calls will request GC.
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap-profiler.cc Thu Jul 5
09:23:14 2012
+++ /branches/bleeding_edge/test/cctest/test-heap-profiler.cc Mon Jul 30
06:09:09 2012
@@ -1468,7 +1468,7 @@
v8::Handle<v8::Value> cHandle = env->Global()->Get(v8::String::New("c"));
CHECK(!cHandle.IsEmpty() && cHandle->IsObject());
- cHandle->ToObject()->GetIdentityHash();
+ cHandle->ToObject()->SetHiddenValue(v8_str("key"), v8_str("val"));
snapshot = v8::HeapProfiler::TakeSnapshot(
v8_str("HiddenPropertiesFastCase2"));
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Mon Jul 30 06:07:29
2012
+++ /branches/bleeding_edge/test/cctest/test-heap.cc Mon Jul 30 06:09:09
2012
@@ -1985,3 +1985,39 @@
g->shared()->PrintLn();
}
#endif // OBJECT_PRINT
+
+
+TEST(Regress2211) {
+ InitializeVM();
+ v8::HandleScope scope;
+
+ v8::Handle<v8::String> value = v8_str("val string");
+ Smi* hash = Smi::FromInt(321);
+ Heap* heap = Isolate::Current()->heap();
+
+ for (int i = 0; i < 2; i++) {
+ // Store identity hash first and common hidden property second.
+ v8::Handle<v8::Object> obj = v8::Object::New();
+ Handle<JSObject> internal_obj = v8::Utils::OpenHandle(*obj);
+ CHECK(internal_obj->HasFastProperties());
+
+ // In the first iteration, set hidden value first and identity hash
second.
+ // In the second iteration, reverse the order.
+ if (i == 0) obj->SetHiddenValue(v8_str("key string"), value);
+ MaybeObject* maybe_obj = internal_obj->SetIdentityHash(hash,
+ ALLOW_CREATION);
+ CHECK(!maybe_obj->IsFailure());
+ if (i == 1) obj->SetHiddenValue(v8_str("key string"), value);
+
+ // Check values.
+ CHECK_EQ(hash,
+
internal_obj->GetHiddenProperty(heap->identity_hash_symbol()));
+ CHECK(value->Equals(obj->GetHiddenValue(v8_str("key string"))));
+
+ // Check size.
+ DescriptorArray* descriptors =
internal_obj->map()->instance_descriptors();
+ ObjectHashTable* hashtable = ObjectHashTable::cast(
+ internal_obj->FastPropertyAt(descriptors->GetFieldIndex(0)));
+ CHECK_LE(hashtable->SizeFor(hashtable->length()), 52);
+ }
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
