Create libsampler as V8 sampler library.

Reland: Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof functionality.
5. Creates SamplerManager to manage the relationship between samplers and threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}

Committed: https://crrev.com/a0198c0f627de392280f00099333f96ee34d2519
Cr-Commit-Position: refs/heads/master@{#36532}

[lpy, 2016-04-26 23:17:59]

Description was changed from

==========
Create libsampler as V8 sampler library.

This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Removes all thread code.
3. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
4. Creates SamplerManager to manage the relationship between samplers and
threads.

BUG=v8:4789
LOG=n
==========

to

==========
Create libsampler as V8 sampler library.

This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Removes all thread code.
3. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
4. Creates SamplerManager to manage the relationship between samplers and
threads.

BUG=v8:4789
LOG=n
==========

[lpy, 2016-05-02 20:07:35]

lpy@chromium.org changed reviewers:
+ fmeawad@chromium.org

[fmeawad, 2016-05-02 23:14:37]

Some comments.

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:235: class SamplerManager {
maybe SamplingManager?

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.h (right):

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.h:35: // Everyone else should override this method in
order to queue samples,
"// All Sampler implementations should override this method in order to queue
samples, the default implementation is used for testing purposes only."

[lpy, 2016-05-03 17:15:44]

lpy@chromium.org changed reviewers:
+ alph@chromium.org, jochen@chromium.org

[lpy, 2016-05-03 17:16:06]

alph@, jochen@, PTAL.

[alph, 2016-05-03 18:48:07]

It's better to not put four things into one patch, but rather have four patches.
This way you can keep the patch size smaller. It is much easier to get smaller
patches through review.
Also I see you created new files by copying code from v8/profiler/sampler.* so
effectively now v8 has some code duplicated. This is not good.

https://codereview.chromium.org/1922303002/diff/20001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1922303002/diff/20001/include/v8.h#newcode6300
include/v8.h:6300: * Check if this isolate is in use.
The description doesn't add much information to the function name. Could you
write more detailed info on what 'in use' means.

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:419: v8::RegisterState state;
You can move register filling out of the loop. Moreover it's better to have it
as a separate function, as it has clean interface and quite lengthy.

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.h (right):

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.h:35: // Everyone else should override this method in
order to queue samples,
On 2016/05/02 23:14:37, fmeawad wrote:
> "// All Sampler implementations should override this method in order to queue
> samples, the default implementation is used for testing purposes only."

Can you then make the method pure virtual and create the TestSampler for the
tests?

[lpy, 2016-05-04 00:09:24]

PTAL.

https://codereview.chromium.org/1922303002/diff/20001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1922303002/diff/20001/include/v8.h#newcode6300
include/v8.h:6300: * Check if this isolate is in use.
On 2016/05/03 18:48:07, alph wrote:
> The description doesn't add much information to the function name. Could you
> write more detailed info on what 'in use' means.

Done.

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:419: v8::RegisterState state;
On 2016/05/03 18:48:07, alph wrote:
> You can move register filling out of the loop. Moreover it's better to have it
> as a separate function, as it has clean interface and quite lengthy.

Done.

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.h (right):

https://codereview.chromium.org/1922303002/diff/20001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.h:35: // Everyone else should override this method in
order to queue samples,
On 2016/05/03 18:48:07, alph wrote:
> On 2016/05/02 23:14:37, fmeawad wrote:
> > "// All Sampler implementations should override this method in order to
queue
> > samples, the default implementation is used for testing purposes only."
> 
> Can you then make the method pure virtual and create the TestSampler for the
> tests?

Done.

[lpy, 2016-05-04 00:14:57]

On 2016/05/03 18:48:07, alph wrote:
> It's better to not put four things into one patch, but rather have four
patches.
> This way you can keep the patch size smaller. It is much easier to get smaller
> patches through review.

Any suggestion?

> Also I see you created new files by copying code from v8/profiler/sampler.* so
> effectively now v8 has some code duplicated. This is not good.


Yes, it's not good to have duplicate code. But to maintain the current sampling
functionality in v8, we need to duplicate it until we migrate the code to use
libsampler, following up CLs will solve this issue.

[alph, 2016-05-04 00:20:18]

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:419: SignalHandler::FillRegisterState(context,
state);
still it is possible to move this out of the loop.

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:425: void SignalHandler::FillRegisterState(void*
context, RegisterState& state) {
please use pointer for out parameters.

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:456: state->fp =
reinterpret_cast<void*>(mcontext.regs[29]);
doesn't look like it compiles.

[lpy, 2016-05-04 00:56:09]

Thanks. PTAL.

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:419: SignalHandler::FillRegisterState(context,
state);
On 2016/05/04 00:20:18, alph wrote:
> still it is possible to move this out of the loop.

Done.

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:425: void SignalHandler::FillRegisterState(void*
context, RegisterState& state) {
On 2016/05/04 00:20:18, alph wrote:
> please use pointer for out parameters.

Done.

https://codereview.chromium.org/1922303002/diff/40001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:456: state->fp =
reinterpret_cast<void*>(mcontext.regs[29]);
On 2016/05/04 00:20:18, alph wrote:
> doesn't look like it compiles.

Done.

[jochen (gone - plz use gerrit), 2016-05-04 07:29:06]

jochen@chromium.org changed reviewers:
+ yangguo@chromium.org

[jochen (gone - plz use gerrit), 2016-05-04 07:29:07]

I like this direction a lot!

I wonder whether libsampler should control spin up the thread itself (essential
move the thread from the unit test directly to libsampler)

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (left):

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:5: #include "src/profiler/sampler.h"
please add a DEPS file to this directory that contains at least

"-src",
"+src/base",
"+src/libsampler",

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:45: #include "src/atomic-utils.h"
I think this file can be moved to src/base

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:48: #include "src/list-inl.h"
please use stl container instead

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:49: #include "src/isolate.h"
we should only use v8::Isolate via include/v8.h

[lpy, 2016-05-05 22:24:17]

PTAL.
Replaced list and hash map with STL.

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (left):

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:5: #include "src/profiler/sampler.h"
On 2016/05/04 07:29:07, jochen wrote:
> please add a DEPS file to this directory that contains at least
> 
> "-src",
> "+src/base",
> "+src/libsampler",

Will do after finishing moving atomic-utils.h into base/

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:45: #include "src/atomic-utils.h"
On 2016/05/04 07:29:07, jochen wrote:
> I think this file can be moved to src/base

atomic-utils.h will be moved into base/ in this CL:
https://codereview.chromium.org/1954603002/

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:48: #include "src/list-inl.h"
On 2016/05/04 07:29:07, jochen wrote:
> please use stl container instead

Done.

https://codereview.chromium.org/1922303002/diff/60001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:49: #include "src/isolate.h"
On 2016/05/04 07:29:07, jochen wrote:
> we should only use v8::Isolate via include/v8.h

Done.

[alph, 2016-05-05 22:39:09]

On 2016/05/04 00:14:57, lpy wrote:
> On 2016/05/03 18:48:07, alph wrote:
> > It's better to not put four things into one patch, but rather have four
> patches.
> > This way you can keep the patch size smaller. It is much easier to get
smaller
> > patches through review.
> 
> Any suggestion?

Can you first split sampler.cc into two files: one implementing GetStackSample,
and the other dealing with threads and sample processing (which will later be
moved into library). 

> 
> > Also I see you created new files by copying code from v8/profiler/sampler.*
so
> > effectively now v8 has some code duplicated. This is not good.
> 
> 
> Yes, it's not good to have duplicate code. But to maintain the current
sampling
> functionality in v8, we need to duplicate it until we migrate the code to use
> libsampler, following up CLs will solve this issue.

Can you please route the legacy API calls through the new code within the single
patch. Otherwise we'll have issues synchronizing these two copies of code.

[lpy, 2016-05-05 22:55:15]

On 2016/05/05 22:39:09, alph wrote:
> On 2016/05/04 00:14:57, lpy wrote:
> > On 2016/05/03 18:48:07, alph wrote:
> > > It's better to not put four things into one patch, but rather have four
> > patches.
> > > This way you can keep the patch size smaller. It is much easier to get
> smaller
> > > patches through review.
> > 
> > Any suggestion?
> 
> Can you first split sampler.cc into two files: one implementing
GetStackSample,
> and the other dealing with threads and sample processing (which will later be
> moved into library). 


Will do.

> > 
> > > Also I see you created new files by copying code from
v8/profiler/sampler.*
> so
> > > effectively now v8 has some code duplicated. This is not good.
> > 
> > 
> > Yes, it's not good to have duplicate code. But to maintain the current
> sampling
> > functionality in v8, we need to duplicate it until we migrate the code to
use
> > libsampler, following up CLs will solve this issue.
> 
> Can you please route the legacy API calls through the new code within the
single
> patch. Otherwise we'll have issues synchronizing these two copies of code.

Sorry I don't understand here. Do you mean when we create the library, we also
make everywhere that is using the sampler use the new one and delete the old
implementation, in the same CL?

[alph, 2016-05-05 23:09:53]

On 2016/05/05 22:55:15, lpy wrote:
> On 2016/05/05 22:39:09, alph wrote:
> > On 2016/05/04 00:14:57, lpy wrote:
> > > On 2016/05/03 18:48:07, alph wrote:
> > > > It's better to not put four things into one patch, but rather have four
> > > patches.
> > > > This way you can keep the patch size smaller. It is much easier to get
> > smaller
> > > > patches through review.
> > > 
> > > Any suggestion?
> > 
> > Can you first split sampler.cc into two files: one implementing
> GetStackSample,
> > and the other dealing with threads and sample processing (which will later
be
> > moved into library). 
> 
> 
> Will do.
> 
> > > 
> > > > Also I see you created new files by copying code from
> v8/profiler/sampler.*
> > so
> > > > effectively now v8 has some code duplicated. This is not good.
> > > 
> > > 
> > > Yes, it's not good to have duplicate code. But to maintain the current
> > sampling
> > > functionality in v8, we need to duplicate it until we migrate the code to
> use
> > > libsampler, following up CLs will solve this issue.
> > 
> > Can you please route the legacy API calls through the new code within the
> single
> > patch. Otherwise we'll have issues synchronizing these two copies of code.
> 
> Sorry I don't understand here. Do you mean when we create the library, we also
> make everywhere that is using the sampler use the new one and delete the old
> implementation, in the same CL?

I see the following steps:
1. create the library, and make the v8-profiler API route through it.
2. ship the library and deprecate the relevant v8-profiler API.
3. update the clients to use the library.
4. remove the old API.

[jochen (gone - plz use gerrit), 2016-05-06 09:43:41]

On 2016/05/05 at 23:09:53, alph wrote:
> On 2016/05/05 22:55:15, lpy wrote:
> > On 2016/05/05 22:39:09, alph wrote:
> > > On 2016/05/04 00:14:57, lpy wrote:
> > > > On 2016/05/03 18:48:07, alph wrote:
> > > > > It's better to not put four things into one patch, but rather have
four
> > > > patches.
> > > > > This way you can keep the patch size smaller. It is much easier to get
> > > smaller
> > > > > patches through review.
> > > > 
> > > > Any suggestion?
> > > 
> > > Can you first split sampler.cc into two files: one implementing
> > GetStackSample,
> > > and the other dealing with threads and sample processing (which will later
be
> > > moved into library). 
> > 
> > 
> > Will do.
> > 
> > > > 
> > > > > Also I see you created new files by copying code from
> > v8/profiler/sampler.*
> > > so
> > > > > effectively now v8 has some code duplicated. This is not good.
> > > > 
> > > > 
> > > > Yes, it's not good to have duplicate code. But to maintain the current
> > > sampling
> > > > functionality in v8, we need to duplicate it until we migrate the code
to
> > use
> > > > libsampler, following up CLs will solve this issue.
> > > 
> > > Can you please route the legacy API calls through the new code within the
> > single
> > > patch. Otherwise we'll have issues synchronizing these two copies of code.
> > 
> > Sorry I don't understand here. Do you mean when we create the library, we
also
> > make everywhere that is using the sampler use the new one and delete the old
> > implementation, in the same CL?
> 
> I see the following steps:
> 1. create the library, and make the v8-profiler API route through it.
> 2. ship the library and deprecate the relevant v8-profiler API.
> 3. update the clients to use the library.
> 4. remove the old API.

btw, shouldn't we finally switch chrome over to the new api?

[lpy, 2016-05-06 17:41:10]

> btw, shouldn't we finally switch chrome over to the new api?

I am confused when saying Chrome.
Chrome has a sampler in content, also it has inspector which is using
CpuProfiler API in include/v8-profiler.h
My thought is we are going to switch the one in content, right?

[alph, 2016-05-06 19:28:38]

On 2016/05/06 17:41:10, lpy wrote:
> > btw, shouldn't we finally switch chrome over to the new api?
> 
> I am confused when saying Chrome.
> Chrome has a sampler in content, also it has inspector which is using
> CpuProfiler API in include/v8-profiler.h
> My thought is we are going to switch the one in content, right?

Let me clarify what we have now.
V8 has two API:
1. GetStackSample + friends
2. old one StartProfiling/StopProfiling/GetProfile that we want to remove out of
V8

content/renderer/devtools implements sampling thread, uses API #1 to get samples
from V8 and generates trace events.

V8ProfilerAgent uses API #2 to collect profiles.

So as soon as the library provides sampler thread and generation of trace events
we can switch content/... to use it.
As soon as the library provides API #2 we can switch V8ProfilerAgent to use it.

[alph, 2016-05-06 19:33:19]

https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
src/libsampler/v8-sampler.cc:365: SamplerManager::sampler_map_.find(thread_id);
Wasn't there an issue with a deadlock in debug mode when you tried to use
std::map here?

[lpy, 2016-05-06 19:38:06]

On 2016/05/06 19:33:19, alph wrote:
>
https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
> File src/libsampler/v8-sampler.cc (right):
> 
>
https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
> src/libsampler/v8-sampler.cc:365:
SamplerManager::sampler_map_.find(thread_id);
> Wasn't there an issue with a deadlock in debug mode when you tried to use
> std::map here?

Yes, there was a deadlock no mater std::map and std::list.
I am not sure if it would happen again once library is running with v8_base. But
if this happens in the future, tests will fail or become flaky.

I actually prefer not to take the risk, any suggestion?

[alph, 2016-05-06 22:33:25]

On 2016/05/06 19:38:06, lpy wrote:
> On 2016/05/06 19:33:19, alph wrote:
> >
>
https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
> > File src/libsampler/v8-sampler.cc (right):
> > 
> >
>
https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
> > src/libsampler/v8-sampler.cc:365:
> SamplerManager::sampler_map_.find(thread_id);
> > Wasn't there an issue with a deadlock in debug mode when you tried to use
> > std::map here?
> 
> Yes, there was a deadlock no mater std::map and std::list.
> I am not sure if it would happen again once library is running with v8_base.
But
> if this happens in the future, tests will fail or become flaky.
> 
> I actually prefer not to take the risk, any suggestion?

I don't have a good solution here.
May be we can use some other not that efficient data structure in debug mode,
e.g. std::vector? Does it have the same issue?

[lpy, 2016-05-06 23:03:08]

On 2016/05/06 22:33:25, alph wrote:
> On 2016/05/06 19:38:06, lpy wrote:
> > On 2016/05/06 19:33:19, alph wrote:
> > >
> >
>
https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
> > > File src/libsampler/v8-sampler.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1922303002/diff/80001/src/libsampler/v8-sampl...
> > > src/libsampler/v8-sampler.cc:365:
> > SamplerManager::sampler_map_.find(thread_id);
> > > Wasn't there an issue with a deadlock in debug mode when you tried to use
> > > std::map here?
> > 
> > Yes, there was a deadlock no mater std::map and std::list.
> > I am not sure if it would happen again once library is running with v8_base.
> But
> > if this happens in the future, tests will fail or become flaky.
> > 
> > I actually prefer not to take the risk, any suggestion?
> 
> I don't have a good solution here.
> May be we can use some other not that efficient data structure in debug mode,
> e.g. std::vector? Does it have the same issue?

Yes, it does.

[lpy, 2016-05-09 17:01:14]

The CQ bit was checked by lpy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-09 17:01:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1922303002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1922303002/100001

[commit-bot: I haz the power, 2016-05-09 17:31:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-09 17:31:33]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-05-10 11:52:41]

can you add a BUILD.gn target as well please?

https://codereview.chromium.org/1922303002/diff/100001/src/v8.gyp
File src/v8.gyp (right):

https://codereview.chromium.org/1922303002/diff/100001/src/v8.gyp#newcode1932
src/v8.gyp:1932: 'v8_base',
you shouldn't depend on an internal target here, but 'v8' and 'v8_libbase'

[lpy, 2016-05-10 18:27:34]

PTAL.

https://codereview.chromium.org/1922303002/diff/100001/src/v8.gyp
File src/v8.gyp (right):

https://codereview.chromium.org/1922303002/diff/100001/src/v8.gyp#newcode1932
src/v8.gyp:1932: 'v8_base',
On 2016/05/10 11:52:40, jochen wrote:
> you shouldn't depend on an internal target here, but 'v8' and 'v8_libbase'

Done.

[jochen (gone - plz use gerrit), 2016-05-11 08:54:40]

https://codereview.chromium.org/1922303002/diff/160001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1922303002/diff/160001/BUILD.gn#newcode2022
BUILD.gn:2022: ":v8_libbase",
should also depend on v8

https://codereview.chromium.org/1922303002/diff/160001/src/v8.gyp
File src/v8.gyp (right):

https://codereview.chromium.org/1922303002/diff/160001/src/v8.gyp#newcode1932
src/v8.gyp:1932: 'v8_libbase',
same here

[lpy, 2016-05-11 20:57:30]

PTAL.

https://codereview.chromium.org/1922303002/diff/160001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1922303002/diff/160001/BUILD.gn#newcode2022
BUILD.gn:2022: ":v8_libbase",
On 2016/05/11 08:54:40, jochen (soon OOO) wrote:
> should also depend on v8

I can't find any target called v8 here, is it v8_base?

https://codereview.chromium.org/1922303002/diff/160001/src/v8.gyp
File src/v8.gyp (right):

https://codereview.chromium.org/1922303002/diff/160001/src/v8.gyp#newcode1932
src/v8.gyp:1932: 'v8_libbase',
On 2016/05/11 08:54:40, jochen (soon OOO) wrote:
> same here

Done.

[lpy, 2016-05-18 20:43:51]

I made a few changes, the main changes is:

1. Deleted sampler.[h|cc]
2. Used libsampler in V8 to keep the functionality as workaround
3. Moved sampling thread to log.cc

The next step here, would be to move Profiler, Ticker and sampling thread in
log.cc to d8, which means to move --prof functionality to d8. And once we finish
this step, there will be only one extra thread, a.k.a. ProfilerEventsProcessor
in cpu-profiler.h, remaining in V8 that needs to move out.

So, PTAL.

Thanks

[lpy, 2016-05-19 01:03:12]

There's deadlock happening on debug build, and it is happening on two different
STL container in the same thread.

Taking JsNativeJsRuntimeJsSampleMultiple on ia32 debug build on Linux for
example, here's the backtrace produced by gdb:
https://paste.googleplex.com/4687488887029760, and here's the log on try bot:
https://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/...

In #9 indicates V8 reached the code in SignalHandler::HandleProfilerSignal,
which is

  SamplerMapConstIterator sampler_entry =
    SamplerManager::sampler_map_.Get().find(thread_id);

However, before the signal handler got called, #16 indicates V8 was in the code
of graph-reducer.cc:81:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/compiler/gr...,
where `reducer_` is a vector.

From the backtrace, it looks like they were using the same mutex, since they
were both calling `pthread_mutex_lock` and it was a deadlock here.

And actually, when v8 is manipulting STL container, and at this moment, the
signal handler gets called, it will lead to a deadlock. But in release build, I
don't spot this issue.

How to reproduce:
1. Apply the patch
2. Build V8 on debug build on Linux, I use gyp
3. Run tests related to CpuProfiler
4. tests will timeout because of the deadlock

I don't have a clue about the reason though. Any idea is appreciated.

Thanks.

[lpy, 2016-05-19 01:22:16]

Description was changed from

==========
Create libsampler as V8 sampler library.

This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Removes all thread code.
3. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
4. Creates SamplerManager to manage the relationship between samplers and
threads.

BUG=v8:4789
LOG=n
==========

to

==========
Create libsampler as V8 sampler library.

This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

[alph, 2016-05-19 21:49:07]

On 2016/05/19 01:03:12, lpy wrote:
> There's deadlock happening on debug build, and it is happening on two
different
> STL container in the same thread.
> 
> Taking JsNativeJsRuntimeJsSampleMultiple on ia32 debug build on Linux for
> example, here's the backtrace produced by gdb:
> https://paste.googleplex.com/4687488887029760, and here's the log on try bot:
>
https://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/...
> 
> In #9 indicates V8 reached the code in SignalHandler::HandleProfilerSignal,
> which is
> 
>   SamplerMapConstIterator sampler_entry =
>     SamplerManager::sampler_map_.Get().find(thread_id);
> 
> However, before the signal handler got called, #16 indicates V8 was in the
code
> of graph-reducer.cc:81:
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/compiler/gr...,
> where `reducer_` is a vector.
> 
> From the backtrace, it looks like they were using the same mutex, since they
> were both calling `pthread_mutex_lock` and it was a deadlock here.
> 
> And actually, when v8 is manipulting STL container, and at this moment, the
> signal handler gets called, it will lead to a deadlock. But in release build,
I
> don't spot this issue.
> 
> How to reproduce:
> 1. Apply the patch
> 2. Build V8 on debug build on Linux, I use gyp
> 3. Run tests related to CpuProfiler
> 4. tests will timeout because of the deadlock
> 
> I don't have a clue about the reason though. Any idea is appreciated.
> 
> Thanks.

I investigated the issue some time ago. In debug mode STL creates a pool of
mutexes for accessing containers and each container uses a mutex in the pool
based on the container's address hash.
So several containers may share the same mutex. The deadlock you're observing is
caused by your map sharing the same mutex as some user map, and that mutex is
happened to be captured by the time the SIGPROF arrives.

Not sure what's the best way to workaround that besides implementing your own
map / taking one from v8.

[lpy, 2016-05-20 17:05:21]

On 2016/05/19 21:49:07, alph wrote:
> On 2016/05/19 01:03:12, lpy wrote:
> > There's deadlock happening on debug build, and it is happening on two
> different
> > STL container in the same thread.
> > 
> > Taking JsNativeJsRuntimeJsSampleMultiple on ia32 debug build on Linux for
> > example, here's the backtrace produced by gdb:
> > https://paste.googleplex.com/4687488887029760, and here's the log on try
bot:
> >
>
https://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/...
> > 
> > In #9 indicates V8 reached the code in SignalHandler::HandleProfilerSignal,
> > which is
> > 
> >   SamplerMapConstIterator sampler_entry =
> >     SamplerManager::sampler_map_.Get().find(thread_id);
> > 
> > However, before the signal handler got called, #16 indicates V8 was in the
> code
> > of graph-reducer.cc:81:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/compiler/gr...,
> > where `reducer_` is a vector.
> > 
> > From the backtrace, it looks like they were using the same mutex, since they
> > were both calling `pthread_mutex_lock` and it was a deadlock here.
> > 
> > And actually, when v8 is manipulting STL container, and at this moment, the
> > signal handler gets called, it will lead to a deadlock. But in release
build,
> I
> > don't spot this issue.
> > 
> > How to reproduce:
> > 1. Apply the patch
> > 2. Build V8 on debug build on Linux, I use gyp
> > 3. Run tests related to CpuProfiler
> > 4. tests will timeout because of the deadlock
> > 
> > I don't have a clue about the reason though. Any idea is appreciated.
> > 
> > Thanks.
> 
> I investigated the issue some time ago. In debug mode STL creates a pool of
> mutexes for accessing containers and each container uses a mutex in the pool
> based on the container's address hash.
> So several containers may share the same mutex. The deadlock you're observing
is
> caused by your map sharing the same mutex as some user map, and that mutex is
> happened to be captured by the time the SIGPROF arrives.
> 
> Not sure what's the best way to workaround that besides implementing your own
> map / taking one from v8.

Thank you for the explanation. I ported HashMap from v8/src.


The Windows compilation hung when it reached
```
LINK_EMBED mksnapshot.exe
STAMP obj\src\v8_external_snapshot.actions_depends.stamp
```
but I can't see any further information. It also happens on my local machine.

yang@, any idea what I do wrong here?
Thanks.

[lpy, 2016-05-22 05:15:07]

By moving `sampling_thread_->Join` out of the destructor of Ticker, the
compilation on Windows won't hang, still wondering why though.

Please take a look at the CL.
Thanks.

[lpy, 2016-05-23 16:50:08]

Description was changed from

==========
Create libsampler as V8 sampler library.

This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

to

==========
Create libsampler as V8 sampler library.
    
This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

[alph, 2016-05-23 23:37:28]

I still don't like that several items (now there are five) are done in a single
patch.

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/hashmap.h
File src/libsampler/hashmap.h (right):

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/hashmap...
src/libsampler/hashmap.h:278: class TemplateHashMap: private HashMapImpl {
Is it used?

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/v8-samp...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/v8-samp...
src/libsampler/v8-sampler.cc:286: static base::LazyInstance<HashMap,
HashMapCreateTrait>::type
nit: I'd make the SamplerManager a singleton instead, but all its members
non-static.

https://codereview.chromium.org/1922303002/diff/360001/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/1922303002/diff/360001/src/log.cc#newcode556
src/log.cc:556: while (true) {
while (sampler_->IsProfiling()) {

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
File test/cctest/libsampler/test-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
test/cctest/libsampler/test-sampler.cc:28: while (true) {
while (sampler_->IsProfiling()) {

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
test/cctest/libsampler/test-sampler.cc:81: void Wait() {
Do you really need to wait. Just empty callbacks should be enough

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
test/cctest/libsampler/test-sampler.cc:164: RunSampler(env.local(), function,
args, arraysize(args), 100, 100);
(100 + 100) * 10ms = 2000ms, which is quite long for a test. Change the interval
to at most 1ms.

[lpy, 2016-05-24 16:25:26]

Description was changed from

==========
Create libsampler as V8 sampler library.
    
This patch does four things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

to

==========
Create libsampler as V8 sampler library.
    
This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

[lpy, 2016-05-24 17:07:57]

Most changes of this patch are: libsampler/ directory, and sampling thread in
log.cc

If we want to land libsampler without code duplication, we need to delete
sampler.[h|cc] at the same time, and replace all usage with libsampler
everywhere. Also since libsampler is using hashmap, hashmap should be landed in
the same patch or before this patch.

I can only see 4 separate patches here:

1. land Isolate::IsInUse in api.cc, a very small API

2. land hashmap.h and utils.h in libsampler, but we will only have these two
files in libsampler at that time, which is strange to me.

3. land sampling thread in log.cc, but it will not be used until libsampler gets
landed, so it's strange to me.

4. land libsampler, as well as replace the usage of sampler with libsampler.

WDYT?

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/hashmap.h
File src/libsampler/hashmap.h (right):

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/hashmap...
src/libsampler/hashmap.h:278: class TemplateHashMap: private HashMapImpl {
On 2016/05/23 23:37:27, alph wrote:
> Is it used?

Done.

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/v8-samp...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/360001/src/libsampler/v8-samp...
src/libsampler/v8-sampler.cc:286: static base::LazyInstance<HashMap,
HashMapCreateTrait>::type
On 2016/05/23 23:37:27, alph wrote:
> nit: I'd make the SamplerManager a singleton instead, but all its members
> non-static.

What's the benefit here?

https://codereview.chromium.org/1922303002/diff/360001/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/1922303002/diff/360001/src/log.cc#newcode556
src/log.cc:556: while (true) {
On 2016/05/23 23:37:27, alph wrote:
> while (sampler_->IsProfiling()) {

Done.

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
File test/cctest/libsampler/test-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
test/cctest/libsampler/test-sampler.cc:28: while (true) {
On 2016/05/23 23:37:27, alph wrote:
> while (sampler_->IsProfiling()) {

Done.

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
test/cctest/libsampler/test-sampler.cc:81: void Wait() {
On 2016/05/23 23:37:27, alph wrote:
> Do you really need to wait. Just empty callbacks should be enough

Done.

https://codereview.chromium.org/1922303002/diff/360001/test/cctest/libsampler...
test/cctest/libsampler/test-sampler.cc:164: RunSampler(env.local(), function,
args, arraysize(args), 100, 100);
On 2016/05/23 23:37:27, alph wrote:
> (100 + 100) * 10ms = 2000ms, which is quite long for a test. Change the
interval
> to at most 1ms.

Done.

[alph, 2016-05-24 22:19:21]

lgtm w/ nits.

Ok, this seems to be a good starting point. Let's land it and work on the minor
things incrementally.

https://codereview.chromium.org/1922303002/diff/440001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1922303002/diff/440001/include/v8.h#newcode6338
include/v8.h:6338: bool IsInUse();
Can GetStackSample just return an empty stack if the isolate is not entered?
I.e. do we really need to expose this in the API?

https://codereview.chromium.org/1922303002/diff/440001/src/libsampler/v8-samp...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/440001/src/libsampler/v8-samp...
src/libsampler/v8-sampler.cc:164: typedef std::vector<Sampler*> SamplerList;
just curious if std::vector has the same issue with locking as std::map?

https://codereview.chromium.org/1922303002/diff/440001/src/libsampler/v8-samp...
src/libsampler/v8-sampler.cc:410: SignalHandler::FillRegisterState(context,
&state);
nit: drop prefix

[lpy, 2016-05-24 23:13:10]

Thank you for reviewing such a big patch.

https://codereview.chromium.org/1922303002/diff/440001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1922303002/diff/440001/include/v8.h#newcode6338
include/v8.h:6338: bool IsInUse();
On 2016/05/24 22:19:21, alph wrote:
> Can GetStackSample just return an empty stack if the isolate is not entered?
> I.e. do we really need to expose this in the API?

I think we can. But I added this API in order to stick the logic in signal
handler since I can't verify the correctness without it.

https://codereview.chromium.org/1922303002/diff/440001/src/libsampler/v8-samp...
File src/libsampler/v8-sampler.cc (right):

https://codereview.chromium.org/1922303002/diff/440001/src/libsampler/v8-samp...
src/libsampler/v8-sampler.cc:164: typedef std::vector<Sampler*> SamplerList;
On 2016/05/24 22:19:21, alph wrote:
> just curious if std::vector has the same issue with locking as std::map?

Instead of using iterator, I use operator[], so I think it doesn't.
\o/

https://codereview.chromium.org/1922303002/diff/440001/src/libsampler/v8-samp...
src/libsampler/v8-sampler.cc:410: SignalHandler::FillRegisterState(context,
&state);
On 2016/05/24 22:19:21, alph wrote:
> nit: drop prefix

Done.

[Yang, 2016-05-25 08:29:39]

LGTM

Question about this though:
"4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality."
Do I understand correctly that this is temporary and eventually the sampling
thread would be started by the embedder?

https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap.h
File src/libsampler/hashmap.h (right):

https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap...
src/libsampler/hashmap.h:5: // This file is ported from src/hashmap.h
I wonder whether it makes sense to move src/hashmap.h to src/base, and use that
from both libsampler and v8.

[lpy, 2016-05-25 13:59:26]

On 2016/05/25 08:29:39, Yang wrote:
> LGTM

Thanks for review this.

> Question about this though:
> "4. Moves sampling thread into log.cc as workaround to keep the --prof
> functionality."
> Do I understand correctly that this is temporary and eventually the sampling
> thread would be started by the embedder?

Yes, this is temporary. The sampling thread and Profiler thread in log.cc will
be moved to D8 in follow up patches.

>
https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap.h
> File src/libsampler/hashmap.h (right):
> 
>
https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap...
> src/libsampler/hashmap.h:5: // This file is ported from src/hashmap.h
> I wonder whether it makes sense to move src/hashmap.h to src/base, and use
that
> from both libsampler and v8.

I think it makes sense. But Looks like we can't since it's using some allocation
policy in src/.

[Yang, 2016-05-25 14:38:11]

On 2016/05/25 13:59:26, lpy wrote:
> On 2016/05/25 08:29:39, Yang wrote:
> > LGTM
> 
> Thanks for review this.
> 
> > Question about this though:
> > "4. Moves sampling thread into log.cc as workaround to keep the --prof
> > functionality."
> > Do I understand correctly that this is temporary and eventually the sampling
> > thread would be started by the embedder?
> 
> Yes, this is temporary. The sampling thread and Profiler thread in log.cc will
> be moved to D8 in follow up patches.
> 
> >
>
https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap.h
> > File src/libsampler/hashmap.h (right):
> > 
> >
>
https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap...
> > src/libsampler/hashmap.h:5: // This file is ported from src/hashmap.h
> > I wonder whether it makes sense to move src/hashmap.h to src/base, and use
> that
> > from both libsampler and v8.
> 
> I think it makes sense. But Looks like we can't since it's using some
allocation
> policy in src/.

Not sure if it's worth investing the time. But we could define the hashmap
without default allocation policy in base, and then derive from it in src and
add that default allocation policy.

Either way lgtm

[lpy, 2016-05-25 16:21:51]

>
https://codereview.chromium.org/1922303002/diff/460001/src/libsampler/hashmap...
> > > src/libsampler/hashmap.h:5: // This file is ported from src/hashmap.h
> > > I wonder whether it makes sense to move src/hashmap.h to src/base, and use
> > that
> > > from both libsampler and v8.
> > 
> > I think it makes sense. But Looks like we can't since it's using some
> allocation
> > policy in src/.
> 
> Not sure if it's worth investing the time. But we could define the hashmap
> without default allocation policy in base, and then derive from it in src and
> add that default allocation policy.
> 
> Either way lgtm

That makes sense to me. I will try to clean this up in another CL.
Thanks.

[lpy, 2016-05-25 19:03:40]

The CQ bit was checked by lpy@chromium.org

[lpy, 2016-05-25 19:03:40]

The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org
Link to the patchset: https://codereview.chromium.org/1922303002/#ps460001
(title: "Address alph's comment")

[commit-bot: I haz the power, 2016-05-25 19:03:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1922303002/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1922303002/460001

[commit-bot: I haz the power, 2016-05-25 19:06:00]

Description was changed from

==========
Create libsampler as V8 sampler library.
    
This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

to

==========
Create libsampler as V8 sampler library.
    
This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

[commit-bot: I haz the power, 2016-05-25 19:06:01]

Committed patchset #24 (id:460001)

[commit-bot: I haz the power, 2016-05-25 19:06:54]

Description was changed from

==========
Create libsampler as V8 sampler library.
    
This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n
==========

to

==========
Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

[commit-bot: I haz the power, 2016-05-25 19:06:56]

Patchset 24 (id:??) landed as
https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}

[lpy, 2016-05-25 20:19:45]

A revert of this CL (patchset #24 id:460001) has been created in
https://codereview.chromium.org/2000323007/ by lpy@chromium.org.

The reason for reverting is: V8 Linux64 TSAN failure because ThreadSanitizer
indicated data race..

[lpy, 2016-05-25 23:23:12]

Description was changed from

==========
Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

to

==========
Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

[lpy, 2016-05-25 23:25:24]

Description was changed from

==========
Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

to

==========
Reland: Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

[lpy, 2016-05-26 02:12:12]

The CQ bit was checked by lpy@chromium.org

[lpy, 2016-05-26 02:12:12]

The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1922303002/#ps480001
(title: "Fix TSAN failure")

[commit-bot: I haz the power, 2016-05-26 02:12:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1922303002/480001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1922303002/480001

[commit-bot: I haz the power, 2016-05-26 02:14:06]

Description was changed from

==========
Reland: Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

to

==========
Reland: Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

[commit-bot: I haz the power, 2016-05-26 02:14:08]

Committed patchset #25 (id:480001)

[commit-bot: I haz the power, 2016-05-26 02:15:00]

Description was changed from

==========
Reland: Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}
==========

to

==========
Reland: Create libsampler as V8 sampler library.

This patch does five things:

1. Extracts sampler as libsampler to provide sampling functionality support.
2. Makes SampleStack virtual so embedders can override the behaviour of sample
collecting.
3. Removes sampler.[h|cc].
4. Moves sampling thread into log.cc as workaround to keep the --prof
functionality.
5. Creates SamplerManager to manage the relationship between samplers and
threads.

The reason we port hashmap.h is that in debug mode, STL containers are using
mutexes from a mutex pool, which may lead to deadlock when using asynchronously
signal handler.

Currently libsampler is used in V8 temporarily.

BUG=v8:4789
LOG=n

Committed: https://crrev.com/06cc9b7c176a6223971deaa9fbcafe1a05058c7b
Cr-Commit-Position: refs/heads/master@{#36527}

Committed: https://crrev.com/a0198c0f627de392280f00099333f96ee34d2519
Cr-Commit-Position: refs/heads/master@{#36532}
==========

[commit-bot: I haz the power, 2016-05-26 02:15:02]

Patchset 25 (id:??) landed as
https://crrev.com/a0198c0f627de392280f00099333f96ee34d2519
Cr-Commit-Position: refs/heads/master@{#36532}