Skip to content

Commit

Permalink
Merge pull request #5362 from johnhaddon/getValueOptimisationPR
Browse files Browse the repository at this point in the history
Improve `ValuePlug::getValue()` performance
  • Loading branch information
johnhaddon authored Jul 11, 2023
2 parents fd2e5c0 + 0945ec9 commit 8e909c8
Show file tree
Hide file tree
Showing 19 changed files with 348 additions and 206 deletions.
3 changes: 3 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------------
Expand Down Expand Up @@ -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
-----
Expand Down
8 changes: 4 additions & 4 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -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" ],
Expand All @@ -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" ],
},
},

Expand Down Expand Up @@ -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" ],
Expand Down Expand Up @@ -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" ],
Expand Down
4 changes: 3 additions & 1 deletion include/Gaffer/NumericPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace Gaffer
{

template<typename T>
class GAFFER_API NumericPlug : public ValuePlug
class IECORE_EXPORT NumericPlug : public ValuePlug
{

public :
Expand Down Expand Up @@ -103,3 +103,5 @@ IE_CORE_DECLAREPTR( FloatPlug );
IE_CORE_DECLAREPTR( IntPlug );

} // namespace Gaffer

#include "Gaffer/NumericPlug.inl"
48 changes: 48 additions & 0 deletions include/Gaffer/NumericPlug.inl
Original file line number Diff line number Diff line change
@@ -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
18 changes: 17 additions & 1 deletion include/Gaffer/Plug.inl
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -52,7 +69,6 @@ const T *Plug::getInput() const
return IECore::runTimeCast<const T>( m_input );
}


template<typename T>
T *Plug::source()
{
Expand Down
2 changes: 2 additions & 0 deletions include/Gaffer/TypedObjectPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,5 @@ IE_CORE_DECLAREPTR( AtomicCompoundDataPlug );
IE_CORE_DECLAREPTR( PathMatcherDataPlug );

} // namespace Gaffer

#include "TypedObjectPlug.inl"
76 changes: 3 additions & 73 deletions include/Gaffer/TypedObjectPlug.inl
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
112 changes: 112 additions & 0 deletions include/Gaffer/TypedObjectPlugImplementation.h
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions include/Gaffer/TypedPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,5 @@ IE_CORE_DECLAREPTR( AtomicBox3fPlug );
IE_CORE_DECLAREPTR( AtomicBox2iPlug );

} // namespace Gaffer

#include "Gaffer/TypedPlug.inl"
Loading

0 comments on commit 8e909c8

Please sign in to comment.