Merge Group Markers into Chromium Traces.
In a previous CL top level group markers were converted to using
chromium traces:
https://codereview.chromium.org/780653007/
The initial thought was that we would standardize on a single trace
type. This is a transitional step which moves the debug marker
manager to be set in the chromium traces, and also makes the group
marker calls mimic the chromium trace calls. Eventually the group
marker calls should be deprecated.
R=sievers@chromium.org, vmiura@chromium.org
BUG=242999, 503166
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/2b089f9f3c4cdeb163aa81b35911ba3d1c53ed9a
Cr-Commit-Position: refs/heads/master@{#339781}
On 2015/05/12 18:14:07, sievers wrote: > On 2015/05/12 17:56:13, David Yen wrote: > > > ...
5 years, 7 months ago
(2015-05-12 18:20:08 UTC)
#7
On 2015/05/12 18:14:07, sievers wrote:
> On 2015/05/12 17:56:13, David Yen wrote:
> >
>
https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu...
> > File cc/resources/scoped_gpu_raster.cc (right):
> >
> >
>
https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu...
> > cc/resources/scoped_gpu_raster.cc:31: gl->TraceBeginCHROMIUM("GroupMarker",
> > "GpuRasterization");
> > On 2015/05/12 17:51:44, sievers wrote:
> > > Why "GroupMarker"?
> >
> > I couldn't really think of a good trace category, maybe just
ScopedGpuRaster?
>
> Maybe just "gpu"? Seems like one would expect to toggle it based on one of the
> existing standard categories.
Sorry I should have been more specific, these are not the trace categories in
the trace-viewer. The trace categories for these are always
"disabled-by-default-gpu.service" for CPU side traces and
"disabled-by-default-gpu-device" for GPU side traces. These categories are the
GL_Categories used to categorize the different traces and cannot be toggled by
default (outside of the trace calls you can still check if the category is
enabled before deciding whether you want to submit a trace call or not, but in
this instance it is not toggled).
For example, in context_provider_in_process.cc is where we have "gpu_toplevel"
traces.
piman
lgtm
5 years, 7 months ago
(2015-05-12 19:22:29 UTC)
#8
lgtm
vmiura
https://codereview.chromium.org/1132283003/diff/60001/cc/resources/scoped_gpu_raster.cc File cc/resources/scoped_gpu_raster.cc (right): https://codereview.chromium.org/1132283003/diff/60001/cc/resources/scoped_gpu_raster.cc#newcode30 cc/resources/scoped_gpu_raster.cc:30: gl->PushGroupMarkerEXT(0, "GpuRasterization"); Markers and traces each add some overhead, ...
5 years, 7 months ago
(2015-05-12 21:47:42 UTC)
#9
Adding Brian to see what he thinks about skia side changes. Do other users utilize ...
5 years, 7 months ago
(2015-05-12 21:52:18 UTC)
#10
Adding Brian to see what he thinks about skia side changes. Do other users
utilize push/pop group markers? Would it be okay to just switch them to
start/stop chromium traces?
bsalomon
On 2015/05/12 21:52:18, David Yen wrote: > Adding Brian to see what he thinks about ...
5 years, 7 months ago
(2015-05-13 13:34:30 UTC)
#11
On 2015/05/12 21:52:18, David Yen wrote:
> Adding Brian to see what he thinks about skia side changes. Do other users
> utilize push/pop group markers? Would it be okay to just switch them to
> start/stop chromium traces?
Passing the buck to egdaniel@ who added the Skis trace marker stuff. My
understanding is that we probably would do better with annotated trace markers
than a marker stack.
egdaniel1
On 2015/05/13 13:34:30, bsalomon wrote: > On 2015/05/12 21:52:18, David Yen wrote: > > Adding ...
5 years, 7 months ago
(2015-05-13 13:46:41 UTC)
#12
On 2015/05/13 13:34:30, bsalomon wrote:
> On 2015/05/12 21:52:18, David Yen wrote:
> > Adding Brian to see what he thinks about skia side changes. Do other users
> > utilize push/pop group markers? Would it be okay to just switch them to
> > start/stop chromium traces?
>
> Passing the buck to egdaniel@ who added the Skis trace marker stuff. My
> understanding is that we probably would do better with annotated trace markers
> than a marker stack.
Yeah I think the ideal model for skia would be to have a start/end system rather
than push/pop. We would just need something exposed as a GL extension to
correctly use it, and then we can figure out what needs to be updated on our
side to use the new model.
vmiura
On 2015/05/13 13:46:41, egdaniel1 wrote: > On 2015/05/13 13:34:30, bsalomon wrote: > > On 2015/05/12 ...
5 years, 7 months ago
(2015-05-13 16:59:11 UTC)
#13
On 2015/05/13 13:46:41, egdaniel1 wrote:
> On 2015/05/13 13:34:30, bsalomon wrote:
> > On 2015/05/12 21:52:18, David Yen wrote:
> > > Adding Brian to see what he thinks about skia side changes. Do other users
> > > utilize push/pop group markers? Would it be okay to just switch them to
> > > start/stop chromium traces?
> >
> > Passing the buck to egdaniel@ who added the Skis trace marker stuff. My
> > understanding is that we probably would do better with annotated trace
markers
> > than a marker stack.
>
>
> Yeah I think the ideal model for skia would be to have a start/end system
rather
> thant push/pop. We would just need something exposed as a GL extension to
> correctly use it, and then we can figure out what needs to be updated on our
> side to use the new model.
Yeah we the extension TraceBeginCHROMIUM(category, name) / TraceEndCHROMIUM().
Tracing for the 'category' can then be turned on & off in 'about:tracing'. In
the future I think we could extend this to pass extra context as JSON data which
we can view in 'about:tracing'.
So it sounds to me that it'd be OK to deprecate use of the Push/Pop APIs in
favor of TraceBegin/EndCHROMIUM.
David Yen
On 2015/05/13 16:59:11, vmiura wrote: > On 2015/05/13 13:46:41, egdaniel1 wrote: > > On 2015/05/13 ...
5 years, 7 months ago
(2015-05-13 17:05:29 UTC)
#14
On 2015/05/13 16:59:11, vmiura wrote:
> On 2015/05/13 13:46:41, egdaniel1 wrote:
> > On 2015/05/13 13:34:30, bsalomon wrote:
> > > On 2015/05/12 21:52:18, David Yen wrote:
> > > > Adding Brian to see what he thinks about skia side changes. Do other
users
> > > > utilize push/pop group markers? Would it be okay to just switch them to
> > > > start/stop chromium traces?
> > >
> > > Passing the buck to egdaniel@ who added the Skis trace marker stuff. My
> > > understanding is that we probably would do better with annotated trace
> markers
> > > than a marker stack.
> >
> >
> > Yeah I think the ideal model for skia would be to have a start/end system
> rather
> > thant push/pop. We would just need something exposed as a GL extension to
> > correctly use it, and then we can figure out what needs to be updated on our
> > side to use the new model.
>
> Yeah we the extension TraceBeginCHROMIUM(category, name) / TraceEndCHROMIUM().
> Tracing for the 'category' can then be turned on & off in 'about:tracing'. In
> the future I think we could extend this to pass extra context as JSON data
which
> we can view in 'about:tracing'.
>
> So it sounds to me that it'd be OK to deprecate use of the Push/Pop APIs in
> favor of TraceBegin/EndCHROMIUM.
Just to clarify, even though the API is Begin/End it still acts as a stack, we
do not currently support overlapped traces.
no sievers
Note that the debug and event markers are a real extension that we implemented (https://www.opengl.org/registry/specs/EXT/EXT_debug_marker.txt). ...
5 years, 7 months ago
(2015-05-13 17:16:40 UTC)
#15
Note that the debug and event markers are a real extension that we implemented
(https://www.opengl.org/registry/specs/EXT/EXT_debug_marker.txt).
On the other hand if nobody wants to use it, we might as well remove it.
The one thing that it's been used for is naming GL contexts for logging from the
decoder service. Of course that can be done much simpler, we can simply pass the
debug name string that the ContextProvider already has to the service and use
that.
no sievers
On 2015/05/13 17:16:40, sievers wrote: > Note that the debug and event markers are a ...
5 years, 7 months ago
(2015-05-13 17:17:28 UTC)
#16
On 2015/05/13 17:16:40, sievers wrote:
> Note that the debug and event markers are a real extension that we implemented
> (https://www.opengl.org/registry/specs/EXT/EXT_debug_marker.txt).
>
> On the other hand if nobody wants to use it, we might as well remove it.
>
> The one thing that it's been used for is naming GL contexts for logging from
the
> decoder service. Of course that can be done much simpler, we can simply pass
the
> debug name string that the ContextProvider already has to the service and use
> that.
at context creation time, I meant to say
David Yen
On 2015/05/13 17:17:28, sievers wrote: > On 2015/05/13 17:16:40, sievers wrote: > > Note that ...
5 years, 7 months ago
(2015-05-13 17:31:15 UTC)
#17
On 2015/05/13 17:17:28, sievers wrote:
> On 2015/05/13 17:16:40, sievers wrote:
> > Note that the debug and event markers are a real extension that we
implemented
> > (https://www.opengl.org/registry/specs/EXT/EXT_debug_marker.txt).
> >
> > On the other hand if nobody wants to use it, we might as well remove it.
> >
> > The one thing that it's been used for is naming GL contexts for logging from
> the
> > decoder service. Of course that can be done much simpler, we can simply pass
> the
> > debug name string that the ContextProvider already has to the service and
use
> > that.
>
> at context creation time, I meant to say
Note, once we switch over group markers to chromium traces, the logger can
simply query the top level chromium trace just as it does with group markers
now.
If we want to support interactions with other tools, we can also make
StartTrace() call PushGroupMarker() and StopTrace() do a PopGroupMarker()
David Yen
On 2015/05/13 17:31:15, David Yen wrote: > On 2015/05/13 17:17:28, sievers wrote: > > On ...
5 years, 6 months ago
(2015-06-22 17:48:20 UTC)
#18
On 2015/05/13 17:31:15, David Yen wrote:
> On 2015/05/13 17:17:28, sievers wrote:
> > On 2015/05/13 17:16:40, sievers wrote:
> > > Note that the debug and event markers are a real extension that we
> implemented
> > > (https://www.opengl.org/registry/specs/EXT/EXT_debug_marker.txt).
> > >
> > > On the other hand if nobody wants to use it, we might as well remove it.
> > >
> > > The one thing that it's been used for is naming GL contexts for logging
from
> > the
> > > decoder service. Of course that can be done much simpler, we can simply
pass
> > the
> > > debug name string that the ContextProvider already has to the service and
> use
> > > that.
> >
> > at context creation time, I meant to say
>
> Note, once we switch over group markers to chromium traces, the logger can
> simply query the top level chromium trace just as it does with group markers
> now.
>
> If we want to support interactions with other tools, we can also make
> StartTrace() call PushGroupMarker() and StopTrace() do a PopGroupMarker()
Lets try this again, I've now changed it so Push/Pop Group Markers simply do the
same thing that Trace Begin/End CHROMIUM does. Trace Begin/End CHROMIUM also now
pushes/pops the debug marker so the logs will display correctly again.
vmiura
https://codereview.chromium.org/1132283003/diff/100001/cc/raster/scoped_gpu_raster.cc File cc/raster/scoped_gpu_raster.cc (right): https://codereview.chromium.org/1132283003/diff/100001/cc/raster/scoped_gpu_raster.cc#newcode30 cc/raster/scoped_gpu_raster.cc:30: gl->TraceBeginCHROMIUM("ScopedGpuRaster", "GpuRasterization"); Can we put this in "gpu_toplevel" category ...
5 years, 6 months ago
(2015-06-25 00:41:57 UTC)
#19
Issue 1132283003: Merge Group Markers into Chromium Traces.
(Closed)
Created 5 years, 7 months ago by David Yen
Modified 5 years, 5 months ago
Reviewers: no sievers, vmiura, piman, bbudge
Base URL: https://chromium.googlesource.com/chromium/src.git@master
Comments: 13