Added the simplest call stack sampling and call profile in tick processor output.

Added the simplest call stack sampling and call profile in tick processor output.

Currently only two stack frames are sampled (current function and its caller).

Output of tick processor looks like this:

 [Call profile]:
   total  call path
   15.2%  LazyCompile: am3 crypto.js:108  <-  LazyCompile: montReduce crypto.js:583
    6.5%  LazyCompile: am3 crypto.js:108  <-  LazyCompile: bnpSquareTo crypto.js:431
    2.9%  Builtin: KeyedStoreIC_Generic  <-  LazyCompile: montReduce crypto.js:583
    2.3%  LazyCompile: am3 crypto.js:108  <-  LazyCompile: bnpMultiplyTo crypto.js:415

Tested under Windows, Linux and OS X.

[Søren Thygesen Gjesse, 2009-02-17 11:29:24]

I like this change very much as it will make a good foundation for both
attributing the cost in IC's etc. to a JS function and locating cost of
different call paths.

I think you should move the walking of the stack call into Ticker::Tick instead
of having it in platform. That way you should be able to avoid copying the stack
when calling Profiler::Insert. Maybe a second parameter to Profiler::Insert
where an allocated array can be passed. Of cause with a dept of two thats not an
issue but I don't like having the copy constructor for TickSample. For now just
having a fixed size stack in TickSample might be sufficient.

The code in platform-all.cc should then move to log.cc. When the stack walking
code becomes architecture dependent we can split it in log-ia32.cc and
log-arm.cc. The stack walking is going to be architecture dependent and not
platform dependent.

http://codereview.chromium.org/21403/diff/1/4
File src/platform-all.cc (right):

http://codereview.chromium.org/21403/diff/1/4#newcode65
Line 65: // than 10,000 butes are bogus
I don't think the constant 10000 is valid for our frames, as using
Function.prototype.apply can create large frames. Try the following in the shell
with --allow-natives-syntax (%DebugPrint prints the frame pointer)

  function f() { DebugPrint('f ' + arguments.length) }
  function g(x) { var a=new Array(x); DebugPrint('g'); f.apply(null, a); }
  g(100000)

I think we just need to store a low-water-mart when V8 is initialized.

http://codereview.chromium.org/21403/diff/1/5
File src/platform-freebsd.cc (right):

http://codereview.chromium.org/21403/diff/1/5#newcode641
Line 641: unsigned int ebp = mcontext.mc_ebp;
Maybe just add the frame pointer to the TickSample class?

http://codereview.chromium.org/21403/diff/1/5#newcode644
Line 644: StackWalker::SampleStack(sample, ebp);
Move stack walking to Ticker::Tick.

http://codereview.chromium.org/21403/diff/1/6
File src/platform-linux.cc (right):

http://codereview.chromium.org/21403/diff/1/6#newcode624
Line 624: unsigned int ebp = mcontext.gregs[REG_EBP];
Maybe just add the frame pointer to the TickSample class?

http://codereview.chromium.org/21403/diff/1/6#newcode627
Line 627: StackWalker::SampleStack(sample, ebp);
Move stack walking to Ticker::Tick.

http://codereview.chromium.org/21403/diff/1/7
File src/platform-macos.cc (right):

http://codereview.chromium.org/21403/diff/1/7#newcode590
Line 590: unsigned int ebp = mcontext->ss.ebp;
Maybe just add the frame pointer to the TickSample class?

http://codereview.chromium.org/21403/diff/1/7#newcode593
Line 593: StackWalker::SampleStack(sample, ebp);
Move stack walking to Ticker::Tick.

http://codereview.chromium.org/21403/diff/1/8
File src/platform-win32.cc (right):

http://codereview.chromium.org/21403/diff/1/8#newcode1589
Line 1589: StackWalker::SampleStack(sample, context.Ebp);
Move stack walking to Ticker::Tick.

http://codereview.chromium.org/21403/diff/1/8#newcode1589
Line 1589: StackWalker::SampleStack(sample, context.Ebp);
Maybe just add the frame pointer to the TickSample class?

[Mikhail Naganov, 2009-02-17 14:07:28]

http://codereview.chromium.org/21403/diff/1/4
File src/platform-all.cc (right):

http://codereview.chromium.org/21403/diff/1/4#newcode65
Line 65: // than 10,000 butes are bogus
On 2009/02/17 11:29:24, Søren Gjesse wrote:
> I don't think the constant 10000 is valid for our frames, as using
> Function.prototype.apply can create large frames. Try the following in the
shell
> with --allow-natives-syntax (%DebugPrint prints the frame pointer)
> 
>   function f() { DebugPrint('f ' + arguments.length) }
>   function g(x) { var a=new Array(x); DebugPrint('g'); f.apply(null, a); }
>   g(100000)
> 
> I think we just need to store a low-water-mart when V8 is initialized.

Ok, done as you're proposing. See Ticker::SampleStack

http://codereview.chromium.org/21403/diff/1/5
File src/platform-freebsd.cc (right):

http://codereview.chromium.org/21403/diff/1/5#newcode641
Line 641: unsigned int ebp = mcontext.mc_ebp;
On 2009/02/17 11:29:24, Søren Gjesse wrote:
> Maybe just add the frame pointer to the TickSample class?

Great idea, thanks! Done (for all platforms).

http://codereview.chromium.org/21403/diff/1/5#newcode644
Line 644: StackWalker::SampleStack(sample, ebp);
On 2009/02/17 11:29:24, Søren Gjesse wrote:
> Move stack walking to Ticker::Tick.

Done (for all platforms).

[Søren Thygesen Gjesse, 2009-02-17 14:14:50]

LGTM

http://codereview.chromium.org/21403/diff/31/32
File src/log.cc (right):

http://codereview.chromium.org/21403/diff/31/32#newcode866
Line 866: fprintf(logfile_, ",0x%x", (unsigned int)(sample->stack[i]));
Please use reinterpret_cast<unsigned int>(...).

http://codereview.chromium.org/21403/diff/31/32#newcode960
Line 960: ticker_ = new Ticker(10, (unsigned int)&stack_var);
Please use reinterpret_cast<unsigned int>(...).

[Mikhail Naganov, 2009-02-17 14:22:41]

Submitting...

Do we need to update tickprocessor.py on the CB machines manually?

http://codereview.chromium.org/21403/diff/31/32
File src/log.cc (right):

http://codereview.chromium.org/21403/diff/31/32#newcode866
Line 866: fprintf(logfile_, ",0x%x", (unsigned int)(sample->stack[i]));
On 2009/02/17 14:14:50, Søren Gjesse wrote:
> Please use reinterpret_cast<unsigned int>(...).

Done.

http://codereview.chromium.org/21403/diff/31/32#newcode960
Line 960: ticker_ = new Ticker(10, (unsigned int)&stack_var);
On 2009/02/17 14:14:50, Søren Gjesse wrote:
> Please use reinterpret_cast<unsigned int>(...).

Done.