Calculates retaining paths through user fields and ignores VM objects

runtime/vm/object_graph.cc

Index: runtime/vm/object_graph.cc
diff --git a/runtime/vm/object_graph.cc b/runtime/vm/object_graph.cc
index 1c3373f4200ed3876fd1286003438cfd02c58e1e..8a9c47538e8983db639869b29888804363b7c68d 100644
--- a/runtime/vm/object_graph.cc
+++ b/runtime/vm/object_graph.cc
@@ -26,12 +26,21 @@ namespace dart {
 class ObjectGraph::Stack : public ObjectPointerVisitor {
  public:
   explicit Stack(Isolate* isolate)
-      : ObjectPointerVisitor(isolate), data_(kInitialCapacity) {}
+      : ObjectPointerVisitor(isolate),
+        include_vm_obj(true),
+        data_(kInitialCapacity) {}
 
   // Marks and pushes. Used to initialize this stack with roots.
   virtual void VisitPointers(RawObject** first, RawObject** last) {
     for (RawObject** current = first; current <= last; ++current) {
       if ((*current)->IsHeapObject() && !(*current)->IsMarked()) {
+        if (!include_vm_obj) {

[cbernaschina, 2017/07/25 21:09:22]

Can we make this more generic?

Maybe storing a function pointer or a functor object that is optionally passed
to ObjectGraph::IterateObjects and will filter out elements?

[danunez, 2017/07/25 22:24:54]

I like this idea and we discussed this a bit offline. We found that
ObjectGraph::IterateObjects will have to take in said function pointer/functor.

I was curious if others had a preferred way of making this change.

+          Object& o = Object::Handle(*current);

[rmacnak, 2017/07/25 21:55:21]

Allocating a handle here is too expensive.

*current->GetClassId()

[danunez, 2017/07/25 22:24:54]

RawObject::GetClassId() is private. I can make it public to avoid the handle.

[rmacnak, 2017/07/26 00:25:40]

Make ObjectGraph::Stack a friend instead. I'm sort of surprised it isn't already
given all the other RawObject going on in this function.

+          intptr_t cid = o.GetClassId();
+          if (cid < kInstanceCid && cid != kContextCid) {
+            continue;
+          }
+        }
         (*current)->SetMarkBit();
         Node node;
         node.ptr = current;
@@ -69,6 +78,8 @@ class ObjectGraph::Stack : public ObjectPointerVisitor {
     }
   }
 
+  bool include_vm_obj;

[rmacnak, 2017/07/25 21:55:21]

include_vm_objects_

[danunez, 2017/07/25 22:24:54]

Done.

+
  private:
   struct Node {
     RawObject** ptr;  // kSentinel for the sentinel node.
@@ -377,7 +388,10 @@ intptr_t ObjectGraph::RetainingPath(Object* obj, const Array& path) {
   RawObject* raw = obj->raw();
   *obj = Object::null();
   RetainingPathVisitor visitor(raw, path);
-  IterateObjects(&visitor);
+  IterateObjectsNoVM(&visitor);
+  if (visitor.length() == 0) {
+    IterateObjects(&visitor);
+  }
   *obj = raw;
   return visitor.length();
 }
@@ -633,4 +647,13 @@ intptr_t ObjectGraph::Serialize(WriteStream* stream,
   return object_count;
 }
 
+void ObjectGraph::IterateObjectsNoVM(ObjectGraph::Visitor* visitor) {

[rmacnak, 2017/07/25 21:55:21]

Place next to ObjectGraph::IterateObjects.

[danunez, 2017/07/25 22:24:54]

I can do this, but I will also have to move IterateUserFields() as well, since
it is a static function declared and defined in object_graph.cc.

+  NoSafepointScope no_safepoint_scope_;
+  Stack stack(isolate());
+  IterateUserFields(&stack);
+  stack.include_vm_obj = false;
+  stack.TraverseGraph(visitor);
+  Unmarker::UnmarkAll(isolate());
+}
+
 }  // namespace dart