mreineck / pocketfft

Fork of https://gitlab.mpcdf.mpg.de/mtr/pocketfft to simplify external contributions
BSD 3-Clause "New" or "Revised" License
71 stars 34 forks source link

Avoid using thread_local to work-around mingw bug #3

Open peterbell10 opened 2 years ago

peterbell10 commented 2 years ago

Ref #1, #2, msys2/MINGW-packages#10164

The mingw issue suggests that thread-local storage is buggy in mingw, so avoiding thread_local might help. I don't have a mingw setup to test though.

cc @carlkl, @rgommers

rgommers commented 2 years ago

Cc @matthew-brett also

mreineck commented 2 years ago

Thanks a lot for this, @peterbell10 ! I don't have access to a MinGW installation either, so let's see what @carlkl says.

mreineck commented 2 years ago

If I read the changes correctly, the evolution of this multi-threading code in https://github.com/mreineck/ducc/blob/ducc0/src/ducc0/infra/threading.cc should be unaffected by this problem; do you agree @peterbell10 ?

Pity we can't switch to this implementation yet, at least for scipy.

peterbell10 commented 2 years ago

Yes, because you use pass scheduler explicitly instead of via thread-local storage it should be equivalent.

carlkl commented 2 years ago

@peterbell10, I will test this as soon as I can with msys2 scipy.

matthew-brett commented 2 years ago

As a data point, I tried naively copying pocketfft_hdronly.h here to the Scipy source tree, and compiling with

but in both cases, Scipy.fft still hangs.

matthew-brett commented 2 years ago

Can I ask - what is the best way of replicating the hang without compiling Scipy? I have not observed a hang on my machine when using mingw-w64 to compile the demo from this repo : https://github.com/mreineck/pocketfft/issues/1#issuecomment-976423438 - even running the demo hundreds of times.

carlkl commented 2 years ago

I now tested this PR with scipy. Sorry for the long time ... With MT scipy still hangs unfortunately. I extracted a stacktrace during the hang: pocketffthang.log.txt (reformatted and commented). However, I don't really get any more insight from it though. It seems to hang on shutdown of the thread pool, but why? Maybe the experts?

mati865 commented 2 years ago

This stacktrace looks like issue LLD had with mingw-w64 GCC: https://reviews.llvm.org/D102944 Replacing exit with _Exit might help.

To clarify:

The mingw issue suggests that thread-local storage is buggy in mingw

Emutls that GCC uses is buggy on Windows but it's totally unrelated to mingw-w64, TLS in Clang+mingw-w64 works just fine.

carlkl commented 2 years ago

I don't think that this hang is related to DLL unloading but I may be wrong. In my opinion it has to do with threads shutdown. And there is no exit in pocketfft code. EDIT: I'm using the latest and greatest UCRT64 gcc-11.2 (msys2)

carlkl commented 2 years ago

I have to correct myself: The hang happens at process termination. @mati865, is this related to https://sourceforge.net/p/mingw-w64/bugs/859/?

carlkl commented 2 years ago

as reference: https://github.com/msys2/MINGW-packages/issues/10164#issuecomment-978033320. Seems to be the __cxa_thread_atexit hell... Still investigating.

mati865 commented 2 years ago

Sorry foe the late reply, this notification got lost in my backlog.

I have to correct myself: The hang happens at process termination. @mati865, is this related to https://sourceforge.net/p/mingw-w64/bugs/859/?

I think this is similar issue that might require another fix.