Revision: 13566
Author:   [email protected]
Date:     Thu Jan 31 02:50:42 2013
Log: Only mark the descriptor that is valid for the map in question. If this map
transitioned from a map with a different descriptor array (or has no back
pointer), mark all valid descriptors from the start.

This fixes the following memory leak: Map A shares a descriptor array
with map B. Map B adds constant function c that in its scope holds on to
an instance of B. If the descriptor array of A would keep all the shared
descriptors alive, including c, this keeps alive both A and c
indefinitely.

This CL also fixes a bug in descriptor array trimming. When trimming
descriptor arrays we need to trim off the slack as well (thus the entire
storage); and since we are trimming a descriptor array, we need to trim
* kDescriptorSize.

Review URL: https://chromiumcodereview.appspot.com/12084066
http://code.google.com/p/v8/source/detail?r=13566

Modified:
 /branches/bleeding_edge/src/objects-inl.h
 /branches/bleeding_edge/src/objects-visiting-inl.h
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/objects.h

=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Fri Jan 25 03:55:29 2013
+++ /branches/bleeding_edge/src/objects-inl.h   Thu Jan 31 02:50:42 2013
@@ -2140,6 +2140,16 @@
       reinterpret_cast<HeapObject*>(this),
       OffsetOfElementAt(ToKeyIndex(descriptor_number)));
 }
+
+
+Object** DescriptorArray::GetDescriptorStartSlot(int descriptor_number) {
+  return GetKeySlot(descriptor_number);
+}
+
+
+Object** DescriptorArray::GetDescriptorEndSlot(int descriptor_number) {
+  return GetValueSlot(descriptor_number - 1) + 1;
+}


 String* DescriptorArray::GetKey(int descriptor_number) {
=======================================
--- /branches/bleeding_edge/src/objects-visiting-inl.h Mon Jan 28 02:25:38 2013 +++ /branches/bleeding_edge/src/objects-visiting-inl.h Thu Jan 31 02:50:42 2013
@@ -395,6 +395,33 @@
     // Already marked by marking map->GetBackPointer() above.
     ASSERT(transitions->IsMap() || transitions->IsUndefined());
   }
+
+  // Since descriptor arrays are potentially shared, ensure that only the
+  // descriptors that appeared for this map are marked. The first time a
+ // non-empty descriptor array is marked, its header is also visited. The slot + // holding the descriptor array will be implicitly recorded when the pointer
+  // fields of this map are visited.
+  DescriptorArray* descriptors = map->instance_descriptors();
+  if (StaticVisitor::MarkObjectWithoutPush(heap, descriptors) &&
+      descriptors->length() > 0) {
+    StaticVisitor::VisitPointers(heap,
+        descriptors->GetFirstElementAddress(),
+        descriptors->GetDescriptorEndSlot(0));
+  }
+  int start = 0;
+  int end = map->NumberOfOwnDescriptors();
+  Object* back_pointer = map->GetBackPointer();
+  if (!back_pointer->IsUndefined()) {
+    Map* parent_map = Map::cast(back_pointer);
+    if (descriptors == parent_map->instance_descriptors()) {
+      start = parent_map->NumberOfOwnDescriptors();
+    }
+  }
+  if (start < end) {
+    StaticVisitor::VisitPointers(heap,
+        descriptors->GetDescriptorStartSlot(start),
+        descriptors->GetDescriptorEndSlot(end));
+  }

   // Mark prototype dependent codes array but do not push it onto marking
   // stack, this will make references from it weak. We will clean dead
=======================================
--- /branches/bleeding_edge/src/objects.cc      Wed Jan 30 13:07:28 2013
+++ /branches/bleeding_edge/src/objects.cc      Thu Jan 31 02:50:42 2013
@@ -7646,11 +7646,12 @@
                                 Map* map,
                                 DescriptorArray* descriptors,
                                 int number_of_own_descriptors) {
-  int number_of_descriptors = descriptors->number_of_descriptors();
+  int number_of_descriptors = descriptors->number_of_descriptors_storage();
   int to_trim = number_of_descriptors - number_of_own_descriptors;
-  if (to_trim <= 0) return;
+  if (to_trim == 0) return;

-  RightTrimFixedArray<FROM_GC>(heap, descriptors, to_trim);
+  RightTrimFixedArray<FROM_GC>(
+      heap, descriptors, to_trim * DescriptorArray::kDescriptorSize);
   descriptors->SetNumberOfDescriptors(number_of_own_descriptors);

   if (descriptors->HasEnumCache()) TrimEnumCache(heap, map, descriptors);
=======================================
--- /branches/bleeding_edge/src/objects.h       Fri Jan 25 02:53:26 2013
+++ /branches/bleeding_edge/src/objects.h       Thu Jan 31 02:50:42 2013
@@ -2609,6 +2609,8 @@
   inline Object** GetKeySlot(int descriptor_number);
   inline Object* GetValue(int descriptor_number);
   inline Object** GetValueSlot(int descriptor_number);
+  inline Object** GetDescriptorStartSlot(int descriptor_number);
+  inline Object** GetDescriptorEndSlot(int descriptor_number);
   inline PropertyDetails GetDetails(int descriptor_number);
   inline PropertyType GetType(int descriptor_number);
   inline int GetFieldIndex(int descriptor_number);

--
--
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/groups/opt_out.


Reply via email to