Thanks!
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
On 2010/03/15 10:32:34, Søren Gjesse wrote:
Please remember the other project files below tools, especially
tools/gyp/v8.gyp.
Ah, yes. Thanks for reminding!
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()),
On 2010/03/15 10:32:34, Søren Gjesse wrote:
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?
Yes, I thought about this. There are no threading issues, because the
analyzer thread only manipulates links to CodeEntries and never touches
their contents. But I decided that putting additional burden on GC that
is only needed for profiler is a bad style. In my approach, the overhead
is added only when profiling is active.
Also, remembering cases when the source of a script -- an external
string -- was disappearing at some point, I decided that it is more
solid just to copy the name at the moment when it's surely available.
Maybe I'll turn to using hashmap when will be dealing with duplicate
names (as in the JS version -- adding '{N}' to duplicate names of
different functions.)
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()
On 2010/03/15 10:32:34, Søren Gjesse wrote:
One line?
Done.
http://codereview.chromium.org/910002/diff/1/5#newcode168
src/profile-generator.h:168: CodeMap() { }
On 2010/03/15 10:32:34, Søren Gjesse wrote:
addCode -> AddCode, moveCode -> MoveCode, ...
Oh, my bad. Fixed.
http://codereview.chromium.org/910002/diff/1/5#newcode177
src/profile-generator.h:177: CodeEntryInfo(CodeEntry* a_entry, unsigned
a_size)
On 2010/03/15 10:32:34, Søren Gjesse wrote:
an_entry?
Done.
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) {
On 2010/03/15 10:32:34, Søren Gjesse wrote:
Is it possible to add an assert here to check that FindInternal has
been called?
It's not mandatory, actually. See Insert implementation.
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.
On 2010/03/15 10:32:34, Søren Gjesse wrote:
This test have no use of ManagedNameCodeEntry.
Here CodeEntry objects are just instruments. The StaticName one is
simpler, so I'm using it.
http://codereview.chromium.org/910002
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
