Improve sampler resolution on Linux.

Improve sampler resolution on Linux.

Instead of relying on itimer signals from kernel, send them
ourselves from a separate thread. This disables an ability
to profile multiple VM threads on Linux, but it anyway doesn't
work on other platforms, so we need a common solution for
it (issue 913 created to track this).

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

[Vitaly Repeshko, 2010-10-25 16:57:25]

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

http://codereview.chromium.org/4000007/diff/1/2#newcode801
src/platform-linux.cc:801: vm_thread_pid_(syscall (SYS_gettid)) {
Please document that libc doesn't provide wrappers for gettid and tgkill. Also
nit: is the extra space after "syscall" required?

http://codereview.chromium.org/4000007/diff/1/2#newcode807
src/platform-linux.cc:807: for ( ; sampler_->IsActive();
usleep(sampler_->interval_ * 1000 - 100)) {
I think "while" loop is more readable here. Also we should check that usleep
returns 0 or EINTR, everything else is a bug.

http://codereview.chromium.org/4000007/diff/1/2#newcode815
src/platform-linux.cc:815: struct itimerval old_timer_value_;
Remove.

http://codereview.chromium.org/4000007/diff/1/2#newcode817
src/platform-linux.cc:817: int vm_thread_pid_;
vm_thread_tid_ or maybe vm_tgid_ and vm_tid_?

http://codereview.chromium.org/4000007/diff/1/2#newcode857
src/platform-linux.cc:857: active_ = true;
Add a comment like "Start a sender thread to periodically send profiling signals
to the VM thread".

http://codereview.chromium.org/4000007/diff/1/2#newcode858
src/platform-linux.cc:858: pthread_attr_t sched_attr;
Passing NULL attr should be equivalent to the default attrs.

http://codereview.chromium.org/4000007/diff/1/2#newcode860
src/platform-linux.cc:860: pthread_create(&data_->signal_sender_thread_,
&sched_attr, SenderEntry, data_);
80 chars.

http://codereview.chromium.org/4000007/diff/1/2#newcode863
src/platform-linux.cc:863: active_sampler_ = this;
I don't understand the connection between active_ and active_sampler_.
Sampler::Start() seems to support the existence of other samplers and returns
early, but Sampler::Stop() always joins the sender thread.

http://codereview.chromium.org/4000007/diff/1/2#newcode874
src/platform-linux.cc:874: setitimer(ITIMER_PROF, &data_->old_timer_value_,
NULL);
Remove.

[mnaganov (inactive), 2010-10-26 10:01:58]

BTW, I tested on ARM, it works, too, with sampling rate close to 1ms (1.17)

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

http://codereview.chromium.org/4000007/diff/1/2#newcode801
src/platform-linux.cc:801: vm_thread_pid_(syscall (SYS_gettid)) {
On 2010/10/25 16:57:25, Vitaly wrote:
> Please document that libc doesn't provide wrappers for gettid and tgkill. Also
> nit: is the extra space after "syscall" required?

Done. Space removed.

http://codereview.chromium.org/4000007/diff/1/2#newcode807
src/platform-linux.cc:807: for ( ; sampler_->IsActive();
usleep(sampler_->interval_ * 1000 - 100)) {
On 2010/10/25 16:57:25, Vitaly wrote:
> I think "while" loop is more readable here. Also we should check that usleep
> returns 0 or EINTR, everything else is a bug.

Converted to while. But I'm not sure what to do, if usleep fails.

http://codereview.chromium.org/4000007/diff/1/2#newcode815
src/platform-linux.cc:815: struct itimerval old_timer_value_;
On 2010/10/25 16:57:25, Vitaly wrote:
> Remove.

Done.

http://codereview.chromium.org/4000007/diff/1/2#newcode817
src/platform-linux.cc:817: int vm_thread_pid_;
On 2010/10/25 16:57:25, Vitaly wrote:
> vm_thread_tid_ or maybe vm_tgid_ and vm_tid_?

Done.

http://codereview.chromium.org/4000007/diff/1/2#newcode857
src/platform-linux.cc:857: active_ = true;
On 2010/10/25 16:57:25, Vitaly wrote:
> Add a comment like "Start a sender thread to periodically send profiling
signals
> to the VM thread".

Done.

http://codereview.chromium.org/4000007/diff/1/2#newcode858
src/platform-linux.cc:858: pthread_attr_t sched_attr;
On 2010/10/25 16:57:25, Vitaly wrote:
> Passing NULL attr should be equivalent to the default attrs.

Done.

http://codereview.chromium.org/4000007/diff/1/2#newcode860
src/platform-linux.cc:860: pthread_create(&data_->signal_sender_thread_,
&sched_attr, SenderEntry, data_);
On 2010/10/25 16:57:25, Vitaly wrote:
> 80 chars.

Done.

http://codereview.chromium.org/4000007/diff/1/2#newcode863
src/platform-linux.cc:863: active_sampler_ = this;
On 2010/10/25 16:57:25, Vitaly wrote:
> I don't understand the connection between active_ and active_sampler_.
> Sampler::Start() seems to support the existence of other samplers and returns
> early, but Sampler::Stop() always joins the sender thread.

Right. Added an explicit variable for tracking sender state.

http://codereview.chromium.org/4000007/diff/1/2#newcode874
src/platform-linux.cc:874: setitimer(ITIMER_PROF, &data_->old_timer_value_,
NULL);
On 2010/10/25 16:57:25, Vitaly wrote:
> Remove.

Done.

[Vitaly Repeshko, 2010-10-26 13:40:36]

LGTM

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

http://codereview.chromium.org/4000007/diff/1/2#newcode807
src/platform-linux.cc:807: for ( ; sampler_->IsActive();
usleep(sampler_->interval_ * 1000 - 100)) {
On 2010/10/26 10:01:58, Michail Naganov wrote:
> On 2010/10/25 16:57:25, Vitaly wrote:
> > I think "while" loop is more readable here. Also we should check that usleep
> > returns 0 or EINTR, everything else is a bug.
> 
> Converted to while. But I'm not sure what to do, if usleep fails.

Something like:
int ret = usleep(...);
ASSERT(ret == 0 || ret == EINTR);
USE(ret);  // To avoid release mode warnings.