New GC APIs, try 2.

With these APIs, the embedder doesn't need to copy Persistent handles around.

BUG=

Review URL: https://codereview.chromium.org/14007008

Patch from Marja Hölttä <marja@chromium.org>.

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

include/v8-profiler.h

@@ -554,6 +554,11 @@ class V8EXPORT HeapProfiler {
   /** Returns memory used for profiler internal data and snapshots. */
   size_t GetProfilerMemorySize();
 
+  /**
+   * Sets a RetainedObjectInfo for an object group (see V8::SetObjectGroupId).
+   */
+  void SetRetainedObjectInfo(UniqueId id, RetainedObjectInfo* info);
+
  private:
   HeapProfiler();
   ~HeapProfiler();

include/v8.h

@@ -145,6 +145,31 @@ class Object;
 }
 
 
+/**
+ * General purpose unique identifier.
+ */
+class UniqueId {
+ public:
+  explicit UniqueId(intptr_t data)
+      : data_(data) {}
+
+  bool operator==(const UniqueId& other) const {
+    return data_ == other.data_;
+  }
+
+  bool operator!=(const UniqueId& other) const {
+    return data_ != other.data_;
+  }
+
+  bool operator<(const UniqueId& other) const {
+    return data_ < other.data_;
+  }
+
+ private:
+  intptr_t data_;
+};
+
+
 // --- Weak Handles ---
 
 
