Reland of [runtime] Do not use the enum-cache for non-prototype objects.

src/objects.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/objects.h"
 
 #include <cmath>
 #include <iomanip>
 #include <sstream>
 
@@ skipping 8080 matching lines @@
           }
         }
       }
     } else {
       // Only deep copy fields from the object literal expression.
       // In particular, don't try to copy the length attribute of
       // an array.
       PropertyFilter filter = static_cast<PropertyFilter>(
           ONLY_WRITABLE | ONLY_ENUMERABLE | ONLY_CONFIGURABLE);
       KeyAccumulator accumulator(isolate, filter);
-      accumulator.NextPrototype();
-      copy->CollectOwnPropertyNames(&accumulator, filter);
+      accumulator.Prepare();
+      copy->CollectOwnPropertyKeys(&accumulator, filter);
       Handle<FixedArray> names = accumulator.GetKeys();
       for (int i = 0; i < names->length(); i++) {
         DCHECK(names->get(i)->IsName());
         Handle<Name> name(Name::cast(names->get(i)));
         Handle<Object> value =
             Object::GetProperty(copy, name).ToHandleChecked();
         if (value->IsJSObject()) {
           Handle<JSObject> result;
           ASSIGN_RETURN_ON_EXCEPTION(
               isolate, result,
@@ skipping 318 matching lines @@
 
   Handle<FixedArray> new_array =
       array->GetIsolate()->factory()->NewFixedArray(length);
   for (int i = 0; i < length; ++i) new_array->set(i, array->get(i));
   return new_array;
 }
 
 
 namespace {
 
-Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
-                                           Handle<JSObject> object,
+Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate, JSObject* object,
                                            bool cache_enum_length) {
-  Handle<Map> map(object->map());
+  Handle<Map> map(object->map(), isolate);
   Handle<DescriptorArray> descs =
       Handle<DescriptorArray>(map->instance_descriptors(), isolate);
   int own_property_count = map->EnumLength();
   // If the enum length of the given map is set to kInvalidEnumCache, this
   // means that the map itself has never used the present enum cache. The
   // first step to using the cache is to set the enum length of the map by
   // counting the number of own descriptors that are ENUMERABLE_STRINGS.
   if (own_property_count == kInvalidEnumCacheSentinel) {
     own_property_count =
         map->NumberOfDescribedProperties(OWN_DESCRIPTORS, ENUMERABLE_STRINGS);
@@ skipping 53 matching lines @@
   DescriptorArray::SetEnumCache(descs, isolate, storage, indices);
   if (cache_enum_length) {
     map->SetEnumLength(own_property_count);
   }
   return storage;
 }
 
 }  // namespace
 
 
-Handle<FixedArray> JSObject::GetEnumPropertyKeys(Handle<JSObject> object,
-                                                 bool cache_enum_length) {
-  Isolate* isolate = object->GetIsolate();
-  if (object->HasFastProperties()) {
-    return GetFastEnumPropertyKeys(isolate, object, cache_enum_length);
-  } else if (object->IsJSGlobalObject()) {
-    Handle<GlobalDictionary> dictionary(object->global_dictionary());
-    int length = dictionary->NumberOfEnumElements();
-    if (length == 0) {
-      return Handle<FixedArray>(isolate->heap()->empty_fixed_array());
-    }
-    Handle<FixedArray> storage = isolate->factory()->NewFixedArray(length);
-    dictionary->CopyEnumKeysTo(*storage);
-    return storage;
-  } else {
-    Handle<NameDictionary> dictionary(object->property_dictionary());
-    int length = dictionary->NumberOfEnumElements();
-    if (length == 0) {
-      return Handle<FixedArray>(isolate->heap()->empty_fixed_array());
-    }
-    Handle<FixedArray> storage = isolate->factory()->NewFixedArray(length);
-    dictionary->CopyEnumKeysTo(*storage);
-    return storage;
-  }
+Handle<FixedArray> JSObject::GetOwnEnumPropertyKeys(Handle<JSObject> object,
+                                                    bool cache_enum_length) {
+  PropertyFilter filter = PropertyFilter::ENUMERABLE_STRINGS;
+  KeyAccumulator keys(object->GetIsolate(), filter);
+  keys.Prepare();
+  object->CollectOwnPropertyKeys(&keys, filter);
+  return keys.GetKeys();
 }
 
 
 enum IndexedOrNamed { kIndexed, kNamed };
 
 
 // Returns |true| on success, |nothing| on exception.
 template <class Callback, IndexedOrNamed type>
 static Maybe<bool> GetKeysFromInterceptor(Isolate* isolate,
                                           Handle<JSReceiver> receiver,
@@ skipping 63 matching lines @@
   }
 
   JSObject::CollectOwnElementKeys(object, accumulator, *filter);
 
   // Add the element keys from the interceptor.
   Maybe<bool> success =
       GetKeysFromInterceptor<v8::IndexedPropertyEnumeratorCallback, kIndexed>(
           isolate, receiver, object, *filter, accumulator);
   MAYBE_RETURN(success, Nothing<bool>());
 
-  if (*filter == ENUMERABLE_STRINGS) {
-    // We can cache the computed property keys if access checks are
-    // not needed and no interceptors are involved.
-    //
-    // We do not use the cache if the object has elements and
-    // therefore it does not make sense to cache the property names
-    // for arguments objects.  Arguments objects will always have
-    // elements.
-    // Wrapped strings have elements, but don't have an elements
-    // array or dictionary.  So the fast inline test for whether to
-    // use the cache says yes, so we should not create a cache.
-    Handle<JSFunction> arguments_function(
-        JSFunction::cast(isolate->sloppy_arguments_map()->GetConstructor()));
-    bool cache_enum_length =
-        ((object->map()->GetConstructor() != *arguments_function) &&
-         !object->IsJSValue() && !object->IsAccessCheckNeeded() &&
-         !object->HasNamedInterceptor() && !object->HasIndexedInterceptor());
-    // Compute the property keys and cache them if possible.
-    Handle<FixedArray> enum_keys =
-        JSObject::GetEnumPropertyKeys(object, cache_enum_length);
-    accumulator->AddKeys(enum_keys);
-  } else {
-    object->CollectOwnPropertyNames(accumulator, *filter);
-  }
+  object->CollectOwnPropertyKeys(accumulator, *filter, type);
 
   // Add the property keys from the interceptor.
   success = GetKeysFromInterceptor<v8::GenericNamedPropertyEnumeratorCallback,
                                    kNamed>(isolate, receiver, object, *filter,
                                            accumulator);
   MAYBE_RETURN(success, Nothing<bool>());
   return Just(true);
 }
 
 
@@ skipping 7715 matching lines @@
         SwapPairs(numbers, i, p);
       }
     }
   } else {
     HeapSortPairs(this, numbers, len);
     return;
   }
 }
 
 
