Object graph visitor: general depth-first search.

Object graph visitor: general depth-first search.

Features:
 * Performs a pre-order, depth-first search.
 * Allows the visitor to direct whether to
   proceed, backtrack, or abort.
 * Provides a complete chain of parents to the root.
   (unlike the GC marking stack).

The plan is to use this in the observatory, e.g.:
 * inspect a path from the isolate roots to a particular object, or
 * determine how much memory is kept alive by some particular object or class of objects.

R=johnmccutchan@google.com

Committed: https://code.google.com/p/dart/source/detail?r=35711

[Cutch, 2014-05-02 18:23:42]

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph.cc
File runtime/vm/object_graph.cc (right):

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph...
runtime/vm/object_graph.cc:41: void Drain(ObjectGraph::Visitor* visitor) {
I would rename this to something like "VisitGraph" or "WalkGraph".

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph.h
File runtime/vm/object_graph.h (right):

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph...
runtime/vm/object_graph.h:67: NoGCScope no_gc_scope_;
Should this NoGCScope be here or inside the IterateObjects / IterateObjectsFrom
call? Putting it here means the user may need to manage scopes, for example:

{
  ObjectGraph og(Isolate::Current());
}

My preference would be to move it into the IterateObjects / IterateObjectsFrom
code.

[koda, 2014-05-02 18:51:34]

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph.cc
File runtime/vm/object_graph.cc (right):

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph...
runtime/vm/object_graph.cc:41: void Drain(ObjectGraph::Visitor* visitor) {
On 2014/05/02 18:23:42, Cutch wrote:
> I would rename this to something like "VisitGraph" or "WalkGraph".

Done.

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph.h
File runtime/vm/object_graph.h (right):

https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph...
runtime/vm/object_graph.h:67: NoGCScope no_gc_scope_;
On 2014/05/02 18:23:42, Cutch wrote:
> Should this NoGCScope be here or inside the IterateObjects /
IterateObjectsFrom
> call? Putting it here means the user may need to manage scopes, for example:
> 
> {
>   ObjectGraph og(Isolate::Current());
> }
> 
> My preference would be to move it into the IterateObjects / IterateObjectsFrom
> code.

Done. Also changed IterateObjectsFrom to take a handle, and added explicit
warning that VisitObject must not allocate on the heap.

[koda, 2014-05-02 20:03:31]

On 2014/05/02 18:51:34, koda wrote:
>
https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph.cc
> File runtime/vm/object_graph.cc (right):
> 
>
https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph...
> runtime/vm/object_graph.cc:41: void Drain(ObjectGraph::Visitor* visitor) {
> On 2014/05/02 18:23:42, Cutch wrote:
> > I would rename this to something like "VisitGraph" or "WalkGraph".
> 
> Done.
> 
>
https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph.h
> File runtime/vm/object_graph.h (right):
> 
>
https://codereview.chromium.org/266643002/diff/100001/runtime/vm/object_graph...
> runtime/vm/object_graph.h:67: NoGCScope no_gc_scope_;
> On 2014/05/02 18:23:42, Cutch wrote:
> > Should this NoGCScope be here or inside the IterateObjects /
> IterateObjectsFrom
> > call? Putting it here means the user may need to manage scopes, for example:
> > 
> > {
> >   ObjectGraph og(Isolate::Current());
> > }
> > 
> > My preference would be to move it into the IterateObjects /
IterateObjectsFrom
> > code.
> 
> Done. Also changed IterateObjectsFrom to take a handle, and added explicit
> warning that VisitObject must not allocate on the heap.

Also fixed some gc-unsafe code in the unit test. PTAL

[Cutch, 2014-05-02 20:09:00]

lgtm

[Ivan Posva, 2014-05-02 21:20:53]

DBC -ip

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph.cc
File runtime/vm/object_graph.cc (right):

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph...
runtime/vm/object_graph.cc:7: #include <vector>
Please use a GrowableArray here.

[koda, 2014-05-02 21:31:40]

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph.cc
File runtime/vm/object_graph.cc (right):

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph...
runtime/vm/object_graph.cc:7: #include <vector>
On 2014/05/02 21:20:53, Ivan Posva wrote:
> Please use a GrowableArray here.

Done.

[koda, 2014-05-02 21:38:34]

Committed patchset #9 manually as r35711 (presubmit successful).

[Ivan Posva, 2014-05-02 22:20:40]

LGTMwC

-Ivan

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph.h
File runtime/vm/object_graph.h (right):

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph...
runtime/vm/object_graph.h:52: // No GC is allowed during the lifetime of an
ObjectGraph instance.
No allocation and no GC is allowed...

https://codereview.chromium.org/266643002/diff/160001/runtime/vm/object_graph.cc
File runtime/vm/object_graph.cc (right):

https://codereview.chromium.org/266643002/diff/160001/runtime/vm/object_graph...
runtime/vm/object_graph.cc:125: isolate_->heap()->WriteProtectCode(true);
To make sure that this destructor is called you need to make the ObjectGraph
extend StackResource.

[koda, 2014-05-02 22:45:18]

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph.h
File runtime/vm/object_graph.h (right):

https://codereview.chromium.org/266643002/diff/140001/runtime/vm/object_graph...
runtime/vm/object_graph.h:52: // No GC is allowed during the lifetime of an
ObjectGraph instance.
On 2014/05/02 22:20:41, Ivan Posva wrote:
> No allocation and no GC is allowed...

Actually, the comment is obsolete, since I moved the NoGCScope into the Iterate*
methods.

I clarified the warning on Visitor::VisitObject in the follow-up CL.

https://codereview.chromium.org/266643002/diff/160001/runtime/vm/object_graph.cc
File runtime/vm/object_graph.cc (right):

https://codereview.chromium.org/266643002/diff/160001/runtime/vm/object_graph...
runtime/vm/object_graph.cc:125: isolate_->heap()->WriteProtectCode(true);
On 2014/05/02 22:20:41, Ivan Posva wrote:
> To make sure that this destructor is called you need to make the ObjectGraph
> extend StackResource.

Done in follow-up CL.