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

Issue 263803003: DM: Push GPU-parent child tasks to the front of the queue. (Closed)

Created:
6 years, 7 months ago by mtklein_C
Modified:
6 years, 7 months ago
Reviewers:
bsalomon, borenet, mtklein
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlesource.com/skia.git@master
Visibility:
Public.

Description

DM: Push GPU-parent child tasks to the front of the queue. Like yesterday's change to run CPU-parent child tasks serially in thread, this reduces peak memory usage by improving the temporaly locality of the bitmaps we create. E.g. Let's say we start with tasks A B C and D Queue: [ A B C D ] Running A creates A' and A", which depend on a bitmap created by A. Queue: [ B C D A' A" * ] That bitmap now needs sit around in RAM while B C and D run pointlessly and can only be destroyed at *. If instead we do this and push dependent child tasks to the front of the queue, the queue and bitmap lifetime looks like this: Queue: [ A' A" * B C D ] This is much, much worse in practice because the queue is often several thousand tasks long. 100s of megs of bitmaps can pile up for 10s of seconds pointlessly. To make this work we add addNext() to SkThreadPool and its cousin DMTaskRunner. I also took the opportunity to swap head and tail in the threadpool implementation so it matches the comments and intuition better: we always pop the head, add() puts it at the tail, addNext() at the head. Before Debug: 49s, 1403352k peak Release: 16s, 2064008k peak After Debug: 49s, 1234788k peak Release: 15s, 1903424k peak BUG=skia:2478 Committed: http://code.google.com/p/skia/source/detail?r=14506

Patch Set 1 #

Patch Set 2 : share code #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -8 lines) Patch
M dm/DMTask.h View 1 chunk +1 line, -1 line 0 comments Download
M dm/DMTask.cpp View 3 chunks +6 lines, -3 lines 0 comments Download
M dm/DMTaskRunner.h View 1 chunk +1 line, -0 lines 0 comments Download
M dm/DMTaskRunner.cpp View 1 chunk +1 line, -1 line 0 comments Download
M include/utils/SkThreadPool.h View 1 5 chunks +22 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
mtklein
6 years, 7 months ago (2014-05-01 15:34:39 UTC) #1
bsalomon
lgtm
6 years, 7 months ago (2014-05-01 15:40:02 UTC) #2
mtklein
The CQ bit was checked by mtklein@google.com
6 years, 7 months ago (2014-05-01 15:43:16 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/263803003/20001
6 years, 7 months ago (2014-05-01 15:43:42 UTC) #4
mtklein
The CQ bit was unchecked by mtklein@google.com
6 years, 7 months ago (2014-05-01 17:40:46 UTC) #5
mtklein
The CQ bit was checked by mtklein@google.com
6 years, 7 months ago (2014-05-01 17:41:02 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/263803003/20001
6 years, 7 months ago (2014-05-01 17:41:15 UTC) #7
commit-bot: I haz the power
6 years, 7 months ago (2014-05-01 17:41:37 UTC) #8
Message was sent while issue was closed.
Change committed as 14506

Powered by Google App Engine
This is Rietveld 408576698