From 8e31371d0ee5d7cf8038c7afdecde759166c7911 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Tue, 4 Oct 2011 07:45:25 +0000 Subject: [PATCH] Move logic for hidden properties into the JSObject. Previously, the logic using the hidden properties backing object was spread accross use sites. Now it's all contained in JSObject, with only simple accessors available. Also change the backing object to be a StringDictionary rather than a JSObject. There's still room for improvement by making a hash-table that don't store property details as well. Review URL: http://codereview.chromium.org/8050013 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9510 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 42 ++------ src/handles.cc | 6 +- src/handles.h | 10 +- src/objects-inl.h | 36 ------- src/objects.cc | 254 ++++++++++++++++++++++++++++++++-------------- src/objects.h | 45 ++++---- 6 files changed, 217 insertions(+), 176 deletions(-) diff --git a/src/api.cc b/src/api.cc index 7ae01d14e4..ee06456516 100644 --- a/src/api.cc +++ b/src/api.cc @@ -3210,21 +3210,10 @@ bool v8::Object::SetHiddenValue(v8::Handle key, ENTER_V8(isolate); i::HandleScope scope(isolate); i::Handle self = Utils::OpenHandle(this); - i::Handle hidden_props(i::GetHiddenProperties( - self, - i::ALLOW_CREATION)); - i::Handle key_obj = Utils::OpenHandle(*key); + i::Handle key_obj = Utils::OpenHandle(*key); i::Handle value_obj = Utils::OpenHandle(*value); - EXCEPTION_PREAMBLE(isolate); - i::Handle obj = i::SetProperty( - hidden_props, - key_obj, - value_obj, - static_cast(None), - i::kNonStrictMode); - has_pending_exception = obj.is_null(); - EXCEPTION_BAILOUT_CHECK(isolate, false); - return true; + i::Handle result = i::SetHiddenProperty(self, key_obj, value_obj); + return *result == *self; } @@ -3234,20 +3223,9 @@ v8::Local v8::Object::GetHiddenValue(v8::Handle key) { return Local()); ENTER_V8(isolate); i::Handle self = Utils::OpenHandle(this); - i::Handle hidden_props(i::GetHiddenProperties( - self, - i::OMIT_CREATION)); - if (hidden_props->IsUndefined()) { - return v8::Local(); - } i::Handle key_obj = Utils::OpenHandle(*key); - EXCEPTION_PREAMBLE(isolate); - i::Handle result = i::GetProperty(hidden_props, key_obj); - has_pending_exception = result.is_null(); - EXCEPTION_BAILOUT_CHECK(isolate, v8::Local()); - if (result->IsUndefined()) { - return v8::Local(); - } + i::Handle result(self->GetHiddenProperty(*key_obj)); + if (result->IsUndefined()) return v8::Local(); return Utils::ToLocal(result); } @@ -3258,15 +3236,9 @@ bool v8::Object::DeleteHiddenValue(v8::Handle key) { ENTER_V8(isolate); i::HandleScope scope(isolate); i::Handle self = Utils::OpenHandle(this); - i::Handle hidden_props(i::GetHiddenProperties( - self, - i::OMIT_CREATION)); - if (hidden_props->IsUndefined()) { - return true; - } - i::Handle js_obj(i::JSObject::cast(*hidden_props)); i::Handle key_obj = Utils::OpenHandle(*key); - return i::DeleteProperty(js_obj, key_obj)->IsTrue(); + self->DeleteHiddenProperty(*key_obj); + return true; } diff --git a/src/handles.cc b/src/handles.cc index ce4258d618..d3fd12635b 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -421,9 +421,11 @@ Handle PreventExtensions(Handle object) { } -Handle GetHiddenProperties(Handle obj, CreationFlag flag) { +Handle SetHiddenProperty(Handle obj, + Handle key, + Handle value) { CALL_HEAP_FUNCTION(obj->GetIsolate(), - obj->GetHiddenProperties(flag), + obj->SetHiddenProperty(*key, *value), Object); } diff --git a/src/handles.h b/src/handles.h index 8dcca3eab8..31be71459a 100644 --- a/src/handles.h +++ b/src/handles.h @@ -265,11 +265,11 @@ Handle GetPrototype(Handle obj); Handle SetPrototype(Handle obj, Handle value); -// Return the object's hidden properties object. If the object has no hidden -// properties and HiddenPropertiesFlag::ALLOW_CREATION is passed, then a new -// hidden property object will be allocated. Otherwise Heap::undefined_value -// is returned. -Handle GetHiddenProperties(Handle obj, CreationFlag flag); +// Sets a hidden property on an object. Returns obj on success, undefined +// if trying to set the property on a detached proxy. +Handle SetHiddenProperty(Handle obj, + Handle key, + Handle value); int GetIdentityHash(Handle obj); diff --git a/src/objects-inl.h b/src/objects-inl.h index baf271fd94..cd78471ce7 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -4347,42 +4347,6 @@ MaybeObject* JSReceiver::GetIdentityHash(CreationFlag flag) { } -bool JSObject::HasHiddenPropertiesObject() { - ASSERT(!IsJSGlobalProxy()); - return GetPropertyAttributePostInterceptor(this, - GetHeap()->hidden_symbol(), - false) != ABSENT; -} - - -Object* JSObject::GetHiddenPropertiesObject() { - ASSERT(!IsJSGlobalProxy()); - 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* result = - GetLocalPropertyPostInterceptor(this, - GetHeap()->hidden_symbol(), - &attributes)->ToObjectUnchecked(); - return result; -} - - -MaybeObject* JSObject::SetHiddenPropertiesObject(Object* hidden_obj) { - ASSERT(!IsJSGlobalProxy()); - return SetPropertyPostInterceptor(GetHeap()->hidden_symbol(), - hidden_obj, - DONT_ENUM, - kNonStrictMode); -} - - -bool JSObject::HasHiddenProperties() { - return !GetHiddenProperties(OMIT_CREATION)->ToObjectChecked()->IsUndefined(); -} - - bool JSReceiver::HasElement(uint32_t index) { if (IsJSProxy()) { return JSProxy::cast(this)->HasElementWithHandler(index); diff --git a/src/objects.cc b/src/objects.cc index 45ae49d222..f2e39523a3 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3279,53 +3279,6 @@ MaybeObject* JSObject::NormalizeElements() { } -MaybeObject* JSObject::GetHiddenProperties(CreationFlag flag) { - Isolate* isolate = GetIsolate(); - Heap* heap = isolate->heap(); - Object* holder = BypassGlobalProxy(); - if (holder->IsUndefined()) return heap->undefined_value(); - JSObject* obj = JSObject::cast(holder); - if (obj->HasFastProperties()) { - // If the object has fast properties, check whether the first slot - // in the descriptor array matches the hidden symbol. Since the - // hidden symbols hash code is zero (and no other string has hash - // code zero) it will always occupy the first entry if present. - DescriptorArray* descriptors = obj->map()->instance_descriptors(); - if ((descriptors->number_of_descriptors() > 0) && - (descriptors->GetKey(0) == heap->hidden_symbol()) && - descriptors->IsProperty(0)) { - ASSERT(descriptors->GetType(0) == FIELD); - return obj->FastPropertyAt(descriptors->GetFieldIndex(0)); - } - } - - // Only attempt to find the hidden properties in the local object and not - // in the prototype chain. - if (!obj->HasHiddenPropertiesObject()) { - // Hidden properties object not found. Allocate a new hidden properties - // object if requested. Otherwise return the undefined value. - if (flag == ALLOW_CREATION) { - Object* hidden_obj; - { MaybeObject* maybe_obj = heap->AllocateJSObject( - isolate->context()->global_context()->object_function()); - if (!maybe_obj->ToObject(&hidden_obj)) return maybe_obj; - } - // Don't allow leakage of the hidden object through accessors - // on Object.prototype. - { - MaybeObject* maybe_obj = - JSObject::cast(hidden_obj)->SetPrototype(heap->null_value(), false); - if (maybe_obj->IsFailure()) return maybe_obj; - } - return obj->SetHiddenPropertiesObject(hidden_obj); - } else { - return heap->undefined_value(); - } - } - return obj->GetHiddenPropertiesObject(); -} - - Smi* JSReceiver::GenerateIdentityHash() { Isolate* isolate = GetIsolate(); @@ -3344,46 +3297,24 @@ Smi* JSReceiver::GenerateIdentityHash() { MaybeObject* JSObject::SetIdentityHash(Object* hash, CreationFlag flag) { - JSObject* hidden_props; - MaybeObject* maybe = GetHiddenProperties(flag); - if (!maybe->To(&hidden_props)) return maybe; - maybe = hidden_props->SetLocalPropertyIgnoreAttributes( - GetHeap()->identity_hash_symbol(), hash, NONE); + MaybeObject* maybe = SetHiddenProperty(GetHeap()->identity_hash_symbol(), + hash); if (maybe->IsFailure()) return maybe; return this; } MaybeObject* JSObject::GetIdentityHash(CreationFlag flag) { - Isolate* isolate = GetIsolate(); - Object* hidden_props_obj; - { MaybeObject* maybe_obj = GetHiddenProperties(flag); - if (!maybe_obj->ToObject(&hidden_props_obj)) return maybe_obj; - } - if (!hidden_props_obj->IsJSObject()) { - // We failed to create hidden properties. That's a detached - // global proxy. - ASSERT(hidden_props_obj->IsUndefined()); - return Smi::FromInt(0); - } - JSObject* hidden_props = JSObject::cast(hidden_props_obj); - String* hash_symbol = isolate->heap()->identity_hash_symbol(); - { - // Note that HasLocalProperty() can cause a GC in the general case in the - // presence of interceptors. - AssertNoAllocation no_alloc; - if (hidden_props->HasLocalProperty(hash_symbol)) { - MaybeObject* hash = hidden_props->GetProperty(hash_symbol); - return Smi::cast(hash->ToObjectChecked()); - } - } + Object* stored_value = GetHiddenProperty(GetHeap()->identity_hash_symbol()); + if (stored_value->IsSmi()) return stored_value; Smi* hash = GenerateIdentityHash(); - { MaybeObject* result = hidden_props->SetLocalPropertyIgnoreAttributes( - hash_symbol, - hash, - static_cast(None)); - if (result->IsFailure()) return result; + MaybeObject* result = SetHiddenProperty(GetHeap()->identity_hash_symbol(), + hash); + if (result->IsFailure()) return result; + if (result->ToObjectUnchecked()->IsUndefined()) { + // Trying to get hash of detached proxy. + return Smi::FromInt(0); } return hash; } @@ -3399,6 +3330,171 @@ MaybeObject* JSProxy::GetIdentityHash(CreationFlag flag) { } +Object* JSObject::GetHiddenProperty(String* key) { + if (IsJSGlobalProxy()) { + // For a proxy, use the prototype as target object. + Object* proxy_parent = GetPrototype(); + // If the proxy is detached, return undefined. + if (proxy_parent->IsNull()) return GetHeap()->undefined_value(); + ASSERT(proxy_parent->IsJSGlobalObject()); + return JSObject::cast(proxy_parent)->GetHiddenProperty(key); + } + ASSERT(!IsJSGlobalProxy()); + MaybeObject* hidden_lookup = GetHiddenPropertiesDictionary(false); + 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); +} + + +MaybeObject* JSObject::SetHiddenProperty(String* key, Object* value) { + if (IsJSGlobalProxy()) { + // For a proxy, use the prototype as target object. + Object* proxy_parent = GetPrototype(); + // If the proxy is detached, return undefined. + if (proxy_parent->IsNull()) return GetHeap()->undefined_value(); + ASSERT(proxy_parent->IsJSGlobalObject()); + return JSObject::cast(proxy_parent)->SetHiddenProperty(key, value); + } + ASSERT(!IsJSGlobalProxy()); + MaybeObject* hidden_lookup = GetHiddenPropertiesDictionary(true); + StringDictionary* dictionary; + if (!hidden_lookup->To(&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(&new_dict)) return insert_result; + if (new_dict != dictionary) { + // 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); + if (store_result->IsFailure()) return store_result; + } + // Return this to mark success. + return this; +} + + +void JSObject::DeleteHiddenProperty(String* key) { + if (IsJSGlobalProxy()) { + // For a proxy, use the prototype as target object. + Object* proxy_parent = GetPrototype(); + // If the proxy is detached, return immediately. + if (proxy_parent->IsNull()) return; + ASSERT(proxy_parent->IsJSGlobalObject()); + JSObject::cast(proxy_parent)->DeleteHiddenProperty(key); + return; + } + MaybeObject* hidden_lookup = GetHiddenPropertiesDictionary(false); + 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); +} + + +bool JSObject::HasHiddenProperties() { + LookupResult lookup; + LocalLookupRealNamedProperty(GetHeap()->hidden_symbol(), &lookup); + return lookup.IsFound(); +} + + +MaybeObject* JSObject::GetHiddenPropertiesDictionary(bool create_if_absent) { + ASSERT(!IsJSGlobalProxy()); + if (HasFastProperties()) { + // If the object has fast properties, check whether the first slot + // in the descriptor array matches the hidden symbol. Since the + // hidden symbols hash code is zero (and no other string has hash + // code zero) it will always occupy the first entry if present. + DescriptorArray* descriptors = this->map()->instance_descriptors(); + if ((descriptors->number_of_descriptors() > 0) && + (descriptors->GetKey(0) == GetHeap()->hidden_symbol()) && + descriptors->IsProperty(0)) { + ASSERT(descriptors->GetType(0) == FIELD); + Object* hidden_store = + this->FastPropertyAt(descriptors->GetFieldIndex(0)); + return StringDictionary::cast(hidden_store); + } + } 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 = + 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(&dictionary)) return dict_alloc; + MaybeObject* store_result = + SetPropertyPostInterceptor(GetHeap()->hidden_symbol(), + dictionary, + DONT_ENUM, + kNonStrictMode); + if (store_result->IsFailure()) return store_result; + return dictionary; +} + + +MaybeObject* JSObject::SetHiddenPropertiesDictionary( + StringDictionary* dictionary) { + ASSERT(!IsJSGlobalProxy()); + ASSERT(HasHiddenProperties()); + if (HasFastProperties()) { + // If the object has fast properties, check whether the first slot + // in the descriptor array matches the hidden symbol. Since the + // hidden symbols hash code is zero (and no other string has hash + // code zero) it will always occupy the first entry if present. + DescriptorArray* descriptors = this->map()->instance_descriptors(); + if ((descriptors->number_of_descriptors() > 0) && + (descriptors->GetKey(0) == GetHeap()->hidden_symbol()) && + descriptors->IsProperty(0)) { + ASSERT(descriptors->GetType(0) == FIELD); + this->FastPropertyAtPut(descriptors->GetFieldIndex(0), dictionary); + return this; + } + } + MaybeObject* store_result = + SetPropertyPostInterceptor(GetHeap()->hidden_symbol(), + dictionary, + DONT_ENUM, + kNonStrictMode); + if (store_result->IsFailure()) return store_result; + return this; +} + + MaybeObject* JSObject::DeletePropertyPostInterceptor(String* name, DeleteMode mode) { // Check local property, ignore interceptor. diff --git a/src/objects.h b/src/objects.h index 5a1a4a3804..7b522e04a3 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1577,27 +1577,25 @@ class JSObject: public JSReceiver { // Accessors for hidden properties object. // // Hidden properties are not local properties of the object itself. - // Instead they are stored on an auxiliary JSObject stored as a local + // Instead they are stored in an auxiliary structure kept as a local // property with a special name Heap::hidden_symbol(). But if the // receiver is a JSGlobalProxy then the auxiliary object is a property - // of its prototype. - // - // Has/Get/SetHiddenPropertiesObject methods don't allow the holder to be - // a JSGlobalProxy. Use BypassGlobalProxy method above to get to the real - // holder. - // - // These accessors do not touch interceptors or accessors. - inline bool HasHiddenPropertiesObject(); - inline Object* GetHiddenPropertiesObject(); - MUST_USE_RESULT inline MaybeObject* SetHiddenPropertiesObject( - Object* hidden_obj); + // of its prototype, and if it's a detached proxy, then you can't have + // hidden properties. - // Retrieves the hidden properties object. - // - // The undefined value might be returned in case no hidden properties object - // is present and creation was omitted. - inline bool HasHiddenProperties(); - MUST_USE_RESULT MaybeObject* GetHiddenProperties(CreationFlag flag); + // Sets a hidden property on this object. Returns this object if successful, + // undefined if called on a detached proxy, and a failure if a GC + // is required + MaybeObject* SetHiddenProperty(String* key, Object* value); + // Gets the value of a hidden property with the given key. Returns undefined + // if the property doesn't exist (or if called on a detached proxy), + // otherwise returns the value set for the key. + Object* GetHiddenProperty(String* key); + // Deletes a hidden property. Deleting a non-existing property is + // considered successful. + void DeleteHiddenProperty(String* key); + // Returns true if the object has a property with the hidden symbol as name. + bool HasHiddenProperties(); MUST_USE_RESULT MaybeObject* GetIdentityHash(CreationFlag flag); MUST_USE_RESULT MaybeObject* SetIdentityHash(Object* hash, CreationFlag flag); @@ -2038,6 +2036,15 @@ class JSObject: public JSReceiver { void LookupInDescriptor(String* name, LookupResult* result); + // 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. + MaybeObject* GetHiddenPropertiesDictionary(bool create_if_absent); + // Updates the existing hidden properties dictionary. + MaybeObject* SetHiddenPropertiesDictionary(StringDictionary* dictionary); + DISALLOW_IMPLICIT_CONSTRUCTORS(JSObject); }; @@ -2863,7 +2870,7 @@ class StringDictionary: public Dictionary { JSObject* obj, int unused_property_fields); - // Find entry for key otherwise return kNotFound. Optimzed version of + // Find entry for key, otherwise return kNotFound. Optimized version of // HashTable::FindEntry. int FindEntry(String* key); };