Use entry rather than index in ElementsAccessor::Get

BUG=v8:4137, v8:4177
LOG=n

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

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

diff --git a/src/elements.cc b/src/elements.cc
index 226ed86..d5b3141 100644 (file)
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -548,30 +548,15 @@ class ElementsAccessorBase : public ElementsAccessor {
                *holder, *backing_store, index) != kMaxUInt32;
   }
 
-  virtual Handle<Object> Get(Handle<JSObject> holder, uint32_t index,
-                             Handle<FixedArrayBase> backing_store) final {
-    if (!IsExternalArrayElementsKind(ElementsTraits::Kind) &&
-        FLAG_trace_js_array_abuse) {
-      CheckArrayAbuse(holder, "elements read", index);
-    }
-
-    if (IsExternalArrayElementsKind(ElementsTraits::Kind) &&
-        FLAG_trace_external_array_abuse) {
-      CheckArrayAbuse(holder, "external elements read", index);
-    }
-
-    return ElementsAccessorSubclass::GetImpl(holder, index, backing_store);
+  virtual Handle<Object> Get(Handle<FixedArrayBase> backing_store,
+                             uint32_t entry) final {
+    return ElementsAccessorSubclass::GetImpl(backing_store, entry);
   }
 
-  static Handle<Object> GetImpl(Handle<JSObject> obj, uint32_t index,
-                                Handle<FixedArrayBase> backing_store) {
-    if (index <
-        ElementsAccessorSubclass::GetCapacityImpl(*obj, *backing_store)) {
-      return BackingStore::get(Handle<BackingStore>::cast(backing_store),
-                               index);
-    } else {
-      return backing_store->GetIsolate()->factory()->the_hole_value();
-    }
+  static Handle<Object> GetImpl(Handle<FixedArrayBase> backing_store,
+                                uint32_t entry) {
+    uint32_t index = GetIndexForEntryImpl(*backing_store, entry);
+    return BackingStore::get(Handle<BackingStore>::cast(backing_store), index);
   }
 
   virtual void Set(FixedArrayBase* backing_store, uint32_t entry,
@@ -758,10 +743,7 @@ class ElementsAccessorBase : public ElementsAccessor {
     uint32_t extra = 0;
     for (uint32_t y = 0; y < len1; y++) {
       if (ElementsAccessorSubclass::HasEntryImpl(*from, y)) {
-        uint32_t index =
-            ElementsAccessorSubclass::GetIndexForEntryImpl(*from, y);
-        Handle<Object> value =
-            ElementsAccessorSubclass::GetImpl(receiver, index, from);
+        Handle<Object> value = ElementsAccessorSubclass::GetImpl(from, y);
 
         DCHECK(!value->IsTheHole());
         DCHECK(!value->IsAccessorPair());
@@ -794,10 +776,7 @@ class ElementsAccessorBase : public ElementsAccessor {
     uint32_t entry = 0;
     for (uint32_t y = 0; y < len1; y++) {
       if (ElementsAccessorSubclass::HasEntryImpl(*from, y)) {
-        uint32_t index =
-            ElementsAccessorSubclass::GetIndexForEntryImpl(*from, y);
-        Handle<Object> value =
-            ElementsAccessorSubclass::GetImpl(receiver, index, from);
+        Handle<Object> value = ElementsAccessorSubclass::GetImpl(from, y);
         DCHECK(!value->IsAccessorPair());
         DCHECK(!value->IsExecutableAccessorInfo());
         if (filter == FixedArray::NON_SYMBOL_KEYS && value->IsSymbol()) {
@@ -834,11 +813,17 @@ class ElementsAccessorBase : public ElementsAccessor {
   static uint32_t GetEntryForIndexImpl(JSObject* holder,
                                        FixedArrayBase* backing_store,
                                        uint32_t index) {
-    return index < ElementsAccessorSubclass::GetCapacityImpl(holder,
-                                                             backing_store) &&
-                   !BackingStore::cast(backing_store)->is_the_hole(index)
-               ? index
-               : kMaxUInt32;
+    if (IsHoleyElementsKind(kind())) {
+      return index < ElementsAccessorSubclass::GetCapacityImpl(holder,
+                                                               backing_store) &&
+                     !BackingStore::cast(backing_store)->is_the_hole(index)
+                 ? index
+                 : kMaxUInt32;
+    } else {
+      Smi* smi_length = Smi::cast(JSArray::cast(holder)->length());
+      uint32_t length = static_cast<uint32_t>(smi_length->value());
+      return index < length ? index : kMaxUInt32;
+    }
   }
 
   virtual uint32_t GetEntryForIndex(JSObject* holder,
@@ -945,16 +930,11 @@ class DictionaryElementsAccessor
     obj->set_elements(*new_elements);
   }
 
-  static Handle<Object> GetImpl(Handle<JSObject> obj, uint32_t index,
-                                Handle<FixedArrayBase> store) {
+  static Handle<Object> GetImpl(Handle<FixedArrayBase> store, uint32_t entry) {
     Handle<SeededNumberDictionary> backing_store =
         Handle<SeededNumberDictionary>::cast(store);
     Isolate* isolate = backing_store->GetIsolate();
-    int entry = backing_store->FindEntry(index);
-    if (entry != SeededNumberDictionary::kNotFound) {
-      return handle(backing_store->ValueAt(entry), isolate);
-    }
-    return isolate->factory()->the_hole_value();
+    return handle(backing_store->ValueAt(entry), isolate);
   }
 
   static void SetImpl(FixedArrayBase* store, uint32_t entry, Object* value) {
@@ -1373,14 +1353,10 @@ class TypedElementsAccessor
   typedef typename ElementsKindTraits<Kind>::BackingStore BackingStore;
   typedef TypedElementsAccessor<Kind> AccessorClass;
 
-  static Handle<Object> GetImpl(Handle<JSObject> obj, uint32_t index,
-                                Handle<FixedArrayBase> backing_store) {
-    if (index < AccessorClass::GetCapacityImpl(*obj, *backing_store)) {
-      return BackingStore::get(Handle<BackingStore>::cast(backing_store),
-                               index);
-    } else {
-      return backing_store->GetIsolate()->factory()->undefined_value();
-    }
+  static Handle<Object> GetImpl(Handle<FixedArrayBase> backing_store,
+                                uint32_t entry) {
+    uint32_t index = GetIndexForEntryImpl(*backing_store, entry);
+    return BackingStore::get(Handle<BackingStore>::cast(backing_store), index);
   }
 
   static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store,
@@ -1398,6 +1374,11 @@ class TypedElementsAccessor
     UNREACHABLE();
   }
 
+  static uint32_t GetIndexForEntryImpl(FixedArrayBase* backing_store,
+                                       uint32_t entry) {
+    return entry;
+  }
+
   static uint32_t GetEntryForIndexImpl(JSObject* holder,
                                        FixedArrayBase* backing_store,
                                        uint32_t index) {
@@ -1439,24 +1420,28 @@ class SloppyArgumentsElementsAccessor
  public:
   explicit SloppyArgumentsElementsAccessor(const char* name)
       : ElementsAccessorBase<SloppyArgumentsElementsAccessorSubclass,
-                             KindTraits>(name) {}
+                             KindTraits>(name) {
+    USE(KindTraits::Kind);
+  }
 
-  static Handle<Object> GetImpl(Handle<JSObject> obj, uint32_t index,
-                                Handle<FixedArrayBase> parameters) {
-    Isolate* isolate = obj->GetIsolate();
+  static Handle<Object> GetImpl(Handle<FixedArrayBase> parameters,
+                                uint32_t entry) {
+    Isolate* isolate = parameters->GetIsolate();
     Handle<FixedArray> parameter_map = Handle<FixedArray>::cast(parameters);
-    Handle<Object> probe(GetParameterMapArg(*parameter_map, index), isolate);
-    if (!probe->IsTheHole()) {
+    uint32_t length = parameter_map->length() - 2;
+    if (entry < length) {
       DisallowHeapAllocation no_gc;
+      Object* probe = parameter_map->get(entry + 2);
       Context* context = Context::cast(parameter_map->get(0));
-      int context_entry = Handle<Smi>::cast(probe)->value();
+      int context_entry = Smi::cast(probe)->value();
       DCHECK(!context->get(context_entry)->IsTheHole());
       return handle(context->get(context_entry), isolate);
     } else {
       // Object is not mapped, defer to the arguments.
       Handle<FixedArray> arguments(FixedArray::cast(parameter_map->get(1)),
                                    isolate);
-      Handle<Object> result = ArgumentsAccessor::GetImpl(obj, index, arguments);
+      Handle<Object> result =
+          ArgumentsAccessor::GetImpl(arguments, entry - length);
       // Elements of the arguments object in slow mode might be slow aliases.
       if (result->IsAliasedArgumentsEntry()) {
         DisallowHeapAllocation no_gc;
@@ -1465,9 +1450,8 @@ class SloppyArgumentsElementsAccessor
         int context_entry = entry->aliased_context_slot();
         DCHECK(!context->get(context_entry)->IsTheHole());
         return handle(context->get(context_entry), isolate);
-      } else {
-        return result;
       }
+      return result;
     }
   }
 

diff --git a/src/elements.h b/src/elements.h
index 584f1b5..511b636 100644 (file)
--- a/src/elements.h
+++ b/src/elements.h
@@ -38,17 +38,8 @@ class ElementsAccessor {
     return HasElement(holder, index, handle(holder->elements()));
   }
 
-  // Returns the element with the specified index or undefined 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 Handle<Object> Get(Handle<JSObject> holder, uint32_t index,
-                             Handle<FixedArrayBase> backing_store) = 0;
-
-  inline Handle<Object> Get(Handle<JSObject> holder, uint32_t index) {
-    return Get(holder, index, handle(holder->elements()));
-  }
+  virtual Handle<Object> Get(Handle<FixedArrayBase> backing_store,
+                             uint32_t entry) = 0;
 
   // Modifies the length data property as specified for JSArrays and resizes the
   // underlying backing store accordingly. The method honors the semantics of

diff --git a/src/lookup.cc b/src/lookup.cc
index eca4419..d731a7c 100644 (file)
--- a/src/lookup.cc
+++ b/src/lookup.cc
@@ -340,7 +340,7 @@ Handle<Object> LookupIterator::FetchValue() const {
     }
 
     ElementsAccessor* accessor = holder->GetElementsAccessor();
-    return accessor->Get(holder, index_);
+    return accessor->Get(handle(holder->elements()), number_);
   } else if (holder_map_->IsGlobalObjectMap()) {
     result = holder->global_dictionary()->ValueAt(number_);
     DCHECK(result->IsPropertyCell());