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

Issue 888041: Review: compute derivatives only when needed (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years ago by larrygritz
Modified:
14 years ago
Reviewers:
ckulla
Base URL:
http://openshadinglanguage.googlecode.com/svn/trunk/
Visibility:
Public.

Description

At long last, this patch switches from the old (intended to be temporary) behavior -- computing derivatives on all non-constant, non-int, non-closure symbols -- to correctly marking ONLY those symbols that truly need derivatives as such. This involves the following steps: 0. oslc already marked which arguments to each op need derivatives. For example, texture(name,s,t,...) takes derivatives of s and t. But now when we load the shader, we correctly read that from the oso file and store it with each op. 1. For each shader layer, after runtime optimization but before temporary coalescing, we analyze the full variable dependencies and therefore mark all dependencies of symbols whose derivatives are taken as needing to have derivatives. 2. We proceed from LAST layer to FIRST (i.e. from downstream to upstream), and as we do the deriv dependency analysis on each layer, we also mark any upstream output parameters of earlier layers that are connected to our layer's input parameters that need derivatives. In this way, derivative-need propagates through the shader network. 3. When we coalesce temporaries, we are careful not to coalesce a variable that needs derivs with one that doesn't! 4. Once this was done, I discovered that spline and texture were both broken when no derivatives were needed. :-) So those are fixed now. 5. Also some other minor refactoring, including moving the necessary dependency analysis from oslc into the runtime library. Results: in our favorite production benchmark frame, after these changes only 7% of symbols are marked as needing derivatives. This greatly reduces memory use, since the vast majority of symbols now only need to store their values, not also two differentials. It also speeds up execution of the shader, since there's less math when you don't compute the derivatives. But, alas, our production frames only speed up by 6-7%, mainly because the deriv math (or, indeed raw shader interpretation) is just not the big bottleneck in our renderer at this point. (We're working on that on a second front, mostly not on the OSL side of the fence.)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address Chris' comments about result vs alpha derivs #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+389 lines, -53 lines) Patch
src/include/osl_pvt.h View 1 chunk +1 line, -0 lines 0 comments Download
src/liboslexec/instance.cpp View 4 chunks +17 lines, -3 lines 0 comments Download
src/liboslexec/loadshader.cpp View 1 chunk +10 lines, -0 lines 0 comments Download
src/liboslexec/opspline.cpp View 1 chunk +7 lines, -8 lines 0 comments Download
src/liboslexec/optexture.cpp View 1 2 chunks +24 lines, -11 lines 1 comment Download
src/liboslexec/oslexec_pvt.h View 4 chunks +16 lines, -0 lines 0 comments Download
src/liboslexec/runtimeoptimize.cpp View 18 chunks +309 lines, -31 lines 0 comments Download
src/liboslexec/shadingsys.cpp View 2 chunks +5 lines, -0 lines 0 comments Download

Messages

Total messages: 7
larrygritz
14 years ago (2010-04-05 04:55:17 UTC) #1
ckulla
LGTM overall, but it looks to me like there are a few missing cases in ...
14 years ago (2010-04-05 17:58:24 UTC) #2
lg_imageworks.com
Good point, Chris. I'll submit an updated patch in a moment that I think addresses ...
14 years ago (2010-04-05 18:35:59 UTC) #3
larrygritz
Address Chris' comments about result vs alpha derivs
14 years ago (2010-04-05 18:38:18 UTC) #4
ckulla
LGTM One minor comment, but it doesn't affect correctness. http://codereview.appspot.com/888041/diff/12001/13003 File src/liboslexec/optexture.cpp (right): http://codereview.appspot.com/888041/diff/12001/13003#newcode259 src/liboslexec/optexture.cpp:259: ...
14 years ago (2010-04-05 18:41:50 UTC) #5
lg_imageworks.com
OK. Thanks for the tip. On Apr 5, 2010, at 11:41 AM, <ckulla@gmail.com> <ckulla@gmail.com> wrote: ...
14 years ago (2010-04-05 18:55:41 UTC) #6
larrygritz
14 years ago (2010-04-05 21:39:33 UTC) #7
One more comment -- even though this only speeds up our production frames by
~6%, on testshade torture tests (lots and lots of math ops, no texture, no
lighting integration) it speeds up by 50%, which is about what I expected when
you cut out the deriv math on most of the symbols.  So this works great and as
intended, it's just that executing the OSL ops themselves is not the main
bottleneck in our renderer at this point.
Sign in to reply to this message.

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