Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(5789)

Issue 6509044: Performance fix for Cycles: Don't wait in the main UI thread when resetting devices (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 8 months ago by lukas.toenne1
Modified:
11 years, 8 months ago
Reviewers:
brechtvl, bf-codereview
Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/
Visibility:
Public.

Description

When the scene is updated Cycles resets the renderer device, cancelling all existing tasks. The main thread would wait for all running tasks to finish before continuing. This is ok when tasks can actually cancel in a timely fashion. For OSL however, this does not work, since the OSL shader group optimization takes quite a bit of time and can not be easily be cancelled once running (on my crappy machine in full debug mode: ~0.12 seconds for simple node trees). This would lead to very laggy UI behavior and make it difficult to accurately control elements such as sliders. This patch removes the wait condition from the device->task_cancel method. Instead it just sets the do_cancel flag and returns. To avoid backlog in the task pool of the device it will return early from the BlenderSession::sync function while the reset is going on (tested in Session::resetting). Once all existing tasks have finished the do_cancel flag is finally cleared again (checked in TaskPool::num_decrease). Care has to be taken to avoid race conditions on the do_cancel flag, since it can now be modified outside the TaskPool::cancel function itself. For this purpose the scope of the TaskPool::num_mutex locks has been extended, in most cases the mutex is now locked by the TaskPool itself before calling TaskScheduler methods, instead of only locking inside the num_increase/num_decrease functions themselves. The only occurrence of a lock outside of the TaskPool methods is in TaskScheduler::thread_run. This patch is most useful in combination with the OSL renderer mode, so it can probably wait until after the 2.64 release. SVM tasks tend to be cancelled quickly, so the effect is less noticeable.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -16 lines) Patch
intern/cycles/blender/blender_session.cpp View 1 chunk +6 lines, -0 lines 0 comments Download
intern/cycles/device/device.h View 1 chunk +1 line, -0 lines 0 comments Download
intern/cycles/device/device_cpu.cpp View 1 chunk +5 lines, -0 lines 0 comments Download
intern/cycles/device/device_cuda.cpp View 1 chunk +5 lines, -0 lines 0 comments Download
intern/cycles/device/device_multi.cpp View 1 chunk +8 lines, -0 lines 0 comments Download
intern/cycles/device/device_opencl.cpp View 1 chunk +5 lines, -0 lines 0 comments Download
intern/cycles/render/session.h View 2 chunks +3 lines, -0 lines 0 comments Download
intern/cycles/render/session.cpp View 3 chunks +19 lines, -0 lines 0 comments Download
intern/cycles/util/util_task.cpp View 4 chunks +17 lines, -16 lines 0 comments Download

Messages

Total messages: 3
lukas.toenne1
11 years, 8 months ago (2012-09-11 10:14:15 UTC) #1
brechtvl
Looks good to me, clever solution :) I don't mind it being committed now, we ...
11 years, 8 months ago (2012-09-11 10:38:04 UTC) #2
lukas.toenne1
11 years, 8 months ago (2012-09-11 11:42:58 UTC) #3
That was a quick review, thanks =)

Committed as r50528.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b