[tracing] Add support for profiling trace events

[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smaller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   will be added to pseudo stack if profiling is enabled.

This change does not add the events to pseudo stack since this is not
yet supported in skia blink and V8

There should be no performance impact on release builds since it just
adds an additional bit flag.
For impact on performance of tracing see https://goo.gl/Q3TZgg.

BUG=598426

[ssid, 2016-03-28 23:45:59]

Description was changed from

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smalller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   are added to pseudo stack if profiling is enabled.

There should be no performance in release builds since it just adds an
additional bit flag.

BUG=598426
==========

to

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smalller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   are added to pseudo stack if profiling is enabled.

There should be no performance in release builds since it just adds an
additional bit flag.

BUG=598426
==========

[ssid, 2016-03-29 01:53:46]

Description was changed from

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smalller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   are added to pseudo stack if profiling is enabled.

There should be no performance in release builds since it just adds an
additional bit flag.

BUG=598426
==========

to

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smalller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   are added to pseudo stack if profiling is enabled.

There should be no performance impact on release builds since it just
adds an additional bit flag.

BUG=598426
==========

[ssid, 2016-03-30 23:28:30]

Description was changed from

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smalller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   are added to pseudo stack if profiling is enabled.

There should be no performance impact on release builds since it just
adds an additional bit flag.

BUG=598426
==========

to

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smaller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   will be added to pseudo stack if profiling is enabled.

There should be no performance impact on release builds since it just
adds an additional bit flag.

BUG=598426
==========

[ssid, 2016-03-30 23:50:43]

Patchset #5 (id:80001) has been deleted

[ssid, 2016-03-30 23:56:31]

Description was changed from

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smaller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   will be added to pseudo stack if profiling is enabled.

There should be no performance impact on release builds since it just
adds an additional bit flag.

BUG=598426
==========

to

==========
[tracing] Add support for profiling trace events

This CL adds a new mode called profiling. In this mode all the
categories are profiled but the trace events are not added. The
profiling lets us get pseudo stacks at any point without actually adding
all the trace events. This is useful for heap profiling since we are
able to get the heap profile with lot of information in smaller trace
files.

Changes:
 - Adds ENABLED_FOR_PROFILING in CategoryGroupEnabledFlags which is set
   when heap profiling is enabled.
 - All scoped trace events irrespective of the categories being enabled
   will be added to pseudo stack if profiling is enabled.

This change does not add the events to pseudo stack since this is not
yet supported in skia blink and V8

There should be no performance impact on release builds since it just
adds an additional bit flag.
For impact on performance of tracing see https://goo.gl/Q3TZgg.

BUG=598426
==========

[ssid, 2016-03-31 00:48:07]

ssid@chromium.org changed reviewers:
+ oysteine@chromium.org, primiano@chromium.org

[ssid, 2016-03-31 00:48:07]

+oysteine I have removed the push and pop part to wait for support in other
projects (skia blink and v8). Looks like it isn't easy change in all projects.
So, I cannot land all at the same time.
This CL just adds the skeleton without stack part. ptal, thanks.

+fmeawad I will add analysis about performance on android and update soon.

[Primiano Tucci (use gerrit), 2016-04-01 14:15:02]

On 2016/03/31 00:48:07, ssid wrote:
> +oysteine I have removed the push and pop part to wait for support in other
> projects (skia blink and v8). Looks like it isn't easy change in all projects.
> So, I cannot land all at the same time.
> This CL just adds the skeleton without stack part. ptal, thanks.
> 
> +fmeawad I will add analysis about performance on android and update soon.

Hmm let's have some discussion before doing this.
Regardless of the performance itself, this adds some extra maintenaince burden
to the tracing codebase.
If we really have to do this, let's brainstorm and figure out if we want a
slightly more general version of this (for instance Fadi/Nat mentioned some
sample profiling of events in the last meeting).
The concrete problem I see here is that there is code around which caches the
category_group_enabled pointer and checks for that without looking for flags.
For instance I just reviewed https://codereview.chromium.org/1776673002/ last
week with IIRC does that. Hopefully that should be self-cotained in trace_event
itself, but would be nice if there was a way to make it opt-out by default
rather than must-be-aware-by-default.
I think this kind of change requires a broader discussion (outside of
codereview). Let's use the next weekly meeting (Performance Instrumentation &
Modelling Weekly) to discuss ?

