Skip to content

Conversation

@TimothyGu
Copy link

No description provided.

@matteo-frigo
Copy link
Member

FFTW does not in itself depend upon pthreads, so you will have to make a case for this change to be accepted.

Threads are a global property of the program, and attempts to hide them in a library don't work. For example, a user may have a single-threaded program and rely on fftw to use multiple cores internally. Conversely, another user may have already created a few threads and expect fftw to run sequentially. A third user may be using MPI processes for compatibility with whatever other software expects MPI.

Historically, FFTW has taken this position:

  1. fftw by itself is not parallel, and does not drag in the pthread library by default.
  2. a user that wants to user threads calls fftw_threads_init() and explicitly links with -lpthread
  3. alternatively, a user can link the openmp runtime, and we do not assume that openmp is compatible with pthreads

It is probably too late to fight this battle, but pthreads have a cost that everybody is now paying whether they are using them or not. For example, at some point in the past, the linux C library added locks to all stdio calls, with the effect that all programs (including sequential ones) became slower. For example, getchar() is 3x slower than getchar_unlocked() (which is what getchar() used to be). We cannot fight this madness, but at least we are trying not to help it.

Copy link
Contributor

@antoniovazquezblanco antoniovazquezblanco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request seems to have been sitting for long enough...

The right approach seems to have been described in #180:

  • For each configuration, a new fft..._threads.pc file should be generated for each variant.
  • That new .pc file may include the threads libs.private dependency.

I believe that this PR can be closed and that the issue should be tracked via #180.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants