Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(237)

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:
bbudge, vmiura, no sievers, piman
CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, penghuang+watch_chromium.org, sievers+watch_chromium.org, jbauman+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, kalyank, piman+watch_chromium.org, cc-bugs_chromium.org, James Su, danakj+watch_chromium.org, bsalomon_chromium
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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}

Patch Set 1 #

Patch Set 2 : Removed references to group markers in unit tests #

Total comments: 4

Patch Set 3 : Fixed syntax error, renamed trace category #

Patch Set 4 : Removed extra c_str() call #

Total comments: 5

Patch Set 5 : Try this again #

Patch Set 6 : Reverted some unnecessary files, fixed some issues #

Total comments: 4

Patch Set 7 : Made push/pop group marker a nop #

Patch Set 8 : rebase #

Patch Set 9 : Converted pepper trace calls #

Patch Set 10 : Make SwapBuffers use the GLES2Decoder GL_Category #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -28 lines) Patch
M cc/raster/scoped_gpu_raster.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M content/browser/android/in_process/context_provider_in_process.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/pepper/video_decoder_shim.cc View 1 2 3 4 5 6 7 8 4 chunks +4 lines, -5 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder.cc View 1 2 3 4 5 6 7 8 9 4 chunks +4 lines, -11 lines 0 comments Download
M gpu/command_buffer/service/gpu_tracer.h View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M gpu/command_buffer/service/gpu_tracer.cc View 1 2 3 4 5 6 1 chunk +3 lines, -4 lines 0 comments Download
M gpu/command_buffer/service/gpu_tracer_unittest.cc View 1 2 3 4 5 6 2 chunks +3 lines, -3 lines 0 comments Download
M ppapi/shared_impl/ppb_graphics_3d_shared.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 29 (4 generated)
David Yen
5 years, 7 months ago (2015-05-12 17:24:27 UTC) #1
David Yen
+piman to review cc/resources/scoped_gpu_raster.cc
5 years, 7 months ago (2015-05-12 17:26:01 UTC) #3
no sievers
lgtm, thanks! https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu_raster.cc File cc/resources/scoped_gpu_raster.cc (right): https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu_raster.cc#newcode31 cc/resources/scoped_gpu_raster.cc:31: gl->TraceBeginCHROMIUM("GroupMarker", "GpuRasterization"); Why "GroupMarker"? https://codereview.chromium.org/1132283003/diff/20001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc ...
5 years, 7 months ago (2015-05-12 17:51:44 UTC) #4
David Yen
https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu_raster.cc File cc/resources/scoped_gpu_raster.cc (right): https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu_raster.cc#newcode31 cc/resources/scoped_gpu_raster.cc:31: gl->TraceBeginCHROMIUM("GroupMarker", "GpuRasterization"); On 2015/05/12 17:51:44, sievers wrote: > Why ...
5 years, 7 months ago (2015-05-12 17:56:13 UTC) #5
no sievers
On 2015/05/12 17:56:13, David Yen wrote: > https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu_raster.cc > File cc/resources/scoped_gpu_raster.cc (right): > > https://codereview.chromium.org/1132283003/diff/20001/cc/resources/scoped_gpu_raster.cc#newcode31 ...
5 years, 7 months ago (2015-05-12 18:14:07 UTC) #6
David Yen
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
piman
lgtm
5 years, 7 months ago (2015-05-12 19:22:29 UTC) #8
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
David Yen
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
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
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
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
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
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
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
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
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
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
David Yen
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"); On 2015/06/25 00:41:57, vmiura wrote: > Can ...
5 years, 6 months ago (2015-06-25 17:40:35 UTC) #20
David Yen
+bbudge for pepper changes.
5 years, 5 months ago (2015-07-21 18:30:23 UTC) #22
bbudge
content/renderer/pepper lgtm
5 years, 5 months ago (2015-07-21 23:01:24 UTC) #23
vmiura
lgtm
5 years, 5 months ago (2015-07-21 23:31:09 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1132283003/180001
5 years, 5 months ago (2015-07-21 23:32:58 UTC) #27
commit-bot: I haz the power
Committed patchset #10 (id:180001)
5 years, 5 months ago (2015-07-21 23:40:23 UTC) #28
commit-bot: I haz the power
5 years, 5 months ago (2015-07-21 23:42:25 UTC) #29
Message was sent while issue was closed.
Patchset 10 (id:??) landed as
https://crrev.com/2b089f9f3c4cdeb163aa81b35911ba3d1c53ed9a
Cr-Commit-Position: refs/heads/master@{#339781}

Powered by Google App Engine
This is Rietveld 408576698