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

Issue 774157 link

Starred by 1 user
Status: Fixed
Owner:
zmo@chromium.org
Closed: Nov 2017
Cc:
█ kbr@chromium.org
vmi...@chromium.org
piman@chromium.org
dsinclair@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 744658



Sign in to add a comment

Encode GpuPreference and pass it from browser to GPU as a single commandline switch

Project Member Reported by zmo@chromium.org, Oct 12 2017 
Right now GpuPrefence isn't available at GPU process launch time. It's only sent over when IPC is established.

Because of this, we have to send bits of info in GpuPreference as individual commandline switch, for example, kDisableAcceleratedVideoDecode, because they are needed before GpuPrefence is sent over.

Such duplicated mechanisms are not good for the long term - they add maintenance cost and are bug prone (changes affect one bit but forget to adjust the other representation).

Also, we should reduce the number of commandline switches sent from one process to another.

Therefore, per discussion with piman, the idea is to encode GpuPreference into one commandline switch (maybe kGpuPreference) and use commandline switch mechanism to sent GpuPreference from browser to GPU process.

Comment 1 by zmo@chromium.org, Oct 12 2017

Blocking: 744658
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 17 2017 

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

commit aad85e5301411557a4b2d9edaec2758773a49307
Author: Zhenyao Mo <zmo@chromium.org>
Date: Tue Oct 17 04:04:45 2017

Add functions to encode GpuPreferences to string and decode it from string.

BUG= 774157 
TEST=gpu_unittests
R=piman@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib4a410443d192cbda369a64d4b59cefb23444f9c
Reviewed-on: https://chromium-review.googlesource.com/717458
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509277}
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/content/browser/BUILD.gn
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/content/browser/gpu/compositor_util.cc
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/BUILD.gn
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/command_buffer/service/gpu_preferences.cc
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/config/gpu_switches.cc
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/config/gpu_switches.h
[add] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/gpu_util_export.h
[modify] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/ipc/common/BUILD.gn
[add] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/ipc/common/gpu_preferences_util.cc
[add] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/ipc/common/gpu_preferences_util.h
[add] https://crrev.com/aad85e5301411557a4b2d9edaec2758773a49307/gpu/ipc/common/gpu_preferences_util_unittest.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 18 2017 

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

commit 83b895e9136947f824c9e7b5f728d193851730b6
Author: Zhenyao Mo <zmo@chromium.org>
Date: Wed Oct 18 18:50:54 2017

Wire GpuPreferences from commandline switch instead of from first IPC.

BUG= 774157 
TEST=bots
R=piman@chromium.org,dcheng@chromium.org,sadrul@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I826dc154b4fb12fbd5642a4b14c085d66e8b507e
Reviewed-on: https://chromium-review.googlesource.com/723847
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509822}
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/components/viz/service/gl/gpu_service_impl.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/components/viz/service/gl/gpu_service_impl.h
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/components/viz/service/gl/gpu_service_impl_unittest.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/content/browser/gpu/gpu_main_thread_factory.h
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/content/gpu/BUILD.gn
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/content/gpu/gpu_main.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/content/gpu/in_process_gpu_thread.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/content/gpu/in_process_gpu_thread.h
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/gpu/ipc/service/gpu_init.h
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/services/ui/gpu/BUILD.gn
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/services/ui/gpu/gpu_main.h
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/services/ui/gpu/interfaces/gpu_main.mojom
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/services/ui/ws/gpu_host.cc
[modify] https://crrev.com/83b895e9136947f824c9e7b5f728d193851730b6/services/ui/ws/gpu_host_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 20 2017 

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

commit e9187a86424d730426eb560a5658ba385658a8c5
Author: Zhenyao Mo <zmo@chromium.org>
Date: Fri Oct 20 04:26:33 2017

Remove two video encode/decode commandline switches from GPU process.

kDisableAcceleratedVideoDecode and kDisableVaapiAcceleratedVideoEncode are
passed through GpuPreferences. Currently only sandbox still uses the two
switches in GPU process, so in this CL we wires them through function params.

kDisableVaapiAcceleratedVideoEncode isn't used in Renderer process at all, so
just remove it.

