Support onetbb and boost 1.87.0 on FreeBSD #1459

Martinfx · 2025-03-11T21:49:51Z

Generally describe what this PR will do, and why it is needed.

I added support for onetbb (Tested with onetbb 2022.0.0)
I added support for boost 1.87.0 (Tested with boost 1.87.0)
I tested on FreeBSD 14.2 amd64 and clang 18

Related Issues

List any Issues this PR addresses or solves

Dependencies

List any other unmerged PRs that this PR depends on

Breaking Changes

I added support for onetbb (Tested with onetbb 2022.0.0)
I added support for boost 1.87.0 (Tested with boost 1.87.0)
I tried to test with the test suite, but I don't know which python library to install (imath python library)
It is an attempt for convert to modern onetbb

Checklist

I have read the contribution guidelines.
I have updated the documentation, if applicable.
I have tested my change(s) in the test suite, and added new test cases where necessary.
My code follows the Cortex project's prevailing coding style and conventions.

Martinfx · 2025-03-12T09:51:40Z

I tried to run some tests, but serialising and deserialising test failed.

scons: Building targets ...
/usr/local/bin/python3.11 test/IECore/All.py --verbose
test (AngleConversionTest.AngleConversionTest.test) ... ok
testCubic (BezierAlgoTest.BezierAlgoTest.testCubic) ... ok
testQuadratic (BezierAlgoTest.BezierAlgoTest.testQuadratic) ... ok
test (SimpleTypedData.BoolDataTest.test) ... ok
testLineSegmentData (SimpleTypedData.BoolDataTest.testLineSegmentData) ... ok
testStreaming (SimpleTypedData.BoolDataTest.testStreaming) ... ok
test (BoolVectorData.BoolVectorDataTest.test) ... ok
testHasBase (BoolVectorData.BoolVectorDataTest.testHasBase) ... ok
testStrAndRepr (BoolVectorData.BoolVectorDataTest.testStrAndRepr) ... ok
testAssignment (ImathVectorData.Box3dVectorDataTest.testAssignment)
Test Box3dVectorData assignment ... ok
testByValueItem (ImathVectorData.Box3dVectorDataTest.testByValueItem)
Test Box3dVectorData by value return type ... ok
testComparison (ImathVectorData.Box3dVectorDataTest.testComparison)
Test Box3dVectorData comparison function ... ok
testConstructors (ImathVectorData.Box3dVectorDataTest.testConstructors)
Test Box3dVectorData constructors ... ok
testContains (ImathVectorData.Box3dVectorDataTest.testContains)
Test Box3dVectorData contains function ... ok
testCopyOnWrite (ImathVectorData.Box3dVectorDataTest.testCopyOnWrite)
Test Box3dVectorData copy-on-write behavior ... ok
testDivOperations (ImathVectorData.Box3dVectorDataTest.testDivOperations)
Test Box3dVectorData division operations ... ok
testEquality (ImathVectorData.Box3dVectorDataTest.testEquality)
Test Box3dVectorData equality function ... ok
testExtend (ImathVectorData.Box3dVectorDataTest.testExtend)
Test Box3dVectorData extend function ... ok
testHasBase (ImathVectorData.Box3dVectorDataTest.testHasBase) ... ok
testMultOperations (ImathVectorData.Box3dVectorDataTest.testMultOperations)
Test Box3dVectorData multiplication operations ... ok
testNegativeIndexing (ImathVectorData.Box3dVectorDataTest.testNegativeIndexing)
Test Box3dVectorData negative indexing ... ok
testResize (ImathVectorData.Box3dVectorDataTest.testResize)
Test Box3dVectorData resizing ... ok
testSerialising (ImathVectorData.Box3dVectorDataTest.testSerialising)
Test serialising and deserialising ... scons: *** [test/IECore/resultsPython.txt] Error -4
scons: building terminated because of errors.

johnhaddon

Thanks for the PR - being able to build against more recent Boost/TBB will be great.

I've made several comments inline on the Boost changes, as I think we can achieve what we want without having two code paths - we use a recent enough boost that the new code will work with our version too. Having just one code path makes things simpler and much easier to maintain.

