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.
https://codereview.chromium.org/21101002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
