Include parent field/list index in retaining path.

R=johnmccutchan@google.com

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

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@37956 260f80e4-7a28-3924-810f-c04153c831b5

runtime/bin/vmservice/client/deployed/web/index.html

@@ -13847,6 +13847,12 @@ hr {
                   <div class="memberName">[{{ element['index']}}]</div>
                   <div class="memberValue">
                     <instance-ref ref="{{ element['value'] }}"></instance-ref>
+                    <template if="{{ element['parentField'] != null }}">
+                      in <field-ref ref="{{ element['parentField'] }}"></field-ref>
+                    </template>
+                    <template if="{{ element['parentListIndex'] != null }}">
+                      at list index {{ element['parentListIndex'] }} of
+                    </template>
                   </div>
                   </div>
                 </template>
@@ -17585,4 +17591,4 @@ hr {
 
   <observatory-application></observatory-application>
 
-<script src="index.html_bootstrap.dart.js" async=""></script></body></html>
+<script type="application/dart" src="index.html_bootstrap.dart"></script><script src="packages/browser/dart.js"></script></body></html>

runtime/bin/vmservice/client/deployed/web/index_devtools.html

@@ -13847,6 +13847,12 @@ hr {
                   <div class="memberName">[{{ element['index']}}]</div>
                   <div class="memberValue">
                     <instance-ref ref="{{ element['value'] }}"></instance-ref>
+                    <template if="{{ element['parentField'] != null }}">
+                      in <field-ref ref="{{ element['parentField'] }}"></field-ref>
+                    </template>
+                    <template if="{{ element['parentListIndex'] != null }}">
+                      at list index {{ element['parentListIndex'] }} of
+                    </template>
                   </div>
                   </div>
                 </template>
@@ -17585,4 +17591,4 @@ hr {
 
   <observatory-application devtools="true"></observatory-application>
 
-<script src="index_devtools.html_bootstrap.dart.js" async=""></script></body></html>
+<script type="application/dart" src="index_devtools.html_bootstrap.dart"></script><script src="packages/browser/dart.js"></script></body></html>

runtime/bin/vmservice/client/deployed/web/packages/observatory/src/elements/instance_view.html

@@ -81,6 +81,12 @@
                   <div class="memberName">[{{ element['index']}}]</div>
                   <div class="memberValue">
                     <instance-ref ref="{{ element['value'] }}"></instance-ref>
+                    <template if="{{ element['parentField'] != null }}">
+                      in <field-ref ref="{{ element['parentField'] }}"></field-ref>
+                    </template>
+                    <template if="{{ element['parentListIndex'] != null }}">
+                      at list index {{ element['parentListIndex'] }} of
+                    </template>
                   </div>
                   </div>
                 </template>

runtime/bin/vmservice/client/lib/src/elements/instance_view.html

@@ -81,6 +81,12 @@
                   <div class="memberName">[{{ element['index']}}]</div>
                   <div class="memberValue">
                     <instance-ref ref="{{ element['value'] }}"></instance-ref>
+                    <template if="{{ element['parentField'] != null }}">
+                      in <field-ref ref="{{ element['parentField'] }}"></field-ref>
+                    </template>
+                    <template if="{{ element['parentListIndex'] != null }}">
+                      at list index {{ element['parentListIndex'] }} of
+                    </template>
                   </div>
                   </div>
                 </template>

runtime/bin/vmservice/client/tests/ui/retainingPath.dart

@@ -0,0 +1,31 @@
+/*
+0.
+dart --observe retainingPath.dart
+1.
+isolate 'root'
+2.
+library 'retainingPath.dart'
+3.
+class 'Foo'
+4.
+under 'instances', find 'strongly reachable' list; it should contain 2 elements, one of which should have a field containing "87"
+5.
+instance "87"
+6.
+find 'retaining path'; it should have length 4, and be annotated as follows:
+ "87" in var a
+ Foo in var b
+ Foo at list index 5 of
+ _List(10)
+*/
+class Foo {
+  var a;
+  var b;
+  Foo(this.a, this.b);
+}
+
+main() {
+  var list = new List<Foo>(10);
+  list[5] = new Foo(42.toString(), new Foo(87.toString(), 17.toString()));
+  while (true) {}
+}

runtime/bin/vmservice/client/tests/ui/retainingPath.txt

@@ -0,0 +1,18 @@
+0.
+dart --observe retainingPath.dart
+1.
+isolate 'root'
+2.
+library 'retainingPath.dart'
+3.
+class 'Foo'
+4.
+under 'instances', find 'strongly reachable' list; it should contain 2 elements, one of which should have a field containing "87"
+5.
+instance "87"
+6.
+find 'retaining path'; it should have length 4, and be annotated as follows:
+ "87" in var a
+ Foo in var b
+ Foo at list index 5 of
+ _List(10)

runtime/vm/object_graph.cc

@@ -20,9 +20,7 @@ namespace dart {
 // its children are processed, to give the visitor a complete chain of parents.
 //
 // TODO(koda): Potential optimizations:
-// - Linked chunks instead of std::vector.
-// - Use smi/heap tag bit instead of full-word sentinel.
-// - Extend RawObject with incremental iteration (one child at a time).
+// - Use tag bits for compact Node and sentinel representations.
 class ObjectGraph::Stack : public ObjectPointerVisitor {
  public:
   explicit Stack(Isolate* isolate)
@@ -33,7 +31,10 @@ class ObjectGraph::Stack : public ObjectPointerVisitor {
     for (RawObject** current = first; current <= last; ++current) {
       if ((*current)->IsHeapObject() && !(*current)->IsMarked()) {
         (*current)->SetMarkBit();
-        data_.Add(*current);
+        Node node;
+        node.ptr = current;
+        node.obj = *current;
+        data_.Add(node);
       }
     }
   }
@@ -41,15 +42,18 @@ class ObjectGraph::Stack : public ObjectPointerVisitor {
   // Traverses the object graph from the current state.
   void TraverseGraph(ObjectGraph::Visitor* visitor) {
     while (!data_.is_empty()) {
-      RawObject* obj = data_.Last();
-      if (obj == kSentinel) {
+      Node node = data_.Last();
+      if (node.ptr == kSentinel) {
         data_.RemoveLast();
         // The node below the sentinel has already been visited.
         data_.RemoveLast();
         continue;
       }
+      RawObject* obj = node.obj;
       ASSERT(obj->IsHeapObject());
-      data_.Add(kSentinel);
+      Node sentinel;
+      sentinel.ptr = kSentinel;
+      data_.Add(sentinel);
       StackIterator it(this, data_.length() - 2);
       switch (visitor->VisitObject(&it)) {
         case ObjectGraph::Visitor::kProceed:
@@ -64,32 +68,70 @@ class ObjectGraph::Stack : public ObjectPointerVisitor {
   }
 
  private:
-  static RawObject* const kSentinel;
+  struct Node {
+    RawObject** ptr;  // kSentinel for the sentinel node.
+    RawObject* obj;
+  };
+
+  static RawObject** const kSentinel;
   static const intptr_t kInitialCapacity = 1024;
-  GrowableArray<RawObject*> data_;
+  static const intptr_t kNoParent = -1;
+
+  intptr_t Parent(intptr_t index) const {
+    // The parent is just below the next sentinel.
+    for (intptr_t i = index; i >= 1; --i) {
+      if (data_[i].ptr == kSentinel) {
+        return i - 1;
+      }
+    }
+    return kNoParent;
+  }
+
+  GrowableArray<Node> data_;
   friend class StackIterator;
   DISALLOW_COPY_AND_ASSIGN(Stack);
 };
 
 
-// We only visit heap objects, so any Smi works as the sentinel.
-RawObject* const ObjectGraph::Stack::kSentinel = Smi::New(0);
+RawObject** const ObjectGraph::Stack::kSentinel = NULL;
 
 
 RawObject* ObjectGraph::StackIterator::Get() const {
-  return stack_->data_[index_];
+  return stack_->data_[index_].obj;
 }
 
 
 bool ObjectGraph::StackIterator::MoveToParent() {
-  // The parent is just below the next sentinel.
-  for (int i = index_; i >= 1; --i) {
-    if (stack_->data_[i] == Stack::kSentinel) {
-      index_ = i - 1;
-      return true;
-    }
+  intptr_t parent = stack_->Parent(index_);
+  if (parent == Stack::kNoParent) {
+    return false;
+  } else {
+    index_ = parent;
+    return true;
+  }
+}
+
+
+intptr_t ObjectGraph::StackIterator::OffsetFromParentInWords() const {
+  intptr_t parent_index = stack_->Parent(index_);
+  if (parent_index == Stack::kNoParent) {
+    return -1;
+  }
+  Stack::Node parent = stack_->data_[parent_index];
+  uword parent_start = RawObject::ToAddr(parent.obj);
+  Stack::Node child = stack_->data_[index_];
+  ASSERT(child.obj == *child.ptr);
+  uword child_ptr_addr = reinterpret_cast<uword>(child.ptr);
+  intptr_t offset = child_ptr_addr - parent_start;
+  if (offset > 0 && offset < parent.obj->Size()) {
+    ASSERT(Utils::IsAligned(offset, kWordSize));
+    return offset >> kWordSizeLog2;
+  } else {
+    // Some internal VM objects visit pointers not contained within the parent.
+    // For instance, RawCode::VisitCodePointers visits pointers in instructions.
+    ASSERT(!parent.obj->IsDartInstance());
+    return -1;
   }
-  return false;
 }
 
 
@@ -224,10 +266,15 @@ class RetainingPathVisitor : public ObjectGraph::Visitor {
     } else {
       HANDLESCOPE(Isolate::Current());
       Object& current = Object::Handle();
+      Smi& offset_from_parent = Smi::Handle();
       do {
-        if (!path_.IsNull() && length_ < path_.Length()) {
+        intptr_t obj_index = length_ * 2;
+        intptr_t offset_index = obj_index + 1;
+        if (!path_.IsNull() && offset_index < path_.Length()) {
           current = it->Get();
-          path_.SetAt(length_, current);
+          path_.SetAt(obj_index, current);
+          offset_from_parent = Smi::New(it->OffsetFromParentInWords());
+          path_.SetAt(offset_index, offset_from_parent);
         }
         ++length_;
       } while (it->MoveToParent());

runtime/vm/object_graph.h

@@ -27,6 +27,8 @@ class ObjectGraph : public StackResource {
     RawObject* Get() const;
     // Returns false if there is no parent.
     bool MoveToParent();
+    // Offset into parent for the pointer to current object. -1 if no parent.
+    intptr_t OffsetFromParentInWords() const;
    private:
     StackIterator(const Stack* stack, intptr_t index)
         : stack_(stack), index_(index) { }
@@ -69,9 +71,10 @@ class ObjectGraph : public StackResource {
   intptr_t SizeRetainedByClass(intptr_t class_id);
 
   // Finds some retaining path from the isolate roots to 'obj'. Populates the
-  // provided array, starting 'obj' itself, up to the smaller of the length of
-  // the array and the length of the path. Returns the length of the path. A
-  // null input array behaves like a zero-length input array.
+  // provided array with pairs of (object, offset from parent in words),
+  // starting with 'obj' itself, as far as there is room. Returns the number
+  // of objects on the full path. A null input array behaves like a zero-length
+  // input array. The 'offset' of a root is -1.
   //
   // To break the trivial path, the handle 'obj' is temporarily cleared during
   // the search, but restored before returning. If no path is found (i.e., the

runtime/vm/object_graph_test.cc

@@ -45,14 +45,14 @@ TEST_CASE(ObjectGraph) {
   //  +   +
   //  |   v
   //  +-->d
-  Array& a = Array::Handle(Array::New(2, Heap::kNew));
+  Array& a = Array::Handle(Array::New(12, Heap::kNew));
   Array& b = Array::Handle(Array::New(2, Heap::kOld));
   Array& c = Array::Handle(Array::New(0, Heap::kOld));
   Array& d = Array::Handle(Array::New(0, Heap::kOld));
-  a.SetAt(0, b);
+  a.SetAt(10, b);
   b.SetAt(0, c);
   b.SetAt(1, d);
-  a.SetAt(1, d);
+  a.SetAt(11, d);
   intptr_t a_size = a.raw()->Size();
   intptr_t b_size = b.raw()->Size();
   intptr_t c_size = c.raw()->Size();
@@ -93,7 +93,7 @@ TEST_CASE(ObjectGraph) {
   }
   {
     // Get hold of c again.
-    b ^= a.At(0);
+    b ^= a.At(10);
     c ^= b.At(0);
     b = Array::null();
     ObjectGraph graph(isolate);
@@ -111,17 +111,26 @@ TEST_CASE(ObjectGraph) {
     }
     {
       HANDLESCOPE(isolate);
-      Array& path = Array::Handle(Array::New(3, Heap::kNew));
+      Array& path = Array::Handle(Array::New(6, Heap::kNew));
       intptr_t length = graph.RetainingPath(&c, path);
       EXPECT_LE(3, length);
       Array& expected_c = Array::Handle();
       expected_c ^= path.At(0);
+      // c is the first element in b.
+      Smi& offset_from_parent = Smi::Handle();
+      offset_from_parent ^= path.At(1);
+      EXPECT_EQ(Array::element_offset(0),
+                offset_from_parent.Value() * kWordSize);
       Array& expected_b = Array::Handle();
-      expected_b ^= path.At(1);
+      expected_b ^= path.At(2);
+      // b is the element with index 10 in a.
+      offset_from_parent ^= path.At(3);
+      EXPECT_EQ(Array::element_offset(10),
+                offset_from_parent.Value() * kWordSize);
       Array& expected_a = Array::Handle();
-      expected_a ^= path.At(2);
+      expected_a ^= path.At(4);
       EXPECT(expected_c.raw() == c.raw());
-      EXPECT(expected_b.raw() == a.At(0));
+      EXPECT(expected_b.raw() == a.At(10));
       EXPECT(expected_a.raw() == a.raw());
     }
   }

runtime/vm/service.cc

@@ -934,6 +934,54 @@ static bool ContainsNonInstance(const Object& obj) {
 }
 
 
+static bool HandleRetainingPath(Isolate* isolate,
+                                Object* obj,
+                                intptr_t limit,
+                                JSONStream* js) {
+  ObjectGraph graph(isolate);
+  Array& path = Array::Handle(Array::New(limit * 2));
+  intptr_t length = graph.RetainingPath(obj, path);
+  JSONObject jsobj(js);
+  jsobj.AddProperty("type", "RetainingPath");
+  jsobj.AddProperty("id", "retaining_path");
+  jsobj.AddProperty("length", length);
+  JSONArray elements(&jsobj, "elements");
+  Object& element = Object::Handle();
+  Object& parent = Object::Handle();
+  Smi& offset_from_parent = Smi::Handle();
+  Class& parent_class = Class::Handle();
+  Array& parent_field_map = Array::Handle();
+  Field& field = Field::Handle();
+  limit = Utils::Minimum(limit, length);
+  for (intptr_t i = 0; i < limit; ++i) {
+    JSONObject jselement(&elements);
+    element = path.At(i * 2);
+    jselement.AddProperty("index", i);
+    jselement.AddProperty("value", element);
+    // Interpret the word offset from parent as list index or instance field.
+    // TODO(koda): User-friendly interpretation for map entries.
+    offset_from_parent ^= path.At((i * 2) + 1);
+    int parent_i = i + 1;
+    if (parent_i < limit) {
+      parent = path.At(parent_i * 2);
+      if (parent.IsArray()) {
+        intptr_t element_index = offset_from_parent.Value() -
+            (Array::element_offset(0) >> kWordSizeLog2);
+        jselement.AddProperty("parentListIndex", element_index);
+      } else if (parent.IsInstance()) {
+        parent_class ^= parent.clazz();
+        parent_field_map = parent_class.OffsetToFieldMap();
+        intptr_t offset = offset_from_parent.Value();
+        if (offset > 0 && offset < parent_field_map.Length()) {
+          field ^= parent_field_map.At(offset);
+          jselement.AddProperty("parentField", field);
+        }
+      }
+    }
+  }
+  return true;
+}
+
 // Takes an Object* only because RetainingPath temporarily clears it.
 static bool HandleInstanceCommands(Isolate* isolate,
                                    Object* obj,
@@ -992,22 +1040,7 @@ static bool HandleInstanceCommands(Isolate* isolate,
                  js->num_arguments());
       return true;
     }
-    ObjectGraph graph(isolate);
-    Array& path = Array::Handle(Array::New(limit));
-    intptr_t length = graph.RetainingPath(obj, path);
-    JSONObject jsobj(js);
-    jsobj.AddProperty("type", "RetainingPath");
-    jsobj.AddProperty("id", "retaining_path");
-    jsobj.AddProperty("length", length);
-    JSONArray elements(&jsobj, "elements");
-    for (intptr_t i = 0; i < path.Length() && i < length; ++i) {
-      JSONObject jselement(&elements);
-      Object& element = Object::Handle();
-      element = path.At(i);
-      jselement.AddProperty("index", i);
-      jselement.AddProperty("value", element);
-    }
-    return true;
+    return HandleRetainingPath(isolate, obj, limit, js);
   }
 
   PrintError(js, "unrecognized action '%s'\n", action);

runtime/vm/service_test.cc

@@ -608,6 +608,119 @@ TEST_CASE(Service_Objects) {
 }
 
 
+TEST_CASE(Service_RetainingPath) {
+  const char* kScript =
+      "var port;\n"    // Set to our mock port by C++.
+      "var id0;\n"     // Set to an object id by C++.
+      "var id1;\n"     // Ditto.
+      "var idElem;\n"  // Ditto.
+      "class Foo {\n"
+      "  String f0;\n"
+      "  String f1;\n"
+      "}\n"
+      "Foo foo;\n"
+      "List<String> lst;\n"
+      "main() {\n"
+      "  foo = new Foo();\n"
+      "  lst = new List<String>(100);\n"
+      "}\n";
+  Isolate* isolate = Isolate::Current();
+  Dart_Handle h_lib = TestCase::LoadTestScript(kScript, NULL);
+  EXPECT_VALID(h_lib);
+  Library& lib = Library::Handle();
+  lib ^= Api::UnwrapHandle(h_lib);
+  EXPECT(!lib.IsNull());
+  Dart_Handle result = Dart_Invoke(h_lib, NewString("main"), 0, NULL);
+  EXPECT_VALID(result);
+  const Class& class_foo = Class::Handle(GetClass(lib, "Foo"));
+  EXPECT(!class_foo.IsNull());
+  Dart_Handle foo = Dart_GetField(h_lib, NewString("foo"));
+  Dart_Handle lst = Dart_GetField(h_lib, NewString("lst"));
+  const intptr_t kElemIndex = 42;
+  {
+    Dart_EnterScope();
+    ObjectIdRing* ring = isolate->object_id_ring();
+    {
+      const String& foo0 = String::Handle(String::New("foo0", Heap::kOld));
+      Dart_Handle h_foo0 = Api::NewHandle(isolate, foo0.raw());
+      EXPECT_VALID(Dart_SetField(foo, NewString("f0"), h_foo0));
+      Dart_Handle id0 = Dart_NewInteger(ring->GetIdForObject(foo0.raw()));
+      EXPECT_VALID(id0);
+      EXPECT_VALID(Dart_SetField(h_lib, NewString("id0"), id0));
+    }
+    {
+      const String& foo1 = String::Handle(String::New("foo1", Heap::kOld));
+      Dart_Handle h_foo1 = Api::NewHandle(isolate, foo1.raw());
+      EXPECT_VALID(Dart_SetField(foo, NewString("f1"), h_foo1));
+      Dart_Handle id1 = Dart_NewInteger(ring->GetIdForObject(foo1.raw()));
+      EXPECT_VALID(id1);
+      EXPECT_VALID(Dart_SetField(h_lib, NewString("id1"), id1));
+    }
+    {
+      const String& elem = String::Handle(String::New("elem", Heap::kOld));
+      Dart_Handle h_elem = Api::NewHandle(isolate, elem.raw());
+      EXPECT_VALID(Dart_ListSetAt(lst, kElemIndex, h_elem));
+      Dart_Handle idElem = Dart_NewInteger(ring->GetIdForObject(elem.raw()));
+      EXPECT_VALID(idElem);
+      EXPECT_VALID(Dart_SetField(h_lib, NewString("idElem"), idElem));
+    }
+    Dart_ExitScope();
+  }
+
+  // Build a mock message handler and wrap it in a dart port.
+  ServiceTestMessageHandler handler;
+  Dart_Port port_id = PortMap::CreatePort(&handler);
+  Dart_Handle port = Api::NewHandle(isolate, SendPort::New(port_id));
+  EXPECT_VALID(port);
+  EXPECT_VALID(Dart_SetField(h_lib, NewString("port"), port));
+  Instance& service_msg = Instance::Handle();
+
+  // Retaining path to 'foo0', limit 2.
+  service_msg = Eval(
+      h_lib,
+      "[0, port, ['objects', '$id0', 'retaining_path'], ['limit'], ['2']]");
+  Service::HandleIsolateMessage(isolate, service_msg);
+  handler.HandleNextMessage();
+  ExpectSubstringF(
+      handler.msg(),
+      "{\"type\":\"RetainingPath\",\"id\":\"retaining_path\",\"length\":2,"
+      "\"elements\":[{\"index\":0,\"value\":{\"type\":\"@String\"");
+  ExpectSubstringF(handler.msg(), "\"parentField\":{\"type\":\"@Field\"");
+  ExpectSubstringF(handler.msg(), "\"name\":\"f0\"");
+  ExpectSubstringF(handler.msg(),
+      "{\"index\":1,\"value\":{\"type\":\"@Instance\"");
+
+  // Retaining path to 'foo1', limit 2.
+  service_msg = Eval(
+      h_lib,
+      "[0, port, ['objects', '$id1', 'retaining_path'], ['limit'], ['2']]");
+  Service::HandleIsolateMessage(isolate, service_msg);
+  handler.HandleNextMessage();
+  ExpectSubstringF(
+      handler.msg(),
+      "{\"type\":\"RetainingPath\",\"id\":\"retaining_path\",\"length\":2,"
+      "\"elements\":[{\"index\":0,\"value\":{\"type\":\"@String\"");
+  ExpectSubstringF(handler.msg(), "\"parentField\":{\"type\":\"@Field\"");
+  ExpectSubstringF(handler.msg(), "\"name\":\"f1\"");
+  ExpectSubstringF(handler.msg(),
+      "{\"index\":1,\"value\":{\"type\":\"@Instance\"");
+
+  // Retaining path to 'elem', limit 2.
+  service_msg = Eval(
+      h_lib,
+      "[0, port, ['objects', '$idElem', 'retaining_path'], ['limit'], ['2']]");
+  Service::HandleIsolateMessage(isolate, service_msg);
+  handler.HandleNextMessage();
+  ExpectSubstringF(
+      handler.msg(),
+      "{\"type\":\"RetainingPath\",\"id\":\"retaining_path\",\"length\":2,"
+      "\"elements\":[{\"index\":0,\"value\":{\"type\":\"@String\"");
+  ExpectSubstringF(handler.msg(), "\"parentListIndex\":%" Pd, kElemIndex);
+  ExpectSubstringF(handler.msg(),
+      "{\"index\":1,\"value\":{\"type\":\"@Array\"");
+}
+
+
 TEST_CASE(Service_Libraries) {
   const char* kScript =
       "var port;\n"  // Set to our mock port by C++.