LGTM

http://codereview.chromium.org/910002/diff/1/2
File src/SConscript (right):

http://codereview.chromium.org/910002/diff/1/2#newcode83
src/SConscript:83: profile-generator.cc
Please remember the other project files below tools, especially
tools/gyp/v8.gyp.

http://codereview.chromium.org/910002/diff/1/3
File src/profile-generator-inl.h (right):

http://codereview.chromium.org/910002/diff/1/3#newcode56
src/profile-generator-inl.h:56: name_(name->ToCString(DISALLOW_NULLS,
ROBUST_STRING_TRAVERSAL).Detach()),
Have you thought about just storing the string object pointer here and
making the GC know about this? Or do we have a threading issue with how
this will be used? Alternatively maybe add these names to the
resource_name hashmap as well?

http://codereview.chromium.org/910002/diff/1/5
File src/profile-generator.h (right):

http://codereview.chromium.org/910002/diff/1/5#newcode123
src/profile-generator.h:123: ProfileTree()
One line?

http://codereview.chromium.org/910002/diff/1/5#newcode168
src/profile-generator.h:168: CodeMap() { }
addCode -> AddCode, moveCode -> MoveCode, ...

http://codereview.chromium.org/910002/diff/1/5#newcode177
src/profile-generator.h:177: CodeEntryInfo(CodeEntry* a_entry, unsigned
a_size)
an_entry?

http://codereview.chromium.org/910002/diff/1/6
File src/splay-tree-inl.h (right):

http://codereview.chromium.org/910002/diff/1/6#newcode69
src/splay-tree-inl.h:69: void SplayTree<Config,
Allocator>::InsertInternal(int cmp, Node* node) {
Is it possible to add an assert here to check that FindInternal has been
called?

http://codereview.chromium.org/910002/diff/1/9
File test/cctest/test-profile-generator.cc (right):

http://codereview.chromium.org/910002/diff/1/9#newcode3
test/cctest/test-profile-generator.cc:3: // Tests of profiles generator
and utilities.
This test have no use of ManagedNameCodeEntry.

http://codereview.chromium.org/910002

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

Reply via email to