imageworks/OpenShadingLanguage — pull request #363 by lgritz (2014-04-30 17:10:00+00:00)
Title: De-virtualize OSLCompiler and ShadingSystem classes!

There are two ways to expose just an abstract class API from a library,
while completely hiding all internals of the class (and in particular,
data internals).

1. Subclassing: Make the Foo public API be an abstract class, with only
pure virtual methods, and no data. Internal to the library, subclass the
parent as 'class FooImpl : public Foo', fully elaborating the
methods. This implementation class is hidden from the client app, so
you'll need create() and destroy() methods.

2. PIMPL idiom: Make the Foo public API a simple concrete class with no
virtual methods, but containing a private FooImpl *m_impl (where FooImpl
is a forward-declared class, so not exposed). The ctr/dtr of the exposed
class will internally create/destroy a FooImpl, and all the methods of
Foo::method() simply call m_impl->method(). This means that the FooImpl
also now no longer needs any virtual methods.

The cost of these are roughly the same -- #1 needs to look up methods in
the vtbl every time, whereas #2 needs to make an extra wrapping function
call every time (though in theory, a good optimizing compiler with LTO
should be able to make this essentially free).

If it's important for the client app to be able to completely substitute
their own custom implementation while following the same API (i.e.,
somebody needs to subclass Foo to specialize it), then #1 is the only
way to go. An example of this is RendererServices -- the whole point of
RS is to allow a renderer implementation to subclass it in order to
provide all the renderer-specific bits of functionality.

However, if the library is providing the one and only possible
implementation, i.e., no subclassing is ever desired/necessary, and the
whole exercise is simply to hide the implementation details behind an
abstrat API, then #2 has a distinct advantage: because there are no
virtual methods, Foo can have new (non-virtual!) public methods added,
and FooImpl can change arbitrarily, without breaking backward
compatibility in linkage with apps! In other words, #2 allows
substantially more change to internals, and adding publicly exposed
features, without violating promises that are usually made about
compatibility for "release branches" of a product. (Whereas, the #1
approach dare not change anything about the API, for fear of altering
the layout of the vtbl.)

So, therefore, this patch "de-virtualizes" the main two public
interfaces for OSL, OSLCompiler and ShadingSystem, switching their API
abstraction strategy from approach #1 to approach #2, and thus serves as
powerful insurance as we move forward, allowing much more freedom in the
range of changes we make may to internals within release branches
without breaking compatibility.

Along the way, I also took a few remaining places in these two APIs
where we passed std::string around and replaced them with string_view.