ValuePlug : Fix getValue() performance regression

Commit 055f07d moved the `computeNode` assignment before the fast path for plugs with static values. This caused significant slowdown when the node was implemented in Python, due to the overhead of the `runTimeCast()`. The excessive Python-specific overhead from this was fixed in the preceding commit, but the additional fix here still yields a 6% improvement in `testStaticNumericValuePerformance()` for the remaining C++-only case. This improvement will become even more significant soon, because I'm sitting on another optimisation which removes the majority of the remaining overhead for this case (the reference counting).

The assignment to `computeNode` within the `if` might be a bit subtle for the reader, but I tried various other ways of factoring it, and they all resulted in at least one more conditional block and some duplicated code. I think this is the clearest way of writing it, particularly with the new assertion that shows the intended result.

Changes.md

@@ -13,6 +13,7 @@ Improvements
 Fixes
 -----
 
+- ValuePlug : Fixed performance regression (introduced in 1.3.1.0) getting values from plugs without an input connection. This could severely affect scene generation times in some cases.
 - NameSwitch : Fixed bug which prevented drag and drop reordering of rows with an input connection.
 - PythonEditor :
   - Fixed output for `print()` calls with multiple arguments, which was previously spread across multiple lines.

python/GafferTest/ValuePlugTest.py

@@ -691,6 +691,15 @@ def testContentionForOneItem( self ) :
 		with GafferTest.TestRunner.PerformanceScope() :
 			GafferTest.parallelGetValue( m["product"], 10000000 )
 
+	@GafferTest.TestRunner.PerformanceTestMethod()
+	def testStaticNumericValuePerformance( self ) :
+
+		node = Gaffer.Node()
+		node["plug"] = Gaffer.IntPlug()
+
+		with GafferTest.TestRunner.PerformanceScope() :
+			GafferTest.parallelGetValue( node["plug"], 10000000 )
+
 	def testIsSetToDefault( self ) :
 
 		n1 = GafferTest.AddNode()
@@ -1012,6 +1021,21 @@ def testNoCachePolicyForConversions( self ) :
 		n["in"].hash()
 		self.assertEqual( n["in"].getValue(), False )
 
+	def testOutputPlugWithConvertingInput( self ) :
+
+		for nodeType in ( Gaffer.Node, Gaffer.ComputeNode ) :
+			with self.subTest( nodeType = nodeType ) :
+
+				node = nodeType()
+				node["in"] = Gaffer.IntPlug()
+				node["out"] = Gaffer.FloatPlug( direction = Gaffer.Plug.Direction.Out )
+				node["out"].setInput( node["in"] )
+
+				for i in range( 0, 10 ) :
+
+					node["in"].setValue( i )
+					self.assertEqual( node["out"].getValue(), i )
+
 	def setUp( self ) :
 
 		GafferTest.TestCase.setUp( self )

src/Gaffer/ValuePlug.cpp

@@ -606,11 +606,13 @@ class ValuePlug::ComputeProcess : public Process
 		{
 			const ValuePlug *p = sourcePlug( plug );
 
-			const ComputeNode *computeNode = IECore::runTimeCast<const ComputeNode>( p->node() );
-
+			const ComputeNode *computeNode = nullptr;
 			if( !p->getInput() )
 			{
-				if( p->direction()==In || !computeNode )
+				// Note the assignment to `computeNode` for use later. It is important that this is short-circuited
+				// for input plugs, as the unnecessary `runTimeCast()` would add measurable overhead for our most
+				// common case.
+				if( p->direction()==In || !(computeNode = IECore::runTimeCast<const ComputeNode>( p->node() )) )
 				{
 					// No input connection, and no means of computing
 					// a value. There can only ever be a single value,
@@ -623,6 +625,8 @@ class ValuePlug::ComputeProcess : public Process
 			// one per context, computed via ComputeNode::compute(). Pull the value out of our cache, or compute
 			// it with a ComputeProcess.
 
+			assert( (plug->getInput() && !computeNode) || computeNode );
+
 			const ThreadState &threadState = ThreadState::current();
 			const Context *currentContext = threadState.context();