Distinguish weak references in heap snapshots, group GC roots.

Distinguish weak references in heap snapshots, group GC roots.

Several changes to better organize snapshot data:

1. Provide information about weak references.
2. Group (GC roots) children.
3. Prettify debug snapshot printing.

BUG=v8:1832
TEST=cctest/test-heap-profiler/*Weak*

Committed: http://code.google.com/p/v8/source/detail?r=10158

[Vitaly Repeshko, 2011-12-02 23:39:47]

LGTM

http://codereview.chromium.org/8716009/diff/1/include/v8-profiler.h
File include/v8-profiler.h (right):

http://codereview.chromium.org/8716009/diff/1/include/v8-profiler.h#newcode224
include/v8-profiler.h:224: kWeak = 6              // A weak persistent handle
reference.
Can this be used for other weak references, e.g., context chaining?

http://codereview.chromium.org/8716009/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/8716009/diff/1/src/objects.h#newcode7894
src/objects.h:7894: static const char* kTags[kNumberOfSyncTags];
One more const.

http://codereview.chromium.org/8716009/diff/1/src/objects.h#newcode7895
src/objects.h:7895: static const char* kTagNames[kNumberOfSyncTags];
Same here.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:1232: for (int i = 0; i <
VisitorSynchronization::kNumberOfSyncTags; ++i)
nit: {}

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:1890: object_count_ += end - start;
Will this require a follow up "Fix win64 build"? :)

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:1905: 
nit: Insert one more blank line.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2338: class RootsReferencesExtractor : public
ObjectVisitor {
nit: Consider adding more vertical whitespace to the body of this class.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2340: struct IndexTag {
Can be private.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2363: while (strong_index <
strong_references_.length()) {
You can have a single loop here (over all references) if you add "strong_index <
strong_references_.length()" to the first "if" below.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2370:
explorer->SetGcSubrootReference(reference_tags_[tags_index].tag,
Hmm, actually, are you sure this loop terminates in its current form?
Strong_index is only incremented in one of the branches.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2381: }
ASSERT(strong_index <= all_index)?

[mnaganov (inactive), 2011-12-05 12:55:25]

http://codereview.chromium.org/8716009/diff/1/include/v8-profiler.h
File include/v8-profiler.h (right):

http://codereview.chromium.org/8716009/diff/1/include/v8-profiler.h#newcode224
include/v8-profiler.h:224: kWeak = 6              // A weak persistent handle
reference.
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> Can this be used for other weak references, e.g., context chaining?

Sure, just a stale comment. Updated.

http://codereview.chromium.org/8716009/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/8716009/diff/1/src/objects.h#newcode7894
src/objects.h:7894: static const char* kTags[kNumberOfSyncTags];
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> One more const.

Done.

http://codereview.chromium.org/8716009/diff/1/src/objects.h#newcode7895
src/objects.h:7895: static const char* kTagNames[kNumberOfSyncTags];
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> Same here.

Done.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:1232: for (int i = 0; i <
VisitorSynchronization::kNumberOfSyncTags; ++i)
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> nit: {}

Done.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:1890: object_count_ += end - start;
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> Will this require a follow up "Fix win64 build"? :)

Thanks. I forgot about this disaster.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:1905: 
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> nit: Insert one more blank line.

Done.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2338: class RootsReferencesExtractor : public
ObjectVisitor {
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> nit: Consider adding more vertical whitespace to the body of this class.

Done.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2340: struct IndexTag {
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> Can be private.

Done.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2363: while (strong_index <
strong_references_.length()) {
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> You can have a single loop here (over all references) if you add "strong_index
<
> strong_references_.length()" to the first "if" below.

Right. I had this in mind but then forgot. Fixed.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2370:
explorer->SetGcSubrootReference(reference_tags_[tags_index].tag,
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> Hmm, actually, are you sure this loop terminates in its current form?
> Strong_index is only incremented in one of the branches.

Yes. strong_references_ is a subset of all_references_. When we meet the item
that is in both sets, we advance both indexes, otherwise only all_references_.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcod...
src/profile-generator.cc:2381: }
On 2011/12/02 23:39:47, Vitaly Repeshko wrote:
> ASSERT(strong_index <= all_index)?

Added to the beginning:

ASSERT(strong_references_.length() <= all_references_.length());

This will "guarantee" that strongs_references_ is a subset of all_references_.