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.

http://codereview.chromium.org/4000007/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to