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

Issue 22831041: cc: Removed low priority bin from tile management. (Closed)

Created:
7 years, 4 months ago by vmpstr
Modified:
7 years, 3 months ago
Reviewers:
reveman
CC:
chromium-reviews, cc-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

cc: Removed low priority bin from tile management. This patch removes low priority bin from tile management, increasing the perftests by about 5% (since the bin comparator doesn't have to check this anymore). TileManager unittests, and all of cc_unittests pass. R=reveman@chromium.org BUG=268887 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219897

Patch Set 1 #

Total comments: 2

Patch Set 2 : addressed comments #

Total comments: 2

Patch Set 3 : #

Total comments: 2

Patch Set 4 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -129 lines) Patch
M cc/resources/managed_tile_state.h View 1 2 3 2 chunks +5 lines, -12 lines 0 comments Download
M cc/resources/managed_tile_state.cc View 1 2 3 3 chunks +37 lines, -47 lines 0 comments Download
M cc/resources/prioritized_tile_set.cc View 1 2 3 2 chunks +3 lines, -5 lines 0 comments Download
M cc/resources/prioritized_tile_set_unittest.cc View 17 chunks +44 lines, -48 lines 0 comments Download
M cc/resources/tile_manager.cc View 1 2 9 chunks +24 lines, -13 lines 0 comments Download
M cc/resources/tile_manager_perftest.cc View 1 2 3 3 chunks +6 lines, -4 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
vmpstr
Please take a look.
7 years, 4 months ago (2013-08-22 22:56:28 UTC) #1
reveman
This looks great if you just cleanup GetPrioritizedTileSet a bit. https://codereview.chromium.org/22831041/diff/1/cc/resources/tile_manager.cc File cc/resources/tile_manager.cc (right): https://codereview.chromium.org/22831041/diff/1/cc/resources/tile_manager.cc#newcode316 ...
7 years, 3 months ago (2013-08-26 16:55:46 UTC) #2
vmpstr
PTAL.
7 years, 3 months ago (2013-08-26 17:30:40 UTC) #3
vmpstr
https://codereview.chromium.org/22831041/diff/1/cc/resources/tile_manager.cc File cc/resources/tile_manager.cc (right): https://codereview.chromium.org/22831041/diff/1/cc/resources/tile_manager.cc#newcode316 cc/resources/tile_manager.cc:316: mts.bin = mts.tree_bin[PENDING_TREE]; On 2013/08/26 16:55:46, David Reveman wrote: ...
7 years, 3 months ago (2013-08-26 17:30:58 UTC) #4
reveman
https://codereview.chromium.org/22831041/diff/6001/cc/resources/tile_manager.cc File cc/resources/tile_manager.cc (right): https://codereview.chromium.org/22831041/diff/6001/cc/resources/tile_manager.cc#newcode334 cc/resources/tile_manager.cc:334: if (mts.bin == NEVER_BIN && !dual_bin_is_never) would: bool is_in_never_bin_on_both_trees ...
7 years, 3 months ago (2013-08-26 20:56:28 UTC) #5
vmpstr
PTAL. https://codereview.chromium.org/22831041/diff/6001/cc/resources/tile_manager.cc File cc/resources/tile_manager.cc (right): https://codereview.chromium.org/22831041/diff/6001/cc/resources/tile_manager.cc#newcode334 cc/resources/tile_manager.cc:334: if (mts.bin == NEVER_BIN && !dual_bin_is_never) On 2013/08/26 ...
7 years, 3 months ago (2013-08-26 21:08:08 UTC) #6
reveman
lgtm https://codereview.chromium.org/22831041/diff/17001/cc/resources/prioritized_tile_set.cc File cc/resources/prioritized_tile_set.cc (right): https://codereview.chromium.org/22831041/diff/17001/cc/resources/prioritized_tile_set.cc#newcode61 cc/resources/prioritized_tile_set.cc:61: case NUM_BINS: nit: "default: NOTREACHED()" would be more ...
7 years, 3 months ago (2013-08-26 23:22:13 UTC) #7
danakj
https://codereview.chromium.org/22831041/diff/17001/cc/resources/prioritized_tile_set.cc File cc/resources/prioritized_tile_set.cc (right): https://codereview.chromium.org/22831041/diff/17001/cc/resources/prioritized_tile_set.cc#newcode61 cc/resources/prioritized_tile_set.cc:61: case NUM_BINS: On 2013/08/26 23:22:13, David Reveman wrote: > ...
7 years, 3 months ago (2013-08-27 02:23:43 UTC) #8
tomhudson
On 2013/08/27 02:23:43, danakj wrote: > https://codereview.chromium.org/22831041/diff/17001/cc/resources/prioritized_tile_set.cc#newcode61 > cc/resources/prioritized_tile_set.cc:61: case NUM_BINS: > On 2013/08/26 23:22:13, ...
7 years, 3 months ago (2013-08-27 10:00:40 UTC) #9
reveman
On 2013/08/27 10:00:40, tomhudson wrote: > On 2013/08/27 02:23:43, danakj wrote: > > > https://codereview.chromium.org/22831041/diff/17001/cc/resources/prioritized_tile_set.cc#newcode61 ...
7 years, 3 months ago (2013-08-27 18:58:23 UTC) #10
vmpstr
On 2013/08/27 18:58:23, David Reveman wrote: > On 2013/08/27 10:00:40, tomhudson wrote: > > On ...
7 years, 3 months ago (2013-08-27 23:35:16 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vmpstr@chromium.org/22831041/25001
7 years, 3 months ago (2013-08-27 23:41:29 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vmpstr@chromium.org/22831041/25001
7 years, 3 months ago (2013-08-28 00:38:45 UTC) #13
commit-bot: I haz the power
7 years, 3 months ago (2013-08-28 03:44:28 UTC) #14
Message was sent while issue was closed.
Change committed as 219897

Powered by Google App Engine
This is Rietveld 408576698