Improve ValuePlug::getValue() performance

Changes.md

@@ -110,6 +110,7 @@ API
 - Handle::AngularDrag : Added `isLinearDrag()` method.
 - Widget : Added per-widget control over colour display transforms via new `setDisplayTransform()`, `getDisplayTransform()` and `displayTransform()` methods.
 - VisibleSet : Added `VisibleSet::Visibility` struct containing `drawMode` and `descendantsVisible` members.
+- ValuePlug : Improved `getValue()` performance, particularly when retrieving previously computed values from the cache. One benchmark shows a 50% reduction in runtime when the cache is under heavy contention from many threads.
 
 Breaking Changes
 ----------------
@@ -160,6 +161,8 @@ Breaking Changes
 - gaffer test : Replaced `-performanceOnly` flag with `-category` argument which may be set to `performance` for the same as the old `-performanceOnly`, or `standard` for the converse.
 - VisibleSet : Renamed `VisibleSet::match()` to `visibility()` and changed return type.
 - SceneView : Removed `gridPlug()` method.
+- TypedPlug : Moved implementation code from `TypedPlug.inl` to `TypedPlugImplementation.h`.
+- TypedObjectPlug : Moved implementation code from `TypedObjectPlug.inl` to `TypedObjectPlugImplementation.h`.
 
 Build
 -----

SConstruct

