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

Issue 691703 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

GpuControlList is very inefficient and blocks startup

Project Member Reported by brettw@chromium.org, Feb 13 2017

Issue description

Blocking startup GpuControlList::LoadList is called twice, once with gpu_blacklist_json (26KB) and once with gpu_driver_bug_list_json (42KB).

First these strings are copied into std::strings for 114KB of allocation and memcpy. Then these strings are parsed as JSON, taking 5 and 6 ms respectively, for a total of 11ms blocked startup time on my Z840.

Then this JSON list is converted to a vector of refcounted objects. Then that vector is filtered, discarding all entries that don't match the current OS.

On my Windows 10 computer, after all of this, my blacklist array is 30 entries and my bug list array is 0 entries.

All of this is REALLY inefficient. I'm seeing LoadList call malloc 23884 times and allocate 998568 bytes. 1214 allocations accounting for 78094 bytes are persistent.

----

I think the design should be stored in static data as structs to require no parsing or allocation, and they should already be filtered by the current OS. The features and bug lists should use constants instead of strings. This initialization of the blacklist should be able to be free.

struct BlackListEntry {  // POD only.
  int id;
  const char* name;
  const char* description;
  ...
  
};
BlackListEntry kSoftwareRenderingList[84] = {
  { 3,
    "GL driver is software rendered. GPU acceleration is disabled",
    ...
  },
  ...
}
 

Comment 1 by brettw@chromium.org, Feb 13 2017

Summary: GpuControlList is very inefficient and blocks startup (was: GpuControl is list is very inefficient and blocks startup)

Comment 2 by zmo@chromium.org, Feb 13 2017

Cc: kbr@chromium.org vmi...@chromium.org cwallez@chromium.org ericrk@chromium.org
It is on the our OKR to move these to GPU process startup
Also after https://bugs.chromium.org/p/angleproject/issues/detail?id=1874 the workarounds detection code in ANGLE will become pure code and will do minimal allocations. Chrome will stop using the gpu_driver_bug_list_json on platforms where it uses ANGLE exclusively.

Comment 4 by brettw@chromium.org, Feb 13 2017

It should still be rewritten not to parse JSON. GPU startup will still block Chrome and so this should be as fast as possible.

Comment 5 by zmo@chromium.org, Feb 13 2017

That's what cwallez said in #3.  We will switch to use ANGLE lib when it's ready, even before Chrome switch to purely ANGLE backend.
zmo: in our plan, workarounds detection moves in ANGLE only when MANGLE is shipped on a platform though. So the old JSON parsing code might stay for a while on platforms other than Windows and Linux. The MANGLE plan doesn't touch software_rendering either.

Comment 7 by zmo@chromium.org, Feb 13 2017

Can you expose the functionality for Chrome to call into ANGLE's utility function? So we can just get rid of the Chrome side code.

Comment 8 by zmo@chromium.org, Feb 13 2017

All in all, blacklist needs the same GPU detection code as workarounds.  So it doesn't make sense for Chrome to have the detection code for blacklist, and for ANGLE to have the detection code for workarounds.  If ANGLE can expose the functions, then we just need one copy of code moving forward.

Comment 9 by brettw@chromium.org, Feb 13 2017

I think fixing this will only take a day or two so if it will be a while, I think we should definitely do it.

Comment 10 by zmo@chromium.org, Feb 13 2017

Sure. I can put it on my TODO list.

Comment 11 by zmo@chromium.org, Feb 13 2017

Originally this json file is designed to be able to push to chrome any time, independent of Chrome update.  Later it turned out to be too much traffic, and GPU drivers become better, so I don't think this need will ever arise again.  Definitely brettw's suggestion is more efficient.
Labels: EM
I had a look at this as well on low-end device memory trace and it seems to be using 53.1KiB of allocations under content::GpuDataManagerImplPrivate::InitializeImpl() on an Android One device. Any memory savings on critical paths are really important for low-end use cases, so if there's a way to make this use less memory and be more performant to boot, that would be great to do.

