Add initial semi-working producers profile.

Add initial semi-working producers profile.

Turned on with '--log-producers' flag, also needs '--noinline-new' (this is temporarily), '--log-code', '--log-gc'. Not all allocations are traced (I'm investigating.)

Stacks are stored using weak handles. Thus, when an object is collected, its allocation stack is deleted.

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

[mnaganov (inactive), 2009-10-13 16:03:53]

Hi Kevin,

Both Soeren and Mads are on vacation. Can you please take a look?

[Kevin Millikin (Chromium), 2009-10-14 15:11:30]

As an initial implementation it LGTM.

http://codereview.chromium.org/267077/diff/2001/22
File src/global-handles.h (right):

http://codereview.chromium.org/267077/diff/2001/22#newcode57
Line 57: typedef void (*WeakReferenceVisitor)(Object* object, void* parameter);
This shouldn't be called ...Visitor unless it's a subclass of the ObjectVisitor.
 I can't think of a better name, though.

http://codereview.chromium.org/267077/diff/2001/23
File src/heap-profiler.cc (right):

http://codereview.chromium.org/267077/diff/2001/23#newcode597
Line 597: static_cast<Address*>(trace)));
Can reinterpret_cast here.

http://codereview.chromium.org/267077/diff/2001/24
File src/heap-profiler.h (right):

http://codereview.chromium.org/267077/diff/2001/24#newcode261
Line 261: static bool can_log;
can_log_.  I'd prefer that it was private, and there was a Setup function that
set it to true.

http://codereview.chromium.org/267077/diff/2001/25
File src/heap.cc (right):

http://codereview.chromium.org/267077/diff/2001/25#newcode3313
Line 3313: ProducerHeapProfile::can_log = true;
If you have a ProducerHeapProfile::Setup function you can call it here (with a
comment that it should be called last during heap setup).

[Vitaly Repeshko, 2009-10-14 17:05:55]

Drive by comments.

http://codereview.chromium.org/267077/diff/2001/23
File src/heap-profiler.cc (right):

http://codereview.chromium.org/267077/diff/2001/23#newcode587
Line 587: object.ClearWeak();
You probably want to Dispose() the weak handle here. ClearWeak() turns it into a
strong handle.

http://codereview.chromium.org/267077/diff/2001/23#newcode643
Line 643: GlobalHandles::IterateWeakRoots(PrintProducerStackTrace,
StackWeakReferenceCallback);
Line too long?

http://codereview.chromium.org/267077/diff/2001/23#newcode665
Line 665: HandleScope scope;
Persistent handles don't need handle scope.

http://codereview.chromium.org/267077/diff/2001/23#newcode666
Line 666: Handle<Object> handle = GlobalHandles::Create(obj);
Why not use include/v8.h API here?

[mnaganov (inactive), 2009-10-15 07:44:16]

Thanks Kevin and Vitaly!

http://codereview.chromium.org/267077/diff/2001/22
File src/global-handles.h (right):

http://codereview.chromium.org/267077/diff/2001/22#newcode57
Line 57: typedef void (*WeakReferenceVisitor)(Object* object, void* parameter);
On 2009/10/14 15:11:31, Kevin Millikin wrote:
> This shouldn't be called ...Visitor unless it's a subclass of the
ObjectVisitor.
>  I can't think of a better name, though.

Renamed to WeakReferenceGuest.

http://codereview.chromium.org/267077/diff/2001/23
File src/heap-profiler.cc (right):

http://codereview.chromium.org/267077/diff/2001/23#newcode587
Line 587: object.ClearWeak();
On 2009/10/14 17:05:55, Vitaly wrote:
> You probably want to Dispose() the weak handle here. ClearWeak() turns it into
a
> strong handle.

Yes, I want the handle to disappear. Thanks for spotting this!

http://codereview.chromium.org/267077/diff/2001/23#newcode597
Line 597: static_cast<Address*>(trace)));
On 2009/10/14 15:11:31, Kevin Millikin wrote:
> Can reinterpret_cast here.

Done.

http://codereview.chromium.org/267077/diff/2001/23#newcode643
Line 643: GlobalHandles::IterateWeakRoots(PrintProducerStackTrace,
StackWeakReferenceCallback);
On 2009/10/14 17:05:55, Vitaly wrote:
> Line too long?

Fixed. I forgot run presubmit.

http://codereview.chromium.org/267077/diff/2001/23#newcode665
Line 665: HandleScope scope;
On 2009/10/14 17:05:55, Vitaly wrote:
> Persistent handles don't need handle scope.

This remained from my attempt to use public API that required lots of handles to
be created. Removed.

http://codereview.chromium.org/267077/diff/2001/23#newcode666
Line 666: Handle<Object> handle = GlobalHandles::Create(obj);
On 2009/10/14 17:05:55, Vitaly wrote:
> Why not use include/v8.h API here?

I tried initially, but this caused me to write ugly redundant chains of handles
conversions back and forth, so I decided to boil it down by using internal API
directly.

http://codereview.chromium.org/267077/diff/2001/24
File src/heap-profiler.h (right):

http://codereview.chromium.org/267077/diff/2001/24#newcode261
Line 261: static bool can_log;
On 2009/10/14 15:11:31, Kevin Millikin wrote:
> can_log_.  I'd prefer that it was private, and there was a Setup function that
> set it to true.

Done.

http://codereview.chromium.org/267077/diff/2001/25
File src/heap.cc (right):

http://codereview.chromium.org/267077/diff/2001/25#newcode3313
Line 3313: ProducerHeapProfile::can_log = true;
On 2009/10/14 15:11:31, Kevin Millikin wrote:
> If you have a ProducerHeapProfile::Setup function you can call it here (with a
> comment that it should be called last during heap setup).

Done.