Merge pull request #667 from johnhaddon/stringSubstitutionOptimisation

String substitution optimisation.
Implements an optimisation for StringPlugs that uses a faster code path when a string contains no variable references, giving a 9% reduction in runtime for John's animation playback test case.

include/Gaffer/Context.h

@@ -106,6 +106,10 @@ class Context : public IECore::RefCounted
  /// then we could have a separate substitute() function capable
  /// of accepting Contexts, CompoundData, CompoundObjects etc.
  std::string substitute( const std::string &input ) const;
+ /// Returns true if the specified string contains substitutions
+ /// which can be performed by the substitute() method. If it returns
+ /// false, it is guaranteed that substitute( input ) == input.
+ static bool hasSubstitutions( const std::string &input );
 
  /// The Scope class is used to push and pop the current context on
  /// the calling thread.

python/GafferTest/ContextTest.py

@@ -244,6 +244,15 @@ def testSubstitute( self ) :
  self.assertEqual( c.substitute( "$a/$dontExist/something.###.tif" ), "apple//something.020.tif" )
  self.assertEqual( c.substitute( "${badlyFormed" ), "" )
 
+ def testHasSubstitutions( self ) :
+
+ c = Gaffer.Context()
+ self.assertFalse( c.hasSubstitutions( "a" ) )
+ self.assertTrue( c.hasSubstitutions( "~something" ) )
+ self.assertTrue( c.hasSubstitutions( "$a" ) )
+ self.assertTrue( c.hasSubstitutions( "${a}" ) )
+ self.assertTrue( c.hasSubstitutions( "###" ) )
+
  def testNames( self ) :
 
  c = Gaffer.Context()

python/GafferTest/StringInOutNode.py

@@ -39,11 +39,11 @@
 
 class StringInOutNode( Gaffer.ComputeNode ) :
 
- def __init__( self, name="StringInOutNode" ) :
+ def __init__( self, name="StringInOutNode", defaultValue="" ) :
 
  Gaffer.ComputeNode.__init__( self, name )
 
- self.addChild( Gaffer.StringPlug( "in" ) )
+ self.addChild( Gaffer.StringPlug( "in", Gaffer.Plug.Direction.In, defaultValue ) )
  self.addChild( Gaffer.StringPlug( "out", Gaffer.Plug.Direction.Out ) )
 
  def affects( self, input ) :

python/GafferTest/StringPlugTest.py

@@ -151,6 +151,14 @@ def testEnvironmentExpansion( self ) :
  self.assertEqual( n["out"].getValue(), "b" )
  self.assertNotEqual( n["out"].hash(), h1 )
  self.assertNotEqual( n["out"].hash(), h2 )
+
+ def testDefaultValueExpansion( self ) :
+
+ n = GafferTest.StringInOutNode( defaultValue = "${A}" )
+ context = Gaffer.Context()
+ context["A"] = "b"
+ with context :
+ self.assertEqual( n["out"].getValue(), "b" )
 
 if __name__ == "__main__":
  unittest.main()

src/Gaffer/Context.cpp

@@ -127,6 +127,23 @@ std::string Context::substitute( const std::string &s ) const
  return result;
 }
 
+bool Context::hasSubstitutions( const std::string &input )
+{
+ for( const char *c = input.c_str(); *c; c++ )
+ {
+ switch( *c )
+ {
+ case '$' :
+ case '#' :
+ case '~' :
+ return true;
+ default :
+ ; // do nothing
+ }
+ }
+ return false;
+}
+
 void Context::substituteInternal( const std::string &s, std::string &result, const int recursionDepth ) const
 {
  if( recursionDepth > 8 )

src/Gaffer/TypedPlug.cpp

@@ -54,29 +54,45 @@ IECORE_RUNTIMETYPED_DEFINETEMPLATESPECIALISATION( Gaffer::AtomicBox2iPlug, Atomi
 template<>
 std::string StringPlug::getValue() const
 { 
- bool performSubstitution = direction()==Plug::In && inCompute() && Plug::getFlags( Plug::PerformsSubstitutions );
-
  IECore::ConstObjectPtr o = getObjectValue();
  const IECore::StringData *s = IECore::runTimeCast<const IECore::StringData>( o.get() );
  if( !s )
  {
  throw IECore::Exception( "StringPlug::getObjectValue() didn't return StringData - is the hash being computed correctly?" );
  }
+
+ bool performSubstitution =
+ direction()==Plug::In &&
+ inCompute() &&
+ Plug::getFlags( Plug::PerformsSubstitutions ) &&
+ Context::hasSubstitutions( s->readable() );
+
  return performSubstitution ? Context::current()->substitute( s->readable() ) : s->readable();
 }
 
 template<>
 IECore::MurmurHash StringPlug::hash() const
 {
  bool performSubstitution = direction()==Plug::In && !getInput<ValuePlug>() && Plug::getFlags( Plug::PerformsSubstitutions );
- if( !performSubstitution )
+ if( performSubstitution )
  {
- return ValuePlug::hash();
+ IECore::ConstObjectPtr o = getObjectValue();
+ const IECore::StringData *s = IECore::runTimeCast<const IECore::StringData>( o.get() );
+ if( !s )
+ {
+ throw IECore::Exception( "StringPlug::getObjectValue() didn't return StringData - is the hash being computed correctly?" );
+ }
+
+ if( Context::hasSubstitutions( s->readable() ) )
+ {
+ IECore::MurmurHash result;
+ result.append( Context::current()->substitute( s->readable() ) );
+ return result;
+ }
  }
-
- IECore::MurmurHash result;
- result.append( Context::current()->substitute( getValue() ) );
- return result;
+
+ // no substitutions
+ return ValuePlug::hash();
 }
 
 // explicit instantiation

src/GafferBindings/ContextBinding.cpp

@@ -155,6 +155,7 @@ void bindContext()
  .def( self == self )
  .def( self != self )
  .def( "substitute", &Context::substitute )
+ .def( "hasSubstitutions", &Context::hasSubstitutions ).staticmethod( "hasSubstitutions" )
  .def( "current", &current ).staticmethod( "current" )
  ;