[fmeawad, 2016-04-01 15:54:45]

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

[fmeawad, 2016-04-01 15:54:46]

Some nits.

https://codereview.chromium.org/1837013003/diff/120001/base/trace_event/trace...
File base/trace_event/trace_log.h (right):

https://codereview.chromium.org/1837013003/diff/120001/base/trace_event/trace...
base/trace_event/trace_log.h:65: ENABLED_FOR_HEAP_PROFILING = 1 << 4,
nit: maybe ENABLED_FOR_PROFILING, the name should convey that it is enabled for
pseudo-stack operations in general and not necessarily heap profiling.

https://codereview.chromium.org/1837013003/diff/120001/base/trace_event/trace...
base/trace_event/trace_log.h:67: ENABLED_FOR_NON_PROFILING_MODE =
ENABLED_FOR_RECORDING |
nit: maybe ENABLED_FOR_TRACING

[ssid, 2016-04-01 17:15:49]

ssid@chromium.org changed reviewers:
+ nduca@chromium.org

[ssid, 2016-04-01 17:15:50]

On 2016/04/01 14:15:02, Primiano Tucci wrote:
> On 2016/03/31 00:48:07, ssid wrote:
> > +oysteine I have removed the push and pop part to wait for support in other
> > projects (skia blink and v8). Looks like it isn't easy change in all
projects.
> > So, I cannot land all at the same time.
> > This CL just adds the skeleton without stack part. ptal, thanks.
> > 
> > +fmeawad I will add analysis about performance on android and update soon.
> 
> Hmm let's have some discussion before doing this.
> Regardless of the performance itself, this adds some extra maintenaince burden
> to the tracing codebase.
> If we really have to do this, let's brainstorm and figure out if we want a
> slightly more general version of this (for instance Fadi/Nat mentioned some
> sample profiling of events in the last meeting).
> The concrete problem I see here is that there is code around which caches the
> category_group_enabled pointer and checks for that without looking for flags.
> For instance I just reviewed https://codereview.chromium.org/1776673002/ last
> week with IIRC does that. Hopefully that should be self-cotained in
trace_event
> itself, but would be nice if there was a way to make it opt-out by default
> rather than must-be-aware-by-default.
> I think this kind of change requires a broader discussion (outside of
> codereview). Let's use the next weekly meeting (Performance Instrumentation &
> Modelling Weekly) to discuss ?

Yes that is correct. I have been trying to have this discussion with each of us
and never got a chance to sync up. The plan is to first add support for
profiling, specific to memory-infra to start with. This can be extended to be
used in for V8 profiling later. To enable this use case, I will also move the
pseudo stack out into a common file or make the AlocationContextTracker a more
generic Tracker which everyone can use. The main change would be to have a
lighter pseudo stack without involving vector operations, since it is needed for
perf measurements.
I am not sure about the naming part, what should the mode be called?

Do you mean to say, there should be a way to specify that do not record all
trace events by default. Why do you think that is useful for memory-infra case?
I am thinking for heap profiler there should always have all the information
possible and it doesn't make the trace that heavy with all events.
Yes, I think there will be use cases for others. But, we still do not know how
the v8 profiling will be started. At that point it will be an easy change for
them to just change TraceLog to enable whatever categories they need for
profiling.

Actually yesterday's meeting we discussed about this and decided lets have a
sync with the 5 of us about this. I tried to find a time when everyone is
available, couldn't find any. Probably next meeting is the best time, also
because oysteine@ had said primiano is a better person to review this CL, but he
is fine with such a change.