BUG= 774157 ,774658
TEST=bots
R=piman@chromium.org,tsepez@chromium.org

kDisableVaapiAcceleratedVideoEncode

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie374633b3291e0ca5b59c5b4dfec612b0c2d4e47
Reviewed-on: https://chromium-review.googlesource.com/728954
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510333}
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.h
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/gpu/gpu_main.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/gpu/gpu_sandbox_hook_linux.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/content/gpu/gpu_sandbox_hook_linux.h
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/gpu/config/gpu_util.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/gpu/ipc/service/gpu_init.h
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/e9187a86424d730426eb560a5658ba385658a8c5/services/ui/gpu/gpu_main.h
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 21 2017 

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

commit e0de55ecc474f015f48fe07a8e20b3f245726761
Author: Zhenyao Mo <zmo@chromium.org>
Date: Sat Oct 21 03:17:11 2017

Move a few switches from GPU process commandline to GpuPreferences

BUG= 774157 
TEST=bots
R=piman@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie41670a0ec8f7eb70535d613ae70dd838bd5edbb
Reviewed-on: https://chromium-review.googlesource.com/731064
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510650}
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/content/gpu/gpu_main.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/content/public/browser/gpu_utils.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/content/public/common/content_switches.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/content/public/common/content_switches.h
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/gpu/command_buffer/service/gpu_preferences.h
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/gpu/ipc/service/switches.cc
[modify] https://crrev.com/e0de55ecc474f015f48fe07a8e20b3f245726761/gpu/ipc/service/switches.h
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 24 2017 

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

commit 19cc1016db8ed8634183511e2eb51364fbfbade7
Author: Ryo Hashimoto <hashimoto@chromium.org>
Date: Tue Oct 24 09:52:32 2017

Revert "Move a few switches from GPU process commandline to GpuPreferences"

This reverts commit e0de55ecc474f015f48fe07a8e20b3f245726761.

Reason for revert: After this change, Chrome doesn't correctly start on some ARM Chromebooks (e.g. kevin, veyron_minnie, peach_pit).

BUG=777250

Original change's description:
> Move a few switches from GPU process commandline to GpuPreferences
> 
> BUG= 774157 
> TEST=bots
> R=​piman@chromium.org
> 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: Ie41670a0ec8f7eb70535d613ae70dd838bd5edbb
> Reviewed-on: https://chromium-review.googlesource.com/731064
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Commit-Queue: Zhenyao Mo <zmo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510650}

TBR=zmo@chromium.org,piman@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  774157 
Change-Id: I48eb066489d1753c842c446484efb297c6a43312
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/735459
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511072}
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/content/gpu/gpu_main.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/content/public/browser/gpu_utils.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/content/public/common/content_switches.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/content/public/common/content_switches.h
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/gpu/command_buffer/service/gpu_preferences.h
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/gpu/ipc/service/switches.cc
[modify] https://crrev.com/19cc1016db8ed8634183511e2eb51364fbfbade7/gpu/ipc/service/switches.h
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 25 2017 

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

commit 910beb8bacc3c31e0f3f9e66ffa3b8328e30902d
Author: Zhenyao Mo <zmo@chromium.org>
Date: Wed Oct 25 03:23:00 2017

[Reland] Move a few switches from GPU process commandline to GpuPreferences

Original review: https://chromium-review.googlesource.com/731064

BUG= 774157 
TEST=bots
R=piman@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Iec41e916f4be93f8edef2b5dc006cd8fc6b8c201
Reviewed-on: https://chromium-review.googlesource.com/735832
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511347}
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/content/gpu/gpu_main.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/content/public/browser/gpu_utils.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/content/public/common/content_switches.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/content/public/common/content_switches.h
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/command_buffer/service/gpu_preferences.h
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/ipc/common/gpu_preferences.mojom
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/ipc/common/gpu_preferences_struct_traits.h
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/ipc/common/gpu_preferences_util_unittest.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/ipc/service/switches.cc
[modify] https://crrev.com/910beb8bacc3c31e0f3f9e66ffa3b8328e30902d/gpu/ipc/service/switches.h

Comment 8 by zmo@chromium.org, Nov 14 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment