Reviewers: Michael Starzinger,

Message:
PTAL

Description:
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.


Please review this at https://chromiumcodereview.appspot.com/12084066/

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

Affected files:
  M src/objects-inl.h
  M src/objects-visiting-inl.h
  M src/objects.h
  M src/objects.cc


Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 28d8762e9e0192ccc6c368e5b9709071ad39fced..18894c1d56123f99fe02df5967ea06ffdac8a857 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -2142,6 +2142,16 @@ Object** DescriptorArray::GetKeySlot(int 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) {
   ASSERT(descriptor_number < number_of_descriptors());
   return String::cast(get(ToKeyIndex(descriptor_number)));
Index: src/objects-visiting-inl.h
diff --git a/src/objects-visiting-inl.h b/src/objects-visiting-inl.h
index 7332a0c8d6ad591df61a2e7c3195e103de1cfdf5..18884933006e6183fbca4a46026ce1b910dcdeb4 100644
--- a/src/objects-visiting-inl.h
+++ b/src/objects-visiting-inl.h
@@ -396,6 +396,25 @@ void StaticMarkingVisitor<StaticVisitor>::MarkMapContents(
     ASSERT(transitions->IsMap() || transitions->IsUndefined());
   }

+  int number_of_own_descriptors = map->NumberOfOwnDescriptors();
+  if (number_of_own_descriptors > 0) {
+    DescriptorArray* descriptors = map->instance_descriptors();
+    if (StaticVisitor::MarkObjectWithoutPush(heap, descriptors)) {
+      StaticVisitor::VisitPointers(heap,
+        descriptors->GetFirstElementAddress(),
+        descriptors->GetDescriptorEndSlot(0));
+    }
+    int end = number_of_own_descriptors;
+    Object* back_pointer = map->GetBackPointer();
+    int start = (back_pointer->IsUndefined() ||
+        Map::cast(back_pointer)->instance_descriptors() != descriptors)
+            ? 0 : map->LastAdded();
+    StaticVisitor::MarkObjectWithoutPush(heap, descriptors);
+    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
   // codes when we iterate over maps in ClearNonLiveTransitions.
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 647e2861c6b5c7505d41b9af21504e80f3aea2bc..8ee3656b23841f03c2721cf89878a25239cea87f 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -7643,11 +7643,12 @@ static void TrimDescriptorArray(Heap* heap,
                                 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);
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index 975fdf8d85a46b98a7d3715c09b798444efaec83..10f74fc7d6a85a264da749899f32ec02b96492e0 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2609,6 +2609,8 @@ class DescriptorArray: public FixedArray {
   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