Improve mark-compact object grouping interface.

The main goal was to improve O(n^2) behavior when there are many object groups.  The old API required the grouping to be done on the v8 side, along with a linear search.  The new interface requires the caller to do the grouping, passing V8 entire groups at a time.  This removes the group id concept on the v8 side.

  - Changed AddObjectToGroup to AddObjectGroup.
  - Removed the group id concept from the V8 side.
  - Remove a static constructor while I'm here, lazily initialize
    the object groups list.
  - Cleaned up return by non-const references to return pointers.

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

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

diff --git a/include/v8.h b/include/v8.h
index accffc0..2a8f86d 100644 (file)
--- a/include/v8.h
+++ b/include/v8.h
@@ -1967,10 +1967,10 @@ class EXPORT V8 {
    * object in the group is alive, all objects in the group are alive.
    * After each garbage collection, object groups are removed. It is
    * intended to be used in the before-garbage-collection callback
-   * function for istance to simulate DOM tree connections among JS
+   * function, for instance to simulate DOM tree connections among JS
    * wrapper objects.
    */
-  static void AddObjectToGroup(void* id, Persistent<Object> obj);
+  static void AddObjectGroup(Persistent<Value>* objects, size_t length);
 
   /**
    * Initializes from snapshot if possible. Otherwise, attempts to

diff --git a/src/api.cc b/src/api.cc
index cc878db..d6453ab 100644 (file)
--- a/src/api.cc
+++ b/src/api.cc
@@ -2641,9 +2641,10 @@ void V8::SetFailedAccessCheckCallbackFunction(
 }
 
 
-void V8::AddObjectToGroup(void* group_id, Persistent<Object> obj) {
-  if (IsDeadCheck("v8::V8::AddObjectToGroup()")) return;
-  i::GlobalHandles::AddToGroup(group_id, reinterpret_cast<i::Object**>(*obj));
+void V8::AddObjectGroup(Persistent<Value>* objects, size_t length) {
+  if (IsDeadCheck("v8::V8::AddObjectGroup()")) return;
+  STATIC_ASSERT(sizeof(Persistent<Value>) == sizeof(i::Object**));
+  i::GlobalHandles::AddGroup(reinterpret_cast<i::Object***>(objects), length);
 }
 
 

diff --git a/src/global-handles.cc b/src/global-handles.cc
index 29ad86e..a92f0df 100644 (file)
--- a/src/global-handles.cc
+++ b/src/global-handles.cc
@@ -352,29 +352,26 @@ void GlobalHandles::Print() {
 
 #endif
 
-List<ObjectGroup*> GlobalHandles::object_groups_(4);
-
-void GlobalHandles::AddToGroup(void* id, Object** handle) {
-  for (int i = 0; i < object_groups_.length(); i++) {
-    ObjectGroup* entry = object_groups_[i];
-    if (entry->id_ == id) {
-      entry->objects_.Add(handle);
-      return;
-    }
-  }
+List<ObjectGroup*>* GlobalHandles::ObjectGroups() {
+  // Lazily initialize the list to avoid startup time static constructors.
+  static List<ObjectGroup*> groups(4);
+  return &groups;
+}
 
-  // not found
-  ObjectGroup* new_entry = new ObjectGroup(id);
-  new_entry->objects_.Add(handle);
-  object_groups_.Add(new_entry);
+void GlobalHandles::AddGroup(Object*** handles, size_t length) {
+  ObjectGroup* new_entry = new ObjectGroup(length);
+  for (size_t i = 0; i < length; ++i)
+    new_entry->objects_.Add(handles[i]);
+  ObjectGroups()->Add(new_entry);
 }
 
 
 void GlobalHandles::RemoveObjectGroups() {
-  for (int i = 0; i< object_groups_.length(); i++) {
-    delete object_groups_[i];
+  List<ObjectGroup*>* object_groups = ObjectGroups();
+  for (int i = 0; i< object_groups->length(); i++) {
+    delete object_groups->at(i);
   }
-  object_groups_.Clear();
+  object_groups->Clear();
 }
 
 

diff --git a/src/global-handles.h b/src/global-handles.h
index 78fb3a1..c5f4450 100644 (file)
--- a/src/global-handles.h
+++ b/src/global-handles.h
@@ -41,15 +41,14 @@ namespace v8 { namespace internal {
 // Callback function on handling weak global handles.
 // typedef bool (*WeakSlotCallback)(Object** pointer);
 
-// An object group is indexed by an id. 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 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 {
  public:
-  explicit ObjectGroup(void* id) : id_(id), objects_(4) {}
+  ObjectGroup() : objects_(4) {}
+  explicit ObjectGroup(size_t capacity) : objects_(capacity) {}
 
-  void* id_;
   List<Object**> objects_;
 };
 
@@ -102,15 +101,13 @@ class GlobalHandles : public AllStatic {
   // Mark the weak pointers based on the callback.
   static void MarkWeakRoots(WeakSlotCallback f);
 
-  // Add an object to a group indexed by an id.
+  // Add an object group.
   // Should only used in GC callback function before a collection.
   // All groups are destroyed after a mark-compact collection.
-  static void AddToGroup(void* id, Object** location);
+  static void AddGroup(Object*** handles, size_t length);
 
   // Returns the object groups.
-  static List<ObjectGroup*>& ObjectGroups() {
-    return object_groups_;
-  }
+  static List<ObjectGroup*>* ObjectGroups();
 
   // Remove bags, this should only happen after GC.
   static void RemoveObjectGroups();
@@ -143,9 +140,6 @@ class GlobalHandles : public AllStatic {
   static Node* first_free_;
   static Node* first_free() { return first_free_; }
   static void set_first_free(Node* value) { first_free_ = value; }
-
-  // A list of object groups.
-  static List<ObjectGroup*> object_groups_;
 };
 
 

diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index de05339..ede5741 100644 (file)
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -564,10 +564,10 @@ void MarkCompactCollector::ProcessRoots(RootMarkingVisitor* visitor) {
 
 
 void MarkCompactCollector::MarkObjectGroups() {
-  List<ObjectGroup*>& object_groups = GlobalHandles::ObjectGroups();
+  List<ObjectGroup*>* object_groups = GlobalHandles::ObjectGroups();
 
-  for (int i = 0; i < object_groups.length(); i++) {
-    ObjectGroup* entry = object_groups[i];
+  for (int i = 0; i < object_groups->length(); i++) {
+    ObjectGroup* entry = object_groups->at(i);
     if (entry == NULL) continue;
 
     List<Object**>& objects = entry->objects_;
@@ -591,8 +591,8 @@ void MarkCompactCollector::MarkObjectGroups() {
     }
     // Once the entire group has been colored gray, set the object group
     // to NULL so it won't be processed again.
-    delete object_groups[i];
-    object_groups[i] = NULL;
+    delete object_groups->at(i);
+    object_groups->at(i) = NULL;
   }
 }
 

diff --git a/test/cctest/test-mark-compact.cc b/test/cctest/test-mark-compact.cc
index d468c24..53cff68 100644 (file)
--- a/test/cctest/test-mark-compact.cc
+++ b/test/cctest/test-mark-compact.cc
@@ -282,10 +282,12 @@ TEST(ObjectGroups) {
   Handle<FixedArray>::cast(g1s2)->set(0, *g2s2);
   Handle<FixedArray>::cast(g2s1)->set(0, *g1s1);
 
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(1), g1s1.location());
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(1), g1s2.location());
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(2), g2s1.location());
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(2), g2s2.location());
+  {
+    Object** g1_objects[] = { g1s1.location(), g1s2.location() };
+    Object** g2_objects[] = { g2s1.location(), g2s2.location() };
+    GlobalHandles::AddGroup(g1_objects, 2);
+    GlobalHandles::AddGroup(g2_objects, 2);
+  }
   // Do a full GC
   CHECK(Heap::CollectGarbage(0, OLD_POINTER_SPACE));
 
@@ -298,10 +300,12 @@ TEST(ObjectGroups) {
                           &WeakPointerCallback);
 
   // Groups are deleted, rebuild groups.
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(1), g1s1.location());
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(1), g1s2.location());
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(2), g2s1.location());
-  GlobalHandles::AddToGroup(reinterpret_cast<void*>(2), g2s2.location());
+  {
+    Object** g1_objects[] = { g1s1.location(), g1s2.location() };
+    Object** g2_objects[] = { g2s1.location(), g2s2.location() };
+    GlobalHandles::AddGroup(g1_objects, 2);
+    GlobalHandles::AddGroup(g2_objects, 2);
+  }
 
   CHECK(Heap::CollectGarbage(0, OLD_POINTER_SPACE));