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#newcode1232
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#newcode1890
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#newcode1905
src/profile-generator.cc:1905:
nit: Insert one more blank line.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcode2338
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#newcode2340
src/profile-generator.cc:2340: struct IndexTag {
Can be private.

http://codereview.chromium.org/8716009/diff/1/src/profile-generator.cc#newcode2363
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#newcode2370
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#newcode2381
src/profile-generator.cc:2381: }
ASSERT(strong_index <= all_index)?

http://codereview.chromium.org/8716009/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to