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.
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.