Comment 13 by zmo@chromium.org, Feb 22 2017

Taking a closer look, the task is not as simple.

This data structure will need to be a struct with fields of
1) arrays of integers with varying size (device_id)
2) arrays of structs with varying size (exceptions)

We (kbr, kainino, zmo) are still looking at how to map the json file to a data structure that is constructed at compile time and yet readable/maintainable.

Comment 14 by zmo@chromium.org, Feb 23 2017

Status: Started (was: Assigned)
So here is a potential solution:

struct Condition {
  int vendor_id;
  std::vector<int> device_ids;
};

struct Entry {
  int ID;
  const char* desc;
  Condition condition;
  std::vector<Condition> exceptions;
};

Then we declare the data as:

static const Entry entries[] = {
  { 1, "NVidia is buggy in ChromeOS", {0x10de, {}},
    { {0x10de, {0x167}} }}
  };
};

brettw, does this look acceptable to you or do you have better suggestions?

Comment 15 by zmo@chromium.org, Feb 23 2017

Per offline discussion, we are going to use a python script to map json data into a data structure without std::vector, so we could apply constexpr.
Labels: -EM LowMemory
Any updates on this?

Comment 18 by zmo@chromium.org, Mar 17 2017

Still working on this.  Absurd amount of testing code change.

Comment 19 by zmo@chromium.org, Mar 17 2017

Here is working progress, but not ready for review yet:

https://codereview.chromium.org/2756793003/
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 1 2017

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

commit 2602bcf4e08d2cb554520b42246f68ac22426bc8
Author: zmo <zmo@chromium.org>
Date: Sat Apr 01 00:12:03 2017

Move GPU blacklist and driver bug workaround list from json to data struct.

BUG= 691703 
TEST=gpu_unittests
R=kbr@chromium.org,brettw@chromium.org,piman@chromium.org
TBR=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

