Skip already finished raster tasks being scheduled again. |
|||
Issue description
As worker thread and compositor thread both run parallely, there is synchnonization issue for the following case.
Compositor thread -> (snippet1)
TileManager::PrepareTiles() {
.
.
// Lock is acquired inside TaskGraphWorkQueue::ScheduleTasks(), till there both threads can run.
TileManager::ScheduleTasks();
.
.
}
Worker Thread -> (snippet2)
RasterWorkerPool::RunTaskInCategoryWithLockAcquired() {
.
.
work_queue_.CompleteTask(prioritized_task);
.
.
}
Even though snippet1 calls CheckForCompletedTasks() before scheduling tasks, as snippet2 is not synchronized with snippet1, while scheduling happens, we may have tasks finished.
Ideal case logs -
[11281:11309:0520/161559:INFO:tile_manager.cc(452)] PRAS::PrepareTiles
[11281:11309:0520/161559:INFO:tile_manager.cc(863)] PRAS::ScheduleTasks
[11281:11321:0520/161559:INFO:task_graph_work_queue.cc(338)] PRAS::CompleteTask
[11281:11320:0520/161559:INFO:task_graph_work_queue.cc(338)] PRAS::CompleteTask
Clash case logs -
[11281:11309:0520/161558:INFO:tile_manager.cc(452)] PRAS::PrepareTiles
[11281:11324:0520/161558:INFO:task_graph_work_queue.cc(338)] PRAS::CompleteTask
[11281:11309:0520/161558:INFO:tile_manager.cc(863)] PRAS::ScheduleTasks
[11281:11322:0520/161558:INFO:task_graph_work_queue.cc(338)] PRAS::CompleteTask
The finished tasks should be skipped during scheduling as on next CheckForCompletedTasks(), those would be collected. If task gets finished after we have checked, then it would be skipped in TaskGraphWorkQueue::ScheduleTasks().
,
May 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06e1561a832083330a193743a427b1354ab3cd39 commit 06e1561a832083330a193743a427b1354ab3cd39 Author: prashant.n <prashant.n@samsung.com> Date: Thu May 26 17:35:21 2016 cc: Remove ScheduleOnOriginThread() and CompleteOnOriginThread(). The task's job is performed in RunOnWorkerThread() and schedule or complete are not needed as a part of task's job. Those are the responsibilities of task's owner (TileManager - compositor thread). This patch removes following functions which were basically needed for async upload. Now raster buffer is provided to task at the time of ctor and functionality of CompleteOnOriginThread() is moved to task's owner. ScheduleOnOriginThread() CompleteOnOriginThread() New OnTaskCompleted() function calls corresponding function of task owner. This patch implements following life cycle for the task. 1. Task's owner creates task with all needed info on origin thread. 2. Task is scheduled and run on worker thread. 3. Completed task is processed on origin thread by task owner. 4. Task is destroyed. This patch also fixes few task related failing cc_perftests (607818). BUG= 499372 , 599863, 607818, 613529 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/1866043006 Cr-Commit-Position: refs/heads/master@{#396218} [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/raster_buffer_provider_perftest.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/raster_buffer_provider_unittest.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/task.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/task.h [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/task_graph_runner_perftest.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/tile_task.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/raster/tile_task.h [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/test/fake_tile_task_manager.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/test/fake_tile_task_manager.h [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/test/task_graph_runner_test_template.h [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/gpu_image_decode_controller.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/gpu_image_decode_controller.h [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/gpu_image_decode_controller_unittest.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/software_image_decode_controller.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/software_image_decode_controller_unittest.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/tile_manager.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/tile_manager.h [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/tile_manager_unittest.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/tile_task_manager.cc [modify] https://crrev.com/06e1561a832083330a193743a427b1354ab3cd39/cc/tiles/tile_task_manager.h
,
May 27 2016
,
May 31 2016
I'd a fix for this bug in above patch. But as this adds data race condition, we need to leave tgwq to handle the scheduling of the tasks. So TileManager::ScheduleTasks() would handle only the tasks which it has collected at the moment. The patch for it is submitted at https://codereview.chromium.org/2018353005/. So in a way this bug is invalid as compared to current implementation. |
|||
►
Sign in to add a comment |
|||
Comment 1 by prashan...@samsung.com
, May 24 2016