Revision: 20321
Author:   [email protected]
Date:     Fri Mar 28 06:59:20 2014 UTC
Log: Reland "Fix property enum cache creation to include only own properties"

Reland r20308 (reverted by r20310).

[email protected]

Review URL: https://codereview.chromium.org/216383003
http://code.google.com/p/v8/source/detail?r=20321

Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-enum-prop-keys-cache-size.js
Modified:
 /branches/bleeding_edge/src/handles.cc

=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-enum-prop-keys-cache-size.js Fri Mar 28 06:59:20 2014 UTC
@@ -0,0 +1,19 @@
+// Copyright 2014 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.
+
+// Flags: --allow-natives-syntax --stress-compaction
+
+%SetAllocationTimeout(100000, 100000);
+
+var x = {};
+x.a = 1;
+x.b = 2;
+x = {};
+
+var y = {};
+y.a = 1;
+
+%SetAllocationTimeout(100000, 0);
+
+for (var z in y) { }
=======================================
--- /branches/bleeding_edge/src/handles.cc      Thu Mar 27 16:18:42 2014 UTC
+++ /branches/bleeding_edge/src/handles.cc      Fri Mar 28 06:59:20 2014 UTC
@@ -627,20 +627,21 @@
                                        bool cache_result) {
   Isolate* isolate = object->GetIsolate();
   if (object->HasFastProperties()) {
-    if (object->map()->instance_descriptors()->HasEnumCache()) {
-      int own_property_count = object->map()->EnumLength();
- // If we have an enum cache, but 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 not DONT_ENUM or SYMBOLIC.
-      if (own_property_count == kInvalidEnumCacheSentinel) {
-        own_property_count = object->map()->NumberOfDescribedProperties(
-            OWN_DESCRIPTORS, DONT_SHOW);
-
-        if (cache_result) object->map()->SetEnumLength(own_property_count);
-      }
+    int own_property_count = object->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 not DONT_ENUM or
+    // SYMBOLIC.
+    if (own_property_count == kInvalidEnumCacheSentinel) {
+      own_property_count = object->map()->NumberOfDescribedProperties(
+          OWN_DESCRIPTORS, DONT_SHOW);
+    } else {
+ ASSERT(own_property_count == object->map()->NumberOfDescribedProperties(
+          OWN_DESCRIPTORS, DONT_SHOW));
+    }

+    if (object->map()->instance_descriptors()->HasEnumCache()) {
       DescriptorArray* desc = object->map()->instance_descriptors();
       Handle<FixedArray> keys(desc->GetEnumCache(), isolate);

@@ -649,6 +650,7 @@
       // enum cache was generated for a previous (smaller) version of the
       // Descriptor Array. In that case we regenerate the enum cache.
       if (own_property_count <= keys->length()) {
+        if (cache_result) object->map()->SetEnumLength(own_property_count);
         isolate->counters()->enum_cache_hits()->Increment();
         return ReduceFixedArrayTo(keys, own_property_count);
       }
@@ -663,23 +665,22 @@
     }

     isolate->counters()->enum_cache_misses()->Increment();
- int num_enum = map->NumberOfDescribedProperties(ALL_DESCRIPTORS, DONT_SHOW);

- Handle<FixedArray> storage = isolate->factory()->NewFixedArray(num_enum); - Handle<FixedArray> indices = isolate->factory()->NewFixedArray(num_enum);
+    Handle<FixedArray> storage = isolate->factory()->NewFixedArray(
+        own_property_count);
+    Handle<FixedArray> indices = isolate->factory()->NewFixedArray(
+        own_property_count);

     Handle<DescriptorArray> descs =
Handle<DescriptorArray>(object->map()->instance_descriptors(), isolate);

-    int real_size = map->NumberOfOwnDescriptors();
-    int enum_size = 0;
+    int size = map->NumberOfOwnDescriptors();
     int index = 0;

-    for (int i = 0; i < descs->number_of_descriptors(); i++) {
+    for (int i = 0; i < size; i++) {
       PropertyDetails details = descs->GetDetails(i);
       Object* key = descs->GetKey(i);
       if (!(details.IsDontEnum() || key->IsSymbol())) {
-        if (i < real_size) ++enum_size;
         storage->set(index, key);
         if (!indices.is_null()) {
           if (details.type() != FIELD) {
@@ -706,10 +707,9 @@
                        indices.is_null() ? Object::cast(Smi::FromInt(0))
                                          : Object::cast(*indices));
     if (cache_result) {
-      object->map()->SetEnumLength(enum_size);
+      object->map()->SetEnumLength(own_property_count);
     }
-
-    return ReduceFixedArrayTo(storage, enum_size);
+    return storage;
   } else {
     Handle<NameDictionary> dictionary(object->property_dictionary());
     int length = dictionary->NumberOfEnumElements();

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to