Use a special EnumLength field to indicate number of valid enum cache values.

src/handles.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 687 matching lines @@
   return isolate->factory()->NewJSArrayWithElements(elements);
 }
 
 
 Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
                                        bool cache_result) {
   int index = 0;
   Isolate* isolate = object->GetIsolate();
   if (object->HasFastProperties()) {
     if (object->map()->instance_descriptors()->HasEnumCache()) {
+      int own_property_count = object->map()->EnumLength();
+
+      // Mark that we have an enum cache if we are allowed to cache it.
+      if (cache_result && own_property_count == Map::kInvalidEnumCache) {
+        int num_enum = object->map()->NumberOfDescribedProperties(DONT_ENUM);
+        object->map()->SetEnumLength(num_enum);
+      }
+
+      DescriptorArray* desc = object->map()->instance_descriptors();
+      Handle<FixedArray> keys(FixedArray::cast(desc->GetEnumCache()), isolate);
+
       isolate->counters()->enum_cache_hits()->Increment();
-      DescriptorArray* desc = object->map()->instance_descriptors();
-      return Handle<FixedArray>(FixedArray::cast(desc->GetEnumCache()),
-                                isolate);
+      return keys;
     }
+
+    Handle<Map> map(object->map());
+
+    if (map->instance_descriptors()->IsEmpty()) {
+      // Count encountering the empty descriptor array as a cache hit.

[Michael Starzinger, 2012/08/06 15:06:22]

Hmm, "encountering" is an adjective, I would just drop the comment completely.

[Toon Verwaest, 2012/08/07 10:49:47]

Done.

+      isolate->counters()->enum_cache_hits()->Increment();
+      if (cache_result) map->SetEnumLength(0);
+      return Handle<FixedArray>(isolate->factory()->empty_fixed_array());

[Michael Starzinger, 2012/08/06 15:06:22]

No need for a copy constructor of the handle here!

[Toon Verwaest, 2012/08/07 10:49:47]

Done.

+    }
+
     isolate->counters()->enum_cache_misses()->Increment();
-    Handle<Map> map(object->map());
-    int num_enum = object->NumberOfLocalProperties(DONT_ENUM);
 
-    Handle<FixedArray> storage = isolate->factory()->NewFixedArray(num_enum);
-    Handle<FixedArray> sort_array = isolate->factory()->NewFixedArray(num_enum);
+    int num_enum = map->NumberOfDescribedProperties(DONT_ENUM);
+
+    Handle<FixedArray> storage;
+    Handle<FixedArray> sort_array;
 
     Handle<FixedArray> indices;
     Handle<FixedArray> sort_array2;
 
-    if (cache_result) {
-      indices = isolate->factory()->NewFixedArray(num_enum);
-      sort_array2 = isolate->factory()->NewFixedArray(num_enum);
-    }
+    storage = isolate->factory()->NewFixedArray(num_enum);

[Michael Starzinger, 2012/08/06 15:06:22]

Can we just merge the handle declarations and the allocations into one line?

[Toon Verwaest, 2012/08/07 10:49:47]

Done.

+    sort_array = isolate->factory()->NewFixedArray(num_enum);
+    indices = isolate->factory()->NewFixedArray(num_enum);
+    sort_array2 = isolate->factory()->NewFixedArray(num_enum);
 
     Handle<DescriptorArray> descs =
         Handle<DescriptorArray>(object->map()->instance_descriptors(), isolate);
 
     for (int i = 0; i < descs->number_of_descriptors(); i++) {
-      if (!descs->GetDetails(i).IsDontEnum()) {
+      PropertyDetails details = descs->GetDetails(i);
+      if (!details.IsDontEnum()) {
         storage->set(index, descs->GetKey(i));
-        PropertyDetails details = descs->GetDetails(i);
         sort_array->set(index, Smi::FromInt(details.index()));
         if (!indices.is_null()) {
           if (details.type() != FIELD) {
             indices = Handle<FixedArray>();
             sort_array2 = Handle<FixedArray>();
           } else {
             int field_index = Descriptor::IndexFromValue(descs->GetValue(i));
             if (field_index >= map->inobject_properties()) {
               field_index = -(field_index - map->inobject_properties() + 1);
             }
             indices->set(index, Smi::FromInt(field_index));
             sort_array2->set(index, Smi::FromInt(details.index()));
           }
         }
         index++;
       }
     }
+    ASSERT(index == storage->length());
+
     storage->SortPairs(*sort_array, sort_array->length());
     if (!indices.is_null()) {
       indices->SortPairs(*sort_array2, sort_array2->length());
     }
+
+    Handle<FixedArray> bridge_storage =
+        isolate->factory()->NewFixedArray(
+            DescriptorArray::kEnumCacheBridgeLength);
+    DescriptorArray* desc = object->map()->instance_descriptors();
+    desc->SetEnumCache(*bridge_storage,
+                       *storage,
+                       indices.is_null() ? Object::cast(Smi::FromInt(0))
+                                         : Object::cast(*indices));
     if (cache_result) {
-      Handle<FixedArray> bridge_storage =
-          isolate->factory()->NewFixedArray(
-              DescriptorArray::kEnumCacheBridgeLength);
-      DescriptorArray* desc = object->map()->instance_descriptors();
-      desc->SetEnumCache(*bridge_storage,
-                         *storage,
-                         indices.is_null() ? Object::cast(Smi::FromInt(0))
-                                           : Object::cast(*indices));
+      object->map()->SetEnumLength(index);
     }
-    ASSERT(storage->length() == index);
     return storage;
   } else {
     int num_enum = object->NumberOfLocalProperties(DONT_ENUM);
     Handle<FixedArray> storage = isolate->factory()->NewFixedArray(num_enum);
     Handle<FixedArray> sort_array = isolate->factory()->NewFixedArray(num_enum);
     object->property_dictionary()->CopyEnumKeysTo(*storage, *sort_array);
     return storage;
   }
 }
 
@@ skipping 218 matching lines @@
   data->next = prev_next_;
   data->limit = prev_limit_;
 #ifdef DEBUG
   handles_detached_ = true;
 #endif
   return deferred;
 }
 
 
 } }  // namespace v8::internal