Remove some of the cache validity checks for for-in enumeration. We

can check for these cases before caching the property names instead.

Review URL: http://codereview.chromium.org/455020

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3389 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/handles.cc

@@ -548,6 +548,12 @@ v8::Handle<v8::Array> GetKeysForIndexedInterceptor(Handle<JSObject> receiver,
 Handle<FixedArray> GetKeysInFixedArrayFor(Handle<JSObject> object,
                                           KeyCollectionType type) {
   Handle<FixedArray> content = Factory::empty_fixed_array();
+  Handle<JSObject> arguments_boilerplate =
+      Handle<JSObject>(
+          Top::context()->global_context()->arguments_boilerplate());
+  Handle<JSFunction> arguments_function =
+      Handle<JSFunction>(
+          JSFunction::cast(arguments_boilerplate->map()->constructor()));
 
   // Only collect keys if access is permitted.
   for (Handle<Object> p = object;
@@ -577,8 +583,21 @@ Handle<FixedArray> GetKeysInFixedArrayFor(Handle<JSObject> object,
         content = AddKeysFromJSArray(content, v8::Utils::OpenHandle(*result));
     }
 
-    // Compute the property keys.
-    content = UnionOfKeys(content, GetEnumPropertyKeys(current));
+    // 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.
+    bool cache_enum_keys =
+        ((current->map()->constructor() != *arguments_function) &&
+         !current->IsAccessCheckNeeded() &&
+         !current->HasNamedInterceptor() &&
+         !current->HasIndexedInterceptor());
+    // Compute the property keys and cache them if possible.
+    content =
+        UnionOfKeys(content, GetEnumPropertyKeys(current, cache_enum_keys));
 
     // Add the property keys from the interceptor.
     if (current->HasNamedInterceptor()) {
@@ -605,7 +624,8 @@ Handle<JSArray> GetKeysFor(Handle<JSObject> object) {
 }
 
 
-Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object) {
+Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
+                                       bool cache_result) {
   int index = 0;
   if (object->HasFastProperties()) {
     if (object->map()->instance_descriptors()->HasEnumCache()) {
@@ -628,10 +648,12 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object) {
       }
     }
     (*storage)->SortPairs(*sort_array, sort_array->length());
+    if (cache_result) {
       Handle<FixedArray> bridge_storage =
           Factory::NewFixedArray(DescriptorArray::kEnumCacheBridgeLength);
       DescriptorArray* desc = object->map()->instance_descriptors();
       desc->SetEnumCache(*bridge_storage, *storage);
+    }
     ASSERT(storage->length() == index);
     return storage;
   } else {

src/handles.h

@@ -277,7 +277,8 @@ enum KeyCollectionType { LOCAL_ONLY, INCLUDE_PROTOS };
 Handle<FixedArray> GetKeysInFixedArrayFor(Handle<JSObject> object,
                                           KeyCollectionType type);
 Handle<JSArray> GetKeysFor(Handle<JSObject> object);
-Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object);
+Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
+                                       bool cache_result);
 
 // Computes the union of keys and return the result.
 // Used for implementing "for (n in object) { }"

src/objects.cc

@@ -2633,33 +2633,24 @@ bool JSObject::ReferencesObject(Object* obj) {
 
 
 // Tests for the fast common case for property enumeration:
-// - this object has an enum cache
-// - this object has no elements
-// - no prototype has enumerable properties/elements
-// - neither this object nor any prototype has interceptors
+// - This object and all prototypes has an enum cache (which means that it has
+//   no interceptors and needs no access checks).
+// - This object has no elements.
+// - No prototype has enumerable properties/elements.
 bool JSObject::IsSimpleEnum() {
-  JSObject* arguments_boilerplate =
-      Top::context()->global_context()->arguments_boilerplate();
-  JSFunction* arguments_function =
-      JSFunction::cast(arguments_boilerplate->map()->constructor());
-  if (IsAccessCheckNeeded()) return false;
-  if (map()->constructor() == arguments_function) return false;
-
   for (Object* o = this;
        o != Heap::null_value();
        o = JSObject::cast(o)->GetPrototype()) {
     JSObject* curr = JSObject::cast(o);
-    if (!curr->HasFastProperties()) return false;
     if (!curr->map()->instance_descriptors()->HasEnumCache()) return false;
+    ASSERT(!curr->HasNamedInterceptor());
+    ASSERT(!curr->HasIndexedInterceptor());
+    ASSERT(!curr->IsAccessCheckNeeded());
     if (curr->NumberOfEnumElements() > 0) return false;
-    if (curr->HasNamedInterceptor()) return false;
-    if (curr->HasIndexedInterceptor()) return false;
     if (curr != this) {
       FixedArray* curr_fixed_array =
           FixedArray::cast(curr->map()->instance_descriptors()->GetEnumCache());
-      if (curr_fixed_array->length() > 0) {
-        return false;
-      }
+      if (curr_fixed_array->length() > 0) return false;
     }
   }
   return true;
@@ -6478,6 +6469,15 @@ int JSObject::NumberOfLocalElements(PropertyAttributes filter) {
 
 
 int JSObject::NumberOfEnumElements() {
+  // Fast case for objects with no elements.
+  if (!IsJSValue() && HasFastElements()) {
+    uint32_t length = IsJSArray() ?
+        static_cast<uint32_t>(
+            Smi::cast(JSArray::cast(this)->length())->value()) :
+        static_cast<uint32_t>(FixedArray::cast(elements())->length());
+    if (length == 0) return 0;
+  }
+  // Compute the number of enumerable elements.
   return NumberOfLocalElements(static_cast<PropertyAttributes>(DONT_ENUM));
 }