Support higher CPU profiler sampling rate on Windows

Support higher CPU profiler sampling rate on Windows

This change moves sampling from SamplerThread to the profiler events processing thread and allows to configure sampling interval on Windows.

Custom tick counter is used instead of OS::Ticks as the latter has maximum presicion of 1ms while we need 100us. QueryPerformanceCounter is used to retrieve high-precision time as described in http://msdn.microsoft.com/en-us/library/ee417693(VS.85).aspx

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

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

[Benedikt Meurer, 2013-08-19 07:58:09]

I don't want to introduce any more #ifdef's here. I think we should simply
import the base/time stuff from chromium instead, which already covers what we
need.

[yurys, 2013-08-19 08:32:40]

On 2013/08/19 07:58:09, Benedikt Meurer wrote:
> I don't want to introduce any more #ifdef's here. I think we should simply
> import the base/time stuff from chromium instead, which already covers what we
> need.

Ho do you see it? Pulling src/base as a third-party library sounds like an
overkill. On ther other hand we cannot simply import base/time as it depends on
other classes in base/ So another option would be to import base/time without
dependencies and change it to use types available in v8. This way we would end
up having two copies of base/time in Chromium.

[Benedikt Meurer, 2013-08-19 08:35:51]

On 2013/08/19 08:32:40, Yury Semikhatsky wrote:
> On 2013/08/19 07:58:09, Benedikt Meurer wrote:
> > I don't want to introduce any more #ifdef's here. I think we should simply
> > import the base/time stuff from chromium instead, which already covers what
we
> > need.
> 
> Ho do you see it? Pulling src/base as a third-party library sounds like an
> overkill. On ther other hand we cannot simply import base/time as it depends
on
> other classes in base/ So another option would be to import base/time without
> dependencies and change it to use types available in v8. This way we would end
> up having two copies of base/time in Chromium.

What I mean is import on a functional level, not just copy the source code. I'll
probably look into that during the week as I plan to cleanup the time delta code
in V8 as well.

[yurys, 2013-08-19 08:50:25]

On 2013/08/19 08:35:51, Benedikt Meurer wrote:
> On 2013/08/19 08:32:40, Yury Semikhatsky wrote:
> > On 2013/08/19 07:58:09, Benedikt Meurer wrote:
> > > I don't want to introduce any more #ifdef's here. I think we should simply
> > > import the base/time stuff from chromium instead, which already covers
what
> we
> > > need.
> > 
> > Ho do you see it? Pulling src/base as a third-party library sounds like an
> > overkill. On ther other hand we cannot simply import base/time as it depends
> on
> > other classes in base/ So another option would be to import base/time
without
> > dependencies and change it to use types available in v8. This way we would
end
> > up having two copies of base/time in Chromium.
> 
> What I mean is import on a functional level, not just copy the source code.
I'll
> probably look into that during the week as I plan to cleanup the time delta
code
> in V8 as well.

I see. But if we don't have a clear plan at the moment on how to proceed with
the time support in v8 it may block this change for indefinitely long time.
Refactoring of the time code is somewhat unrelated to the profiler changes here
and I'd rather land the profiler modifications based on the existing time API
and switch to the new time API later when we agree on it. That way we would be
able to do further improvements in the profiler.

[Benedikt Meurer, 2013-08-19 08:52:47]

On 2013/08/19 08:50:25, Yury Semikhatsky wrote:
> On 2013/08/19 08:35:51, Benedikt Meurer wrote:
> > On 2013/08/19 08:32:40, Yury Semikhatsky wrote:
> > > On 2013/08/19 07:58:09, Benedikt Meurer wrote:
> > > > I don't want to introduce any more #ifdef's here. I think we should
simply
> > > > import the base/time stuff from chromium instead, which already covers
> what
> > we
> > > > need.
> > > 
> > > Ho do you see it? Pulling src/base as a third-party library sounds like an
> > > overkill. On ther other hand we cannot simply import base/time as it
depends
> > on
> > > other classes in base/ So another option would be to import base/time
> without
> > > dependencies and change it to use types available in v8. This way we would
> end
> > > up having two copies of base/time in Chromium.
> > 
> > What I mean is import on a functional level, not just copy the source code.
> I'll
> > probably look into that during the week as I plan to cleanup the time delta
> code
> > in V8 as well.
> 
> I see. But if we don't have a clear plan at the moment on how to proceed with
> the time support in v8 it may block this change for indefinitely long time.
> Refactoring of the time code is somewhat unrelated to the profiler changes
here
> and I'd rather land the profiler modifications based on the existing time API
> and switch to the new time API later when we agree on it. That way we would be
> able to do further improvements in the profiler.

