diff --git a/Changes.md b/Changes.md index 927854660c1..ee8ec5db7b7 100644 --- a/Changes.md +++ b/Changes.md @@ -11,6 +11,7 @@ Features Improvements ------------ +- CollectScenes : Improved performance when computing sets, with a 3x speedup being seen in one particular benchmark. - LightTool : Changed spot light and quad light edge tool tip locations so that they follow the cone and edge during drag. - Arnold : Improved speed of translation of encapsulated scenes when using many threads. diff --git a/include/GafferScene/CollectScenes.h b/include/GafferScene/CollectScenes.h index 37b9714ad99..43a6a8e2392 100644 --- a/include/GafferScene/CollectScenes.h +++ b/include/GafferScene/CollectScenes.h @@ -77,6 +77,9 @@ class GAFFERSCENE_API CollectScenes : public SceneProcessor void hash( const Gaffer::ValuePlug *output, const Gaffer::Context *context, IECore::MurmurHash &h ) const override; void compute( Gaffer::ValuePlug *output, const Gaffer::Context *context ) const override; + Gaffer::ValuePlug::CachePolicy hashCachePolicy( const Gaffer::ValuePlug *output ) const override; + Gaffer::ValuePlug::CachePolicy computeCachePolicy( const Gaffer::ValuePlug *output ) const override; + void hashBound( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const override; Imath::Box3f computeBound( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent ) const override; diff --git a/python/GafferSceneTest/CollectScenesTest.py b/python/GafferSceneTest/CollectScenesTest.py index 53878206403..b5e703abf25 100644 --- a/python/GafferSceneTest/CollectScenesTest.py +++ b/python/GafferSceneTest/CollectScenesTest.py @@ -42,6 +42,7 @@ import IECore import Gaffer +import GafferTest import GafferScene import GafferSceneTest @@ -509,5 +510,57 @@ def testNoContextVariable( self ) : { "frame", "framesPerSecond", "scene:path" } ) + @GafferTest.TestRunner.PerformanceTestMethod() + def testSetPerformance( self ) : + + # Collecting sets from 1000 instancers, each with a differing + # number of points. + + random = Gaffer.Random() + random["seedVariable"].setValue( "collect:rootName" ) + random["floatRange"][0].setValue( 10 ) + random["floatRange"][0].setValue( 1000 ) + + plane = GafferScene.Plane() + plane["divisions"]["y"].setInput( random["outFloat"] ) + + sphere = GafferScene.Sphere() + sphere["sets"].setValue( "A" ) + + planeFilter = GafferScene.PathFilter() + planeFilter["paths"].setValue( IECore.StringVectorData( [ "/plane" ] ) ) + + instancer = GafferScene.Instancer() + instancer["in"].setInput( plane["out"] ) + instancer["filter"].setInput( planeFilter["out"] ) + instancer["prototypes"].setInput( sphere["out"] ) + + collect = GafferScene.CollectScenes() + collect["in"].setInput( instancer["out"] ) + collect["rootNames"].setValue( IECore.StringVectorData( [ "root{}".format( i ) for i in range( 0, 1000 ) ] ) ) + + with GafferTest.TestRunner.PerformanceScope() : + collect["out"].set( "A" ) + + def testSetHashStability( self ) : + + randomChoice = Gaffer.RandomChoice() + randomChoice.setup( Gaffer.StringPlug() ) + randomChoice["choices"]["values"].setValue( IECore.StringVectorData( [ "A", "" ] ) ) + randomChoice["choices"]["weights"].setValue( IECore.FloatVectorData( [ 1, 1 ] ) ) + randomChoice["seedVariable"].setValue( "collect:rootName" ) + + cube = GafferScene.Cube() + cube["sets"].setInput( randomChoice["out"] ) + + collect = GafferScene.CollectScenes() + collect["in"].setInput( cube["out"] ) + collect["rootNames"].setValue( IECore.StringVectorData( [ "root{}".format( i ) for i in range( 0, 1000 ) ] ) ) + + h = collect["out"].setHash( "A" ) + for i in range( 0, 100 ) : + Gaffer.ValuePlug.clearHashCache() + self.assertEqual( collect["out"].setHash( "A" ), h ) + if __name__ == "__main__": unittest.main() diff --git a/src/GafferScene/CollectScenes.cpp b/src/GafferScene/CollectScenes.cpp index ee55d668462..5b75e18c9c3 100644 --- a/src/GafferScene/CollectScenes.cpp +++ b/src/GafferScene/CollectScenes.cpp @@ -45,6 +45,8 @@ #include "boost/container/flat_map.hpp" +#include "tbb/parallel_reduce.h" + #include "fmt/format.h" using namespace std; @@ -164,6 +166,12 @@ class RootTree : public IECore::Data return m_roots; } + using RootRange = tbb::blocked_range::const_iterator>; + RootRange rootRange() const + { + return RootRange( m_roots.begin(), m_roots.end() ); + } + private : LocationPtr m_treeRoot; @@ -189,6 +197,11 @@ class CollectScenes::SourceScope : public Context::EditableScope { } + SourceScope( const ThreadState &threadState, const InternedString &rootVariable ) + : EditableScope( threadState ), m_rootVariable( rootVariable ) + { + } + void setRoot( const std::string *root ) { if( !m_rootVariable.string().empty() ) @@ -425,6 +438,24 @@ void CollectScenes::compute( Gaffer::ValuePlug *output, const Gaffer::Context *c SceneProcessor::compute( output, context ); } +Gaffer::ValuePlug::CachePolicy CollectScenes::hashCachePolicy( const Gaffer::ValuePlug *output ) const +{ + if( output == outPlug()->setPlug() ) + { + return ValuePlug::CachePolicy::TaskCollaboration; + } + return SceneProcessor::hashCachePolicy( output ); +} + +Gaffer::ValuePlug::CachePolicy CollectScenes::computeCachePolicy( const Gaffer::ValuePlug *output ) const +{ + if( output == outPlug()->setPlug() ) + { + return ValuePlug::CachePolicy::TaskCollaboration; + } + return SceneProcessor::computeCachePolicy( output ); +} + void CollectScenes::hashBound( const ScenePath &path, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const { SourcePathScope sourcePathScope( context, this, path ); @@ -681,15 +712,46 @@ void CollectScenes::hashSet( const IECore::InternedString &setName, const Gaffer const PathMatcherDataPlug *inSetPlug = inPlug()->setPlug(); const StringPlug *sourceRootPlug = this->sourceRootPlug(); + const std::string rootNameVariable = rootNameVariablePlug()->getValue(); - SourceScope sourceScope( context, rootNameVariablePlug()->getValue() ); - for( const auto &root : rootTree->roots() ) - { - sourceScope.setRoot( &root ); - inSetPlug->hash( h ); - sourceRootPlug->hash( h ); - h.append( root ); - } + const ThreadState &threadState = ThreadState::current(); + tbb::task_group_context taskGroupContext( tbb::task_group_context::isolated ); + + const IECore::MurmurHash setsHash = parallel_deterministic_reduce( + + rootTree->rootRange(), + + IECore::MurmurHash(), + + [&] ( const RootTree::RootRange &range, const MurmurHash &x ) { + + SourceScope sourceScope( threadState, rootNameVariable ); + + MurmurHash result = x; + for( auto it = range.begin(); it != range.end(); ++it ) + { + const string &root = *it; + sourceScope.setRoot( &root ); + inSetPlug->hash( result ); + sourceRootPlug->hash( result ); + result.append( root ); + } + return result; + + }, + + [] ( const MurmurHash &x, const MurmurHash &y ) { + + MurmurHash result = x; + result.append( y ); + return result; + }, + + taskGroupContext + + ); + + h.append( setsHash ); } IECore::ConstPathMatcherDataPtr CollectScenes::computeSet( const IECore::InternedString &setName, const Gaffer::Context *context, const ScenePlug *parent ) const @@ -700,33 +762,59 @@ IECore::ConstPathMatcherDataPtr CollectScenes::computeSet( const IECore::Interne rootTree = boost::static_pointer_cast( rootTreePlug()->getValue() ); } - PathMatcherDataPtr setData = new PathMatcherData; - PathMatcher &set = setData->writable(); - const PathMatcherDataPlug *inSetPlug = inPlug()->setPlug(); const StringPlug *sourceRootPlug = this->sourceRootPlug(); + const std::string rootNameVariable = rootNameVariablePlug()->getValue(); - SourceScope sourceScope( context, rootNameVariablePlug()->getValue() ); - ScenePlug::ScenePath prefix; - for( const auto &root : rootTree->roots() ) - { - sourceScope.setRoot( &root ); - ConstPathMatcherDataPtr inSetData = inSetPlug->getValue(); - const PathMatcher &inSet = inSetData->readable(); - if( !inSet.isEmpty() ) - { - ScenePlug::stringToPath( root, prefix ); - const string root = sourceRootPlug->getValue(); - if( !root.empty() ) - { - set.addPaths( inSet.subTree( root ), prefix ); - } - else + const ThreadState &threadState = ThreadState::current(); + tbb::task_group_context taskGroupContext( tbb::task_group_context::isolated ); + + IECore::PathMatcher set = parallel_reduce( + + rootTree->rootRange(), + + PathMatcher(), + + [&] ( const RootTree::RootRange &range, const IECore::PathMatcher &x ) { + + SourceScope sourceScope( threadState, rootNameVariable ); + + PathMatcher result = x; + ScenePlug::ScenePath prefix; + for( auto it = range.begin(); it != range.end(); ++it ) { - set.addPaths( inSet, prefix ); + const string &root = *it; + sourceScope.setRoot( &root ); + ConstPathMatcherDataPtr inSetData = inSetPlug->getValue(); + const PathMatcher &inSet = inSetData->readable(); + if( !inSet.isEmpty() ) + { + ScenePlug::stringToPath( root, prefix ); + const string sourceRoot = sourceRootPlug->getValue(); + if( !sourceRoot.empty() ) + { + result.addPaths( inSet.subTree( sourceRoot ), prefix ); + } + else + { + result.addPaths( inSet, prefix ); + } + } } - } - } + return result; + + }, + + [] ( const PathMatcher &x, const PathMatcher &y ) { + + PathMatcher result = x; + result.addPaths( y ); + return result; + }, + + taskGroupContext + + ); - return setData; + return new PathMatcherData( set ); }