-void JSObject::CollectOwnPropertyNames(KeyAccumulator* keys,
-                                       PropertyFilter filter) {
+namespace {
+
+void CollectEnumCacheTo(KeyAccumulator* keys, JSObject* object) {
+  // We can cache the computed property keys if access checks are
+  // not needed and no interceptors are involved.
+  //
+  // We do not use the cache if the object has elements and
+  // therefore it does not make sense to cache the property names
+  // for arguments objects.  Arguments objects will always have
+  // elements.
+  // Wrapped strings have elements, but don't have an elements
+  // array or dictionary.  So the fast inline test for whether to
+  // use the cache says yes, so we should not create a cache.
+  Isolate* isolate = keys->isolate();
+  Handle<JSFunction> arguments_function(
+      JSFunction::cast(isolate->sloppy_arguments_map()->GetConstructor()));
+  bool cache_enum_length =
+      ((object->map()->GetConstructor() != *arguments_function) &&
+       !object->IsJSValue() && !object->IsAccessCheckNeeded() &&
+       !object->HasNamedInterceptor() && !object->HasIndexedInterceptor());
+  // Compute the property keys and cache them if possible.
+  Handle<FixedArray> enum_keys =
+      GetFastEnumPropertyKeys(isolate, object, cache_enum_length);
+  keys->AddKeys(enum_keys);
+}
+
+}  // namespace
+
+
+void JSObject::CollectFastPropertyKeysTo(KeyAccumulator* keys,
+                                         PropertyFilter filter,
+                                         JSReceiver::KeyCollectionType type) {
+  // Avoid using the enum cache if we have to walk the prototype chain due
+  // to shadowing properties.
+  if (filter == ENUMERABLE_STRINGS && type == JSReceiver::OWN_ONLY) {
+    CollectEnumCacheTo(keys, this);
+    return;
+  }
+
+  int real_size = map()->NumberOfOwnDescriptors();
+  Handle<DescriptorArray> descs(map()->instance_descriptors());
+  for (int i = 0; i < real_size; i++) {
+    PropertyDetails details = descs->GetDetails(i);
+    if ((details.attributes() & filter) != 0) {
+      if (type == JSReceiver::OWN_ONLY) continue;
+      if (details.attributes() & DONT_ENUM) {
+        keys->HideKey(descs->GetKey(i));
+      }
+      continue;
+    }
+    if (filter & ONLY_ALL_CAN_READ) {
+      if (details.kind() != kAccessor) continue;
+      Object* accessors = descs->GetValue(i);
+      if (!accessors->IsAccessorInfo()) continue;
+      if (!AccessorInfo::cast(accessors)->all_can_read()) continue;
+    }
+    Name* key = descs->GetKey(i);
+    if (key->FilterKey(filter)) continue;
+    keys->AddKey(key);
+  }
+}
+
+
+void JSObject::CollectOwnPropertyKeys(KeyAccumulator* keys,
+                                      PropertyFilter filter,
+                                      JSReceiver::KeyCollectionType type) {
   if (HasFastProperties()) {
-    int real_size = map()->NumberOfOwnDescriptors();
-    Handle<DescriptorArray> descs(map()->instance_descriptors());
-    for (int i = 0; i < real_size; i++) {
-      PropertyDetails details = descs->GetDetails(i);
-      if ((details.attributes() & filter) != 0) continue;
-      if (filter & ONLY_ALL_CAN_READ) {
-        if (details.kind() != kAccessor) continue;
-        Object* accessors = descs->GetValue(i);
-        if (!accessors->IsAccessorInfo()) continue;
-        if (!AccessorInfo::cast(accessors)->all_can_read()) continue;
-      }
-      Name* key = descs->GetKey(i);
-      if (key->FilterKey(filter)) continue;
-      keys->AddKey(key);
-    }
+    CollectFastPropertyKeysTo(keys, filter, type);
   } else if (IsJSGlobalObject()) {
     GlobalDictionary::CollectKeysTo(handle(global_dictionary()), keys, filter);
   } else {
     NameDictionary::CollectKeysTo(handle(property_dictionary()), keys, filter);
   }
 }
 
 
 int JSObject::NumberOfOwnElements(PropertyFilter filter) {
   // Fast case for objects with no elements.
@@ skipping 2013 matching lines @@
   }
   DCHECK(storage->length() >= index);
   return index - start_index;
 }
 
 
 template <typename Derived, typename Shape, typename Key>
 void Dictionary<Derived, Shape, Key>::CollectKeysTo(
     Handle<Dictionary<Derived, Shape, Key> > dictionary, KeyAccumulator* keys,
     PropertyFilter filter) {
+  if (dictionary->NumberOfElements() == 0) return;
   int capacity = dictionary->Capacity();
   Handle<FixedArray> array =
       keys->isolate()->factory()->NewFixedArray(dictionary->NumberOfElements());
   int array_size = 0;
+  std::vector<int> hidden_key_indices;
 
   {
     DisallowHeapAllocation no_gc;
     Dictionary<Derived, Shape, Key>* raw_dict = *dictionary;
     for (int i = 0; i < capacity; i++) {
-      Object* k = raw_dict->KeyAt(i);
-      if (!raw_dict->IsKey(k) || k->FilterKey(filter)) continue;
+      Object* key = raw_dict->KeyAt(i);
+      if (!raw_dict->IsKey(key) || key->FilterKey(filter)) continue;
       if (raw_dict->IsDeleted(i)) continue;
       PropertyDetails details = raw_dict->DetailsAt(i);
-      if ((details.attributes() & filter) != 0) continue;
+      if ((details.attributes() & filter) != 0) {
+        if (details.attributes() & DONT_ENUM) {
+          hidden_key_indices.push_back(i);
+        }
+        continue;
+      }
       if (filter & ONLY_ALL_CAN_READ) {
         if (details.kind() != kAccessor) continue;
         Object* accessors = raw_dict->ValueAt(i);
         if (accessors->IsPropertyCell()) {
           accessors = PropertyCell::cast(accessors)->value();
         }
         if (!accessors->IsAccessorInfo()) continue;
         if (!AccessorInfo::cast(accessors)->all_can_read()) continue;
       }
       array->set(array_size++, Smi::FromInt(i));
     }
 
     EnumIndexComparator<Derived> cmp(static_cast<Derived*>(raw_dict));
     Smi** start = reinterpret_cast<Smi**>(array->GetFirstElementAddress());
     std::sort(start, start + array_size, cmp);
   }
-
+  for (uint32_t i = 0; i < hidden_key_indices.size(); i++) {
+    keys->HideKey(dictionary->KeyAt(hidden_key_indices[i]));
+  }
   for (int i = 0; i < array_size; i++) {
     int index = Smi::cast(array->get(i))->value();
     keys->AddKey(dictionary->KeyAt(index));
   }
 }
 
 
 // Backwards lookup (slow).
 template<typename Derived, typename Shape, typename Key>
 Object* Dictionary<Derived, Shape, Key>::SlowReverseLookup(Object* value) {
@@ skipping 1239 matching lines @@
   if (cell->value() != *new_value) {
     cell->set_value(*new_value);
     Isolate* isolate = cell->GetIsolate();
     cell->dependent_code()->DeoptimizeDependentCodeGroup(
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
 
 }  // namespace internal
 }  // namespace v8