Annotate runtime call stats with trace events

Annotate runtime call stats with trace events

I used a new category "v8.runtime" and all events are disabled by
default, so there shouldn't be any perf impact.

BUG=none
R=fmeawad@chromium.org,cbruni@chromium.org

Committed: https://crrev.com/34a47649e89bc7027843f779a1baa46fab185edf
Cr-Commit-Position: refs/heads/master@{#34620}

[Camillo Bruni, 2016-03-08 13:36:14]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2016-03-08 13:36:24]

The CQ bit was unchecked by cbruni@chromium.org

[commit-bot: I haz the power, 2016-03-08 13:36:24]

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

[commit-bot: I haz the power, 2016-03-08 13:36:25]

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

[commit-bot: I haz the power, 2016-03-08 13:36:26]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[Camillo Bruni, 2016-03-08 13:37:38]

On 2016/03/08 at 10:41:30, jochen wrote:
> 

I still have to investigate a performance regression due to the timer-scopes,
thus I think it would be better to send the trace events from within the
RuntimeCallTimerScopes to avoid adding yet another check.

[jochen (gone - plz use gerrit), 2016-03-08 13:43:52]

On 2016/03/08 at 13:37:38, cbruni wrote:
> On 2016/03/08 at 10:41:30, jochen wrote:
> > 
> 
> I still have to investigate a performance regression due to the timer-scopes,
thus I think it would be better to send the trace events from within the
RuntimeCallTimerScopes to avoid adding yet another check.

well, ideally, the runtime call timer checks would be removed, no? In the end,
it should be possible to send the trace events without specifying additional
flags to v8.

Another issue is that inside the RuntimeCallTimerScope, I don't have access to
the scope's name...

[jochen (gone - plz use gerrit), 2016-03-08 13:44:01]

Description was changed from

==========
Annotate runtime call stats with trace events

I used a new category "v8-runtime" and all events are disabled by
default, so there shouldn't be any perf impact.

BUG=none
R=fmeawad@chromium.org,cbruni@chromium.org
==========

to

==========
Annotate runtime call stats with trace events

I used a new category "v8.runtime" and all events are disabled by
default, so there shouldn't be any perf impact.

BUG=none
R=fmeawad@chromium.org,cbruni@chromium.org
==========

[Camillo Bruni, 2016-03-08 14:05:01]

On 2016/03/08 at 13:43:52, jochen wrote:
> On 2016/03/08 at 13:37:38, cbruni wrote:
> > On 2016/03/08 at 10:41:30, jochen wrote:
> > > 
> > 
> > I still have to investigate a performance regression due to the
timer-scopes, thus I think it would be better to send the trace events from
within the RuntimeCallTimerScopes to avoid adding yet another check.
> 
> well, ideally, the runtime call timer checks would be removed, no? In the end,
it should be possible to send the trace events without specifying additional
flags to v8.

yes ideally yes, but I fear that the counters we stay longer than expected and
they tank some benchmarks currently by 6% ;)

> Another issue is that inside the RuntimeCallTimerScope, I don't have access to
the scope's name...

sure, the counters have the name :)

[jochen (gone - plz use gerrit), 2016-03-08 14:07:11]

On 2016/03/08 at 14:05:01, cbruni wrote:
> On 2016/03/08 at 13:43:52, jochen wrote:
> > On 2016/03/08 at 13:37:38, cbruni wrote:
> > > On 2016/03/08 at 10:41:30, jochen wrote:
> > > > 
> > > 
> > > I still have to investigate a performance regression due to the
timer-scopes, thus I think it would be better to send the trace events from
within the RuntimeCallTimerScopes to avoid adding yet another check.
> > 
> > well, ideally, the runtime call timer checks would be removed, no? In the
end, it should be possible to send the trace events without specifying
additional flags to v8.
> 
> yes ideally yes, but I fear that the counters we stay longer than expected and
they tank some benchmarks currently by 6% ;)
> 
> > Another issue is that inside the RuntimeCallTimerScope, I don't have access
to the scope's name...
> 
> sure, the counters have the name :)

well, as char* - so I'd have to strcat them to "V8.Runtime_" which is kinda
expensive :-/

[jochen (gone - plz use gerrit), 2016-03-08 15:42:40]

btw, which benchmark do they tank?

[Camillo Bruni, 2016-03-08 17:04:33]

On 2016/03/08 at 15:42:40, jochen wrote:
> btw, which benchmark do they tank?

v8-perf/benchmarks/SunSpiderGolem/SunSpiderGolemStringFasta.js seems to be
heavily affected (due to the massive runtime function calls).
Seems like I get it more or less under control with
https://codereview.chromium.org/1771323003 but I introduced much more overhead
than I thought (even without actually enabling the runtime stats).

[Camillo Bruni, 2016-03-08 17:04:36]

lgtm

[fmeawad, 2016-03-08 17:46:29]

LGTM

v8.runtime is the correct format.
I will monitor the perf waterfall for any regression caused by that CL.

[jochen (gone - plz use gerrit), 2016-03-09 12:46:38]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-03-09 12:46:39]

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

[commit-bot: I haz the power, 2016-03-09 12:46:43]

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

[commit-bot: I haz the power, 2016-03-09 13:09:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-09 13:09:47]

Description was changed from

==========
Annotate runtime call stats with trace events

I used a new category "v8.runtime" and all events are disabled by
default, so there shouldn't be any perf impact.

BUG=none
R=fmeawad@chromium.org,cbruni@chromium.org
==========

to

==========
Annotate runtime call stats with trace events

I used a new category "v8.runtime" and all events are disabled by
default, so there shouldn't be any perf impact.

BUG=none
R=fmeawad@chromium.org,cbruni@chromium.org

Committed: https://crrev.com/34a47649e89bc7027843f779a1baa46fab185edf
Cr-Commit-Position: refs/heads/master@{#34620}
==========

[commit-bot: I haz the power, 2016-03-09 13:09:48]

Patchset 3 (id:??) landed as
https://crrev.com/34a47649e89bc7027843f779a1baa46fab185edf
Cr-Commit-Position: refs/heads/master@{#34620}