MINOR: threads: add more consistency between certain variables in no-thread case

When threads are disabled, some variables such as tid and tid_bit are
still checked everywhere, the MAX_THREADS_MASK macro is ~0UL while
MAX_THREADS is 1, and the all_threads_mask variable is replaced with a
macro forced to zero. The compiler cannot optimize away all this code
involving checks on tid and tid_bit, and we end up in special cases
where all_threads_mask has to be specifically tested for being zero or
not. It is not even certain the code paths are always equivalent when
testing without threads and with nbthread 1.

Let's change this to make sure we always present a single thread when
threads are disabled, and have the relevant values declared as constants
so that the compiler can optimize all the tests away. Now we have
MAX_THREADS_MASK set to 1, all_threads_mask set to 1, tid set to zero
and tid_bit set to 1. Doing just this has removed 4 kB of code in the
no-thread case.

A few checks for all_threads_mask==0 have been removed since it never
happens anymore.

diff --git a/include/common/hathreads.h b/include/common/hathreads.h
index 068b2a1530a7b9bb3b3fed1996010bb6b84714d0..4cf3db9bee839672d9c40eab7e8ead81cbe771e3 100644 (file)
--- a/include/common/hathreads.h
+++ b/include/common/hathreads.h
@@ -24,10 +24,6 @@
 
 #include <common/config.h>
 
-#define MAX_THREADS_MASK ((unsigned long)-1)
-extern THREAD_LOCAL unsigned int tid;     /* The thread id */
-extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the thread id */
-
 /* Note about all_threads_mask :
  *    - with threads support disabled, this symbol is defined as zero (0UL).
  *    - with threads enabled, this variable is never zero, it contains the mask
@@ -37,7 +33,14 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the threa
 #ifndef USE_THREAD
 
 #define MAX_THREADS 1
-#define all_threads_mask 0UL
+#define MAX_THREADS_MASK 1
+
+/* Only way found to replace variables with constants that are optimized away
+ * at build time.
+ */
+enum { all_threads_mask = 1UL };
+enum { tid_bit = 1UL };
+enum { tid = 0 };
 
 #define __decl_hathreads(decl)
 
@@ -116,6 +119,9 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the threa
 
 #define ha_sigmask(how, set, oldset)  sigprocmask(how, set, oldset)
 
+static inline void ha_set_tid(unsigned int tid)
+{
+}
 
 static inline void __ha_barrier_load(void)
 {
@@ -138,6 +144,7 @@ static inline void __ha_barrier_full(void)
 #include <import/plock.h>
 
 #define MAX_THREADS LONGBITS
+#define MAX_THREADS_MASK ((unsigned long)-1)
 
 #define __decl_hathreads(decl) decl
 
@@ -266,10 +273,19 @@ void thread_exit_sync(void);
 int  thread_no_sync(void);
 int  thread_need_sync(void);
 
+extern THREAD_LOCAL unsigned int tid;     /* The thread id */
+extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the thread id */
 extern volatile unsigned long all_threads_mask;
 
 #define ha_sigmask(how, set, oldset)  pthread_sigmask(how, set, oldset)
 
+/* sets the thread ID and the TID bit for the current thread */
+static inline void ha_set_tid(unsigned int data)
+{
+       tid     = data;
+       tid_bit = (1UL << tid);
+}
+
 
 #if defined(DEBUG_THREAD) || defined(DEBUG_FULL)
 

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 5abe9fa0ca396d3573a99aa8107d1d5947cd1369..1451243adcf54bc7ccd5d3bdefbe9140c00ff361 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -7587,11 +7587,11 @@ int check_config_validity()
                                nbproc = my_ffsl(bind_conf->bind_proc);
 
                        mask = bind_conf->bind_thread[nbproc - 1];
-                       if (mask && !(mask & (all_threads_mask ? all_threads_mask : 1UL))) {
+                       if (mask && !(mask & all_threads_mask)) {
                                unsigned long new_mask = 0;
 
                                while (mask) {
-                                       new_mask |= mask & (all_threads_mask ? all_threads_mask : 1UL);
+                                       new_mask |= mask & all_threads_mask;
                                        mask >>= global.nbthread;
                                }
 

diff --git a/src/haproxy.c b/src/haproxy.c
index 810d5983890f221a227114be333352a0551490ad..42f1f3e64f8a9591bcb04eeeb150e04dd0c92d20 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2459,8 +2459,7 @@ static void *run_thread_poll_loop(void *data)
        struct per_thread_deinit_fct *ptdf;
        __decl_hathreads(static HA_SPINLOCK_T start_lock);
 
-       tid     = *((unsigned int *)data);
-       tid_bit = (1UL << tid);
+       ha_set_tid(*((unsigned int *)data));
        tv_update_date(-1,-1);
 
        list_for_each_entry(ptif, &per_thread_init_list, list) {

diff --git a/src/hathreads.c b/src/hathreads.c
index bfd055aa9b9c56c2078a8175fe117d6272b2c9d4..595a71717c4015212a6cf3218b74a75716060143 100644 (file)
--- a/src/hathreads.c
+++ b/src/hathreads.c
@@ -19,8 +19,6 @@
 #include <common/standard.h>
 #include <proto/fd.h>
 
-THREAD_LOCAL unsigned int tid      = 0;
-THREAD_LOCAL unsigned long tid_bit = (1UL << 0);
 
 /* Dummy I/O handler used by the sync pipe.*/
 void thread_sync_io_handler(int fd)
@@ -33,6 +31,9 @@ static HA_SPINLOCK_T sync_lock;
 static int           threads_sync_pipe[2];
 static unsigned long threads_want_sync = 0;
 volatile unsigned long all_threads_mask  = 1; // nbthread 1 assumed by default
+THREAD_LOCAL unsigned int  tid           = 0;
+THREAD_LOCAL unsigned long tid_bit       = (1UL << 0);
+
 
 #if defined(DEBUG_THREAD) || defined(DEBUG_FULL)
 struct lock_stat lock_stats[LOCK_LABELS];
@@ -127,7 +128,7 @@ void thread_enter_sync()
 {
        static volatile unsigned long barrier = 0;
 
-       if (!all_threads_mask)
+       if (!(all_threads_mask & (all_threads_mask - 1)))
                return;
 
        thread_sync_barrier(&barrier);
@@ -143,7 +144,7 @@ void thread_exit_sync()
 {
        static volatile unsigned long barrier = 0;
 
-       if (!all_threads_mask)
+       if (!(all_threads_mask & (all_threads_mask - 1)))
                return;
 
        if (threads_want_sync & tid_bit)