Start migrating profiles processing to C++.

Start migrating profiles processing to C++.

Adding code for maintaining (address -> name) mapping and building
call trees.

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

[Søren Thygesen Gjesse, 2010-03-15 10:32:34]

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.

[mnaganov (inactive), 2010-03-15 12:54:37]

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.

[Finnur, 2011-03-25 15:18:48]

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

http://codereview.chromium.org/910002/diff/10001/src/profile-generator.cc#new...
src/profile-generator.cc:143: callback->AfterChildTraversed(parent.node,
current.node);
If the callback is a DeleteNodesCallback, then we're passing a deleted node here
into AfterChildTraversed due to line 140, no? I don't know how this code works,
can that situation occur? (CID: 9378)

[mnaganov (inactive), 2011-03-28 08:16:37]

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

http://codereview.chromium.org/910002/diff/10001/src/profile-generator.cc#new...
src/profile-generator.cc:143: callback->AfterChildTraversed(parent.node,
current.node);
On 2011/03/25 15:18:48, Finnur wrote:
> If the callback is a DeleteNodesCallback, then we're passing a deleted node
here
> into AfterChildTraversed due to line 140, no? I don't know how this code
works,
> can that situation occur? (CID: 9378)

You are right. But in DeleteNodesCallback, AfterChildTraversed is empty, so
passing a stale pointer to it doesn't cause any harm. But I should add a comment
about it, I think. Thanks for noticing!

[Søren Thygesen Gjesse, 2011-03-28 09:24:03]

LGTM