From 828ff9e4ffa85c4c033aa42c412765395029a366 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 29 May 2026 10:06:25 +0100 Subject: [PATCH 1/4] ClassParameter, ClassVectorParameter : Use dynamically allocated TypeId There is nothing in Cortex itself that depends on the statically allocated `TypeId::Class*ParameterTypeId` enumeration values. It is possible that recently removed modules such as IECoreNuke might depend on them as keys in parameter-handling registries. But two options are available there : 1. Use `RunTimeTyped::typeIdFromTypeName()` to get the TypeId. This will require that the `IECore` Python module is imported first. 2. Key the registry using type names instead of IDs. --- Changes | 3 +++ include/IECore/TypeIds.h | 4 ++-- python/IECore/ClassParameter.py | 2 +- python/IECore/ClassVectorParameter.py | 2 +- src/IECorePython/TypeIdBinding.cpp | 2 -- test/IECore/ClassParameterTest.py | 12 ++++++++++++ test/IECore/ClassVectorParameterTest.py | 4 ++++ 7 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Changes b/Changes index 854bcd7ea6..807c4a0031 100644 --- a/Changes +++ b/Changes @@ -1,7 +1,10 @@ 10.7.x.x (relative to 10.7.0.0a10) ======== +Breaking Changes +---------------- +- ClassParameter, ClassVectorParameter : Replaced statically allocated TypeIds with ids allocated dynamically by `IECore.registerRunTimeTyped()`. If the id is needed from C++, it can be queried at runtime with `RunTimeTyped::typeIdFromTypeName()`. 10.7.0.0a10 (relative to 10.7.0.0a9) =========== diff --git a/include/IECore/TypeIds.h b/include/IECore/TypeIds.h index 82fb4252b1..25c0652d7d 100644 --- a/include/IECore/TypeIds.h +++ b/include/IECore/TypeIds.h @@ -220,8 +220,8 @@ enum TypeId FileSequenceParameterTypeId = 289, FileSequenceVectorParameterTypeId = 290, CompoundDataBaseTypeId = 311, - ClassParameterTypeId = 313, - ClassVectorParameterTypeId = 314, + ObsoleteClassParameterTypeId = 313, // Replaced by dynamically registered TypeId + ObsoleteClassVectorParameterTypeId = 314, // Replaced by dynamically registered TypeId TransformationMatrixfParameterTypeId = 334, TransformationMatrixdParameterTypeId = 335, LineSegment3fDataTypeId = 344, diff --git a/python/IECore/ClassParameter.py b/python/IECore/ClassParameter.py index ab04206078..5db61f33c6 100644 --- a/python/IECore/ClassParameter.py +++ b/python/IECore/ClassParameter.py @@ -103,6 +103,6 @@ def _parse( args, parameter ) : parameter.setClass( args[0], int( args[1] ), args[2] ) del args[0:3] -IECore.registerRunTimeTyped( ClassParameter, IECore.TypeId.ClassParameter ) +IECore.registerRunTimeTyped( ClassParameter ) IECore.ParameterParser.registerType( ClassParameter.staticTypeId(), ClassParameter._parse, ClassParameter._serialise ) diff --git a/python/IECore/ClassVectorParameter.py b/python/IECore/ClassVectorParameter.py index c62187e7f0..1f8a95ea17 100644 --- a/python/IECore/ClassVectorParameter.py +++ b/python/IECore/ClassVectorParameter.py @@ -213,6 +213,6 @@ def _parse( args, parameter ) : parameter.setClasses( list( zip( parameterNames, classNames, classVersions ) ) ) -IECore.registerRunTimeTyped( ClassVectorParameter, IECore.TypeId.ClassVectorParameter ) +IECore.registerRunTimeTyped( ClassVectorParameter ) IECore.ParameterParser.registerType( ClassVectorParameter.staticTypeId(), ClassVectorParameter._parse, ClassVectorParameter._serialise ) diff --git a/src/IECorePython/TypeIdBinding.cpp b/src/IECorePython/TypeIdBinding.cpp index a286043e7f..da6fa89a4c 100644 --- a/src/IECorePython/TypeIdBinding.cpp +++ b/src/IECorePython/TypeIdBinding.cpp @@ -259,8 +259,6 @@ void bindTypeId() .value( "FileSequenceParameter", FileSequenceParameterTypeId ) .value( "FileSequenceVectorParameter", FileSequenceVectorParameterTypeId ) .value( "CompoundDataBase", CompoundDataBaseTypeId ) - .value( "ClassParameter", ClassParameterTypeId ) - .value( "ClassVectorParameter", ClassVectorParameterTypeId ) .value( "TransformationMatrixfParameter", TransformationMatrixfParameterTypeId ) .value( "TransformationMatrixdParameter", TransformationMatrixdParameterTypeId ) .value( "LineSegment3fData", LineSegment3fDataTypeId ) diff --git a/test/IECore/ClassParameterTest.py b/test/IECore/ClassParameterTest.py index 9b6400e44c..b12bc6e76f 100644 --- a/test/IECore/ClassParameterTest.py +++ b/test/IECore/ClassParameterTest.py @@ -148,5 +148,17 @@ def testSerialiseAndParse( self ) : self.assertEqual( c["a"].getNumericValue(), 10 ) self.assertEqual( c["b"].getNumericValue(), 20 ) + def testTypeId( self ) : + + parameter = IECore.ClassParameter( + "n", + "d", + "IECORE_OP_PATHS", + os.path.join( "maths", "multiply" ), + 2 + ) + + self.assertEqual( IECore.RunTimeTyped.typeIdFromTypeName( "ClassParameter" ), parameter.typeId() ) + if __name__ == "__main__" : unittest.main() diff --git a/test/IECore/ClassVectorParameterTest.py b/test/IECore/ClassVectorParameterTest.py index 59ebd483c5..4a4586b046 100644 --- a/test/IECore/ClassVectorParameterTest.py +++ b/test/IECore/ClassVectorParameterTest.py @@ -514,6 +514,10 @@ def testUserData( self ) : self.assertEqual( c["p0"].userData(), IECore.CompoundObject() ) + def testTypeId( self ) : + + parameter = IECore.ClassVectorParameter( "n", "d", "IECORE_OP_PATHS" ) + self.assertEqual( IECore.RunTimeTyped.typeIdFromTypeName( "ClassVectorParameter" ), parameter.typeId() ) if __name__ == "__main__" : unittest.main() From 77cbede5c8b28d512616847ceab2a40f3334cc86 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 29 May 2026 10:40:39 +0100 Subject: [PATCH 2/4] RunTimeTyped : Always allocate dynamic TypeIds for Python classes This guarantees that all RunTimeTyped subclasses implemented in Python have TypeIds in the Range FirstPythonTypeId-LastPythonTypeId. We'll make use of that property in the next commit. --- Changes | 1 + include/IECore/TypeIds.h | 8 ++-- python/IECore/registerRunTimeTyped.py | 64 +++++++++------------------ test/IECore/RunTimeTyped.py | 7 --- 4 files changed, 27 insertions(+), 53 deletions(-) diff --git a/Changes b/Changes index 807c4a0031..60ba4ca764 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,7 @@ Breaking Changes ---------------- - ClassParameter, ClassVectorParameter : Replaced statically allocated TypeIds with ids allocated dynamically by `IECore.registerRunTimeTyped()`. If the id is needed from C++, it can be queried at runtime with `RunTimeTyped::typeIdFromTypeName()`. +- RunTimeTyped : Removed `typId` argument from `registerRunTimeTyped()` Python function. TypeIds are now always allocated dynamically for Python classes. 10.7.0.0a10 (relative to 10.7.0.0a9) =========== diff --git a/include/IECore/TypeIds.h b/include/IECore/TypeIds.h index 25c0652d7d..9a02b4108c 100644 --- a/include/IECore/TypeIds.h +++ b/include/IECore/TypeIds.h @@ -315,9 +315,11 @@ enum TypeId FirstAtomsGafferTypeId = 128000, // Used by AtomsGaffer LastAtomsGafferTypeId = 128999, - // TypeIds dynamically allocated by registerRunTimeTyped (IECore Python) - FirstDynamicTypeId = 300000, - LastDynamicTypeId = 399999, + // TypeIds dynamically allocated for Python classes by + // `IECore.registerRunTimeTyped()`. Do not use for any + // other purpose. + FirstPythonTypeId = 300000, + LastPythonTypeId = 399999, LastExtensionTypeId = 399999, // Any TypeIds beyond this point can be considered safe for private internal use. diff --git a/python/IECore/registerRunTimeTyped.py b/python/IECore/registerRunTimeTyped.py index 8608629d08..54f56ba681 100644 --- a/python/IECore/registerRunTimeTyped.py +++ b/python/IECore/registerRunTimeTyped.py @@ -60,72 +60,50 @@ def __registerTypeId( typeId, typeName, baseTypeId ) : # register the new type id IECore.RunTimeTyped.registerType( typeId, typeName, baseTypeId ) -__nextDynamicRunTimeTypedId = None +__nextTypeId = 300000 # Same as `TypeId::FirstPythonTypeId` defined in TypeIds.h ## This function adds the necessary function definitions to a python # class for it to properly implement the RunTimeTyped interface. It should -# be called once for all python classes inheriting from RunTimeTyped. It also -# calls registerTypeId() for you. -# typId is optional and if not defined, this function will associate a dynamic Id -# in the range FirstDynamicTypeId and LastDynamicTypeId from TypeIds.h. -# It's necessary to specify type Id for Object derived class or anything that -# is serializable. -# If typeName is not specified then the name of the class itself is used - you may wish -# to provide an explicit typeName in order to prefix the name with a module name. -def registerRunTimeTyped( typ, typId = None, typeName = None ) : +# be called once for all python classes inheriting from RunTimeTyped. +# If `typeName` is not specified then the name of the class itself is used - +# you may wish to provide an explicit typeName in order to prefix the name +# with a module name. +def registerRunTimeTyped( typ, typeName = None ) : if typeName is None : typeName = typ.__name__ runTypedBaseClass = next( c for c in typ.__bases__ if issubclass( c, IECore.RunTimeTyped ) ) - # constants below are the same as in TypeIds.h - FirstDynamicTypeId = 300000 - LastDynamicTypeId = 399999 + # As defined in TypeIds.h + LastPythonTypeId = 399999 - # check if overwritting registration. if not hasattr( IECore.TypeId, typeName ) : - if typId is None : + # First registration. - global __nextDynamicRunTimeTypedId + global __nextTypeId + if __nextTypeId > LastPythonTypeId : + raise Exception( "Too many dynamic RunTimeTyped registered classes! You must change TypeIds.h and rebuild Cortex." ) - if __nextDynamicRunTimeTypedId is None : - __nextDynamicRunTimeTypedId = FirstDynamicTypeId - elif __nextDynamicRunTimeTypedId > LastDynamicTypeId: - raise Exception( "Too many dynamic RunTimeTyped registered classes! You must change TypeIds.h and rebuild Cortex." ) + typeId = IECore.TypeId( __nextTypeId ) + __nextTypeId += 1 - typId = __nextDynamicRunTimeTypedId + __registerTypeId( typeId, typeName, IECore.TypeId( runTypedBaseClass.staticTypeId() ) ) - __nextDynamicRunTimeTypedId += 1 + else : - __registerTypeId( IECore.TypeId( typId ), typeName, IECore.TypeId( runTypedBaseClass.staticTypeId() ) ) + # Re-registration - this can happen when reloading an Op for example. - else : - # check if the new type Id is compatible with the previously registered one. - prevTypId = getattr( IECore.TypeId, typeName ) - if prevTypId in range( FirstDynamicTypeId, LastDynamicTypeId+1 ) : - if not typId is None : - raise Exception( "Trying to set a type ID for %s previously registered as a dynamic type Id!" % typeName ) - else : - if typId is None : - raise Exception( "Trying to re-register type %s as dynamic type Id!" % typeName ) - elif typId != prevTypId : - raise Exception( "Trying to re-register %s under different type Id: %s != %s" % ( typeName, str(typId), prevTypId ) ) - # necessary when the typeid is defined in IECore/TypeIds.h and bound in TypeIdBinding.cpp, but then - # the class for that typeid is implemented in python (currently ClassParameter does this). - if IECore.RunTimeTyped.typeNameFromTypeId( prevTypId )=="" : - IECore.RunTimeTyped.registerType( prevTypId, typeName, IECore.TypeId( runTypedBaseClass.staticTypeId() ) ) - - # Retrieve the correct value from the enum - tId = getattr( IECore.TypeId, typeName ) + typeId = getattr( IECore.TypeId, typeName ) + assert( IECore.RunTimeTyped.typeNameFromTypeId( typeId ) != "" ) # add the typeId and typeName method overrides - typ.typeId = lambda x : tId + typ.typeId = lambda x : typeId typ.typeName = lambda x: typeName # add the staticTypeId, staticTypeName, baseTypeId, and baseTypeName overrides - typ.staticTypeId = staticmethod( lambda : tId ) + typ.staticTypeId = staticmethod( lambda : typeId ) typ.staticTypeName = staticmethod( lambda : typeName ) typ.baseTypeId = staticmethod( lambda : runTypedBaseClass.staticTypeId() ) typ.baseTypeName = staticmethod( lambda : runTypedBaseClass.staticTypeName() ) diff --git a/test/IECore/RunTimeTyped.py b/test/IECore/RunTimeTyped.py index 2a51bd063e..883a30c94c 100644 --- a/test/IECore/RunTimeTyped.py +++ b/test/IECore/RunTimeTyped.py @@ -131,13 +131,6 @@ def testStaticTypeBindings( self ) : def testRegisterRunTimeTyped( self ) : - # should raise because given type ID is different than the FileSequenceParameter type id - self.assertRaises( Exception, IECore.registerRunTimeTyped, IECore.FileSequenceParameter, 100009 ) - # should raise because SequenceLsOp is registered with dynamic type id. - self.assertRaises( Exception, IECore.registerRunTimeTyped, IECore.SequenceLsOp, 100009 ) - # should raise because FileSequenceParameter is registered with a non-dynamic type id - self.assertRaises( Exception, IECore.registerRunTimeTyped, IECore.FileSequenceParameter ) - # should not raise because SequenceLsOp was already registered with a dynamic type id IECore.registerRunTimeTyped( IECore.SequenceLsOp ) From ef99e1970d85bb527d3c8a386684d3e40bc20223 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 29 May 2026 11:17:51 +0100 Subject: [PATCH 3/4] RunTimeTypedWrapper : Avoid unnecessary GIL acquisitions These could absolutely destroy performance in Gaffer, resulting in workarounds of the sort seen in https://github.com/GafferHQ/gaffer/commit/11b7361171bd041a0ec8545a35aa3d6d02ea715c. Despite these, we're still finding casts that are hitting Python unnecessarily, now during ShaderNetwork generation. Now we have guaranteed that all Python classes have TypeIds in a specific range, we can avoid the GIL lock entirely for any casts to types known to be implemented in C++. This will let us remove all the workarounds in Gaffer, and be confident that we won't find other examples of performance-destroying casts in future. --- Changes | 5 +++++ include/IECorePython/RunTimeTypedBinding.inl | 10 +++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Changes b/Changes index 60ba4ca764..7977f808a5 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,11 @@ 10.7.x.x (relative to 10.7.0.0a10) ======== +Fixes +----- + +- RunTimeTyped : Fixed unnecessary overhead of `runTimeCast()` and `isInstanceOf()` for subclasses implemented in Python. The GIL is now never needed when the cast is to a C++ type. + Breaking Changes ---------------- diff --git a/include/IECorePython/RunTimeTypedBinding.inl b/include/IECorePython/RunTimeTypedBinding.inl index c6129573bc..3485e31c4b 100644 --- a/include/IECorePython/RunTimeTypedBinding.inl +++ b/include/IECorePython/RunTimeTypedBinding.inl @@ -137,9 +137,13 @@ const char *RunTimeTypedWrapper::typeName() const template bool RunTimeTypedWrapper::isInstanceOf( IECore::TypeId typeId ) const { - if( T::isInstanceOf( typeId ) ) + if( typeId < IECore::FirstPythonTypeId || typeId > IECore::LastPythonTypeId ) { - return true; + // TypeId belongs to a C++ class, so there is no need to check the + // result of Python override of `isInstanceOf()`. This is a crucial + // performance optimisation for `runTimeCast()` calls in C++, as it + // avoids taking the GIL unnecessarily. + return T::isInstanceOf( typeId ); } if( this->isSubclassed() ) @@ -182,7 +186,7 @@ bool RunTimeTypedWrapper::isInstanceOf( const char *typeName ) const return boost::python::extract( res ); } } - + catch( const boost::python::error_already_set & ) { ExceptionAlgo::translatePythonException(); From fd9bb276ca278b0ad97cb8f9821075a17f122926 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 29 May 2026 11:21:21 +0100 Subject: [PATCH 4/4] RunTimeTyped : Add todo --- python/IECore/registerRunTimeTyped.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/IECore/registerRunTimeTyped.py b/python/IECore/registerRunTimeTyped.py index 54f56ba681..34982209b5 100644 --- a/python/IECore/registerRunTimeTyped.py +++ b/python/IECore/registerRunTimeTyped.py @@ -68,6 +68,8 @@ def __registerTypeId( typeId, typeName, baseTypeId ) : # If `typeName` is not specified then the name of the class itself is used - # you may wish to provide an explicit typeName in order to prefix the name # with a module name. +## \todo It feels like this could probably be done automatically using a +# custom metaclass for RunTimeTyped? def registerRunTimeTyped( typ, typeName = None ) : if typeName is None :