MINGW-packages
MINGW-packages copied to clipboard
msys2
igraph: disable OpenMP support
This PR disables OpenMP support in igraph.
igraph in MSYS2 is currently set up to link to OpenBLAS, both directly and indirectly (through ARPACK). OpenBLAS conflicts with OpenMP, unless it is built specifically to be compatible. Otherwise there is a risk of a hang. See #8399 for details.
To avoid this issue in igraph, we have two choices:
- Disable OpenMP
- Avoid linking to OpenBLAS (use bundled BLAS or reference BLAS). Since ARPACK in MSYS2 links to OpenBLAS, this means that we must also avoid ARPACK, and use the ARPACK that is bundled with igraph. This is an old version that is less reliable.
The first choice has a smaller impact (fewer igraph functions affected, smaller performance impact, smaller reliability impact), so this is what I recommend.
If #8399 is addressed at some point in the future, this change can be reverted. The change I proposed in #8399 has wide impact, so I expect that it will take some time until a decision is reached about it. Until then, it would be nice to have a reliable igraph in MSYS2 that does not hang.
You mentioned that the crash only occuers on MINGW32, so why disabling OpenMP it on all environments?
This is not a crash, but a deadlock caused by a documented race condition.
It is the nature of race conditions that the bug may or may not manifest itself depending on circumstances outside of one's control (such as relative timing between threads). For example, I cannot reproduce the hang with MinGW32 locally, but I see it consistently on GitHub Actions. Just because I have not yet observed it on 64-bit does not mean that there is no risk of hang.
As stated in #8399, the OpenBLAS docs are clear that unless OpenBLAS was compiled with USE_OPENMP=1 (which it isn't in MSYS2), it should not be used together with OpenMP. https://github.com/xianyi/OpenBLAS/wiki/faq#multi-threaded
I'm not sure what the impact is on performance. But would this (from the FAQ you linked) also work?
Call
openblas_set_num_threads(1)in the application on runtime.
But would this (from the FAQ you linked) also work?
If you are suggesting to patch igraph, that is not possible. This is an OpenBLAS-specific call. igraph does not know what BLAS implementation it is using. Some systems (like Fedora) even support dynamically switchable BLAS.
Also, just to avoid misunderstandings: igraph is not incompatible with OpenBLAS. It is OpenMP that is incompatible with a specific configuration of OpenBLAS, which MSYS2 happens to be using. I assume that this is why some Linux distros configure OpenBLAS to be compatible with OpenMP (Arch) and some others provide multiple variants (Debian, Fedora).
I'm not sure what the impact is on performance.
I am a member of the igraph team. In my opinion, from a performance consideration, disabling OpenMP for igraph is much better than disabling multithreading in OpenBLAS.
In the end, my goal with this PR is to ensure that MSYS2 users get a reliable build of igraph.
We also could give OpenMP another shot and keep it only for subsystems where is does work but I have no strong opinion here.
OpenMP support has already been disabled for 32-bit. Now that the problem is solved, I suggest re-enabling it.
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-igraph/PKGBUILD#L41