Review-Url: https://codereview.chromium.org/2756793003
Cr-Commit-Position: refs/heads/master@{#461282}

[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/chrome/browser/extensions/extension_service_unittest.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/chrome/browser/extensions/requirements_checker_browsertest.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/compositor_util.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_impl.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_impl.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_impl_private.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_impl_private.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_impl_private_unittest.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_testing.json
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_testing_arrays_and_structs_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_testing_autogen.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_testing_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_testing_entry_enums_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/gpu/gpu_data_manager_testing_exceptions_autogen.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/browser/resources/gpu/info_view.js
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/public/browser/gpu_data_manager.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/content/test/BUILD.gn
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/BUILD.gn
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/BUILD.gn
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_blacklist.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_blacklist.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_blacklist_unittest.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_entry_unittest.cc
[delete] https://crrev.com/1f2f12faf6b42983525f917bd5fabc2f02bdf8a3/gpu/config/gpu_control_list_jsons.h
[delete] https://crrev.com/1f2f12faf6b42983525f917bd5fabc2f02bdf8a3/gpu/config/gpu_control_list_number_info_unittest.cc
[delete] https://crrev.com/1f2f12faf6b42983525f917bd5fabc2f02bdf8a3/gpu/config/gpu_control_list_os_info_unittest.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing.json
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing_arrays_and_structs_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing_autogen.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing_data.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing_entry_enums_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_testing_exceptions_autogen.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_unittest.cc
[delete] https://crrev.com/1f2f12faf6b42983525f917bd5fabc2f02bdf8a3/gpu/config/gpu_control_list_version_info_unittest.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_control_list_version_unittest.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list.README
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list.h
[rename] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list.json
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list_arrays_and_structs_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list_autogen.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list_exceptions_autogen.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_list_unittest.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_driver_bug_workaround_type.h
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_util.cc
[modify] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/gpu_util_unittest.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/process_json.py
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/software_rendering_list.README
[rename] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/software_rendering_list.json
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/software_rendering_list_arrays_and_structs_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/software_rendering_list_autogen.cc
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/software_rendering_list_autogen.h
[add] https://crrev.com/2602bcf4e08d2cb554520b42246f68ac22426bc8/gpu/config/software_rendering_list_exceptions_autogen.h

Cc: aelias@chromium.org
It's easy to forget to run gpu/config/process_json.py and I'm especially worried this will lead to mistakes and additional conflicts when cherry-picking blacklist entries to release branches.  Please make it part of the build process before closing this issue.
Project Member

Comment 22 by bugdroid1@chromium.org, Apr 13 2017

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

commit ede160c50d85e5fd546ec8134714b35c71c0e2f1
Author: zmo <zmo@chromium.org>
Date: Thu Apr 13 01:46:38 2017

Move gpu jason file data generation to build time.

BUG= 691703 
TEST=chrome
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

Review-Url: https://codereview.chromium.org/2812003006
Cr-Commit-Position: refs/heads/master@{#464235}

[modify] https://crrev.com/ede160c50d85e5fd546ec8134714b35c71c0e2f1/gpu/config/BUILD.gn
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/gpu_driver_bug_list_arrays_and_structs_autogen.h
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/gpu_driver_bug_list_autogen.cc
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/gpu_driver_bug_list_autogen.h
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/gpu_driver_bug_list_exceptions_autogen.h
[modify] https://crrev.com/ede160c50d85e5fd546ec8134714b35c71c0e2f1/gpu/config/process_json.py
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/software_rendering_list_arrays_and_structs_autogen.h
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/software_rendering_list_autogen.cc
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/software_rendering_list_autogen.h
[delete] https://crrev.com/7d5d3c6aca8a1155fc9449a7b6669d4fb2724328/gpu/config/software_rendering_list_exceptions_autogen.h

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 15 2017

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

commit a3071ce2bd0a82923d33a73a5c0c46408b837a14
Author: zmo <zmo@chromium.org>
Date: Sat Apr 15 00:50:17 2017

Only generate GPU json data entries related to a given platform at build time.

This is for SoftwareRenderingList and GpuDriverBugList.

BUG= 691703 
TEST=chrome
R=kbr@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

Review-Url: https://codereview.chromium.org/2823503002
Cr-Commit-Position: refs/heads/master@{#464840}

[modify] https://crrev.com/a3071ce2bd0a82923d33a73a5c0c46408b837a14/gpu/config/BUILD.gn
[modify] https://crrev.com/a3071ce2bd0a82923d33a73a5c0c46408b837a14/gpu/config/gpu_driver_bug_list.json
[modify] https://crrev.com/a3071ce2bd0a82923d33a73a5c0c46408b837a14/gpu/config/gpu_driver_bug_list_unittest.cc
[modify] https://crrev.com/a3071ce2bd0a82923d33a73a5c0c46408b837a14/gpu/config/process_json.py
[modify] https://crrev.com/a3071ce2bd0a82923d33a73a5c0c46408b837a14/gpu/config/software_rendering_list.json

Comment 24 by zmo@chromium.org, Apr 15 2017

Status: Fixed (was: Started)

Comment 25 by kbr@chromium.org, Apr 17 2017

Great work Mo! This not only makes the blacklist faster but also makes it easier to maintain.

Project Member

Comment 26 by bugdroid1@chromium.org, Apr 18 2017

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

commit 604a0257a13c25d59165dfac380be1b60c6c9686
Author: qiankun.miao <qiankun.miao@intel.com>
Date: Tue Apr 18 17:23:02 2017

Update some autogen files

These files are generated by running gpu/config/process_json.py.
These are missing in the following CL:
https://codereview.chromium.org/2823503002.

BUG= 691703 
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

Review-Url: https://codereview.chromium.org/2828463002
Cr-Commit-Position: refs/heads/master@{#465270}

[modify] https://crrev.com/604a0257a13c25d59165dfac380be1b60c6c9686/content/browser/gpu/gpu_data_manager_testing_autogen.cc
[modify] https://crrev.com/604a0257a13c25d59165dfac380be1b60c6c9686/gpu/config/gpu_control_list_testing_autogen.cc

Sign in to add a comment