CollectScenes : Multithread hashSet() and computeSet()

This gives a 3x speedup in `CollectScenesTest.testSetPerformance`.

Perhaps more interestingly, it also gives almost a 2x speedup in `SetQueryTest.testScaling()`, but almost half of that speedup is due to the change in hash cache policy _alone_. In `testScaling()`, the same set is required by every location in the scene, but we are visiting enough locations that the hash cache is under significant pressure. By moving `out.set` to the TaskCollaboration policy, the set hash is stored in the shared central cache, from which it is exceedingly unlikely to be evicted (because per-location hashes are not stored in the global cache).

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.
 

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;
 

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()

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<vector<string>::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<const RootTree>( 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 );
 }