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#newcode1232
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#newcode1890
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#newcode1905
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#newcode2338
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#newcode2340
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#newcode2363
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#newcode2370
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#newcode2381
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_.
http://codereview.chromium.org/8716009/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
