Implicit references are missing in heap profiles

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

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

include/v8.h

@@ -3216,7 +3216,7 @@ class V8EXPORT V8 {
    * or delete properties for example) since it is possible such
    * operations will result in the allocation of objects.
    */
-  static void SetGlobalGCPrologueCallback(GCCallback);
+  V8_DEPRECATED(static void SetGlobalGCPrologueCallback(GCCallback));
 
   /**
    * Enables the host application to receive a notification after a
@@ -3245,7 +3245,7 @@ class V8EXPORT V8 {
    * or delete properties for example) since it is possible such
    * operations will result in the allocation of objects.
    */
-  static void SetGlobalGCEpilogueCallback(GCCallback);
+  V8_DEPRECATED(static void SetGlobalGCEpilogueCallback(GCCallback));
 
   /**
    * Enables the host application to provide a mechanism to be notified

src/heap.cc

@@ -882,24 +882,13 @@ bool Heap::PerformGarbageCollection(GarbageCollector collector,
   }
 #endif
 
-  if (collector == MARK_COMPACTOR && global_gc_prologue_callback_) {
-    ASSERT(!allocation_allowed_);
-    GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL);
-    VMState state(isolate_, EXTERNAL);
-    global_gc_prologue_callback_();
-  }
-
   GCType gc_type =
       collector == MARK_COMPACTOR ? kGCTypeMarkSweepCompact : kGCTypeScavenge;
 
   {
     GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL);
     VMState state(isolate_, EXTERNAL);
-    for (int i = 0; i < gc_prologue_callbacks_.length(); ++i) {
-      if (gc_type & gc_prologue_callbacks_[i].gc_type) {
-        gc_prologue_callbacks_[i].callback(gc_type, kNoGCCallbackFlags);
-      }
-    }
+    CallGCPrologueCallbacks(gc_type);
   }
 
   EnsureFromSpaceIsCommitted();
@@ -1009,19 +998,7 @@ bool Heap::PerformGarbageCollection(GarbageCollector collector,
   {
     GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL);
     VMState state(isolate_, EXTERNAL);
-    GCCallbackFlags callback_flags = kNoGCCallbackFlags;
-    for (int i = 0; i < gc_epilogue_callbacks_.length(); ++i) {
-      if (gc_type & gc_epilogue_callbacks_[i].gc_type) {
-        gc_epilogue_callbacks_[i].callback(gc_type, callback_flags);
-      }
-    }
-  }
-
-  if (collector == MARK_COMPACTOR && global_gc_epilogue_callback_) {
-    ASSERT(!allocation_allowed_);
-    GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL);
-    VMState state(isolate_, EXTERNAL);
-    global_gc_epilogue_callback_();
+    CallGCEpilogueCallbacks(gc_type);
   }
 
 #ifdef VERIFY_HEAP
@@ -1034,6 +1011,30 @@ bool Heap::PerformGarbageCollection(GarbageCollector collector,
 }
 
 
+void Heap::CallGCPrologueCallbacks(GCType gc_type) {
+  if (gc_type == kGCTypeMarkSweepCompact && global_gc_prologue_callback_) {
+    global_gc_prologue_callback_();
+  }
+  for (int i = 0; i < gc_prologue_callbacks_.length(); ++i) {
+    if (gc_type & gc_prologue_callbacks_[i].gc_type) {
+      gc_prologue_callbacks_[i].callback(gc_type, kNoGCCallbackFlags);
+    }
+  }
+}
+
+
+void Heap::CallGCEpilogueCallbacks(GCType gc_type) {
+  for (int i = 0; i < gc_epilogue_callbacks_.length(); ++i) {
+    if (gc_type & gc_epilogue_callbacks_[i].gc_type) {
+      gc_epilogue_callbacks_[i].callback(gc_type, kNoGCCallbackFlags);
+    }
+  }
+  if (gc_type == kGCTypeMarkSweepCompact && global_gc_epilogue_callback_) {
+    global_gc_epilogue_callback_();
+  }
+}
+
+
 void Heap::MarkCompact(GCTracer* tracer) {
   gc_state_ = MARK_COMPACT;
   LOG(isolate_, ResourceEvent("markcompact", "begin"));
@@ -1376,6 +1377,7 @@ void Heap::Scavenge() {
     new_space_front = DoScavenge(&scavenge_visitor, new_space_front);
   }
   isolate()->global_handles()->RemoveObjectGroups();
+  isolate()->global_handles()->RemoveImplicitRefGroups();
 
   isolate_->global_handles()->IdentifyNewSpaceWeakIndependentHandles(
       &IsUnscavengedHeapObject);

src/heap.h

@@ -1638,13 +1638,8 @@ class Heap {
 
   inline Isolate* isolate();
 
-  inline void CallGlobalGCPrologueCallback() {
-    if (global_gc_prologue_callback_ != NULL) global_gc_prologue_callback_();
-  }
-
-  inline void CallGlobalGCEpilogueCallback() {
-    if (global_gc_epilogue_callback_ != NULL) global_gc_epilogue_callback_();
-  }
+  void CallGCPrologueCallbacks(GCType gc_type);
+  void CallGCEpilogueCallbacks(GCType gc_type);
 
   inline bool OldGenerationAllocationLimitReached();
 

src/profile-generator.cc

@@ -2826,8 +2826,9 @@ int NativeObjectsExplorer::EstimateObjectsCount() {
 void NativeObjectsExplorer::FillRetainedObjects() {
   if (embedder_queried_) return;
   Isolate* isolate = Isolate::Current();
+  const GCType major_gc_type = kGCTypeMarkSweepCompact;
   // Record objects that are joined into ObjectGroups.
-  isolate->heap()->CallGlobalGCPrologueCallback();
+  isolate->heap()->CallGCPrologueCallbacks(major_gc_type);
   List<ObjectGroup*>* groups = isolate->global_handles()->object_groups();
   for (int i = 0; i < groups->length(); ++i) {
     ObjectGroup* group = groups->at(i);
@@ -2841,7 +2842,7 @@ void NativeObjectsExplorer::FillRetainedObjects() {
     group->info_ = NULL;  // Acquire info object ownership.
   }
   isolate->global_handles()->RemoveObjectGroups();
-  isolate->heap()->CallGlobalGCEpilogueCallback();
+  isolate->heap()->CallGCEpilogueCallbacks(major_gc_type);
   // Record objects that are not in ObjectGroups, but have class ID.
   GlobalHandlesExtractor extractor(this);
   isolate->global_handles()->IterateAllRootsWithClassIds(&extractor);
@@ -2870,6 +2871,7 @@ void NativeObjectsExplorer::FillImplicitReferences() {
           child_entry);
     }
   }
+  isolate->global_handles()->RemoveImplicitRefGroups();
 }
 
 List<HeapObject*>* NativeObjectsExplorer::GetListMaybeDisposeInfo(

test/cctest/test-heap-profiler.cc

@@ -1116,7 +1116,7 @@ class GraphWithImplicitRefs {
     instance_ = NULL;
   }
 
-  static void gcPrologue() {
+  static void gcPrologue(v8::GCType type, v8::GCCallbackFlags flags) {
     instance_->AddImplicitReferences();
   }
 
@@ -1142,7 +1142,7 @@ TEST(HeapSnapshotImplicitReferences) {
   LocalContext env;
 
   GraphWithImplicitRefs graph(&env);
-  v8::V8::SetGlobalGCPrologueCallback(&GraphWithImplicitRefs::gcPrologue);
+  v8::V8::AddGCPrologueCallback(&GraphWithImplicitRefs::gcPrologue);
 
   const v8::HeapSnapshot* snapshot =
       v8::HeapProfiler::TakeSnapshot(v8_str("implicit_refs"));
@@ -1165,7 +1165,7 @@ TEST(HeapSnapshotImplicitReferences) {
     }
   }
   CHECK_EQ(2, implicit_targets_count);
-  v8::V8::SetGlobalGCPrologueCallback(NULL);
+  v8::V8::RemoveGCPrologueCallback(&GraphWithImplicitRefs::gcPrologue);
 }