@@ -3148,6 +3173,39 @@ class V8EXPORT Isolate {
   /** Returns the context that is on the top of the stack. */
   Local<Context> GetCurrentContext();
 
+  /**
+   * Allows the host application to group objects together. If one
+   * 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 instance to simulate DOM tree connections among JS
+   * wrapper objects. Object groups for all dependent handles need to
+   * be provided for kGCTypeMarkSweepCompact collections, for all other
+   * garbage collection types it is sufficient to provide object groups
+   * for partially dependent handles only.
+   */
+  void SetObjectGroupId(const Persistent<Value>& object,
+                        UniqueId id);
+
+  /**
+   * Allows the host application to declare implicit references from an object
+   * group to an object. If the objects of the object group are alive, the child
+   * object is alive too. After each garbage collection, all implicit references
+   * are removed. It is intended to be used in the before-garbage-collection
+   * callback function.
+   */
+  void SetReferenceFromGroup(UniqueId id,
+                             const Persistent<Value>& child);
+
+  /**
+   * Allows the host application to declare implicit references from an object
+   * to another object. If the parent object is alive, the child object is alive
+   * too. After each garbage collection, all implicit references are removed. It
+   * is intended to be used in the before-garbage-collection callback function.
+   */
+  void SetReference(const Persistent<Object>& parent,
+                    const Persistent<Value>& child);
+
  private:
   Isolate();
   Isolate(const Isolate&);
@@ -3552,6 +3610,8 @@ class V8EXPORT V8 {
    * for partially dependent handles only.
    * See v8-profiler.h for RetainedObjectInfo interface description.
    */
+  // TODO(marja): deprecate AddObjectGroup. Use Isolate::SetObjectGroupId and
+  // HeapProfiler::SetRetainedObjectInfo instead.
   static void AddObjectGroup(Persistent<Value>* objects,
                              size_t length,
                              RetainedObjectInfo* info = NULL);
@@ -3567,6 +3627,8 @@ class V8EXPORT V8 {
    * are removed.  It is intended to be used in the before-garbage-collection
    * callback function.
    */
+  // TODO(marja): Deprecate AddImplicitReferences. Use
+  // Isolate::SetReferenceFromGroup instead.
   static void AddImplicitReferences(Persistent<Object> parent,
                                     Persistent<Value>* children,
                                     size_t length);

src/api.cc

@@ -6019,6 +6019,31 @@ v8::Local<v8::Context> Isolate::GetCurrentContext() {
 }
 
 
+void Isolate::SetObjectGroupId(const Persistent<Value>& object,
+                               UniqueId id) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(this);
+  internal_isolate->global_handles()->SetObjectGroupId(
+      reinterpret_cast<i::Object**>(*object), id);
+}
+
+
+void Isolate::SetReferenceFromGroup(UniqueId id,
+                                    const Persistent<Value>& object) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(this);
+  internal_isolate->global_handles()
+      ->SetReferenceFromGroup(id, reinterpret_cast<i::Object**>(*object));
+}
+
+
+void Isolate::SetReference(const Persistent<Object>& parent,
+                           const Persistent<Value>& child) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(this);
+  internal_isolate->global_handles()->SetReference(
+      i::Handle<i::HeapObject>::cast(Utils::OpenHandle(*parent)).location(),
+      reinterpret_cast<i::Object**>(*child));
+}
+
+
 void V8::SetGlobalGCPrologueCallback(GCCallback callback) {
   i::Isolate* isolate = i::Isolate::Current();
   if (IsDeadCheck(isolate, "v8::V8::SetGlobalGCPrologueCallback()")) return;
@@ -7218,6 +7243,12 @@ size_t HeapProfiler::GetProfilerMemorySize() {
 }
 
 
+void HeapProfiler::SetRetainedObjectInfo(UniqueId id,
+                                         RetainedObjectInfo* info) {
+  reinterpret_cast<i::HeapProfiler*>(this)->SetRetainedObjectInfo(id, info);
+}
+
+
 v8::Testing::StressType internal::Testing::stress_type_ =
     v8::Testing::kStressTypeOpt;
 

src/global-handles.cc

@@ -37,7 +37,13 @@ namespace internal {
 
 
 ObjectGroup::~ObjectGroup() {
-  if (info_ != NULL) info_->Dispose();
+  if (info != NULL) info->Dispose();
+  delete[] objects;
+}
+
+
+ImplicitRefGroup::~ImplicitRefGroup() {
+  delete[] children;
 }
 
 
@@ -438,7 +444,8 @@ GlobalHandles::GlobalHandles(Isolate* isolate)
       first_block_(NULL),
       first_used_block_(NULL),
       first_free_(NULL),
-      post_gc_processing_count_(0) {}
+      post_gc_processing_count_(0),
+      object_group_connections_(kObjectGroupConnectionsCapacity) {}
 
 
 GlobalHandles::~GlobalHandles() {
@@ -578,15 +585,16 @@ void GlobalHandles::IterateNewSpaceWeakIndependentRoots(ObjectVisitor* v) {
 
 bool GlobalHandles::IterateObjectGroups(ObjectVisitor* v,
                                         WeakSlotCallbackWithHeap can_skip) {
+  ComputeObjectGroupsAndImplicitReferences();
   int last = 0;
   bool any_group_was_visited = false;
   for (int i = 0; i < object_groups_.length(); i++) {
     ObjectGroup* entry = object_groups_.at(i);
     ASSERT(entry != NULL);
 
-    Object*** objects = entry->objects_;
+    Object*** objects = entry->objects;
     bool group_should_be_visited = false;
-    for (size_t j = 0; j < entry->length_; j++) {
+    for (size_t j = 0; j < entry->length; j++) {
       Object* object = *objects[j];
       if (object->IsHeapObject()) {
         if (!can_skip(isolate_->heap(), &object)) {
@@ -603,7 +611,7 @@ bool GlobalHandles::IterateObjectGroups(ObjectVisitor* v,
 
     // An object in the group requires visiting, so iterate over all
     // objects in the group.
-    for (size_t j = 0; j < entry->length_; ++j) {
+    for (size_t j = 0; j < entry->length; ++j) {
       Object* object = *objects[j];
       if (object->IsHeapObject()) {
         v->VisitPointer(&object);
@@ -613,7 +621,7 @@ bool GlobalHandles::IterateObjectGroups(ObjectVisitor* v,
 
     // Once the entire group has been iterated over, set the object
     // group to NULL so it won't be processed again.
-    entry->Dispose();
+    delete entry;
     object_groups_.at(i) = NULL;
   }
   object_groups_.Rewind(last);
@@ -824,7 +832,23 @@ void GlobalHandles::AddObjectGroup(Object*** handles,
     if (info != NULL) info->Dispose();
     return;
   }
-  object_groups_.Add(ObjectGroup::New(handles, length, info));
+  ObjectGroup* group = new ObjectGroup(length);
+  for (size_t i = 0; i < length; ++i)
+    group->objects[i] = handles[i];
+  group->info = info;
+  object_groups_.Add(group);
+}
+
+
+void GlobalHandles::SetObjectGroupId(Object** handle,
+                                     UniqueId id) {
+  object_group_connections_.Add(ObjectGroupConnection(id, handle));
+}
+
+
+void GlobalHandles::SetRetainedObjectInfo(UniqueId id,
+                                          RetainedObjectInfo* info) {
+  retainer_infos_.Add(ObjectGroupRetainerInfo(id, info));
 }
 
 
@@ -838,23 +862,45 @@ void GlobalHandles::AddImplicitReferences(HeapObject** parent,
   }
 #endif
   if (length == 0) return;
-  implicit_ref_groups_.Add(ImplicitRefGroup::New(parent, children, length));
+  ImplicitRefGroup* group = new ImplicitRefGroup(parent, length);
+  for (size_t i = 0; i < length; ++i)
+    group->children[i] = children[i];
+  implicit_ref_groups_.Add(group);
+}
+
+
+void GlobalHandles::SetReferenceFromGroup(UniqueId id, Object** child) {
+  ASSERT(!Node::FromLocation(child)->is_independent());
+  implicit_ref_connections_.Add(ObjectGroupConnection(id, child));
+}
+
+
+void GlobalHandles::SetReference(HeapObject** parent, Object** child) {
+  ASSERT(!Node::FromLocation(child)->is_independent());
+  ImplicitRefGroup* group = new ImplicitRefGroup(parent, 1);
+  group->children[0] = child;
+  implicit_ref_groups_.Add(group);
 }
 
 
 void GlobalHandles::RemoveObjectGroups() {
-  for (int i = 0; i < object_groups_.length(); i++) {
+  for (int i = 0; i < object_groups_.length(); i++)
-    object_groups_.at(i)->Dispose();
+    delete object_groups_.at(i);
-  }
   object_groups_.Clear();
+  for (int i = 0; i < retainer_infos_.length(); ++i)
+    retainer_infos_[i].info->Dispose();
+  retainer_infos_.Clear();
+  object_group_connections_.Clear();
+  object_group_connections_.Initialize(kObjectGroupConnectionsCapacity);
 }
 
 
 void GlobalHandles::RemoveImplicitRefGroups() {
   for (int i = 0; i < implicit_ref_groups_.length(); i++) {
-    implicit_ref_groups_.at(i)->Dispose();
+    delete implicit_ref_groups_.at(i);
   }
   implicit_ref_groups_.Clear();
+  implicit_ref_connections_.Clear();
 }
 
 
@@ -863,4 +909,108 @@ void GlobalHandles::TearDown() {
 }
 
 
+void GlobalHandles::ComputeObjectGroupsAndImplicitReferences() {
+  if (object_group_connections_.length() == 0) {
+    for (int i = 0; i < retainer_infos_.length(); ++i)
+      retainer_infos_[i].info->Dispose();
+    retainer_infos_.Clear();
+    implicit_ref_connections_.Clear();
+    return;
+  }
+
+  object_group_connections_.Sort();
+  retainer_infos_.Sort();
+  implicit_ref_connections_.Sort();
+
+  int info_index = 0;  // For iterating retainer_infos_.
+  UniqueId current_group_id(0);
+  int current_group_start = 0;
+
+  int current_implicit_refs_start = 0;
+  int current_implicit_refs_end = 0;
+  for (int i = 0; i <= object_group_connections_.length(); ++i) {
+    if (i == 0)
+      current_group_id = object_group_connections_[i].id;
+    if (i == object_group_connections_.length() ||
+        current_group_id != object_group_connections_[i].id) {
+      // Group detected: objects in indices [current_group_start, i[.
+
+      // Find out which implicit references are related to this group. (We want
+      // to ignore object groups which only have 1 object, but that object is
+      // needed as a representative object for the implicit refrerence group.)
+      while (current_implicit_refs_start < implicit_ref_connections_.length() &&
+             implicit_ref_connections_[current_implicit_refs_start].id <
+                 current_group_id)
+        ++current_implicit_refs_start;
+      current_implicit_refs_end = current_implicit_refs_start;
+      while (current_implicit_refs_end < implicit_ref_connections_.length() &&
+             implicit_ref_connections_[current_implicit_refs_end].id ==
+                 current_group_id)
+        ++current_implicit_refs_end;
+
+      if (current_implicit_refs_end > current_implicit_refs_start) {
+        // Find a representative object for the implicit references.
+        HeapObject** representative = NULL;
+        for (int j = current_group_start; j < i; ++j) {
+          Object** object = object_group_connections_[j].object;
+          if ((*object)->IsHeapObject()) {
+            representative = reinterpret_cast<HeapObject**>(object);
+            break;
+          }
+        }
+        if (representative) {
+          ImplicitRefGroup* group = new ImplicitRefGroup(
+              representative,
+              current_implicit_refs_end - current_implicit_refs_start);
+          for (int j = current_implicit_refs_start;
+               j < current_implicit_refs_end;
+               ++j) {
+            group->children[j - current_implicit_refs_start] =
+                implicit_ref_connections_[j].object;
+          }
+          implicit_ref_groups_.Add(group);
+        }
+        current_implicit_refs_start = current_implicit_refs_end;
+      }
+
+      // Find a RetainedObjectInfo for the group.
+      RetainedObjectInfo* info = NULL;
+      while (info_index < retainer_infos_.length() &&
+             retainer_infos_[info_index].id < current_group_id) {
+        retainer_infos_[info_index].info->Dispose();
+        ++info_index;
+      }
+      if (info_index < retainer_infos_.length() &&
+          retainer_infos_[info_index].id == current_group_id) {
+        // This object group has an associated ObjectGroupRetainerInfo.
+        info = retainer_infos_[info_index].info;
+        ++info_index;
+      }
+
+      // Ignore groups which only contain one object.
+      if (i > current_group_start + 1) {
+        ObjectGroup* group = new ObjectGroup(i - current_group_start);
+        for (int j = current_group_start; j < i; ++j) {
+          group->objects[j - current_group_start] =
+              object_group_connections_[j].object;
+        }
+        group->info = info;
+        object_groups_.Add(group);
+      } else if (info) {
+        info->Dispose();
+      }
+
+      if (i < object_group_connections_.length()) {
+        current_group_id = object_group_connections_[i].id;
+        current_group_start = i;
+      }
+    }
+  }
+  object_group_connections_.Clear();
+  object_group_connections_.Initialize(kObjectGroupConnectionsCapacity);
+  retainer_infos_.Clear();
+  implicit_ref_connections_.Clear();
+}
+
+
 } }  // namespace v8::internal

src/global-handles.h

@@ -28,6 +28,7 @@
 #ifndef V8_GLOBAL_HANDLES_H_
 #define V8_GLOBAL_HANDLES_H_
 
+#include "../include/v8.h"
 #include "../include/v8-profiler.h"
 
 #include "list.h"
@@ -46,70 +47,76 @@ class ObjectVisitor;
 // At GC the destroyed global handles are removed from the free list
 // and deallocated.
 
+// Data structures for tracking object groups and implicit references.
+
 // 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:
+// An implicit references group consists of two parts: a parent object and a
-  static ObjectGroup* New(Object*** handles,
+// list of children objects.  If the parent is alive, all the children are alive
-                          size_t length,
+// too.
-                          v8::RetainedObjectInfo* info) {
+
+struct ObjectGroup {
+  explicit ObjectGroup(size_t length)
+      : info(NULL), length(length) {
     ASSERT(length > 0);
-    ObjectGroup* group = reinterpret_cast<ObjectGroup*>(
+    objects = new Object**[length];
-        malloc(OFFSET_OF(ObjectGroup, objects_[length])));
-    group->length_ = length;
-    group->info_ = info;
-    CopyWords(group->objects_, handles, static_cast<int>(length));
-    return group;
   }
-
-  void Dispose() {
-    if (info_ != NULL) info_->Dispose();
-    free(this);
-  }
-
-  size_t length_;
-  v8::RetainedObjectInfo* info_;
-  Object** objects_[1];  // Variable sized array.
-
- private:
-  void* operator new(size_t size);
-  void operator delete(void* p);
   ~ObjectGroup();
-  DISALLOW_IMPLICIT_CONSTRUCTORS(ObjectGroup);
+
+  v8::RetainedObjectInfo* info;
+  Object*** objects;
+  size_t length;
 };
 
 
-// An implicit references group consists of two parts: a parent object and
+struct ImplicitRefGroup {
-// a list of children objects.  If the parent is alive, all the children
+  ImplicitRefGroup(HeapObject** parent, size_t length)
-// are alive too.
+      : parent(parent), length(length) {
-class ImplicitRefGroup {
- public:
-  static ImplicitRefGroup* New(HeapObject** parent,
-                               Object*** children,
-                               size_t length) {
     ASSERT(length > 0);
-    ImplicitRefGroup* group = reinterpret_cast<ImplicitRefGroup*>(
+    children = new Object**[length];
-        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_;
-  size_t length_;
-  Object** children_[1];  // Variable sized array.
-
- private:
-  void* operator new(size_t size);
-  void operator delete(void* p);
   ~ImplicitRefGroup();
-  DISALLOW_IMPLICIT_CONSTRUCTORS(ImplicitRefGroup);
+
+  HeapObject** parent;
+  Object*** children;
+  size_t length;
+};
+
+
+// For internal bookkeeping.
+struct ObjectGroupConnection {
+  ObjectGroupConnection(UniqueId id, Object** object)
+      : id(id), object(object) {}
+
+  bool operator==(const ObjectGroupConnection& other) const {
+    return id == other.id;
+  }
+
+  bool operator<(const ObjectGroupConnection& other) const {
+    return id < other.id;
+  }
+
+  UniqueId id;
+  Object** object;
+};
+
+
+struct ObjectGroupRetainerInfo {
+  ObjectGroupRetainerInfo(UniqueId id, RetainedObjectInfo* info)
+      : id(id), info(info) {}
+
+  bool operator==(const ObjectGroupRetainerInfo& other) const {
+    return id == other.id;
+  }
+
+  bool operator<(const ObjectGroupRetainerInfo& other) const {
+    return id < other.id;
+  }
+
+  UniqueId id;
+  RetainedObjectInfo* info;
 };
 
 
@@ -218,6 +225,16 @@ class GlobalHandles {
                       size_t length,
                       v8::RetainedObjectInfo* info);
 
+  // Associates handle with the object group represented by id.
+  // Should be only used in GC callback function before a collection.
+  // All groups are destroyed after a garbage collection.
+  void SetObjectGroupId(Object** handle, UniqueId id);
+
+  // Set RetainedObjectInfo for an object group. Should not be called more than
+  // once for a group. Should not be called for a group which contains no
+  // handles.
+  void SetRetainedObjectInfo(UniqueId id, RetainedObjectInfo* info);
+
   // 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.
@@ -225,11 +242,23 @@ class GlobalHandles {
                              Object*** children,
                              size_t length);
 
-  // Returns the object groups.
+  // Adds an implicit reference from a group to an object. Should be only used
-  List<ObjectGroup*>* object_groups() { return &object_groups_; }
+  // in GC callback function before a collection. All implicit references are
+  // destroyed after a mark-compact collection.
+  void SetReferenceFromGroup(UniqueId id, Object** child);
+
+  // Adds an implicit reference from a parent object to a child object. Should
+  // be only used in GC callback function before a collection. All implicit
+  // references are destroyed after a mark-compact collection.
+  void SetReference(HeapObject** parent, Object** child);
+
+  List<ObjectGroup*>* object_groups() {
+    ComputeObjectGroupsAndImplicitReferences();
+    return &object_groups_;
+  }
 
-  // Returns the implicit references' groups.
   List<ImplicitRefGroup*>* implicit_ref_groups() {
+    ComputeObjectGroupsAndImplicitReferences();
     return &implicit_ref_groups_;
   }
 
@@ -250,6 +279,15 @@ class GlobalHandles {
  private:
   explicit GlobalHandles(Isolate* isolate);
 
+  // Migrates data from the internal representation (object_group_connections_,
+  // retainer_infos_ and implicit_ref_connections_) to the public and more
+  // efficient representation (object_groups_ and implicit_ref_groups_).
+  void ComputeObjectGroupsAndImplicitReferences();
+
+  // v8::internal::List is inefficient even for small number of elements, if we
+  // don't assign any initial capacity.
+  static const int kObjectGroupConnectionsCapacity = 20;
+
   // Internal node structures.
   class Node;
   class NodeBlock;
@@ -275,9 +313,17 @@ class GlobalHandles {
 
   int post_gc_processing_count_;
 
+  // Object groups and implicit references, public and more efficient
+  // representation.
   List<ObjectGroup*> object_groups_;
   List<ImplicitRefGroup*> implicit_ref_groups_;
 
+  // Object groups and implicit references, temporary representation while
+  // constructing the groups.
+  List<ObjectGroupConnection> object_group_connections_;
+  List<ObjectGroupRetainerInfo> retainer_infos_;
+  List<ObjectGroupConnection> implicit_ref_connections_;
+
   friend class Isolate;
 
   DISALLOW_COPY_AND_ASSIGN(GlobalHandles);

src/heap-profiler.cc

@@ -140,5 +140,10 @@ void HeapProfiler::ObjectMoveEvent(Address from, Address to) {
   snapshots_->ObjectMoveEvent(from, to);
 }
 
+void HeapProfiler::SetRetainedObjectInfo(UniqueId id,
+                                         RetainedObjectInfo* info) {
+  // TODO(yurus, marja): Don't route this information through GlobalHandles.
+  heap()->isolate()->global_handles()->SetRetainedObjectInfo(id, info);
+}
 
 } }  // namespace v8::internal

src/heap-profiler.h

@@ -80,6 +80,8 @@ class HeapProfiler {
     return snapshots_->is_tracking_objects();
   }
 
+  void SetRetainedObjectInfo(UniqueId id, RetainedObjectInfo* info);
+
  private:
   Heap* heap() const { return snapshots_->heap(); }
 

src/heap-snapshot-generator.cc

@@ -1940,14 +1940,14 @@ void NativeObjectsExplorer::FillRetainedObjects() {
   List<ObjectGroup*>* groups = isolate->global_handles()->object_groups();
   for (int i = 0; i < groups->length(); ++i) {
     ObjectGroup* group = groups->at(i);
-    if (group->info_ == NULL) continue;
+    if (group->info == NULL) continue;
-    List<HeapObject*>* list = GetListMaybeDisposeInfo(group->info_);
+    List<HeapObject*>* list = GetListMaybeDisposeInfo(group->info);
-    for (size_t j = 0; j < group->length_; ++j) {
+    for (size_t j = 0; j < group->length; ++j) {
-      HeapObject* obj = HeapObject::cast(*group->objects_[j]);
+      HeapObject* obj = HeapObject::cast(*group->objects[j]);
       list->Add(obj);
       in_groups_.Insert(obj);
     }
-    group->info_ = NULL;  // Acquire info object ownership.
+    group->info = NULL;  // Acquire info object ownership.
   }
   isolate->global_handles()->RemoveObjectGroups();
   isolate->heap()->CallGCEpilogueCallbacks(major_gc_type);
@@ -1963,12 +1963,12 @@ void NativeObjectsExplorer::FillImplicitReferences() {
       isolate->global_handles()->implicit_ref_groups();
   for (int i = 0; i < groups->length(); ++i) {
     ImplicitRefGroup* group = groups->at(i);
-    HeapObject* parent = *group->parent_;
+    HeapObject* parent = *group->parent;
     int parent_entry =
         filler_->FindOrAddEntry(parent, native_entries_allocator_)->index();
     ASSERT(parent_entry != HeapEntry::kNoEntry);
-    Object*** children = group->children_;
+    Object*** children = group->children;
-    for (size_t j = 0; j < group->length_; ++j) {
+    for (size_t j = 0; j < group->length; ++j) {
       Object* child = *children[j];
       HeapEntry* child_entry =
           filler_->FindOrAddEntry(child, native_entries_allocator_);

src/mark-compact.cc

@@ -1939,14 +1939,14 @@ void MarkCompactCollector::MarkImplicitRefGroups() {
     ImplicitRefGroup* entry = ref_groups->at(i);
     ASSERT(entry != NULL);
 
-    if (!IsMarked(*entry->parent_)) {
+    if (!IsMarked(*entry->parent)) {
       (*ref_groups)[last++] = entry;
       continue;
     }
 
-    Object*** children = entry->children_;
+    Object*** children = entry->children;
     // A parent object is marked, so mark all child heap objects.
-    for (size_t j = 0; j < entry->length_; ++j) {
+    for (size_t j = 0; j < entry->length; ++j) {
       if ((*children[j])->IsHeapObject()) {
         HeapObject* child = HeapObject::cast(*children[j]);
         MarkBit mark = Marking::MarkBitFrom(child);
@@ -1956,7 +1956,7 @@ void MarkCompactCollector::MarkImplicitRefGroups() {
 
     // Once the entire group has been marked, dispose it because it's
     // not needed anymore.
-    entry->Dispose();
+    delete entry;
   }
   ref_groups->Rewind(last);
 }

test/cctest/test-api.cc

@@ -67,6 +67,7 @@ using ::v8::StackTrace;
 using ::v8::String;
 using ::v8::TryCatch;
 using ::v8::Undefined;
+using ::v8::UniqueId;
 using ::v8::V8;
 using ::v8::Value;
 
@@ -2445,7 +2446,7 @@ static void WeakPointerCallback(v8::Isolate* isolate,
 }
 
 
-THREADED_TEST(ApiObjectGroups) {
+THREADED_TEST(OldApiObjectGroups) {
   LocalContext env;
   v8::Isolate* iso = env->GetIsolate();
   HandleScope scope(iso);
@@ -2490,7 +2491,7 @@ THREADED_TEST(ApiObjectGroups) {
     V8::AddObjectGroup(g1_objects, 2);
     V8::AddImplicitReferences(g1s1, g1_children, 1);
     V8::AddObjectGroup(g2_objects, 2);
-    V8::AddImplicitReferences(g2s2, g2_children, 1);
+    V8::AddImplicitReferences(g2s1, g2_children, 1);
   }
   // Do a single full GC, ensure incremental marking is stopped.
   HEAP->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
@@ -2514,7 +2515,7 @@ THREADED_TEST(ApiObjectGroups) {
     V8::AddObjectGroup(g1_objects, 2);
     V8::AddImplicitReferences(g1s1, g1_children, 1);
     V8::AddObjectGroup(g2_objects, 2);
-    V8::AddImplicitReferences(g2s2, g2_children, 1);
+    V8::AddImplicitReferences(g2s1, g2_children, 1);
   }
 
   HEAP->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
@@ -2531,7 +2532,95 @@ THREADED_TEST(ApiObjectGroups) {
 }
 
 
-THREADED_TEST(ApiObjectGroupsCycle) {
+THREADED_TEST(ApiObjectGroups) {
+  LocalContext env;
+  v8::Isolate* iso = env->GetIsolate();
+  HandleScope scope(iso);
+
+  Persistent<Object> g1s1;
+  Persistent<Object> g1s2;
+  Persistent<Object> g1c1;
+  Persistent<Object> g2s1;
+  Persistent<Object> g2s2;
+  Persistent<Object> g2c1;
+
+  WeakCallCounter counter(1234);
+
+  {
+    HandleScope scope(iso);
+    g1s1 = Persistent<Object>::New(iso, Object::New());
+    g1s2 = Persistent<Object>::New(iso, Object::New());
+    g1c1 = Persistent<Object>::New(iso, Object::New());
+    g1s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g1s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g1c1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+
+    g2s1 = Persistent<Object>::New(iso, Object::New());
+    g2s2 = Persistent<Object>::New(iso, Object::New());
+    g2c1 = Persistent<Object>::New(iso, Object::New());
+    g2s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g2s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g2c1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+  }
+
+  Persistent<Object> root = Persistent<Object>::New(iso, g1s1);  // make a root.
+
+  // Connect group 1 and 2, make a cycle.
+  CHECK(g1s2->Set(0, g2s2));
+  CHECK(g2s1->Set(0, g1s1));
+
+  {
+    UniqueId id1(reinterpret_cast<intptr_t>(*g1s1));
+    UniqueId id2(reinterpret_cast<intptr_t>(*g2s2));
+    iso->SetObjectGroupId(g1s1, id1);
+    iso->SetObjectGroupId(g1s2, id1);
+    iso->SetReferenceFromGroup(id1, g1c1);
+    iso->SetObjectGroupId(g2s1, id2);
+    iso->SetObjectGroupId(g2s2, id2);
+    iso->SetReferenceFromGroup(id2, g2c1);
+  }
+  // Do a single full GC, ensure incremental marking is stopped.
+  v8::internal::Heap* heap = reinterpret_cast<v8::internal::Isolate*>(
+      iso)->heap();
+  heap->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+
+  // All object should be alive.
+  CHECK_EQ(0, counter.NumberOfWeakCalls());
+
+  // Weaken the root.
+  root.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+  // But make children strong roots---all the objects (except for children)
+  // should be collectable now.
+  g1c1.ClearWeak(iso);
+  g2c1.ClearWeak(iso);
+
+  // Groups are deleted, rebuild groups.
+  {
+    UniqueId id1(reinterpret_cast<intptr_t>(*g1s1));
+    UniqueId id2(reinterpret_cast<intptr_t>(*g2s2));
+    iso->SetObjectGroupId(g1s1, id1);
+    iso->SetObjectGroupId(g1s2, id1);
+    iso->SetReferenceFromGroup(id1, g1c1);
+    iso->SetObjectGroupId(g2s1, id2);
+    iso->SetObjectGroupId(g2s2, id2);
+    iso->SetReferenceFromGroup(id2, g2c1);
+  }
+
+  heap->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+
+  // All objects should be gone. 5 global handles in total.
+  CHECK_EQ(5, counter.NumberOfWeakCalls());
+
+  // And now make children weak again and collect them.
+  g1c1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+  g2c1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+
+  heap->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+  CHECK_EQ(7, counter.NumberOfWeakCalls());
+}
+
+
+THREADED_TEST(OldApiObjectGroupsCycle) {
   LocalContext env;
   v8::Isolate* iso = env->GetIsolate();
   HandleScope scope(iso);
@@ -2637,9 +2726,117 @@ THREADED_TEST(ApiObjectGroupsCycle) {
 }
 
 
+THREADED_TEST(ApiObjectGroupsCycle) {
+  LocalContext env;
+  v8::Isolate* iso = env->GetIsolate();
+  HandleScope scope(iso);
+
+  WeakCallCounter counter(1234);
+
+  Persistent<Object> g1s1;
+  Persistent<Object> g1s2;
+  Persistent<Object> g2s1;
+  Persistent<Object> g2s2;
+  Persistent<Object> g3s1;
+  Persistent<Object> g3s2;
+  Persistent<Object> g4s1;
+  Persistent<Object> g4s2;
+
+  {
+    HandleScope scope(iso);
+    g1s1 = Persistent<Object>::New(iso, Object::New());
+    g1s2 = Persistent<Object>::New(iso, Object::New());
+    g1s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g1s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    CHECK(g1s1.IsWeak(iso));
+    CHECK(g1s2.IsWeak(iso));
+
+    g2s1 = Persistent<Object>::New(iso, Object::New());
+    g2s2 = Persistent<Object>::New(iso, Object::New());
+    g2s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g2s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    CHECK(g2s1.IsWeak(iso));
+    CHECK(g2s2.IsWeak(iso));
+
+    g3s1 = Persistent<Object>::New(iso, Object::New());
+    g3s2 = Persistent<Object>::New(iso, Object::New());
+    g3s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g3s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    CHECK(g3s1.IsWeak(iso));
+    CHECK(g3s2.IsWeak(iso));
+
+    g4s1 = Persistent<Object>::New(iso, Object::New());
+    g4s2 = Persistent<Object>::New(iso, Object::New());
+    g4s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g4s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    CHECK(g4s1.IsWeak(iso));
+    CHECK(g4s2.IsWeak(iso));
+  }
+
+  Persistent<Object> root = Persistent<Object>::New(iso, g1s1);  // make a root.
+
+  // Connect groups.  We're building the following cycle:
+  // G1: { g1s1, g2s1 }, g1s1 implicitly references g2s1, ditto for other
+  // groups.
+  {
+    UniqueId id1(reinterpret_cast<intptr_t>(*g1s1));
+    UniqueId id2(reinterpret_cast<intptr_t>(*g2s1));
+    UniqueId id3(reinterpret_cast<intptr_t>(*g3s1));
+    UniqueId id4(reinterpret_cast<intptr_t>(*g4s1));
+    iso->SetObjectGroupId(g1s1, id1);
+    iso->SetObjectGroupId(g1s2, id1);
+    iso->SetReferenceFromGroup(id1, g2s1);
+    iso->SetObjectGroupId(g2s1, id2);
+    iso->SetObjectGroupId(g2s2, id2);
+    iso->SetReferenceFromGroup(id2, g3s1);
+    iso->SetObjectGroupId(g3s1, id3);
+    iso->SetObjectGroupId(g3s2, id3);
+    iso->SetReferenceFromGroup(id3, g4s1);
+    iso->SetObjectGroupId(g4s1, id4);
+    iso->SetObjectGroupId(g4s2, id4);
+    iso->SetReferenceFromGroup(id4, g1s1);
+  }
+  // Do a single full GC
+  v8::internal::Heap* heap = reinterpret_cast<v8::internal::Isolate*>(
+      iso)->heap();
+  heap->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+
+  // All object should be alive.
+  CHECK_EQ(0, counter.NumberOfWeakCalls());
+
+  // Weaken the root.
+  root.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+
+  // Groups are deleted, rebuild groups.
+  {
+    UniqueId id1(reinterpret_cast<intptr_t>(*g1s1));
+    UniqueId id2(reinterpret_cast<intptr_t>(*g2s1));
+    UniqueId id3(reinterpret_cast<intptr_t>(*g3s1));
+    UniqueId id4(reinterpret_cast<intptr_t>(*g4s1));
+    iso->SetObjectGroupId(g1s1, id1);
+    iso->SetObjectGroupId(g1s2, id1);
+    iso->SetReferenceFromGroup(id1, g2s1);
+    iso->SetObjectGroupId(g2s1, id2);
+    iso->SetObjectGroupId(g2s2, id2);
+    iso->SetReferenceFromGroup(id2, g3s1);
+    iso->SetObjectGroupId(g3s1, id3);
+    iso->SetObjectGroupId(g3s2, id3);
+    iso->SetReferenceFromGroup(id3, g4s1);
+    iso->SetObjectGroupId(g4s1, id4);
+    iso->SetObjectGroupId(g4s2, id4);
+    iso->SetReferenceFromGroup(id4, g1s1);
+  }
+
+  heap->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+
+  // All objects should be gone. 9 global handles in total.
+  CHECK_EQ(9, counter.NumberOfWeakCalls());
+}
+
+
 // TODO(mstarzinger): This should be a THREADED_TEST but causes failures
 // on the buildbots, so was made non-threaded for the time being.
-TEST(ApiObjectGroupsCycleForScavenger) {
+TEST(OldApiObjectGroupsCycleForScavenger) {
   i::FLAG_stress_compaction = false;
   i::FLAG_gc_global = false;
   LocalContext env;
@@ -2734,6 +2931,105 @@ TEST(ApiObjectGroupsCycleForScavenger) {
 }
 
 
+// TODO(mstarzinger): This should be a THREADED_TEST but causes failures
+// on the buildbots, so was made non-threaded for the time being.
+TEST(ApiObjectGroupsCycleForScavenger) {
+  i::FLAG_stress_compaction = false;
+  i::FLAG_gc_global = false;
+  LocalContext env;
+  v8::Isolate* iso = env->GetIsolate();
+  HandleScope scope(iso);
+
+  WeakCallCounter counter(1234);
+
+  Persistent<Object> g1s1;
+  Persistent<Object> g1s2;
+  Persistent<Object> g2s1;
+  Persistent<Object> g2s2;
+  Persistent<Object> g3s1;
+  Persistent<Object> g3s2;
+
+  {
+    HandleScope scope(iso);
+    g1s1 = Persistent<Object>::New(iso, Object::New());
+    g1s2 = Persistent<Object>::New(iso, Object::New());
+    g1s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g1s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+
+    g2s1 = Persistent<Object>::New(iso, Object::New());
+    g2s2 = Persistent<Object>::New(iso, Object::New());
+    g2s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g2s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+
+    g3s1 = Persistent<Object>::New(iso, Object::New());
+    g3s2 = Persistent<Object>::New(iso, Object::New());
+    g3s1.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+    g3s2.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+  }
+
+  // Make a root.
+  Persistent<Object> root = Persistent<Object>::New(iso, g1s1);
+  root.MarkPartiallyDependent(iso);
+
+  // Connect groups.  We're building the following cycle:
+  // G1: { g1s1, g2s1 }, g1s1 implicitly references g2s1, ditto for other
+  // groups.
+  {
+    g1s1.MarkPartiallyDependent(iso);
+    g1s2.MarkPartiallyDependent(iso);
+    g2s1.MarkPartiallyDependent(iso);
+    g2s2.MarkPartiallyDependent(iso);
+    g3s1.MarkPartiallyDependent(iso);
+    g3s2.MarkPartiallyDependent(iso);
+    iso->SetObjectGroupId(g1s1, UniqueId(1));
+    iso->SetObjectGroupId(g1s2, UniqueId(1));
+    g1s1->Set(v8_str("x"), g2s1);
+    iso->SetObjectGroupId(g2s1, UniqueId(2));
+    iso->SetObjectGroupId(g2s2, UniqueId(2));
+    g2s1->Set(v8_str("x"), g3s1);
+    iso->SetObjectGroupId(g3s1, UniqueId(3));
+    iso->SetObjectGroupId(g3s2, UniqueId(3));
+    g3s1->Set(v8_str("x"), g1s1);
+  }
+
+  v8::internal::Heap* heap = reinterpret_cast<v8::internal::Isolate*>(
+      iso)->heap();
+  heap->CollectGarbage(i::NEW_SPACE);
+
+  // All objects should be alive.
+  CHECK_EQ(0, counter.NumberOfWeakCalls());
+
+  // Weaken the root.
+  root.MakeWeak(iso, reinterpret_cast<void*>(&counter), &WeakPointerCallback);
+  root.MarkPartiallyDependent(iso);
+
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  // Groups are deleted, rebuild groups.
+  {
+    g1s1.MarkPartiallyDependent(isolate);
+    g1s2.MarkPartiallyDependent(isolate);
+    g2s1.MarkPartiallyDependent(isolate);
+    g2s2.MarkPartiallyDependent(isolate);
+    g3s1.MarkPartiallyDependent(isolate);
+    g3s2.MarkPartiallyDependent(isolate);
+    iso->SetObjectGroupId(g1s1, UniqueId(1));
+    iso->SetObjectGroupId(g1s2, UniqueId(1));
+    g1s1->Set(v8_str("x"), g2s1);
+    iso->SetObjectGroupId(g2s1, UniqueId(2));
+    iso->SetObjectGroupId(g2s2, UniqueId(2));
+    g2s1->Set(v8_str("x"), g3s1);
+    iso->SetObjectGroupId(g3s1, UniqueId(3));
+    iso->SetObjectGroupId(g3s2, UniqueId(3));
+    g3s1->Set(v8_str("x"), g1s1);
+  }
+
+  heap->CollectGarbage(i::NEW_SPACE);
+
+  // All objects should be gone. 7 global handles in total.
+  CHECK_EQ(7, counter.NumberOfWeakCalls());
+}
+
+
 THREADED_TEST(ScriptException) {
   LocalContext env;
   v8::HandleScope scope(env->GetIsolate());

test/cctest/test-global-handles.cc

@@ -30,29 +30,25 @@
 #include "cctest.h"
 
 using namespace v8::internal;
+using v8::UniqueId;
 
-static int NumberOfWeakCalls = 0;
-static void WeakPointerCallback(v8::Isolate* isolate,
-                                v8::Persistent<v8::Value> handle,
-                                void* id) {
-  ASSERT(id == reinterpret_cast<void*>(1234));
-  NumberOfWeakCalls++;
-  handle.Dispose(isolate);
-}
 
 static List<Object*> skippable_objects;
 static List<Object*> can_skip_called_objects;
 
+
 static bool CanSkipCallback(Heap* heap, Object** pointer) {
   can_skip_called_objects.Add(*pointer);
   return skippable_objects.Contains(*pointer);
 }
 
+
 static void ResetCanSkipData() {
   skippable_objects.Clear();
   can_skip_called_objects.Clear();
 }
 
+
 class TestRetainedObjectInfo : public v8::RetainedObjectInfo {
  public:
   TestRetainedObjectInfo() : has_been_disposed_(false) {}
@@ -76,6 +72,7 @@ class TestRetainedObjectInfo : public v8::RetainedObjectInfo {
   bool has_been_disposed_;
 };
 
+
 class TestObjectVisitor : public ObjectVisitor {
  public:
   virtual void VisitPointers(Object** start, Object** end) {
@@ -86,6 +83,7 @@ class TestObjectVisitor : public ObjectVisitor {
   List<Object*> visited;
 };
 
+
 TEST(IterateObjectGroupsOldApi) {
   CcTest::InitializeVM();
   GlobalHandles* global_handles = Isolate::Current()->global_handles();
@@ -96,27 +94,11 @@ TEST(IterateObjectGroupsOldApi) {
       global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
   Handle<Object> g1s2 =
       global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
-  global_handles->MakeWeak(g1s1.location(),
-                           reinterpret_cast<void*>(1234),
-                           NULL,
-                           &WeakPointerCallback);
-  global_handles->MakeWeak(g1s2.location(),
-                           reinterpret_cast<void*>(1234),
-                           NULL,
-                           &WeakPointerCallback);
 
   Handle<Object> g2s1 =
       global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
   Handle<Object> g2s2 =
       global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
-  global_handles->MakeWeak(g2s1.location(),
-                           reinterpret_cast<void*>(1234),
-                           NULL,
-                           &WeakPointerCallback);
-  global_handles->MakeWeak(g2s2.location(),
-                           reinterpret_cast<void*>(1234),
-                           NULL,
-                           &WeakPointerCallback);
 
   TestRetainedObjectInfo info1;
   TestRetainedObjectInfo info2;
@@ -184,7 +166,6 @@ TEST(IterateObjectGroupsOldApi) {
     global_handles->IterateObjectGroups(&visitor, &CanSkipCallback);
 
     // CanSkipCallback was called for all objects.
-    fprintf(stderr, "can skip len %d\n", can_skip_called_objects.length());
     ASSERT(can_skip_called_objects.length() == 1);
     ASSERT(can_skip_called_objects.Contains(*g2s1.location()) ||
            can_skip_called_objects.Contains(*g2s2.location()));
@@ -196,3 +177,141 @@ TEST(IterateObjectGroupsOldApi) {
     ASSERT(info2.has_been_disposed());
   }
 }
+
+
+TEST(IterateObjectGroups) {
+  CcTest::InitializeVM();
+  GlobalHandles* global_handles = Isolate::Current()->global_handles();
+
+  v8::HandleScope handle_scope(CcTest::isolate());
+
+  Handle<Object> g1s1 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+  Handle<Object> g1s2 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
+  Handle<Object> g2s1 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+  Handle<Object> g2s2 =
+    global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
+  TestRetainedObjectInfo info1;
+  TestRetainedObjectInfo info2;
+  global_handles->SetObjectGroupId(g2s1.location(), UniqueId(2));
+  global_handles->SetObjectGroupId(g2s2.location(), UniqueId(2));
+  global_handles->SetRetainedObjectInfo(UniqueId(2), &info2);
+  global_handles->SetObjectGroupId(g1s1.location(), UniqueId(1));
+  global_handles->SetObjectGroupId(g1s2.location(), UniqueId(1));
+  global_handles->SetRetainedObjectInfo(UniqueId(1), &info1);
+
+  // Iterate the object groups. First skip all.
+  {
+    ResetCanSkipData();
+    skippable_objects.Add(*g1s1.location());
+    skippable_objects.Add(*g1s2.location());
+    skippable_objects.Add(*g2s1.location());
+    skippable_objects.Add(*g2s2.location());
+    TestObjectVisitor visitor;
+    global_handles->IterateObjectGroups(&visitor, &CanSkipCallback);
+
+    // CanSkipCallback was called for all objects.
+    ASSERT(can_skip_called_objects.length() == 4);
+    ASSERT(can_skip_called_objects.Contains(*g1s1.location()));
+    ASSERT(can_skip_called_objects.Contains(*g1s2.location()));
+    ASSERT(can_skip_called_objects.Contains(*g2s1.location()));
+    ASSERT(can_skip_called_objects.Contains(*g2s2.location()));
+
+    // Nothing was visited.
+    ASSERT(visitor.visited.length() == 0);
+    ASSERT(!info1.has_been_disposed());
+    ASSERT(!info2.has_been_disposed());
+  }
+
+  // Iterate again, now only skip the second object group.
+  {
+    ResetCanSkipData();
+    // The first grough should still be visited, since only one object is
+    // skipped.
+    skippable_objects.Add(*g1s1.location());
+    skippable_objects.Add(*g2s1.location());
+    skippable_objects.Add(*g2s2.location());
+    TestObjectVisitor visitor;
+    global_handles->IterateObjectGroups(&visitor, &CanSkipCallback);
+
+    // CanSkipCallback was called for all objects.
+    ASSERT(can_skip_called_objects.length() == 3 ||
+           can_skip_called_objects.length() == 4);
+    ASSERT(can_skip_called_objects.Contains(*g1s2.location()));
+    ASSERT(can_skip_called_objects.Contains(*g2s1.location()));
+    ASSERT(can_skip_called_objects.Contains(*g2s2.location()));
+
+    // The first group was visited.
+    ASSERT(visitor.visited.length() == 2);
+    ASSERT(visitor.visited.Contains(*g1s1.location()));
+    ASSERT(visitor.visited.Contains(*g1s2.location()));
+    ASSERT(info1.has_been_disposed());
+    ASSERT(!info2.has_been_disposed());
+  }
+
+  // Iterate again, don't skip anything.
+  {
+    ResetCanSkipData();
+    TestObjectVisitor visitor;
+    global_handles->IterateObjectGroups(&visitor, &CanSkipCallback);
+
+    // CanSkipCallback was called for all objects.
+    ASSERT(can_skip_called_objects.length() == 1);
+    ASSERT(can_skip_called_objects.Contains(*g2s1.location()) ||
+           can_skip_called_objects.Contains(*g2s2.location()));
+
+    // The second group was visited.
+    ASSERT(visitor.visited.length() == 2);
+    ASSERT(visitor.visited.Contains(*g2s1.location()));
+    ASSERT(visitor.visited.Contains(*g2s2.location()));
+    ASSERT(info2.has_been_disposed());
+  }
+}
+
+
+TEST(ImplicitReferences) {
+  CcTest::InitializeVM();
+  GlobalHandles* global_handles = Isolate::Current()->global_handles();
+
+  v8::HandleScope handle_scope(CcTest::isolate());
+
+  Handle<Object> g1s1 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+  Handle<Object> g1c1 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+  Handle<Object> g1c2 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
+
+  Handle<Object> g2s1 =
+      global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+  Handle<Object> g2s2 =
+    global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+  Handle<Object> g2c1 =
+    global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
+  global_handles->SetObjectGroupId(g1s1.location(), UniqueId(1));
+  global_handles->SetObjectGroupId(g2s1.location(), UniqueId(2));
+  global_handles->SetObjectGroupId(g2s2.location(), UniqueId(2));
+  global_handles->SetReferenceFromGroup(UniqueId(1), g1c1.location());
+  global_handles->SetReferenceFromGroup(UniqueId(1), g1c2.location());
+  global_handles->SetReferenceFromGroup(UniqueId(2), g2c1.location());
+
+  List<ImplicitRefGroup*>* implicit_refs =
+      global_handles->implicit_ref_groups();
+  USE(implicit_refs);
+  ASSERT(implicit_refs->length() == 2);
+  ASSERT(implicit_refs->at(0)->parent ==
+         reinterpret_cast<HeapObject**>(g1s1.location()));
+  ASSERT(implicit_refs->at(0)->length == 2);
+  ASSERT(implicit_refs->at(0)->children[0] == g1c1.location());
+  ASSERT(implicit_refs->at(0)->children[1] == g1c2.location());
+  ASSERT(implicit_refs->at(1)->parent ==
+         reinterpret_cast<HeapObject**>(g2s1.location()));
+  ASSERT(implicit_refs->at(1)->length == 1);
+  ASSERT(implicit_refs->at(1)->children[0] == g2c1.location());
+}

test/cctest/test-heap-profiler.cc

@@ -1191,9 +1191,10 @@ class GraphWithImplicitRefs {
   explicit GraphWithImplicitRefs(LocalContext* env) {
     CHECK_EQ(NULL, instance_);
     instance_ = this;
-    v8::Isolate* isolate = (*env)->GetIsolate();
+    isolate_ = (*env)->GetIsolate();
     for (int i = 0; i < kObjectsCount; i++) {
-      objects_[i] = v8::Persistent<v8::Object>::New(isolate, v8::Object::New());
+      objects_[i] =
+          v8::Persistent<v8::Object>::New(isolate_, v8::Object::New());
     }
     (*env)->Global()->Set(v8_str("root_object"), objects_[0]);
   }
@@ -1208,15 +1209,20 @@ class GraphWithImplicitRefs {
  private:
   void AddImplicitReferences() {
     // 0 -> 1
-    v8::V8::AddImplicitReferences(
+    isolate_->SetObjectGroupId(v8::Persistent<v8::Object>::Cast(objects_[0]),
-        v8::Persistent<v8::Object>::Cast(objects_[0]), &objects_[1], 1);
+                               v8::UniqueId(1));
-    // Adding two more references(note length=2 in params): 1 -> 2, 1 -> 3
+    isolate_->SetReferenceFromGroup(
-    v8::V8::AddImplicitReferences(
+        v8::UniqueId(1), v8::Persistent<v8::Object>::Cast(objects_[1]));
-        v8::Persistent<v8::Object>::Cast(objects_[1]), &objects_[2], 2);
+    // Adding two more references: 1 -> 2, 1 -> 3
+    isolate_->SetReference(v8::Persistent<v8::Object>::Cast(objects_[1]),
+                           v8::Persistent<v8::Object>::Cast(objects_[2]));
+    isolate_->SetReference(v8::Persistent<v8::Object>::Cast(objects_[1]),
+                           v8::Persistent<v8::Object>::Cast(objects_[3]));
   }
 
   v8::Persistent<v8::Value> objects_[kObjectsCount];
   static GraphWithImplicitRefs* instance_;
+  v8::Isolate* isolate_;
 };
 
 GraphWithImplicitRefs* GraphWithImplicitRefs::instance_ = NULL;

test/cctest/test-mark-compact.cc

@@ -368,7 +368,7 @@ TEST(ObjectGroups) {
         Handle<HeapObject>::cast(g1s1).location(), g1_children, 1);
     global_handles->AddObjectGroup(g2_objects, 2, NULL);
     global_handles->AddImplicitReferences(
-        Handle<HeapObject>::cast(g2s2).location(), g2_children, 1);
+        Handle<HeapObject>::cast(g2s1).location(), g2_children, 1);
   }
   // Do a full GC
   HEAP->CollectGarbage(OLD_POINTER_SPACE);
@@ -397,7 +397,7 @@ TEST(ObjectGroups) {
         Handle<HeapObject>::cast(g1s1).location(), g1_children, 1);
     global_handles->AddObjectGroup(g2_objects, 2, NULL);
     global_handles->AddImplicitReferences(
-        Handle<HeapObject>::cast(g2s2).location(), g2_children, 1);
+        Handle<HeapObject>::cast(g2s1).location(), g2_children, 1);
   }
 
   HEAP->CollectGarbage(OLD_POINTER_SPACE);