vigra icon indicating copy to clipboard operation
vigra copied to clipboard

fix thread safety in multi_fft.hxx and fftw3.hxx

Open ukoethe opened this issue 14 years ago • 5 comments

a mutex lock is required around calls to the planner functions (see http://www.fftw.org/fftw3_doc/Thread-safety.html)

ukoethe avatar Nov 23 '11 18:11 ukoethe

Is thread safety an important goal for VIGRA functions, or should this be documented and left to the user? I assume you want this because the API of some functions hides the fact that the FFTW is involved, and it appears as if the functions worked on (thread-owned) image arrays only?!

hmeine avatar Nov 24 '11 10:11 hmeine

Maybe it's easier to just document the behavior and tell people what to do for multi-threading. In any case, fourierTransform(), convolveFFT() etc. are currently not thread-safe because FFTW plan creation is not.

ukoethe avatar Nov 24 '11 14:11 ukoethe

I don't think, mutex locks are the direction to go. They block execution and thus reduce the overall speedup that can be achieved by parallelism. Documentation is good, but you can achieve thread-safety without mutex locks by re-using plans. I've implemented that as FFTFilter class: see https://github.com/jschleic/simple-STORM/blob/master/storm/fftfilter.hxx The constructor creates the plans needed and applyFourierFilter() only calls fftw_execute() that is thread safe.

Another possible API could be adding an optional PlanObject, so that a user can create a plan from the master thread via a function

plan = createForwardPlan(srcImageRange(exampleImage));

and pass this plan to the actual execute-function as optional object from different threads

void fourierTransform(srcUpperLeft, srcLowerRight, srcAcc,
                      FFTWComplexImage::traverser destUpperLeft, FFTWComplexImage::Accessor dest,
                      FFTWPlan plan = FFTWPlan());

jschleic avatar Nov 28 '11 11:11 jschleic

But even if plans are reused, their creation must be protected by a mutex. BTW, the plan class you propose already exists in multi_fft.hxx.

ukoethe avatar Nov 28 '11 12:11 ukoethe

Oh, multi_fft.hxx in fact has all the functionality I wanted. But it's not thread-safe (even when locking the init functions), since the execute-functions modify class object variables realArray, realKernel etc. Thus execute() cannot be called simultaneously from different threads, if I understand the code correctly. Qt has a good section about reentrant vs. thread-safe: http://doc.qt.nokia.com/latest/threads-reentrancy.html.

Using multi_fft-functionality one would need a different FFTWPlan object for every thread instead of one shared object as in my proposal. Then you indeed need to lock the init functions.

But how would you implement a mutex lock? Is there a general concept for portable mutexes? As far as I know this depends on the threading library used. For example OpenMP would use "#pragma omp critical", Qt has a "QMutexLocker" class and pthreads have a function for mutexes. What functions are used for native Win32 threads? ;-)

jschleic avatar Nov 28 '11 13:11 jschleic