Use std::thread instead of boost::thread?

[sercero]

Hello @paroj, I'm trying to replace the use of boost::thread with that of std::thread and I want to ask you some questions.

On one hand if you think that boost::thread should remain as a third option or be replaced by std::thread entirely.

Also, can you confirm that this replacement is valid?

//boost::recursive_mutex::scoped_lock soundLock(mSoundMutex);
std::lock_guard<std::recursive_mutex> soundLock(mSoundMutex);

Another thing is that I'm trying to find a replacement for this statement, but it goes over my head:

mUpdateThread = OGRE_NEW_T(boost::thread, Ogre::MEMCATEGORY_GENERAL)(boost::function0<void>(&OgreOggSoundManager::threadUpdate, this));

Can you help?

Thanks.

NOTE: If you want I can submit the pull request and you review there. But I wasn't able to test anything because for now the code is not compiling due to the mUpdateThread creation.

[paroj]

you could use OgreThreadDefines.h, which would make this follow whatever Ogre is configured with.

Your example would then become:

OGRE_LOCK_MUTEX_NAMED(soundLock, mSoundMutex);

your second example should be (untested):

OGRE_THREAD_CREATE(tmp, std::bind(&OgreOggSoundManager::threadUpdate, this));
mUpdateThread = tmp;

the bind call does the equivalent of:

static OgreOggSoundManager* object = this;

void foo() { object->threadUpdate(); }

std::thread tmp(foo);

[sercero]

@paroj I haved reviewed this option and there doesn't seem to be a preprocessor define for std::lock_guard<std::recursive_mutex> lock(mMutex); which is used extensively in OgreOggSound.

Do you think I should add it to OgreThreadDefines.h?

The problem with that is OgreOggSound will now depend on Ogre version >= 1.13

[paroj]

std::lock_guard lock(mMutex); which is used extensively in OgreOggSound.

wont the plain OGRE_LOCK_MUTEX do? e.g.

https://github.com/OGRECave/ogre/blob/7b03a64b848c0fa82cb15c7f20eb5a9efc723783/OgreMain/src/OgreHardwareBufferManager.cpp#L72-L78

[sercero]

No, because for STD OGRE_LOCK_MUTEX maps to OGRE_WQ_LOCK_MUTEX which is a mutex of type std::unique_lock<std::recursive_mutex> and I need std::lock_guard<std::recursive_mutex> (otherwise I have to make significant changes to the code).

Anyway, I ended up putting a variable in CMake with a number:

 * Specifies whether to use threads for streaming
 * 0 - No multithreading
 * 1 - POCO multithreaded
 * 2 - boost:thread multithreaded
 * 3 - std::thread multithreaded

The other threading models are going to be deprecated anyway...

[paroj]

No, because for STD OGRE_LOCK_MUTEX maps to OGRE_WQ_LOCK_MUTEX which is a mutex of type std::unique_lock and I need std::lock_guard (otherwise I have to make significant changes to the code).

but as far as I see std::lock_guard has no API and is just sits in the scope without being passed around - so std::unique_lock should be a strict superset and thus could be used in place.

Anyway.. I think it is also fine to just use std::thread without making anything configurable here, if that makes your life easier.

[sercero]

but as far as I see std::lock_guard has no API and is just sits in the scope without being passed around - so std::unique_lock should be a strict superset and thus could be used in place.

From my googling std::unique_lock and std::lock_guard were used in different circumstances, what I saw at the moment was that in order to replace std::lock_guard I would need to do an extensive refactoring that would be very difficult for me (very basic knowledge of threads).

Anyway.. I think it is also fine to just use std::thread without making anything configurable here, if that makes your life easier.

Yes, it would have made my life easier 😅, but the actual solution I think is not that bad, the code had minimal changes. My main concern is with testing because I don't want to install boost nor POCO so I didn't want to touch that code.

[paroj]

From my googling std::unique_lock and std::lock_guard were used in different circumstances, what I saw at the moment was that in order to replace std::lock_guard I would need to do an extensive refactoring that would be very difficult for me (very basic knowledge of threads).

I also just googled around: https://stackoverflow.com/a/20516876/927543

all we need these things to do is to lock a mutex on construction and unlock it when it goes out of scope. All of unique_lock, lock_guard and scoped_lock (c++17) do that. There is only a difference when you want to lock multiple things at once - which we dont.

[sercero]

I understand, but my objective was to find a replacement for what is being used in the code which is boost::recursive_mutex::scoped_lock.

And the replacement I found was std::lock_guard<std::recursive_mutex> because it has the same behaivour of releasing the lock automatically. I am not checking the code to see if that feature is actually being used so I looked for the most similar replacement.

std::lock_guard boost scoped_lock vs plain lock/unlock

On the other hand the most voted answer here supports what you are saying: BOOST scoped_lock replacement in C++11

I will see what happens when using std::unique_lock vs std::scoped_lock, but my fear is that threading is not something easy to test for bugs and naive tests might not find a bug.

In my mind the current threading code with boost has been tested and that is why I want to find the most similar equivalent in C++11 std::thread.

[paroj]

my objective was to find a replacement for what is being used in the code which is boost::recursive_mutex::scoped_lock.

yes, thats why I suggested using OGRE_THREAD* defines in the first place:

https://github.com/OGRECave/ogre/blob/5e6260fa5783d3c9e1504f1cd6bb372843ca1032/OgreMain/include/Threading/OgreThreadDefinesBoost.h#L49

the STD variant is currently used in Ogre Main without any issues so far..

I have the feeling we are talking past one another somehow, so let me try again:

• you could look at https://github.com/OGRECave/ogre/blob/master/OgreMain/include/Threading/OgreThreadDefinesBoost.h for a OGRE_ define that matches the current boost usage and replace them
• compile Ogre & audiovideo with boost to see whether things still work
• compile Ogre & audiovideo with STD and things should still work, but use STD now

[sercero]

Oh boy, now I see that you already determined that they were the same now I understand...

OK, I will do a second try of using OgreThreadDefines using the Boost one as an example, good idea...

[sercero]

@paroj I have a working version with a switch to select a number: 1 - POCO, 2 - BOOST, 3 - STD that has the minimal changes to the original code.

And I am also trying to develop the version that uses OgreThreadDefines.h <--- this version is almost ready but I have a very strange problem.

For some reason the defines are being taken into account (I know this because if there is a redefinition the compiler gives a warning) but they are being added in a strange way like interpreting OGRE_THREAD_PROVIDER in different ways along different files.

What do you prefer?

1) I submit the working code, but without using OgreThreadDefines.h

2) I submit the non-working code using OgreThreadDefines.h and you help me fix the problem.

[paroj]

Nr. 2