[v8-dev] Fix property enum cache creation to include only own properties (issue 212673011)

[jarin]

Reviewers: Toon Verwaest,

Description:
With this fix, we only create the enum cache for own property descriptors
(originally we cached all descriptors in the map).  The problem was that the
size of all descriptors could be trimmed during GC triggered by allocating  
the
storage for the cache, so we could have ended up with a wrong storage size.
This is really Toon's fix, I have only created a small repro case.

BUG=

Please review this at https://codereview.chromium.org/212673011/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+24, -8 lines):
  M src/handles.cc
  A test/mjsunit/regress/regress-enum-prop-keys-cache-size.js


Index: src/handles.cc
diff --git a/src/handles.cc b/src/handles.cc
index  
398a68265cdf1c65d2b98a4d459ab972bbe34cef..ec4f7291b5bcd44f1d30ff273c89e474229d3a81  
100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -663,7 +663,7 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject>  
object,
     }

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

     Handle<FixedArray> storage =  
isolate->factory()->NewFixedArray(num_enum);
     Handle<FixedArray> indices =  
isolate->factory()->NewFixedArray(num_enum);
@@ -671,15 +671,13 @@ Handle<FixedArray>  
GetEnumPropertyKeys(Handle<JSObject> object,
     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 +704,9 @@ Handle<FixedArray>  
GetEnumPropertyKeys(Handle<JSObject> object,
                        indices.is_null() ? Object::cast(Smi::FromInt(0))
                                          : Object::cast(*indices));
     if (cache_result) {
-      object->map()->SetEnumLength(enum_size);
+      object->map()->SetEnumLength(index);
     }
-
-    return ReduceFixedArrayTo(storage, enum_size);
+    return storage;
   } else {
     Handle<NameDictionary> dictionary(object->property_dictionary());
     int length = dictionary->NumberOfEnumElements();
Index: test/mjsunit/regress/regress-enum-prop-keys-cache-size.js
diff --git a/test/mjsunit/regress/regress-enum-prop-keys-cache-size.js  
b/test/mjsunit/regress/regress-enum-prop-keys-cache-size.js
new file mode 100644
index  
0000000000000000000000000000000000000000..1227500eeaf45b7b696ab6b6f3a7e87275569a27
--- /dev/null
+++ b/test/mjsunit/regress/regress-enum-prop-keys-cache-size.js
@@ -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) { }


--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.