Revert of Remove GetAttributes from the mix to avoid another virtual dispatch. (patch...

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

Review URL: https://codereview.chromium.org/1179933002

Cr-Commit-Position: refs/heads/master@{#28958}

diff --git a/src/elements.cc b/src/elements.cc
index 8cfb01b525c17a0de35438c0e2cae498d00fd78f..2734823852896d9379bcd870f10c32cb263de373 100644 (file)
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -583,10 +583,15 @@ class ElementsAccessorBase : public ElementsAccessor {
     ElementsAccessorSubclass::ValidateImpl(holder);
   }
 
+  static bool HasElementImpl(Handle<JSObject> holder, uint32_t key,
+                             Handle<FixedArrayBase> backing_store) {
+    return ElementsAccessorSubclass::GetAttributesImpl(
+               *holder, key, *backing_store) != ABSENT;
+  }
+
   virtual bool HasElement(Handle<JSObject> holder, uint32_t key,
                           Handle<FixedArrayBase> backing_store) final {
-    return ElementsAccessorSubclass::GetIndexForKeyImpl(*holder, *backing_store,
-                                                        key) != kMaxUInt32;
+    return ElementsAccessorSubclass::HasElementImpl(holder, key, backing_store);
   }
 
   virtual Handle<Object> Get(Handle<JSObject> holder, uint32_t key,
@@ -628,6 +633,20 @@ class ElementsAccessorBase : public ElementsAccessor {
         obj, Handle<BackingStore>::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<AccessorPair> GetAccessorPair(
       Handle<JSObject> holder, uint32_t key,
       Handle<FixedArrayBase> backing_store) final {
@@ -823,21 +842,14 @@ class ElementsAccessorBase : public ElementsAccessor {
     return ElementsAccessorSubclass::GetKeyForIndexImpl(backing_store, index);
   }
 
-  static uint32_t GetIndexForKeyImpl(JSObject* holder,
-                                     FixedArrayBase* backing_store,
+  static uint32_t GetIndexForKeyImpl(FixedArrayBase* backing_store,
                                      uint32_t key) {
-    return key < ElementsAccessorSubclass::GetCapacityImpl(holder,
-                                                           backing_store) &&
-                   !BackingStore::cast(backing_store)->is_the_hole(key)
-               ? key
-               : kMaxUInt32;
+    return key;
   }
 
-  virtual uint32_t GetIndexForKey(JSObject* holder,
-                                  FixedArrayBase* backing_store,
+  virtual uint32_t GetIndexForKey(FixedArrayBase* backing_store,
                                   uint32_t key) final {
-    return ElementsAccessorSubclass::GetIndexForKeyImpl(holder, backing_store,
-                                                        key);
+    return ElementsAccessorSubclass::GetIndexForKeyImpl(backing_store, key);
   }
 
   static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store,
@@ -982,6 +994,16 @@ class FastElementsAccessor
     DeleteCommon(obj, key, language_mode);
   }
 
+  static bool HasElementImpl(
+      Handle<JSObject> holder,
+      uint32_t key,
+      Handle<FixedArrayBase> backing_store) {
+    if (key >= static_cast<uint32_t>(backing_store->length())) {
+      return false;
+    }
+    return !Handle<BackingStore>::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);
   }
@@ -1271,6 +1293,13 @@ 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);
@@ -1290,12 +1319,10 @@ class TypedElementsAccessor
     // External arrays always ignore deletes.
   }
 
-  static uint32_t GetIndexForKeyImpl(JSObject* holder,
-                                     FixedArrayBase* backing_store,
-                                     uint32_t key) {
-    return key < AccessorClass::GetCapacityImpl(holder, backing_store)
-               ? key
-               : kMaxUInt32;
+  static bool HasElementImpl(Handle<JSObject> holder, uint32_t key,
+                             Handle<FixedArrayBase> backing_store) {
+    uint32_t capacity = AccessorClass::GetCapacityImpl(*holder, *backing_store);
+    return key < capacity;
   }
 
   static uint32_t GetCapacityImpl(JSObject* holder,
@@ -1458,6 +1485,17 @@ 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<AccessorPair> GetAccessorPairImpl(
       Handle<JSObject> obj, uint32_t key, Handle<FixedArrayBase> store) {
     Handle<SeededNumberDictionary> backing_store =
@@ -1471,6 +1509,13 @@ class DictionaryElementsAccessor
     return MaybeHandle<AccessorPair>();
   }
 
+  static bool HasElementImpl(Handle<JSObject> holder, uint32_t key,
+                             Handle<FixedArrayBase> store) {
+    Handle<SeededNumberDictionary> backing_store =
+        Handle<SeededNumberDictionary>::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);
@@ -1485,14 +1530,14 @@ class DictionaryElementsAccessor
     return Smi::cast(key)->value();
   }
 
-  static uint32_t GetIndexForKeyImpl(JSObject* holder, FixedArrayBase* store,
-                                     uint32_t key) {
+  static uint32_t GetIndexForKeyImpl(FixedArrayBase* store, uint32_t key) {
     DisallowHeapAllocation no_gc;
     SeededNumberDictionary* dict = SeededNumberDictionary::cast(store);
     int entry = dict->FindEntry(key);
-    return entry == SeededNumberDictionary::kNotFound
-               ? kMaxUInt32
-               : static_cast<uint32_t>(entry);
+    if (entry == SeededNumberDictionary::kNotFound) {
+      return kMaxUInt32;
+    }
+    return static_cast<uint32_t>(entry);
   }
 
   static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store,
@@ -1563,6 +1608,20 @@ 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<AccessorPair> GetAccessorPairImpl(
       Handle<JSObject> obj, uint32_t key, Handle<FixedArrayBase> parameters) {
     Handle<FixedArray> parameter_map = Handle<FixedArray>::cast(parameters);
@@ -1647,17 +1706,16 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase<
     return ForArray(arguments)->GetKeyForIndex(arguments, index - length);
   }
 
-  static uint32_t GetIndexForKeyImpl(JSObject* holder,
-                                     FixedArrayBase* parameters, uint32_t key) {
+  static uint32_t GetIndexForKeyImpl(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));
-    uint32_t index = ElementsAccessor::ForArray(arguments)
-                         ->GetIndexForKey(holder, arguments, key);
-    if (index == kMaxUInt32) return index;
-    return (parameter_map->length() - 2) + index;
+    return length +
+           ElementsAccessor::ForArray(arguments)
+               ->GetIndexForKey(arguments, key);
   }
 
   static PropertyDetails GetDetailsImpl(FixedArrayBase* parameters,

diff --git a/src/elements.h b/src/elements.h
index 310891ab1e030be14f8413dcef21680f4471a2cd..60ff130d2998ce59089567e218ee2716de6cae9b 100644 (file)
--- a/src/elements.h
+++ b/src/elements.h
@@ -60,6 +60,19 @@ 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<JSObject> 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
@@ -173,8 +186,7 @@ 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(JSObject* holder,
-                                  FixedArrayBase* backing_store,
+  virtual uint32_t GetIndexForKey(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 5fddccec2bc5bf18aeca4316733634beef972f10..e169e288caf7b0d4af05ffc60b3665cc7dd3344d 100644 (file)
--- a/src/lookup-inl.h
+++ b/src/lookup-inl.h
@@ -72,8 +72,12 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* const map,
 
           ElementsAccessor* accessor = js_object->GetElementsAccessor();
           FixedArrayBase* backing_store = js_object->elements();
-          number_ = accessor->GetIndexForKey(js_object, backing_store, index_);
+          number_ = accessor->GetIndexForKey(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()) {