I haven't reviewed the TBB changes in any detail, because we won't be able to maintain two code paths there - the code is too complex to even consider it. What we need is a single version that will work with either TBB version (possibly with very small, isolated #if blocks). I would also want to split these changes out into separate commits - one commit per topic makes things easier to review and debug.

If you are willing, I suggest we focus on simplifying the Boost changes and getting those merged on their own first. Then once we have that established we can look at the thornier issue of TBB.

Many thanks once again,
John

johnhaddon · 2025-03-13T12:05:21Z

include/IECoreScene/private/PrimitiveAlgoUtils.h

@@ -48,8 +48,15 @@

 #include "boost/format.hpp"

+#include <tbb/version.h>


The tbb/version.h header doesn't seem to exist in TBB 2020, so is breaking the build here :

src/IECore/IndexedIOAlgo.cpp:36:10: fatal error: tbb/version.h: No such file or directory 36 | #include <tbb/version.h>

johnhaddon · 2025-03-13T12:07:08Z

src/IECoreGL/Selector.cpp

+#include <boost/version.hpp>
+#if BOOST_VERSION >= 107000
+#include <boost/timer/timer.hpp>
+#else
+#include <boost/timer.hpp>
+#endif


We're not actually using boost::timer in this file, so we can simply delete the include instead.

johnhaddon · 2025-03-13T12:28:41Z

src/IECoreImage/ClientDisplayDriver.cpp

+#include <boost/version.hpp>
+#if BOOST_VERSION >= 106600
+#include <boost/asio.hpp>
+#include <boost/asio/io_context.hpp>
+#else
+#include <boost/asio.hpp>
+#include <boost/asio/io_service.hpp>
+#endif


I don't think this change is necessary - in both boost versions, asio.hpp includes everything.

johnhaddon · 2025-03-13T12:31:17Z

src/IECoreImage/ClientDisplayDriver.cpp

+#if BOOST_VERSION >= 106600
+		boost::asio::io_context m_service;
+#else
 		boost::asio::io_service m_service;
+#endif


I think this can be simpler. io_context is available in 1.80 (the version we're building with), and io_service is just a typedef referencing it. So I think we can just change to io_context unconditionally, and have one version of the code that works for both boost versions.

johnhaddon · 2025-03-13T12:33:40Z

src/IECoreImage/ClientDisplayDriver.cpp

+#if BOOST_VERSION >= 106600
+	boost::asio::io_context io_context;
+	tcp::resolver resolver(io_context);
+	boost::system::error_code error;
+	auto endpoints = resolver.resolve(m_data->m_host, m_data->m_port, error);
+
+	if (!error)
+	{
+	    error = boost::asio::error::host_not_found;
+   	 	for (auto it = endpoints.begin(); it != endpoints.end() && error; ++it)
+    	{
+        	m_data->m_socket.close();
+        	m_data->m_socket.connect(*it, error);
+    	}
+	}
+
+	if (error)
+	{
+    	throw Exception("Could not connect to remote display driver server: " + error.message());
+	}
+#else


As mentioned above, I think we should be able to have one version of the code here, that works for both Boost versions.

johnhaddon · 2025-03-13T12:35:11Z

src/IECoreImage/DisplayDriverServer.cpp

+#if BOOST_VERSION >= 106600
+class DisplayDriverServer::Session : public RefCounted
+{
+	public:
+
+		Session( boost::asio::io_context& io_service, MergeMap& mergeMap );
+        ~Session() override;
+
+        boost::asio::ip::tcp::socket& socket();
+        void start();
+
+    private:
+
+  		void handleReadHeader( const boost::system::error_code& error );
+      	void handleReadOpenParameters( const boost::system::error_code& error );
+        void handleReadDataParameters( const boost::system::error_code& error );
+        void sendResult( DisplayDriverServerHeader::MessageType msg, size_t dataSize );
+        void sendException( const char *message );
+
+    private:
+		boost::asio::ip::tcp::socket m_socket;
+		DisplayDriverPtr m_displayDriver;
+		DisplayDriverServerHeader m_header;
+		CharVectorDataPtr m_buffer;
+  		MergeMap& m_mergeMap;
+		std::optional<int> m_mergeId;
+};
+#else


It will be too hard to maintain this amount of almost-identical code in separate #if blocks. As above, I think we can simply change to io_context unconditionally.

johnhaddon · 2025-03-13T12:41:25Z

src/IECore/FileNameParameter.cpp

-#include "boost/filesystem/convenience.hpp"
+#if BOOST_VERSION >= 108500
+#include <boost/filesystem/path.hpp>
+#else
+#include <boost/filesystem/convenience.hpp>
+#endif


We can just drop the convenience.hpp include completely - see below.

johnhaddon · 2025-03-13T12:42:32Z

src/IECore/FileNameParameter.cpp

@@ -87,8 +91,11 @@ bool FileNameParameter::valueValid( const Object *value, std::string *reason ) c
 	// extensions check
 	if( extensions().size() )
 	{
+#if BOOST_VERSION >= 108500
+		string ext = boost::filesystem::path(boost::filesystem::path( s->readable())).extension().string();


I believe this line will work in Boost 1.80 as well, so we can simply replace the old code completely. I think we have one-too-many boost::filesystem::path() calls though - we only need one.

support onetbb and boost 1.87.0

ac441a7

Martinfx changed the title ~~support onetbb and boost 1.87.0~~ support onetbb and boost 1.87.0 on FreeBSD Mar 11, 2025

Martinfx changed the title ~~support onetbb and boost 1.87.0 on FreeBSD~~ Support onetbb and boost 1.87.0 on FreeBSD Mar 11, 2025

support onetbb and boost 1.87.0

1dd417f

johnhaddon reviewed Mar 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support onetbb and boost 1.87.0 on FreeBSD #1459

Support onetbb and boost 1.87.0 on FreeBSD #1459

Martinfx commented Mar 11, 2025 •

edited

Loading

Martinfx commented Mar 12, 2025 •

edited

Loading

johnhaddon left a comment

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

johnhaddon Mar 13, 2025

		@@ -48,8 +48,15 @@

		#include "boost/format.hpp"

		#include <tbb/version.h>

Support onetbb and boost 1.87.0 on FreeBSD #1459

Are you sure you want to change the base?

Support onetbb and boost 1.87.0 on FreeBSD #1459

Conversation

Martinfx commented Mar 11, 2025 • edited Loading

Related Issues

Dependencies

Breaking Changes

Checklist

Martinfx commented Mar 12, 2025 • edited Loading

johnhaddon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Martinfx commented Mar 11, 2025 •

edited

Loading

Martinfx commented Mar 12, 2025 •

edited

Loading