@@ -1093,7 +1093,7 @@ libraries = {
 
 	"GafferSceneTest" : {
 		"envAppends" : {
-			"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "IECoreScene$CORTEX_LIB_SUFFIX" ],
+			"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "GafferImage", "IECoreScene$CORTEX_LIB_SUFFIX" ],
 		},
 		"pythonEnvAppends" : {
 			"LIBS" : [ "Gaffer", "GafferDispatch", "GafferBindings", "GafferScene", "GafferSceneTest" ],
@@ -1106,7 +1106,7 @@ libraries = {
 			"LIBS" : [ "Gaffer", "GafferUI", "GafferImage", "GafferImageUI", "GafferScene", "Iex$IMATH_LIB_SUFFIX", "IECoreGL$CORTEX_LIB_SUFFIX", "IECoreImage$CORTEX_LIB_SUFFIX", "IECoreScene$CORTEX_LIB_SUFFIX" ],
 		},
 		"pythonEnvAppends" : {
-			"LIBS" : [ "IECoreGL$CORTEX_LIB_SUFFIX", "GafferBindings", "GafferScene", "GafferUI", "GafferImageUI", "GafferSceneUI", "IECoreScene$CORTEX_LIB_SUFFIX" ],
+			"LIBS" : [ "IECoreGL$CORTEX_LIB_SUFFIX", "GafferBindings", "GafferScene", "GafferImage", "GafferUI", "GafferImageUI", "GafferSceneUI", "IECoreScene$CORTEX_LIB_SUFFIX" ],
 		},
 	},
 
@@ -1246,7 +1246,7 @@ libraries = {
 
 	"GafferOSLUI" : {
 		"envAppends" : {
-			"LIBS" : [ "Gaffer", "GafferUI", "GafferOSL" ],
+			"LIBS" : [ "Gaffer", "GafferImage", "GafferUI", "GafferOSL" ],
 		},
 		"pythonEnvAppends" : {
 			"LIBS" : [ "IECoreGL$CORTEX_LIB_SUFFIX", "GafferBindings", "GafferScene", "GafferUI", "GafferImageUI", "GafferOSLUI" ],
@@ -1351,7 +1351,7 @@ libraries = {
 
 	"GafferUSD" : {
 		"envAppends" : {
-			"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "IECoreScene$CORTEX_LIB_SUFFIX" ] + [ "${USD_LIB_PREFIX}" + x for x in ( [ "sdf", "arch", "tf", "vt" ] if not env["USD_MONOLITHIC"] else [ "usd_ms" ] ) ],
+			"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "GafferImage", "IECoreScene$CORTEX_LIB_SUFFIX" ] + [ "${USD_LIB_PREFIX}" + x for x in ( [ "sdf", "arch", "tf", "vt" ] if not env["USD_MONOLITHIC"] else [ "usd_ms" ] ) ],
 			# USD includes "at least one deprecated or antiquated header", so we
 			# have to drop our usual strict warning levels.
 			"CXXFLAGS" : [ "-Wno-deprecated" if env["PLATFORM"] != "win32" else "/wd4996" ],

include/Gaffer/NumericPlug.h

@@ -45,7 +45,7 @@ namespace Gaffer
 {
 
 template<typename T>
-class GAFFER_API NumericPlug : public ValuePlug
+class IECORE_EXPORT NumericPlug : public ValuePlug
 {
 
 	public :
@@ -103,3 +103,5 @@ IE_CORE_DECLAREPTR( FloatPlug );
 IE_CORE_DECLAREPTR( IntPlug );
 
 } // namespace Gaffer
+
+#include "Gaffer/NumericPlug.inl"

include/Gaffer/NumericPlug.inl

@@ -0,0 +1,48 @@
+//////////////////////////////////////////////////////////////////////////
+//
+//  Copyright (c) 2023, Cinesite VFX Ltd. All rights reserved.
+//
+//  Redistribution and use in source and binary forms, with or without
+//  modification, are permitted provided that the following conditions are
+//  met:
+//
+//      * Redistributions of source code must retain the above
+//        copyright notice, this list of conditions and the following
+//        disclaimer.
+//
+//      * Redistributions in binary form must reproduce the above
+//        copyright notice, this list of conditions and the following
+//        disclaimer in the documentation and/or other materials provided with
+//        the distribution.
+//
+//      * Neither the name of John Haddon nor the names of
+//        any other contributors to this software may be used to endorse or
+//        promote products derived from this software without specific prior
+//        written permission.
+//
+//  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+//  IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+//  THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+//  PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+//  CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+//  EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+//  PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+//  PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+//  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+//  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+//  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+//////////////////////////////////////////////////////////////////////////
+
+#pragma once
+
+namespace Gaffer
+{
+
+template<typename T>
+inline T NumericPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
+{
+	return getObjectValue<DataType>( precomputedHash )->readable();
+}
+
+} // namespace Gaffer

include/Gaffer/Plug.inl

@@ -37,9 +37,26 @@
 
 #pragma once
 
+#include "Gaffer/Node.h"
+
 namespace Gaffer
 {
 
+inline Node *Plug::node()
+{
+	return ancestor<Node>();
+}
+
+inline const Node *Plug::node() const
+{
+	return ancestor<Node>();
+}
+
+inline Plug::Direction Plug::direction() const
+{
+	return m_direction;
+}
+
 template<typename T>
 T *Plug::getInput()
 {
@@ -52,7 +69,6 @@ const T *Plug::getInput() const
 	return IECore::runTimeCast<const T>( m_input );
 }
 
-
 template<typename T>
 T *Plug::source()
 {

include/Gaffer/TypedObjectPlug.h

@@ -181,3 +181,5 @@ IE_CORE_DECLAREPTR( AtomicCompoundDataPlug );
 IE_CORE_DECLAREPTR( PathMatcherDataPlug );
 
 } // namespace Gaffer
+
+#include "TypedObjectPlug.inl"

include/Gaffer/TypedObjectPlug.inl

@@ -1,7 +1,6 @@
 //////////////////////////////////////////////////////////////////////////
 //
-//  Copyright (c) 2011-2012, John Haddon. All rights reserved.
-//  Copyright (c) 2011-2015, Image Engine Design Inc. All rights reserved.
+//  Copyright (c) 2023, Cinesite VFX Ltd. All rights reserved.
 //
 //  Redistribution and use in source and binary forms, with or without
 //  modification, are permitted provided that the following conditions are
@@ -35,84 +34,15 @@
 //
 //////////////////////////////////////////////////////////////////////////
 
-/// This file contains the implementation of TypedObjectPlug. Rather than include it
-/// in a public header it is #included in TypedObjectPlug.cpp, and the relevant template
-/// classes are explicitly instantiated there. This prevents a host of problems to do with
-/// the definition of the same symbols in multiple object files. Additional TypedObjectPlug
-/// instantations may be created in similar .cpp files in other libraries.
+#pragma once
 
 namespace Gaffer
 {
 
 template<class T>
-const IECore::RunTimeTyped::TypeDescription<TypedObjectPlug<T> > TypedObjectPlug<T>::g_typeDescription;
-
-template<class T>
-TypedObjectPlug<T>::TypedObjectPlug(
-	const std::string &name,
-	Direction direction,
-	ConstValuePtr defaultValue,
-	unsigned flags
-)
-	:	ValuePlug( name, direction, defaultValue->copy(), flags )
-{
-}
-
-template<class T>
-TypedObjectPlug<T>::~TypedObjectPlug()
-{
-}
-
-template<class T>
-bool TypedObjectPlug<T>::acceptsInput( const Plug *input ) const
-{
-	if( !ValuePlug::acceptsInput( input ) )
-	{
-		return false;
-	}
-	if( input )
-	{
-		return input->isInstanceOf( staticTypeId() );
-	}
-	return true;
-}
-
-template<class T>
-PlugPtr TypedObjectPlug<T>::createCounterpart( const std::string &name, Direction direction ) const
-{
-	return new TypedObjectPlug<T>( name, direction, defaultValue(), getFlags() );
-}
-
-template<class T>
-const typename TypedObjectPlug<T>::ValueType *TypedObjectPlug<T>::defaultValue() const
-{
-	return static_cast<const ValueType *>( defaultObjectValue() );
-}
-
-template<class T>
-void TypedObjectPlug<T>::setValue( ConstValuePtr value )
-{
-	setObjectValue( value );
-}
-
-template<class T>
-typename TypedObjectPlug<T>::ConstValuePtr TypedObjectPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
+inline typename TypedObjectPlug<T>::ConstValuePtr TypedObjectPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
 {
 	return getObjectValue<ValueType>( precomputedHash );
 }
 
-template<class T>
-void TypedObjectPlug<T>::setFrom( const ValuePlug *other )
-{
-	const TypedObjectPlug<T> *tOther = IECore::runTimeCast<const TypedObjectPlug>( other );
-	if( tOther )
-	{
-		setValue( tOther->getValue() );
-	}
-	else
-	{
-		throw IECore::Exception( "Unsupported plug type" );
-	}
-}
-
 } // namespace Gaffer

include/Gaffer/TypedObjectPlugImplementation.h

@@ -0,0 +1,112 @@
+//////////////////////////////////////////////////////////////////////////
+//
+//  Copyright (c) 2011-2012, John Haddon. All rights reserved.
+//  Copyright (c) 2011-2015, Image Engine Design Inc. All rights reserved.
+//
+//  Redistribution and use in source and binary forms, with or without
+//  modification, are permitted provided that the following conditions are
+//  met:
+//
+//      * Redistributions of source code must retain the above
+//        copyright notice, this list of conditions and the following
+//        disclaimer.
+//
+//      * Redistributions in binary form must reproduce the above
+//        copyright notice, this list of conditions and the following
+//        disclaimer in the documentation and/or other materials provided with
+//        the distribution.
+//
+//      * Neither the name of John Haddon nor the names of
+//        any other contributors to this software may be used to endorse or
+//        promote products derived from this software without specific prior
+//        written permission.
+//
+//  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
+//  IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+//  THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+//  PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+//  CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+//  EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+//  PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+//  PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+//  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+//  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+//  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+//////////////////////////////////////////////////////////////////////////
+
+/// This file contains the implementation of TypedObjectPlug. Rather than include it
+/// in a public header it is #included in TypedObjectPlug.cpp, and the relevant template
+/// classes are explicitly instantiated there. This prevents a host of problems to do with
+/// the definition of the same symbols in multiple object files. Additional TypedObjectPlug
+/// instantations may be created in similar .cpp files in other libraries.
+
+namespace Gaffer
+{
+
+template<class T>
+const IECore::RunTimeTyped::TypeDescription<TypedObjectPlug<T> > TypedObjectPlug<T>::g_typeDescription;
+
+template<class T>
+TypedObjectPlug<T>::TypedObjectPlug(
+	const std::string &name,
+	Direction direction,
+	ConstValuePtr defaultValue,
+	unsigned flags
+)
+	:	ValuePlug( name, direction, defaultValue->copy(), flags )
+{
+}
+
+template<class T>
+TypedObjectPlug<T>::~TypedObjectPlug()
+{
+}
+
+template<class T>
+bool TypedObjectPlug<T>::acceptsInput( const Plug *input ) const
+{
+	if( !ValuePlug::acceptsInput( input ) )
+	{
+		return false;
+	}
+	if( input )
+	{
+		return input->isInstanceOf( staticTypeId() );
+	}
+	return true;
+}
+
+template<class T>
+PlugPtr TypedObjectPlug<T>::createCounterpart( const std::string &name, Direction direction ) const
+{
+	return new TypedObjectPlug<T>( name, direction, defaultValue(), getFlags() );
+}
+
+template<class T>
+const typename TypedObjectPlug<T>::ValueType *TypedObjectPlug<T>::defaultValue() const
+{
+	return static_cast<const ValueType *>( defaultObjectValue() );
+}
+
+template<class T>
+void TypedObjectPlug<T>::setValue( ConstValuePtr value )
+{
+	setObjectValue( value );
+}
+
+template<class T>
+void TypedObjectPlug<T>::setFrom( const ValuePlug *other )
+{
+	const TypedObjectPlug<T> *tOther = IECore::runTimeCast<const TypedObjectPlug>( other );
+	if( tOther )
+	{
+		setValue( tOther->getValue() );
+	}
+	else
+	{
+		throw IECore::Exception( "Unsupported plug type" );
+	}
+}
+
+} // namespace Gaffer

include/Gaffer/TypedPlug.h

@@ -107,3 +107,5 @@ IE_CORE_DECLAREPTR( AtomicBox3fPlug );
 IE_CORE_DECLAREPTR( AtomicBox2iPlug );
 
 } // namespace Gaffer
+
+#include "Gaffer/TypedPlug.inl"