[Mesa-dev] [PATCH] c11/threads: create mutexattrs only when needed

[Emil Velikov]

Any comments on the patch and/or discussion after the --- line ?

On 24 April 2016 at 16:14, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
>
> If the mutexattrs are the default one can just pass NULL to
> pthread_mutex_init. As the compiler does not know this detail it
> unnecessarily creates/destroys the attrs.
>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> While going through GLVND, I've noticed that it (sort of) breaks its
> assumptions/goals - 'we don't want the heavy locking/etc. brought by
> pthreads' [for single threaded uses]
>
> Thus I gave mesa a quick look and the following popped up:
>
> - pthread_once - libglapi, classic + dri
> Replace with an atomic test & set combo ?
>
> - pthread_mutexattr_* - all dri modules, libGL-apple
> Using a recursive lock in src/mesa/main/shared.c and
> src/glx/apple/apple_glx_drawable.c
>
> - pthread_key_* - EGL
> - pthread_.etspecific - EGL
> Extend pthread-stubs explicitly required it by mesa ?
> Note: the original code that pthread-stubs is based on (libX11) does
> have these ;-)
>
> - pthread_barrier_* - llvmpipe
> Fall-back to the mutex + cond implementation ?
>
> - pthread_setname_np - llvmpipe
> Do we need this ? Afaict the Windows build does not have an equivalent.
>
> - pthread_join - nine, llvmpipe, radeon(s), rbug, omx (thanks bellagio)
> - pthread_create - nine, llvmpipe, radeon(s), rbug
> - pthread_sigmask - nine, llvmpipe, radeon(s), rbug
>
> These four (five inc bellagio/omx) want more than one thread. How do we
> get others pthread free, while keeping these happy ?
>
> Please let me know how you feel on the topic. Do you see this as worthy
> goal ? Does the proposed solutions sound OK ? Can you think of any
> alternatives?
>
> -Emil
>
> P.S. For anyone who wonders, libc (GNU one only iirc) provides
> lightweight stubs, thus single-threaded apps work without the overhead.
> ---
>  include/c11/threads_posix.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/c11/threads_posix.h b/include/c11/threads_posix.h
> index ce9853b..11d36e4 100644
> --- a/include/c11/threads_posix.h
> +++ b/include/c11/threads_posix.h
> @@ -180,9 +180,14 @@ mtx_init(mtx_t *mtx, int type)
>        && type != (mtx_timed|mtx_recursive)
>        && type != (mtx_try|mtx_recursive))
>          return thrd_error;
> +
> +    if ((type & mtx_recursive) == 0) {
> +        pthread_mutex_init(mtx, NULL);
> +        return thrd_success;
> +    }
> +
>      pthread_mutexattr_init(&attr);
> -    if ((type & mtx_recursive) != 0)
> -        pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
> +    pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
>      pthread_mutex_init(mtx, &attr);
>      pthread_mutexattr_destroy(&attr);
>      return thrd_success;
> --
> 2.8.0
>