New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 832243 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
█ fsam...@chromium.org
vmi...@chromium.org
piman@chromium.org
dsinclair@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocked on:
issue 868476


Sign in to add a comment

Unify InProcessCommandBuffer and GLES2CommandBufferStub logic

Project Member Reported by kylec...@chromium.org, Apr 12 2018 
InProcessCommandBuffer was previously only used in tests and Android WebView. InProcessCommandBuffer was not always kept up to date with GLES2CommandBufferStub as a result.

With OOP-D the display compositor uses InProcessCommandBuffer. The code will run in many more cases and the divergent logic might be a problem.

Ideally both classes could be refactored so code can be shared between them. This is an non-trivial refactor so in the short term some of the divergent logic can just be fixed in InProcessCommandBuffer.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 13 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22b5641e249102f203176077163f1152d09a254f

commit 22b5641e249102f203176077163f1152d09a254f
Author: kylechar <kylechar@chromium.org>
Date: Fri Apr 13 15:58:21 2018

Improve InProcessCommandBuffer logic.

This CL implements logic from GLES2CommandBufferStub::Initialize() in
InProcessCommandBuffer::InitializeOnGpuThread(). Some differences are
not addressed and I've left TODOs instead.

Bug: 832243,  811979 
Change-Id: I1139a33f594280ea4c17b3a7223391e33bc15ad6
Reviewed-on: https://chromium-review.googlesource.com/1006038
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550630}
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/android_webview/browser/deferred_gpu_command_service.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/android_webview/browser/deferred_gpu_command_service.h
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/gpu_in_process_thread_service.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/gpu_in_process_thread_service.h
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/in_process_command_buffer.h
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22b5641e249102f203176077163f1152d09a254f

commit 22b5641e249102f203176077163f1152d09a254f
Author: kylechar <kylechar@chromium.org>
Date: Fri Apr 13 15:58:21 2018

Improve InProcessCommandBuffer logic.

This CL implements logic from GLES2CommandBufferStub::Initialize() in
InProcessCommandBuffer::InitializeOnGpuThread(). Some differences are
not addressed and I've left TODOs instead.

Bug: 832243,  811979 
Change-Id: I1139a33f594280ea4c17b3a7223391e33bc15ad6
Reviewed-on: https://chromium-review.googlesource.com/1006038
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550630}
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/android_webview/browser/deferred_gpu_command_service.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/android_webview/browser/deferred_gpu_command_service.h
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/gpu_in_process_thread_service.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/gpu_in_process_thread_service.h
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/22b5641e249102f203176077163f1152d09a254f/gpu/ipc/in_process_command_buffer.h
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e24532bb48094205dd63cd74377964d3206c2154

commit e24532bb48094205dd63cd74377964d3206c2154
Author: kylechar <kylechar@chromium.org>
Date: Tue Jul 03 15:44:36 2018

Devirtualize GLInProcessContext.

There is only one implementation so there doesn't need to be a virtual
base class. Also remove some unused functions.

Bug: 832243
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I581d47639448efa4a0812f604808a03587269320
Reviewed-on: https://chromium-review.googlesource.com/1119122
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572234}
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/android_webview/browser/aw_render_thread_context_provider.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/cc/test/test_in_process_context_provider.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/components/viz/common/gl_helper_benchmark.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/components/viz/common/gl_helper_unittest.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/components/viz/common/yuv_readback_unittest.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/components/viz/service/display_embedder/viz_process_context_provider.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/components/viz/service/display_embedder/viz_process_context_provider.h
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/gpu/ipc/client/gpu_in_process_context_tests.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/gpu/ipc/gl_in_process_context.cc
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/gpu/ipc/gl_in_process_context.h
[modify] https://crrev.com/e24532bb48094205dd63cd74377964d3206c2154/ui/compositor/test/in_process_context_provider.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2fdaa04465acfbd1feb5cb164029961ea2c4a18b

commit 2fdaa04465acfbd1feb5cb164029961ea2c4a18b
Author: kylechar <kylechar@chromium.org>
Date: Tue Jul 03 17:36:23 2018

Refactor InProcessCommandBuffer::Service.

There is a lot of complexity in InProcessCommandBuffer and the internal
Service class makes things more complex. This CL attempts to clean this
up a bit.

1. Move InProcessCommandBuffer::Service into it's own file and rename to
   GpuCommandService. Improve comments and modernize code slightly.
2. Make GpuCommandService a subclass of RefCountedThreadSafe, rather
   than all of the subclasses of GpuCommandService and then manually
   forwarding calls to AddRef() and Release(). This was originally done
   because GpuCommandService was a pure interface, however that is no
   longer the case.
3. Remove passing around const refs to scoped_refptrs. We can just pass
   the scoped_refptr by value in these cases.

Bug: 832243
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib6dc5212ba71e3645e2649b5db43f2c06fbb14e2
Reviewed-on: https://chromium-review.googlesource.com/1118618
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572272}
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/android_webview/browser/aw_render_thread_context_provider.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/android_webview/browser/aw_render_thread_context_provider.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/android_webview/browser/deferred_gpu_command_service.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/android_webview/browser/deferred_gpu_command_service.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/cc/test/pixel_test.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/cc/test/pixel_test.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/components/viz/service/display_embedder/gpu_display_provider.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/components/viz/service/display_embedder/gpu_display_provider.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/components/viz/service/display_embedder/viz_process_context_provider.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/components/viz/service/display_embedder/viz_process_context_provider.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/components/viz/service/main/viz_main_impl.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/BUILD.gn
[add] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/command_buffer_task_executor.cc
[add] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/command_buffer_task_executor.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/gl_in_process_context.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/gl_in_process_context.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/gpu_in_process_thread_service.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/gpu_in_process_thread_service.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/in_process_command_buffer.h
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/raster_in_process_context.cc
[modify] https://crrev.com/2fdaa04465acfbd1feb5cb164029961ea2c4a18b/gpu/ipc/raster_in_process_context.h

Comment 5 by ericrk@chromium.org, Jul 27

Blockedon: 868476
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 27 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73a7ca461b7538c8c178af601e9c587490daa26c

commit 73a7ca461b7538c8c178af601e9c587490daa26c
Author: kylechar <kylechar@chromium.org>
Date: Thu Sep 27 22:44:26 2018

Add trace events to InProcessCommandBuffer.

InProcessCommandBuffer is being used for the display compositor with
OOP-D. Add trace events for key functions, similar to the existing
traces in CommandBufferProxyImpl and [GLES2]CommandBufferStub, to help
diagnose performance issues.

Bug: 832243,  890013 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I34b962b48ad4eb10b112a9c979056993868637a4
Reviewed-on: https://chromium-review.googlesource.com/1250043
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594902}
[modify] https://crrev.com/73a7ca461b7538c8c178af601e9c587490daa26c/gpu/ipc/in_process_command_buffer.cc
►

Sign in to add a comment