Ignore CpuProfiler::SetIdle call when not profiling.

Ignore CpuProfiler::SetIdle call when not profiling.

It truned out we can enter nested message loop and call
SetIdle from inside the compilation in some obscure
situations. To not whitelist all the possible StateTag values,
we'd better ignore this call when not profiling as it has
no effect anyway.

This patch also reverts DCHECK change from https://codereview.chromium.org/1922703005/.

BUG=none
LOG=N

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

[dgozman, 2016-04-29 21:52:21]

dgozman@chromium.org changed reviewers:
+ adamk@chromium.org, alph@chromium.org

[dgozman, 2016-04-29 21:52:21]

Hi,

Could you please a look? Unfortunately, previous approach didn't work so I have
to bother you again :-)

Thanks,
Dmitry

[adamk, 2016-04-29 21:56:23]

adamk@chromium.org changed reviewers:
+ yangguo@chromium.org
- adamk@chromium.org

[adamk, 2016-04-29 21:56:24]

If it's not simple, I think that I'm not the right V8 OWNER here, adding Yang.

[alph, 2016-04-30 00:03:08]

https://codereview.chromium.org/1936703002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1936703002/diff/1/src/api.cc#newcode8356
src/api.cc:8356: if (!profiler->is_profiling()) return;
Sorry, didn't get how it helps. If we're not profiling, but it's called from
within a compilation it'll get the state of v8::COMPILER and fail the DCHECK. Am
I missing something?

[dgozman, 2016-04-30 00:10:40]

https://codereview.chromium.org/1936703002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1936703002/diff/1/src/api.cc#newcode8356
src/api.cc:8356: if (!profiler->is_profiling()) return;
On 2016/04/30 00:03:08, alph wrote:
> Sorry, didn't get how it helps. If we're not profiling, but it's called from
> within a compilation it'll get the state of v8::COMPILER and fail the DCHECK.
Am
> I missing something?

This does not happen, since compilation cannot trigger idle state unless we run
nested message loop.
Do you think we should instead allow to call this at any time and just remove
the DCHECK? That's a possible solution.

[alph, 2016-04-30 00:29:20]

https://codereview.chromium.org/1936703002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1936703002/diff/1/src/api.cc#newcode8356
src/api.cc:8356: if (!profiler->is_profiling()) return;
On 2016/04/30 00:10:39, dgozman wrote:
> On 2016/04/30 00:03:08, alph wrote:
> > Sorry, didn't get how it helps. If we're not profiling, but it's called from
> > within a compilation it'll get the state of v8::COMPILER and fail the
DCHECK.
> Am
> > I missing something?
> 
> This does not happen, since compilation cannot trigger idle state unless we
run
> nested message loop.
> Do you think we should instead allow to call this at any time and just remove
> the DCHECK? That's a possible solution.

Looks more robust to me. The problem is that when you call setIdle(false) you
have to restore the previous state e.g. COMPILER, rather than set it to
EXTERNAL.

Relying on the fact that profiling disables nested messages loops looks fragile.
Despite it was done this way before.

[alph, 2016-04-30 01:04:01]

ok, let's leave it this way.

So assumptions are:
1. SetIdle does not have any effect when not profiling.
2. When profiling the caller has to be responsible not to do the nested state
changes.

lgtm.

[Yang, 2016-05-02 10:21:04]

lgtm

[alph, 2016-05-02 17:06:00]

The CQ bit was checked by alph@chromium.org

[commit-bot: I haz the power, 2016-05-02 17:06:04]

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

[commit-bot: I haz the power, 2016-05-02 17:32:08]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-02 17:33:36]

Description was changed from

==========
Ignore CpuProfiler::SetIdle call when not profiling.

It truned out we can enter nested message loop and call
SetIdle from inside the compilation in some obscure
situations. To not whitelist all the possible StateTag values,
we'd better ignore this call when not profiling as it has
no effect anyway.

This patch also reverts DCHECK change from
https://codereview.chromium.org/1922703005/.

BUG=none
LOG=N
==========

to

==========
Ignore CpuProfiler::SetIdle call when not profiling.

It truned out we can enter nested message loop and call
SetIdle from inside the compilation in some obscure
situations. To not whitelist all the possible StateTag values,
we'd better ignore this call when not profiling as it has
no effect anyway.

This patch also reverts DCHECK change from
https://codereview.chromium.org/1922703005/.

BUG=none
LOG=N

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

[commit-bot: I haz the power, 2016-05-02 17:33:37]

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