I'll see if I can manage to import the stuff this week.

[yurys, 2013-08-26 12:01:58]

On 2013/08/19 08:52:47, Benedikt Meurer wrote:
> On 2013/08/19 08:50:25, Yury Semikhatsky wrote:
> > On 2013/08/19 08:35:51, Benedikt Meurer wrote:
> > > On 2013/08/19 08:32:40, Yury Semikhatsky wrote:
> > > > On 2013/08/19 07:58:09, Benedikt Meurer wrote:
> > > > > I don't want to introduce any more #ifdef's here. I think we should
> simply
> > > > > import the base/time stuff from chromium instead, which already covers
> > what
> > > we
> > > > > need.
> > > > 
> > > > Ho do you see it? Pulling src/base as a third-party library sounds like
an
> > > > overkill. On ther other hand we cannot simply import base/time as it
> depends
> > > on
> > > > other classes in base/ So another option would be to import base/time
> > without
> > > > dependencies and change it to use types available in v8. This way we
would
> > end
> > > > up having two copies of base/time in Chromium.
> > > 
> > > What I mean is import on a functional level, not just copy the source
code.
> > I'll
> > > probably look into that during the week as I plan to cleanup the time
delta
> > code
> > > in V8 as well.
> > 
> > I see. But if we don't have a clear plan at the moment on how to proceed
with
> > the time support in v8 it may block this change for indefinitely long time.
> > Refactoring of the time code is somewhat unrelated to the profiler changes
> here
> > and I'd rather land the profiler modifications based on the existing time
API
> > and switch to the new time API later when we agree on it. That way we would
be
> > able to do further improvements in the profiler.
> 
> I'll see if I can manage to import the stuff this week.

Benedikt, any update on this?

[yurys, 2013-08-27 08:21:29]

Rebased this change on the patch adding Chromium-style TimeDelta support
https://codereview.chromium.org/23295034/. 

PTAL

[Benedikt Meurer, 2013-08-27 14:02:43]

Looks way better now. :-)

LGTM with comment

https://codereview.chromium.org/23271003/diff/20001/src/sampler.cc
File src/sampler.cc (right):

https://codereview.chromium.org/23271003/diff/20001/src/sampler.cc#newcode614
src/sampler.cc:614: #elif defined(_WIN32) || defined(_WIN64) ||
defined(__CYGWIN__)
You don't need to test for _WIN64, since _WIN32 is defined for both 32 and 64
bit windows.
Please convert to use V8_OS_* macros, so we can get rid of this platform
specific defines at some point:

#elif V8_OS_WIN || V8_OS_CYGWIN

[yurys, 2013-08-27 15:56:27]

https://codereview.chromium.org/23271003/diff/20001/src/sampler.cc
File src/sampler.cc (right):

https://codereview.chromium.org/23271003/diff/20001/src/sampler.cc#newcode614
src/sampler.cc:614: #elif defined(_WIN32) || defined(_WIN64) ||
defined(__CYGWIN__)
On 2013/08/27 14:02:43, Benedikt Meurer wrote:
> You don't need to test for _WIN64, since _WIN32 is defined for both 32 and 64
> bit windows.
> Please convert to use V8_OS_* macros, so we can get rid of this platform
> specific defines at some point:
> 
> #elif V8_OS_WIN || V8_OS_CYGWIN

Done.

[yurys, 2013-08-29 13:44:34]

Committed patchset #6 manually as r16428 (presubmit successful).