Move compile and parse trace events to a separate category.

Move compile and parse trace events to a separate category.

For motivation see the bug.

BUG=625865
LOG=NO

Committed: https://crrev.com/6bf6ab79e35a91bca5ea9728690cd10009e464a1
Cr-Commit-Position: refs/heads/master@{#37581}

[ulan, 2016-07-06 09:07:28]

Description was changed from

==========
Move compile and parse trace events to a separate catagory.

For motivation see the bug.

BUG=625865
LOG=NO
==========

to

==========
Move compile and parse trace events to a separate category.

For motivation see the bug.

BUG=625865
LOG=NO
==========

[ulan, 2016-07-06 09:07:47]

ulan@chromium.org changed reviewers:
+ rmcilroy@chromium.org, vogelheim@chromium.org, yangguo@chromium.org

[ulan, 2016-07-06 09:07:48]

vogelheim@, ptal at parsing/*
rmcilroy@, ptal at interpreter/*
yangguo@, ptal at the rest.

This CL depends on https://codereview.chromium.org/2121263002/

[Yang, 2016-07-06 09:18:51]

lgtm

[vogelheim, 2016-07-06 09:26:34]

lgtm

Two questions, though:
- The "v8.compile" event itself (generated in Blink's bindings) will still be in
the default set?
- Will the disabled-by-default-v8.compile category automatically show up in
about:tracing's Record dialog?

[rmcilroy, 2016-07-06 09:28:27]

Interpreter lgtm.

[ulan, 2016-07-06 09:52:52]

Thank you.

On 2016/07/06 09:26:34, vogelheim wrote:
> lgtm
> 
> Two questions, though:
> - The "v8.compile" event itself (generated in Blink's bindings) will still be
in
> the default set?
Yes.

> - Will the disabled-by-default-v8.compile category automatically show up in
> about:tracing's Record dialog?
Yes.

[fmeawad, 2016-07-06 14:42:12]

On 2016/07/06 09:52:52, ulan wrote:
> Thank you.
> 
> On 2016/07/06 09:26:34, vogelheim wrote:
> > lgtm
> > 
> > Two questions, though:
> > - The "v8.compile" event itself (generated in Blink's bindings) will still
be
> in
> > the default set?
> Yes.
> 
> > - Will the disabled-by-default-v8.compile category automatically show up in
> > about:tracing's Record dialog?
> Yes.

I believe that the overhead comes mostly from the parse and compiles that
happens lazily, I would like to keep the top level events in 'v8' category (like
top level Compile, Parse, and Execute) like the one in ParseProgram for example
so that we can still do timeline breakdown without having to enable the
'disabled-by-default-v8.compile' category.

+nedneguyen

[rmcilroy, 2016-07-06 15:11:31]

On 2016/07/06 14:42:12, fmeawad wrote:
> On 2016/07/06 09:52:52, ulan wrote:
> > Thank you.
> > 
> > On 2016/07/06 09:26:34, vogelheim wrote:
> > > lgtm
> > > 
> > > Two questions, though:
> > > - The "v8.compile" event itself (generated in Blink's bindings) will still
> be
> > in
> > > the default set?
> > Yes.
> > 
> > > - Will the disabled-by-default-v8.compile category automatically show up
in
> > > about:tracing's Record dialog?
> > Yes.
> 
> I believe that the overhead comes mostly from the parse and compiles that
> happens lazily, I would like to keep the top level events in 'v8' category
(like
> top level Compile, Parse, and Execute) like the one in ParseProgram for
example
> so that we can still do timeline breakdown without having to enable the
> 'disabled-by-default-v8.compile' category.
> 
> +nedneguyen

I don't think we should spilt up lazy and eager compilation into seperate
categories - that would make it very confusing if you enable one category and
think you are seeing all time spent in compile, but there is a whole bunch of
hidden work being hidden in another disabled category.

[ulan, 2016-07-06 18:27:45]

On 2016/07/06 14:42:12, fmeawad wrote:
> I believe that the overhead comes mostly from the parse and compiles that
> happens lazily, I would like to keep the top level events in 'v8' category
(like
> top level Compile, Parse, and Execute) like the one in ParseProgram for
example

ParseProgram is called for both lazy and eager compilation.

V8.Parse is nested in a compile event (e.g. V8.CompileCode), which in turn is
usually nested in V8.Execute for both eager and lazy compilations.

Do you mean by top level event, a compile event that is not nested in
V8.Execute? If so maybe "v8.run" and "v8.compile" top level events generated by
Blink would help?

In any case, I would agree with Ross. If a user is interested in eager
compilations, they are probably also interested in lazy compilations (and vice
versa).

> so that we can still do timeline breakdown without having to enable the
> 'disabled-by-default-v8.compile' category.
I am trying to understand trade-off between this and crbug.com/625865.
What is the main disadvantage of enabling 'disabled-by-default-v8.compile' on
demand?

[fmeawad, 2016-07-06 20:14:21]

On 2016/07/06 18:27:45, ulan wrote:
> On 2016/07/06 14:42:12, fmeawad wrote:
> > I believe that the overhead comes mostly from the parse and compiles that
> > happens lazily, I would like to keep the top level events in 'v8' category
> (like
> > top level Compile, Parse, and Execute) like the one in ParseProgram for
> example
> 
> ParseProgram is called for both lazy and eager compilation.
> 
> V8.Parse is nested in a compile event (e.g. V8.CompileCode), which in turn is
> usually nested in V8.Execute for both eager and lazy compilations.

Sorry to confuse you with eager vs. lazy, I was mostly pointing out that we
compile before and within execution, we should ignore the one within execution,
and capture only the top level for the one before execution.

> 
> Do you mean by top level event, a compile event that is not nested in
> V8.Execute? If so maybe "v8.run" and "v8.compile" top level events generated
by
> Blink would help?

I also want V8 to provide those top level events in standalone mode, I guess we
can re-add some of them to the api.

> 
> In any case, I would agree with Ross. If a user is interested in eager
> compilations, they are probably also interested in lazy compilations (and vice
> versa).
> 
> > so that we can still do timeline breakdown without having to enable the
> > 'disabled-by-default-v8.compile' category.
> I am trying to understand trade-off between this and crbug.com/625865.
> What is the main disadvantage of enabling 'disabled-by-default-v8.compile' on
> demand?

My concern was that the 'v8' category would not have useful information if we
remove all parse and compile, while I was hoping that the 'v8' category would be
able to give us a overview of v8 behavior.

with that said, LGTM.

[ulan, 2016-07-07 08:56:55]

> I also want V8 to provide those top level events in standalone mode, I guess
we
> can re-add some of them to the api.
Filed https://bugs.chromium.org/p/chromium/issues/detail?id=626256

> My concern was that the 'v8' category would not have useful information if we
> remove all parse and compile, while I was hoping that the 'v8' category would
be
> able to give us a overview of v8 behavior.
Yes, I agree. I will land this CL after I add trace events to public compiler
API.

[ulan, 2016-07-07 10:51:21]

The CQ bit was checked by ulan@chromium.org

[commit-bot: I haz the power, 2016-07-07 10:51:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ulan, 2016-07-07 10:51:32]

The CQ bit was unchecked by ulan@chromium.org

[ulan, 2016-07-07 10:54:06]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-07-07 10:54:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
vogelheim@chromium.org, yangguo@chromium.org, rmcilroy@chromium.org,
fmeawad@chromium.org
Link to the patchset: https://codereview.chromium.org/2121273002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2016-07-07 10:54:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 11:18:18]

Description was changed from

==========
Move compile and parse trace events to a separate category.

For motivation see the bug.

BUG=625865
LOG=NO
==========

to

==========
Move compile and parse trace events to a separate category.

For motivation see the bug.

BUG=625865
LOG=NO
==========

[commit-bot: I haz the power, 2016-07-07 11:18:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-07 11:20:48]

Description was changed from

==========
Move compile and parse trace events to a separate category.

For motivation see the bug.

BUG=625865
LOG=NO
==========

to

==========
Move compile and parse trace events to a separate category.

For motivation see the bug.

BUG=625865
LOG=NO

Committed: https://crrev.com/6bf6ab79e35a91bca5ea9728690cd10009e464a1
Cr-Commit-Position: refs/heads/master@{#37581}
==========

[commit-bot: I haz the power, 2016-07-07 11:20:49]

Patchset 2 (id:??) landed as
https://crrev.com/6bf6ab79e35a91bca5ea9728690cd10009e464a1
Cr-Commit-Position: refs/heads/master@{#37581}