From 360e0204087c458db1d7e7635b38b7430bfde873 Mon Sep 17 00:00:00 2001 From: verwaest Date: Thu, 11 Jun 2015 13:17:25 -0700 Subject: [PATCH] Revert of Revert of Remove GetAttributes from the mix to avoid another virtual dispatch. (patchset #1 id:1 of https://codereview.chromium.org/1179933002/) Reason for revert: Reland, this didn't break anything. Original issue's description: > Revert of Remove GetAttributes from the mix to avoid another virtual dispatch. (patchset #2 id:40001 of https://codereview.chromium.org/1175973002/) > > Reason for revert: > It broke webkit_unit_tests > > Original issue's description: > > Remove GetAttributes from the mix to avoid another virtual dispatch. > > > > BUG=chromium:495949,v8:4137 > > LOG=n > > > > Committed: https://crrev.com/2269b8b5a696bf4eef13590093151bff624d4175 > > Cr-Commit-Position: refs/heads/master@{#28953} > > TBR=verwaest@chromium.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=chromium:495949,v8:4137 > > Committed: https://crrev.com/ae639d2ad646237e2f413259a0f116845ef96440 > Cr-Commit-Position: refs/heads/master@{#28958} TBR=ishell@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=chromium:495949,v8:4137 Review URL: https://codereview.chromium.org/1182603002 Cr-Commit-Position: refs/heads/master@{#28966} --- src/elements.cc | 118 ++++++++++++++----------------------------------------- src/elements.h | 16 +------- src/lookup-inl.h | 6 +-- 3 files changed, 33 insertions(+), 107 deletions(-) diff --git a/src/elements.cc b/src/elements.cc index 2734823..8cfb01b 100644 --- a/src/elements.cc +++ b/src/elements.cc @@ -583,15 +583,10 @@ class ElementsAccessorBase : public ElementsAccessor { ElementsAccessorSubclass::ValidateImpl(holder); } - static bool HasElementImpl(Handle holder, uint32_t key, - Handle backing_store) { - return ElementsAccessorSubclass::GetAttributesImpl( - *holder, key, *backing_store) != ABSENT; - } - virtual bool HasElement(Handle holder, uint32_t key, Handle backing_store) final { - return ElementsAccessorSubclass::HasElementImpl(holder, key, backing_store); + return ElementsAccessorSubclass::GetIndexForKeyImpl(*holder, *backing_store, + key) != kMaxUInt32; } virtual Handle Get(Handle holder, uint32_t key, @@ -633,20 +628,6 @@ class ElementsAccessorBase : public ElementsAccessor { obj, Handle::cast(backing_store), key, value); } - virtual PropertyAttributes GetAttributes( - JSObject* holder, uint32_t key, FixedArrayBase* backing_store) final { - return ElementsAccessorSubclass::GetAttributesImpl(holder, key, - backing_store); - } - - static PropertyAttributes GetAttributesImpl(JSObject* obj, uint32_t key, - FixedArrayBase* backing_store) { - if (key >= ElementsAccessorSubclass::GetCapacityImpl(obj, backing_store)) { - return ABSENT; - } - return BackingStore::cast(backing_store)->is_the_hole(key) ? ABSENT : NONE; - } - virtual MaybeHandle GetAccessorPair( Handle holder, uint32_t key, Handle backing_store) final { @@ -842,14 +823,21 @@ class ElementsAccessorBase : public ElementsAccessor { return ElementsAccessorSubclass::GetKeyForIndexImpl(backing_store, index); } - static uint32_t GetIndexForKeyImpl(FixedArrayBase* backing_store, + static uint32_t GetIndexForKeyImpl(JSObject* holder, + FixedArrayBase* backing_store, uint32_t key) { - return key; + return key < ElementsAccessorSubclass::GetCapacityImpl(holder, + backing_store) && + !BackingStore::cast(backing_store)->is_the_hole(key) + ? key + : kMaxUInt32; } - virtual uint32_t GetIndexForKey(FixedArrayBase* backing_store, + virtual uint32_t GetIndexForKey(JSObject* holder, + FixedArrayBase* backing_store, uint32_t key) final { - return ElementsAccessorSubclass::GetIndexForKeyImpl(backing_store, key); + return ElementsAccessorSubclass::GetIndexForKeyImpl(holder, backing_store, + key); } static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store, @@ -994,16 +982,6 @@ class FastElementsAccessor DeleteCommon(obj, key, language_mode); } - static bool HasElementImpl( - Handle holder, - uint32_t key, - Handle backing_store) { - if (key >= static_cast(backing_store->length())) { - return false; - } - return !Handle::cast(backing_store)->is_the_hole(key); - } - static bool HasIndexImpl(FixedArrayBase* backing_store, uint32_t index) { return !BackingStore::cast(backing_store)->is_the_hole(index); } @@ -1293,13 +1271,6 @@ class TypedElementsAccessor } } - static PropertyAttributes GetAttributesImpl(JSObject* obj, uint32_t key, - FixedArrayBase* backing_store) { - return key < AccessorClass::GetCapacityImpl(obj, backing_store) - ? DONT_DELETE - : ABSENT; - } - static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store, uint32_t index) { return PropertyDetails(DONT_DELETE, DATA, 0, PropertyCellType::kNoCell); @@ -1319,10 +1290,12 @@ class TypedElementsAccessor // External arrays always ignore deletes. } - static bool HasElementImpl(Handle holder, uint32_t key, - Handle backing_store) { - uint32_t capacity = AccessorClass::GetCapacityImpl(*holder, *backing_store); - return key < capacity; + static uint32_t GetIndexForKeyImpl(JSObject* holder, + FixedArrayBase* backing_store, + uint32_t key) { + return key < AccessorClass::GetCapacityImpl(holder, backing_store) + ? key + : kMaxUInt32; } static uint32_t GetCapacityImpl(JSObject* holder, @@ -1485,17 +1458,6 @@ class DictionaryElementsAccessor return value; } - static PropertyAttributes GetAttributesImpl(JSObject* obj, uint32_t key, - FixedArrayBase* backing_store) { - SeededNumberDictionary* dictionary = - SeededNumberDictionary::cast(backing_store); - int entry = dictionary->FindEntry(key); - if (entry != SeededNumberDictionary::kNotFound) { - return dictionary->DetailsAt(entry).attributes(); - } - return ABSENT; - } - static MaybeHandle GetAccessorPairImpl( Handle obj, uint32_t key, Handle store) { Handle backing_store = @@ -1509,13 +1471,6 @@ class DictionaryElementsAccessor return MaybeHandle(); } - static bool HasElementImpl(Handle holder, uint32_t key, - Handle store) { - Handle backing_store = - Handle::cast(store); - return backing_store->FindEntry(key) != SeededNumberDictionary::kNotFound; - } - static bool HasIndexImpl(FixedArrayBase* store, uint32_t index) { DisallowHeapAllocation no_gc; SeededNumberDictionary* dict = SeededNumberDictionary::cast(store); @@ -1530,14 +1485,14 @@ class DictionaryElementsAccessor return Smi::cast(key)->value(); } - static uint32_t GetIndexForKeyImpl(FixedArrayBase* store, uint32_t key) { + static uint32_t GetIndexForKeyImpl(JSObject* holder, FixedArrayBase* store, + uint32_t key) { DisallowHeapAllocation no_gc; SeededNumberDictionary* dict = SeededNumberDictionary::cast(store); int entry = dict->FindEntry(key); - if (entry == SeededNumberDictionary::kNotFound) { - return kMaxUInt32; - } - return static_cast(entry); + return entry == SeededNumberDictionary::kNotFound + ? kMaxUInt32 + : static_cast(entry); } static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store, @@ -1608,20 +1563,6 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase< ->Set(obj, key, arguments, value); } - static PropertyAttributes GetAttributesImpl(JSObject* obj, uint32_t key, - FixedArrayBase* backing_store) { - FixedArray* parameter_map = FixedArray::cast(backing_store); - Object* probe = GetParameterMapArg(parameter_map, key); - if (!probe->IsTheHole()) { - return NONE; - } else { - // If not aliased, check the arguments. - FixedArray* arguments = FixedArray::cast(parameter_map->get(1)); - return ElementsAccessor::ForArray(arguments) - ->GetAttributes(obj, key, arguments); - } - } - static MaybeHandle GetAccessorPairImpl( Handle obj, uint32_t key, Handle parameters) { Handle parameter_map = Handle::cast(parameters); @@ -1706,16 +1647,17 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase< return ForArray(arguments)->GetKeyForIndex(arguments, index - length); } - static uint32_t GetIndexForKeyImpl(FixedArrayBase* parameters, uint32_t key) { + static uint32_t GetIndexForKeyImpl(JSObject* holder, + FixedArrayBase* parameters, uint32_t key) { FixedArray* parameter_map = FixedArray::cast(parameters); Object* probe = GetParameterMapArg(parameter_map, key); if (!probe->IsTheHole()) return key; - uint32_t length = parameter_map->length() - 2; FixedArray* arguments = FixedArray::cast(parameter_map->get(1)); - return length + - ElementsAccessor::ForArray(arguments) - ->GetIndexForKey(arguments, key); + uint32_t index = ElementsAccessor::ForArray(arguments) + ->GetIndexForKey(holder, arguments, key); + if (index == kMaxUInt32) return index; + return (parameter_map->length() - 2) + index; } static PropertyDetails GetDetailsImpl(FixedArrayBase* parameters, diff --git a/src/elements.h b/src/elements.h index 60ff130..310891a 100644 --- a/src/elements.h +++ b/src/elements.h @@ -60,19 +60,6 @@ class ElementsAccessor { return Get(holder, key, handle(holder->elements())); } - // Returns an element's attributes, or ABSENT if there is no such - // element. This method doesn't iterate up the prototype chain. The caller - // can optionally pass in the backing store to use for the check, which must - // be compatible with the ElementsKind of the ElementsAccessor. If - // backing_store is NULL, the holder->elements() is used as the backing store. - virtual PropertyAttributes GetAttributes(JSObject* holder, uint32_t key, - FixedArrayBase* backing_store) = 0; - - inline PropertyAttributes GetAttributes(Handle holder, - uint32_t key) { - return GetAttributes(*holder, key, holder->elements()); - } - // Returns an element's accessors, or NULL if the element does not exist or // is plain. This method doesn't iterate up the prototype chain. The caller // can optionally pass in the backing store to use for the check, which must @@ -186,7 +173,8 @@ class ElementsAccessor { // the index to a key using the KeyAt method on the NumberDictionary. virtual uint32_t GetKeyForIndex(FixedArrayBase* backing_store, uint32_t index) = 0; - virtual uint32_t GetIndexForKey(FixedArrayBase* backing_store, + virtual uint32_t GetIndexForKey(JSObject* holder, + FixedArrayBase* backing_store, uint32_t key) = 0; virtual PropertyDetails GetDetails(FixedArrayBase* backing_store, uint32_t index) = 0; diff --git a/src/lookup-inl.h b/src/lookup-inl.h index e169e28..5fddcce 100644 --- a/src/lookup-inl.h +++ b/src/lookup-inl.h @@ -72,12 +72,8 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* const map, ElementsAccessor* accessor = js_object->GetElementsAccessor(); FixedArrayBase* backing_store = js_object->elements(); - number_ = accessor->GetIndexForKey(backing_store, index_); + number_ = accessor->GetIndexForKey(js_object, backing_store, index_); if (number_ == kMaxUInt32) return NOT_FOUND; - if (accessor->GetAttributes(js_object, index_, backing_store) == - ABSENT) { - return NOT_FOUND; - } property_details_ = accessor->GetDetails(backing_store, number_); } } else if (holder->IsGlobalObject()) { -- 2.7.4