Make object groups and implicit references a bit more lightweight.

We can only call malloc/free once per group and we can avoid scanning
through a list of NULLs if we keep unprocessed groups in the beginning.

I also changed the internal representation of implicit references to
hold a handle to the parent (instead of a direct pointer). The
prologue callback must not trigger a GC, but it's better to be safe.

Review URL: http://codereview.chromium.org/6800003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7521 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -4473,7 +4473,7 @@ void V8::AddImplicitReferences(Persistent<Object> parent,
   if (IsDeadCheck(isolate, "v8::V8::AddImplicitReferences()")) return;
   STATIC_ASSERT(sizeof(Persistent<Value>) == sizeof(i::Object**));
   isolate->global_handles()->AddImplicitReferences(
-      *Utils::OpenHandle(*parent),
+      i::Handle<i::HeapObject>::cast(Utils::OpenHandle(*parent)).location(),
       reinterpret_cast<i::Object***>(children), length);
 }
 

src/global-handles.cc

@@ -558,28 +558,25 @@ void GlobalHandles::Print() {
 void GlobalHandles::AddObjectGroup(Object*** handles,
                                    size_t length,
                                    v8::RetainedObjectInfo* info) {
-  ObjectGroup* new_entry = new ObjectGroup(length, info);
-  for (size_t i = 0; i < length; ++i) {
-    new_entry->objects_.Add(handles[i]);
+  if (length == 0) {
+    if (info != NULL) info->Dispose();
+    return;
   }
-  object_groups_.Add(new_entry);
+  object_groups_.Add(ObjectGroup::New(handles, length, info));
 }
 
 
-void GlobalHandles::AddImplicitReferences(HeapObject* parent,
+void GlobalHandles::AddImplicitReferences(HeapObject** parent,
                                           Object*** children,
                                           size_t length) {
-  ImplicitRefGroup* new_entry = new ImplicitRefGroup(parent, length);
-  for (size_t i = 0; i < length; ++i) {
-    new_entry->children_.Add(children[i]);
-  }
-  implicit_ref_groups_.Add(new_entry);
+  if (length == 0) return;
+  implicit_ref_groups_.Add(ImplicitRefGroup::New(parent, children, length));
 }
 
 
 void GlobalHandles::RemoveObjectGroups() {
   for (int i = 0; i < object_groups_.length(); i++) {
-    delete object_groups_.at(i);
+    object_groups_.at(i)->Dispose();
   }
   object_groups_.Clear();
 }
@@ -587,7 +584,7 @@ void GlobalHandles::RemoveObjectGroups() {
 
 void GlobalHandles::RemoveImplicitRefGroups() {
   for (int i = 0; i < implicit_ref_groups_.length(); i++) {
-    delete implicit_ref_groups_.at(i);
+    implicit_ref_groups_.at(i)->Dispose();
   }
   implicit_ref_groups_.Clear();
 }

src/global-handles.h

@@ -42,37 +42,66 @@ namespace internal {
 // An object group is treated like a single JS object: if one of object in
 // the group is alive, all objects in the same group are considered alive.
 // An object group is used to simulate object relationship in a DOM tree.
-class ObjectGroup : public Malloced {
+class ObjectGroup {
  public:
-  ObjectGroup() : objects_(4) {}
-  ObjectGroup(size_t capacity, v8::RetainedObjectInfo* info)
-      : objects_(static_cast<int>(capacity)),
-        info_(info) { }
-  ~ObjectGroup();
+  static ObjectGroup* New(Object*** handles,
+                          size_t length,
+                          v8::RetainedObjectInfo* info) {
+    ASSERT(length > 0);
+    ObjectGroup* group = reinterpret_cast<ObjectGroup*>(
+        malloc(OFFSET_OF(ObjectGroup, objects_[length])));
+    group->length_ = length;
+    group->info_ = info;
+    CopyWords(group->objects_, handles, length);
+    return group;
+  }
+
+  void Dispose() {
+    free(this);
+  }
 
-  List<Object**> objects_;
+  size_t length_;
   v8::RetainedObjectInfo* info_;
+  Object** objects_[1];  // Variable sized array.
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(ObjectGroup);
+  void* operator new(size_t size);
+  void operator delete(void* p);
+  ~ObjectGroup();
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ObjectGroup);
 };
 
 
 // An implicit references group consists of two parts: a parent object and
 // a list of children objects.  If the parent is alive, all the children
 // are alive too.
-class ImplicitRefGroup : public Malloced {
+class ImplicitRefGroup {
  public:
-  ImplicitRefGroup() : children_(4) {}
-  ImplicitRefGroup(HeapObject* parent, size_t capacity)
-      : parent_(parent),
-        children_(static_cast<int>(capacity)) { }
+  static ImplicitRefGroup* New(HeapObject** parent,
+                               Object*** children,
+                               size_t length) {
+    ASSERT(length > 0);
+    ImplicitRefGroup* group = reinterpret_cast<ImplicitRefGroup*>(
+        malloc(OFFSET_OF(ImplicitRefGroup, children_[length])));
+    group->parent_ = parent;
+    group->length_ = length;
+    CopyWords(group->children_, children, length);
+    return group;
+  }
+
+  void Dispose() {
+    free(this);
+  }
 
-  HeapObject* parent_;
-  List<Object**> children_;
+  HeapObject** parent_;
+  size_t length_;
+  Object** children_[1];  // Variable sized array.
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(ImplicitRefGroup);
+  void* operator new(size_t size);
+  void operator delete(void* p);
+  ~ImplicitRefGroup();
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ImplicitRefGroup);
 };
 
 
@@ -154,7 +183,7 @@ class GlobalHandles {
   // Add an implicit references' group.
   // Should be only used in GC callback function before a collection.
   // All groups are destroyed after a mark-compact collection.
-  void AddImplicitReferences(HeapObject* parent,
+  void AddImplicitReferences(HeapObject** parent,
                              Object*** children,
                              size_t length);
 

src/mark-compact.cc

@@ -1217,13 +1217,14 @@ void MarkCompactCollector::MarkObjectGroups() {
   List<ObjectGroup*>* object_groups =
       heap()->isolate()->global_handles()->object_groups();
 
+  int last = 0;
   for (int i = 0; i < object_groups->length(); i++) {
     ObjectGroup* entry = object_groups->at(i);
-    if (entry == NULL) continue;
+    ASSERT(entry != NULL);
 
-    List<Object**>& objects = entry->objects_;
+    Object*** objects = entry->objects_;
     bool group_marked = false;
-    for (int j = 0; j < objects.length(); j++) {
+    for (size_t j = 0; j < entry->length_; j++) {
       Object* object = *objects[j];
       if (object->IsHeapObject() && HeapObject::cast(object)->IsMarked()) {
         group_marked = true;
@@ -1231,21 +1232,24 @@ void MarkCompactCollector::MarkObjectGroups() {
       }
     }
 
-    if (!group_marked) continue;
+    if (!group_marked) {
+      (*object_groups)[last++] = entry;
+      continue;
+    }
 
-    // An object in the group is marked, so mark as gray all white heap
-    // objects in the group.
-    for (int j = 0; j < objects.length(); ++j) {
+    // An object in the group is marked, so mark all heap objects in
+    // the group.
+    for (size_t j = 0; j < entry->length_; ++j) {
       if ((*objects[j])->IsHeapObject()) {
         MarkObject(HeapObject::cast(*objects[j]));
       }
     }
 
-    // Once the entire group has been colored gray, set the object group
-    // to NULL so it won't be processed again.
-    delete entry;
-    object_groups->at(i) = NULL;
+    // Once the entire group has been marked, dispose it because it's
+    // not needed anymore.
+    entry->Dispose();
   }
+  object_groups->Rewind(last);
 }
 
 
@@ -1253,26 +1257,29 @@ void MarkCompactCollector::MarkImplicitRefGroups() {
   List<ImplicitRefGroup*>* ref_groups =
       heap()->isolate()->global_handles()->implicit_ref_groups();
 
+  int last = 0;
   for (int i = 0; i < ref_groups->length(); i++) {
     ImplicitRefGroup* entry = ref_groups->at(i);
-    if (entry == NULL) continue;
+    ASSERT(entry != NULL);
 
-    if (!entry->parent_->IsMarked()) continue;
+    if (!(*entry->parent_)->IsMarked()) {
+      (*ref_groups)[last++] = entry;
+      continue;
+    }
 
-    List<Object**>& children = entry->children_;
-    // A parent object is marked, so mark as gray all child white heap
-    // objects.
-    for (int j = 0; j < children.length(); ++j) {
+    Object*** children = entry->children_;
+    // A parent object is marked, so mark all child heap objects.
+    for (size_t j = 0; j < entry->length_; ++j) {
       if ((*children[j])->IsHeapObject()) {
         MarkObject(HeapObject::cast(*children[j]));
       }
     }
 
-    // Once the entire group has been colored gray, set the  group
-    // to NULL so it won't be processed again.
-    delete entry;
-    ref_groups->at(i) = NULL;
+    // Once the entire group has been marked, dispose it because it's
+    // not needed anymore.
+    entry->Dispose();
   }
+  ref_groups->Rewind(last);
 }
 
 

src/profile-generator.cc

@@ -2295,7 +2295,7 @@ void NativeObjectsExplorer::FillRetainedObjects() {
     ObjectGroup* group = groups->at(i);
     if (group->info_ == NULL) continue;
     List<HeapObject*>* list = GetListMaybeDisposeInfo(group->info_);
-    for (int j = 0; j < group->objects_.length(); ++j) {
+    for (size_t j = 0; j < group->length_; ++j) {
       HeapObject* obj = HeapObject::cast(*group->objects_[j]);
       list->Add(obj);
       in_groups_.Insert(obj);

src/v8utils.h

@@ -120,7 +120,9 @@ inline Vector< Handle<Object> > HandleVector(v8::internal::Handle<T>* elms,
 // Memory
 
 // Copies data from |src| to |dst|.  The data spans MUST not overlap.
-inline void CopyWords(Object** dst, Object** src, int num_words) {
+template <typename T>
+inline void CopyWords(T* dst, T* src, int num_words) {
+  STATIC_ASSERT(sizeof(T) == kPointerSize);
   ASSERT(Min(dst, src) + num_words <= Max(dst, src));
   ASSERT(num_words > 0);
 

test/cctest/test-mark-compact.cc

@@ -299,8 +299,8 @@ static void WeakPointerCallback(v8::Persistent<v8::Value> handle, void* id) {
 }
 
 TEST(ObjectGroups) {
-  GlobalHandles* global_handles = Isolate::Current()->global_handles();
   InitializeVM();
+  GlobalHandles* global_handles = Isolate::Current()->global_handles();
 
   NumberOfWeakCalls = 0;
   v8::HandleScope handle_scope;
@@ -308,9 +308,9 @@ TEST(ObjectGroups) {
   Handle<Object> g1s1 =
       global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
   Handle<Object> g1s2 =
-    global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
   Handle<Object> g1c1 =
-    global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
   global_handles->MakeWeak(g1s1.location(),
                            reinterpret_cast<void*>(1234),
                            &WeakPointerCallback);
@@ -349,11 +349,11 @@ TEST(ObjectGroups) {
     Object** g2_objects[] = { g2s1.location(), g2s2.location() };
     Object** g2_children[] = { g2c1.location() };
     global_handles->AddObjectGroup(g1_objects, 2, NULL);
-    global_handles->AddImplicitReferences(HeapObject::cast(*g1s1),
-                                          g1_children, 1);
+    global_handles->AddImplicitReferences(
+        Handle<HeapObject>::cast(g1s1).location(), g1_children, 1);
     global_handles->AddObjectGroup(g2_objects, 2, NULL);
-    global_handles->AddImplicitReferences(HeapObject::cast(*g2s2),
-                                          g2_children, 1);
+    global_handles->AddImplicitReferences(
+        Handle<HeapObject>::cast(g2s2).location(), g2_children, 1);
   }
   // Do a full GC
   HEAP->CollectGarbage(OLD_POINTER_SPACE);
@@ -377,11 +377,11 @@ TEST(ObjectGroups) {
     Object** g2_objects[] = { g2s1.location(), g2s2.location() };
     Object** g2_children[] = { g2c1.location() };
     global_handles->AddObjectGroup(g1_objects, 2, NULL);
-    global_handles->AddImplicitReferences(HeapObject::cast(*g1s1),
-                                         g1_children, 1);
+    global_handles->AddImplicitReferences(
+        Handle<HeapObject>::cast(g1s1).location(), g1_children, 1);
     global_handles->AddObjectGroup(g2_objects, 2, NULL);
-    global_handles->AddImplicitReferences(HeapObject::cast(*g2s2),
-                                         g2_children, 1);
+    global_handles->AddImplicitReferences(
+        Handle<HeapObject>::cast(g2s2).location(), g2_children, 1);
   }
 
   HEAP->CollectGarbage(OLD_POINTER_SPACE);
@@ -400,3 +400,45 @@ TEST(ObjectGroups) {
   HEAP->CollectGarbage(OLD_POINTER_SPACE);
   CHECK_EQ(7, NumberOfWeakCalls);
 }
+
+
+class TestRetainedObjectInfo : public v8::RetainedObjectInfo {
+ public:
+  TestRetainedObjectInfo() : has_been_disposed_(false) {}
+
+  bool has_been_disposed() { return has_been_disposed_; }
+
+  virtual void Dispose() {
+    ASSERT(!has_been_disposed_);
+    has_been_disposed_ = true;
+  }
+
+  virtual bool IsEquivalent(v8::RetainedObjectInfo* other) {
+    return other == this;
+  }
+
+  virtual intptr_t GetHash() { return 0; }
+
+  virtual const char* GetLabel() { return "whatever"; }
+
+ private:
+  bool has_been_disposed_;
+};
+
+
+TEST(EmptyObjectGroups) {
+  InitializeVM();
+  GlobalHandles* global_handles = Isolate::Current()->global_handles();
+
+  v8::HandleScope handle_scope;
+
+  Handle<Object> object =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
+  TestRetainedObjectInfo info;
+  global_handles->AddObjectGroup(NULL, 0, &info);
+  ASSERT(info.has_been_disposed());
+
+  global_handles->AddImplicitReferences(
+        Handle<HeapObject>::cast(object).location(), NULL, 0);
+}