Parallelization bug #23 and cmake_constant_file #27
Conversation
MartKro
commented
Jan 14, 2020
MartKro
commented
- Fixed the parallelization bug Parallelization with RELIC #23 which prevents possible parallelization due to the introduced mutex. Now the mutex is also protected using __thread but since it is implementation dependent the flag can be easily disabled
- Changed the introduction of constants from CMAKE to an extra file cmake_constants.h.in
- Fixed the parallelization bug which prevents possible parallelization due to the introduced mutex. Now the mutex is also protected using __thread but since it is implementation dependent the flag can be easily disabled - Changed the introduction of constants from CMAKE to an extra file cmake_constants.h.in Signed-off-by: Martin Kromm <[email protected]>
| static std::mutex relic_mutex; | ||
| #endif |
There was a problem hiding this comment.
So if multithreading via pthreads is enabled in Relic, then you give every thread its own mutex in thread local storage. Hence, every thread locks its own mutex to access its own relic context object (also in TLS). I do not see the need for this mutex then.
There was a problem hiding this comment.
I've already considered it to disable the mutex completely if the preprocessor flag is defined (and this was done in a previous implementation of mine). However after some considerations I decided against it since the method only enables thread safety for a single thread method; that is, if a user decides to use pthread but then also uses openmp concurrently thread safety cannot be guranteed then, and it is not obvious to see for a user if he doesn't study the code.
I rather prefer to implement concurrency conserative since errors are often easy to make and hard to find later. Besides that there speaks nothing against your suggestion.
|
This doesn't introduce thread local mutexes when using OpenMP, right? Could you also declare the mutex threadprivate if OpenMP is enabled? (like it's done here with the context object: https://github.com/relic-toolkit/relic/blob/c25edf6520e5e4b39663f0d0594db7186440be5c/src/relic_core.c#L95) |
