Reviewers: Mikhail Naganov (Chromium), Søren Gjesse,
Message:
Mikhail,
Here's a first step at refactoring the debug MarkRootObjectVisitor
mechanism in
heap.cc. I've now encapsulated it into a PathTracer class and provided a
virtual ProcessResults() method that can be overridden to do customized work
with the results found from the path tracing. This will be used by the LOL
code
later.
I thought I'd submit this change independently first so that you can see
that it
is semantically equivalent to the pre-existing code with only minor changes.
I've added comments at various places to let you know what and why I made
those
changes where relevant.
Please take a look. Thanks.
Mark
http://codereview.chromium.org/6541044/diff/1/src/heap.cc
File src/heap.cc (right):
http://codereview.chromium.org/6541044/diff/1/src/heap.cc#newcode5186
src/heap.cc:5186: void PathTracer::TracePathFrom(Object** root) {
TracePathFrom() used to be MarkRootObjectRecursively().
http://codereview.chromium.org/6541044/diff/1/src/heap.cc#newcode5197
src/heap.cc:5197: ProcessResults();
ProcessResults() is introduced as a virtual method to allow a subclass
to customize it. Technically, this tracer class can be used to do other
things instead of just printing a path. LOL will use it to print more
verbose information with some filtering added. There are other uses for
it as well that I'm still exploring.
http://codereview.chromium.org/6541044/diff/1/src/heap.cc#newcode5201
src/heap.cc:5201: void PathTracer::MarkRecursively(Object** p) {
MarkRecursively() used to be MarkObjectRecursively() in heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.cc#newcode5238
src/heap.cc:5238: }
The pre-existing code would scan all entries in the object body.
However, the Context object contains implicit 2 weak ref pointers that
are treated specially by GC. For LOL's leak debugging, the weak refs
should not be scanned either. Hence, I added the option to skip them
here.
http://codereview.chromium.org/6541044/diff/1/src/heap.cc#newcode5242
src/heap.cc:5242: MarkRecursively(&map);
I moved the tracing of the map after the tracing of the body because in
the memory leak debugging work of LOL, the user is more likely to be
interested in the reference paths via the body properties than then map.
I don't expect this to cause any problems for the existing heap
debugging tracers. If there's an issue, please let me know and I can
abstract this further. Thanks.
http://codereview.chromium.org/6541044/diff/1/src/heap.cc#newcode5249
src/heap.cc:5249: void PathTracer::UnmarkRecursively(Object** p) {
UnmarkRecursively() used to be called UnmarkObjectRecursively() in
heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h
File src/heap.h (right):
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2164
src/heap.h:2164: class PathTracer: public ObjectVisitor {
PathTracer used to be called MarkRootVisitor in heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2185
src/heap.h:2185: done = ((what_to_find_ == kFindFirst) &&
found_target_);
I added an optimization to bail here if we've already found what we
want.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2190
src/heap.h:2190: void TracePathFrom(Object** root);
TracePathFrom() used to be called MarkRootObjectRecursively() in
heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2199
src/heap.h:2199: class MarkVisitor: public ObjectVisitor {
MarkVisitor used to be called MarkObjectVisitor in heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2204
src/heap.h:2204: for (Object** p = start; !tracer_->found() && (p <
end); p++) {
The check for !tracer->found() is an optimization that allows us to bail
from the loop if we've already found the target. Otherwise, it would
call MarkRecursively() (previously MarkObjectRecursively()) which does
returns and does nothing if the target is found anyway. So, there is no
semantic change here.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2215
src/heap.h:2215: class UnmarkVisitor: public ObjectVisitor {
UnmarkVisitor used to be called UnmarkObjectVisitor in heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2232
src/heap.h:2232: void MarkRecursively(Object** p);
MarkRecursively() used to be called MarkObjectRecursively() in heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2233
src/heap.h:2233: void UnmarkRecursively(Object** p);
UnmarkRecursively() used to be called UnmarkObjectRecursively() in
heap.cc.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2244
src/heap.h:2244: List<Object*> object_stack_;
search_target_, search_for_any_global_, found_target_, and object_stack_
used to be global statics. I've encapsulated them in the tracer object
now.
http://codereview.chromium.org/6541044/diff/1/src/heap.h#newcode2247
src/heap.h:2247: UnmarkVisitor unmark_visitor;
There used to be global static instances of these 2 visitors in heap.cc
too. Now, their lifecycle is bound to the tracer's.
Description:
Refactored PathTracer in heap.cc.
This is so that it can be reused by LOL code later.
Please review this at http://codereview.chromium.org/6541044/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/heap.h
M src/heap.cc
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
