Support higher CPU profiler sampling rate on posix systems

Support higher CPU profiler sampling rate on posix systems

New flag is added that allows to specify CPU profiler sampling rate in microseconds as command line argument. It was tested to work fine with 100us interval(currently it is 1ms). Default values are kept the same as in the current implementation. The new implementation is enabled only on POSIX platforms which use signals to collect samples. Other platforms that pause thread being sampled are to follow.

SIGPROF signals are now sent on the profiler event processor thread to make sure that the processing thread does fall far behind the sampling.

The patch is based on the previous one that was rolled out in r13851. The main difference is that the circular queue is not modified for now. 

On Linux sampling for CPU profiler is initiated on the profiler event processor thread, other platforms to follow. 

CPU profiler continues to use SamplingCircularQueue, we will probably replace it with a single sample buffer when Mac and Win ports support profiling on the event processing thread. 

When --prof option is specified profiling is initiated either on the profiler event processor thread if CPU profiler is on or on the SignalSender thread as it used to be if no CPU profiles are being collected. 

ProfilerEventsProcessor::ProcessEventsAndDoSample now waits in a tight loop, processing collected samples until sampling interval expires. To save CPU resources I'm planning to change that to use nanosleep as only one sample is expected in the queue at any point. 


BUG=v8:2814
R=bmeurer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=16310

[Benedikt Meurer, 2013-07-30 06:41:04]

https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc#newcode175
src/cpu-profiler.cc:175: ProcessEventsAndYield();
I think there are two severe issues with this loop, first of all running_ is not
volatile and there's no memory barrier for running_. And YieldCPU() should not
be used. Maybe unrelated to the change at hand, but how about using proper
synchronization primitives (i.e. monitors/condition variables or semaphores)
instead?

[yurys, 2013-07-30 08:52:25]

https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc#newcode175
src/cpu-profiler.cc:175: ProcessEventsAndYield();
On 2013/07/30 06:41:04, Benedikt Meurer wrote:
> I think there are two severe issues with this loop, first of all running_ is
not
> volatile and there's no memory barrier for running_. And YieldCPU() should not
> be used. Maybe unrelated to the change at hand, but how about using proper
> synchronization primitives (i.e. monitors/condition variables or semaphores)
> instead?

The profiler code tries to avoid usage of synchronization primitives to minimize
effect it imposes on the thread that is being profiled. In particular both
queues used to pass code events and tick samples from the VM thread to the
processing one are lock-free (I didn't see any measurements that would justify
this approach though). Adding a mutex/semaphore here may have a noticeable
performance impact so I'd prefer to stay with a more light-weight solution. I
believe changing running_ into Atomic32 and using NoBarrier_Load to read it here
should be enough, wdyt?

Also can you elaborate on why you think that YieldCPU should not be used? Among
other things I believe that a side-effect of using it here may be that it
implies a memory barrier and makes this code work correctly:)

Anyways I'm going to address this in a separate CL as all the raised issues are
applicable to the current implementation.

[Benedikt Meurer, 2013-07-30 09:58:31]

On 2013/07/30 08:52:25, Yury Semikhatsky wrote:
> https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc
> File src/cpu-profiler.cc (right):
> 
> https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc#newcode175
> src/cpu-profiler.cc:175: ProcessEventsAndYield();
> On 2013/07/30 06:41:04, Benedikt Meurer wrote:
> > I think there are two severe issues with this loop, first of all running_ is
> not
> > volatile and there's no memory barrier for running_. And YieldCPU() should
not
> > be used. Maybe unrelated to the change at hand, but how about using proper
> > synchronization primitives (i.e. monitors/condition variables or semaphores)
> > instead?
> 
> The profiler code tries to avoid usage of synchronization primitives to
minimize
> effect it imposes on the thread that is being profiled. In particular both
> queues used to pass code events and tick samples from the VM thread to the
> processing one are lock-free (I didn't see any measurements that would justify
> this approach though). Adding a mutex/semaphore here may have a noticeable
> performance impact so I'd prefer to stay with a more light-weight solution. I
> believe changing running_ into Atomic32 and using NoBarrier_Load to read it
here
> should be enough, wdyt?

