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

Issue 1733703002: content: Allow multiple raster threads on Android. (Closed)

Created:
4 years, 10 months ago by reveman
Modified:
3 years, 9 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, piman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

content: Allow multiple raster threads on Android. This allows the same amount of foreground raster threads on Android as on other platforms. BUG=504515

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -7 lines) Patch
M content/browser/gpu/compositor_util.cc View 1 chunk +0 lines, -7 lines 2 comments Download

Messages

Total messages: 6 (3 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1733703002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1733703002/1
4 years, 10 months ago (2016-02-25 04:57:14 UTC) #2
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 10 months ago (2016-02-25 06:05:15 UTC) #4
aelias_OOO_until_Jul13
4 years, 10 months ago (2016-02-26 03:12:03 UTC) #6
https://codereview.chromium.org/1733703002/diff/1/content/browser/gpu/composi...
File content/browser/gpu/compositor_util.cc (left):

https://codereview.chromium.org/1733703002/diff/1/content/browser/gpu/composi...
content/browser/gpu/compositor_util.cc:189: num_raster_threads = 1;
vmpstr@ and ericrk@ mentioned today they'd also prefer to hold off on increasing
the number of foreground threads on Android until their image decode work is
further along.  I'm open to revisiting this later when we have more confidence
in our task policy.

As long as the code that feeds tasks into the raster system is entirely sane, I
have no real grounds to object on raising the number of threads here, I'm just
not sure we're at that point quite yet.

https://codereview.chromium.org/1733703002/diff/1/content/browser/gpu/composi...
File content/browser/gpu/compositor_util.cc (right):

https://codereview.chromium.org/1733703002/diff/1/content/browser/gpu/composi...
content/browser/gpu/compositor_util.cc:180: num_processors =
std::min(num_processors, 4);
How about moving this to the base::SysInfo::NumberOfProcessors() implementation?
 I think it's reasonable to define that to refer to the number of big cores. 
Every use case of the core count information I can think of is similar to this
one, to balance workloads and avoid oversaturation.

Ideally we would also use a system API that allows us to distinguish the cores,
instead of capping to the magic number 4 (but I'm not sure if anybody bothered
to provide that yet).

Powered by Google App Engine
This is Rietveld 408576698