bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: new module 'thread-optim'


From: Bruno Haible
Subject: Re: new module 'thread-optim'
Date: Sun, 09 Aug 2020 11:44:53 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-186-generic; KDE/5.18.0; x86_64; ; )

Hi Paul,

> > +# define IF_MT_DECL  char optimize_for_single_thread = 
> > __libc_single_threaded
> > +# define IF_MT       if (optimize_for_single_thread)
> 
> That second line looks backward

Oops, what a blunder! Thanks for noticing it. Fixed through the patch below.

> Could you explain why macros like IF_MT_DECL and IF_MT are needed? That is, 
> why 
> wouldn't it suffice to have a Gnulib-supplied <sys/single_threaded.h> on 
> non-glibc platforms, and have "enum { __libc_single_threaded = 0 };" in the 
> Gnulib replacement? Typically it's better to avoid language-bending macros 
> like 
> IF_MT if we can.

This would be possible. However, it adds 4 more lines of code to each function
that does a lock/unlock of any lock. I hate the fact that the glibc authors
recommend a solution that requires source code modifications all over the place
at all (while the previous solution with '#pragma weak' required source code
modifications in a single place only). Then at least I wish to minimize the
impact on the source code: 1-2 more lines of code in each such function.

To me, this variable '__libc_single_threaded' is not something that should
be visible in source code
  - because it is so low-level,
  - because only one platform has it.
It is just an ugly artifact that we better hide under some layer. For example,
we may want to add a __builtin_expect call in the 'if'.

For this layer, I also considered a macro pair that expands to a do-while
syntax, but this does not work when the lock statement is duplicated in
two 'if' branches:

   if (...)
     {
       ...
       gl_lock_lock (&lock);
       ...
     }
   else
     {
       ...
       gl_lock_lock (&lock);
       ...
     }
  ...
  gl_lock_unlock (&lock);

I also considered to introduce macro variants of pthread_mutex_lock/unlock,
pthread_rwlock_lock/unlock, glthread_lock_lock/unlock, gl_lock_lock/unlock, etc.
but the global impact on source code readability would be worse.


2020-08-09  Bruno Haible  <bruno@clisp.org>

        thread-optim: Fix logic error.
        Reported by Paul Eggert.
        * lib/thread-optim.h (IF_MT): Fix logic error.

diff --git a/lib/thread-optim.h b/lib/thread-optim.h
index 8040d53..5d9a499 100644
--- a/lib/thread-optim.h
+++ b/lib/thread-optim.h
@@ -51,7 +51,7 @@
 #if HAVE_SYS_SINGLE_THREADED_H /* glibc >= 2.32 */
 # include <sys/single_threaded.h>
 # define IF_MT_DECL  char optimize_for_single_thread = __libc_single_threaded
-# define IF_MT       if (optimize_for_single_thread)
+# define IF_MT       if (!optimize_for_single_thread)
 #else
 # define IF_MT_DECL  (void *)0
 # define IF_MT




reply via email to

[Prev in Thread] Current Thread [Next in Thread]