Using monitors or semaphores here would avoid both the busy waiting and the
synchronization issue. 

> Also can you elaborate on why you think that YieldCPU should not be used?
Among
> other things I believe that a side-effect of using it here may be that it
> implies a memory barrier and makes this code work correctly:)

It _may_ indeed act as a memory barrier, but there's no warranty. YieldCPU() is
used to reduce the negative effects of busy waiting. Even with YieldCPU(),
you're still burning one core with your loop most of the time.

> Anyways I'm going to address this in a separate CL as all the raised issues
are
> applicable to the current implementation.

Yeah, it's not really related to this CL. So this is LGTM.

[yurys, 2013-07-30 12:33:28]

On 2013/07/30 09:58:31, Benedikt Meurer wrote:
> On 2013/07/30 08:52:25, Yury Semikhatsky wrote:
> > https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc
> > File src/cpu-profiler.cc (right):
> > 
> >
https://codereview.chromium.org/21101002/diff/1/src/cpu-profiler.cc#newcode175
> > src/cpu-profiler.cc:175: ProcessEventsAndYield();
> > On 2013/07/30 06:41:04, Benedikt Meurer wrote:
> > > I think there are two severe issues with this loop, first of all running_
is
> > not
> > > volatile and there's no memory barrier for running_. And YieldCPU() should
> not
> > > be used. Maybe unrelated to the change at hand, but how about using proper
> > > synchronization primitives (i.e. monitors/condition variables or
semaphores)
> > > instead?
> > 
> > The profiler code tries to avoid usage of synchronization primitives to
> minimize
> > effect it imposes on the thread that is being profiled. In particular both
> > queues used to pass code events and tick samples from the VM thread to the
> > processing one are lock-free (I didn't see any measurements that would
justify
> > this approach though). Adding a mutex/semaphore here may have a noticeable
> > performance impact so I'd prefer to stay with a more light-weight solution.
I
> > believe changing running_ into Atomic32 and using NoBarrier_Load to read it
> here
> > should be enough, wdyt?
> 
> Using monitors or semaphores here would avoid both the busy waiting and the
> synchronization issue. 
> 
Correct. With current implementation using monitors/semaphors may make sense.
But note that we cannot use monitors/semaphors to access sample buffer as signal
handler may be called when the monitor is acquired by some thread and then we
would end up with a dead lock.

One of the aspects of this change is to send SIGPROF signal on the profile event
processing thread and eventually get rid of SamplerThread. We need to do that
because:

1) We don't want the processing thread to fall behind the samples buffer. One
simple solution for this is not to send next signal before we have processed at
least one sample. On the other hand we'd like to be able to send them at a fixed
rate (100us), so  we need to wait until some timeout expires.

2) OS::Sleep used on the sampler thread on some systems will be limited by the
scheduler quantum and won't work for very short intervals so we cannot sleep for
100us.


> > Also can you elaborate on why you think that YieldCPU should not be used?
> Among
> > other things I believe that a side-effect of using it here may be that it
> > implies a memory barrier and makes this code work correctly:)
> 
> It _may_ indeed act as a memory barrier, but there's no warranty. YieldCPU()
is
> used to reduce the negative effects of busy waiting. Even with YieldCPU(),
> you're still burning one core with your loop most of the time.
> 
Our assumption is that with a high sampling rate we can afford allocating one
CPU core to the profile event processor and the sampling rate would be limited
by the performance of that thread. So we intentionally want to have the core be
busy exclusively with the profile events processing.


> > Anyways I'm going to address this in a separate CL as all the raised issues
> are
> > applicable to the current implementation.
> 
> Yeah, it's not really related to this CL. So this is LGTM.

[yurys, 2013-08-26 07:17:24]

Committed patchset #2 manually as r16310.