Use signals for cpu profiling on Mac OS X

Use signals for cpu profiling on Mac OS X

A while ago in r2315 Mac OS X cpu profiler implementation was changed to pause sampled thread instead of sending SIGPROF signal. That was done because at that point profiler send the signal to the whole process and it was handled on a random thread. Now that signal-based implementation uses pthread_kill it may well be used on Mac OS X too.

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

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

[Benedikt Meurer, 2013-08-20 11:58:56]

What's the benefit of this change, other than trying to somehow unify the code
paths?

[yurys, 2013-08-20 12:07:36]

On 2013/08/20 11:58:56, Benedikt Meurer wrote:
> What's the benefit of this change, other than trying to somehow unify the code
> paths?

Just unification of the code no obvious performance gains at the moment. After
this change all but Windows port use the same mechanism based on signals.

[Benedikt Meurer, 2013-08-20 12:13:02]

On 2013/08/20 12:07:36, Yury Semikhatsky wrote:
> On 2013/08/20 11:58:56, Benedikt Meurer wrote:
> > What's the benefit of this change, other than trying to somehow unify the
code
> > paths?
> 
> Just unification of the code no obvious performance gains at the moment. After
> this change all but Windows port use the same mechanism based on signals.

Well previously Mac and Windows ports were using the same mechanism and everyone
else was using signals because of the lack of thread_suspend/resume.

Personally I think the thread_suspend/resume approach is cleaner and less likely
to break compared to the SIGPROF approach, so I'm not in favor of using signals
on the Mac unless there's a good reason to do so.

[yurys, 2013-08-20 12:25:05]

On 2013/08/20 12:13:02, Benedikt Meurer wrote:
> On 2013/08/20 12:07:36, Yury Semikhatsky wrote:
> > On 2013/08/20 11:58:56, Benedikt Meurer wrote:
> > > What's the benefit of this change, other than trying to somehow unify the
> code
> > > paths?
> > 
> > Just unification of the code no obvious performance gains at the moment.
After
> > this change all but Windows port use the same mechanism based on signals.
> 
> Well previously Mac and Windows ports were using the same mechanism and
everyone
> else was using signals because of the lack of thread_suspend/resume.
> 
> Personally I think the thread_suspend/resume approach is cleaner and less
likely
> to break compared to the SIGPROF approach, so I'm not in favor of using
signals
> on the Mac unless there's a good reason to do so.

Thread suspend/resume code is platform dependent while signal API is the same on
all platforms which means that we have fewer platform-specific pieces in the
sampler implementation. Why do you think that suspend/resume is less likely to
break compared to the SIGPROF approach?

[Benedikt Meurer, 2013-08-20 12:29:19]

On 2013/08/20 12:25:05, Yury Semikhatsky wrote:
> On 2013/08/20 12:13:02, Benedikt Meurer wrote:
> > On 2013/08/20 12:07:36, Yury Semikhatsky wrote:
> > > On 2013/08/20 11:58:56, Benedikt Meurer wrote:
> > > > What's the benefit of this change, other than trying to somehow unify
the
> > code
> > > > paths?
> > > 
> > > Just unification of the code no obvious performance gains at the moment.
> After
> > > this change all but Windows port use the same mechanism based on signals.
> > 
> > Well previously Mac and Windows ports were using the same mechanism and
> everyone
> > else was using signals because of the lack of thread_suspend/resume.
> > 
> > Personally I think the thread_suspend/resume approach is cleaner and less
> likely
> > to break compared to the SIGPROF approach, so I'm not in favor of using
> signals
> > on the Mac unless there's a good reason to do so.
> 
> Thread suspend/resume code is platform dependent while signal API is the same
on
> all platforms which means that we have fewer platform-specific pieces in the
> sampler implementation. Why do you think that suspend/resume is less likely to
> break compared to the SIGPROF approach?

Because only very few functions are actually signal-safe. For example, I'm
pretty sure that backtrace() and friends is NOT signal-safe, so calling this
from a signal handler might cause trouble every once in a while. Unfortunately
Linux lacks a "portable way" (in the sense that it's portable across different
libpthread/libc implementations) to suspend/resume a thread, so we have to use
the POSIX signals here and pray that we don't crash even tho we don't pay
attention not to call any non-signal-safe functions.

[yurys, 2013-08-20 12:42:13]

On 2013/08/20 12:29:19, Benedikt Meurer wrote:
> On 2013/08/20 12:25:05, Yury Semikhatsky wrote:
> > On 2013/08/20 12:13:02, Benedikt Meurer wrote:
> > > On 2013/08/20 12:07:36, Yury Semikhatsky wrote:
> > > > On 2013/08/20 11:58:56, Benedikt Meurer wrote:
> > > > > What's the benefit of this change, other than trying to somehow unify
> the
> > > code
> > > > > paths?
> > > > 
> > > > Just unification of the code no obvious performance gains at the moment.
> > After
> > > > this change all but Windows port use the same mechanism based on
signals.
> > > 
> > > Well previously Mac and Windows ports were using the same mechanism and
> > everyone
> > > else was using signals because of the lack of thread_suspend/resume.
> > > 
> > > Personally I think the thread_suspend/resume approach is cleaner and less
> > likely
> > > to break compared to the SIGPROF approach, so I'm not in favor of using
> > signals
> > > on the Mac unless there's a good reason to do so.
> > 
> > Thread suspend/resume code is platform dependent while signal API is the
same
> on
> > all platforms which means that we have fewer platform-specific pieces in the
> > sampler implementation. Why do you think that suspend/resume is less likely
to
> > break compared to the SIGPROF approach?
> 
> Because only very few functions are actually signal-safe. For example, I'm
> pretty sure that backtrace() and friends is NOT signal-safe, so calling this
> from a signal handler might cause trouble every once in a while. Unfortunately
> Linux lacks a "portable way" (in the sense that it's portable across different
> libpthread/libc implementations) to suspend/resume a thread, so we have to use
> the POSIX signals here and pray that we don't crash even tho we don't pay
> attention not to call any non-signal-safe functions.
The profiler code is designed in a way so that it doesn't call such unsafe
functions from the signal handler and doesn't do any heap allocations when it
collects stack traces. I don't see why it should be more dangerous than using
thread suspend which combined with synchronization objects might cause a dead
lock. Of cause programmer errors are possible here but they are possible with
thread_suspend too. If we think that signals are not reliable then we should not
be using them on posix systems too but so far they proved not to be worse than
thread suspend/resume and I don't think we should avoid them because there are
some functions which are definitely not used in the profiler might not be
signal-safe.

[Benedikt Meurer, 2013-08-21 08:50:41]

Ok, I'm fine with this. It would be nice to have only one way to do the
sampling, but I see that there's currently no way to achieve this, since Windows
lacks proper POSIX signal support and Linux lacks thread_suspend/resume.

LGTM

[Sven Panne, 2013-08-21 09:53:24]

LGTM from me too. As Benedikt already pointed out, there is probably no hope for
a unified implementation across all platforms, so let's choose things according
to ease of implementation/code size. Regarding this, the CL at hand is a win by
removing several dozen lines of code.

[yurys, 2013-08-26 11:53:38]

Committed patchset #3 manually as r16320 (presubmit successful).