This patch fixes performance problems on multi-core machines as reported at <https://bugs.gnu.org/26441>. See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in Gnulib by Bruno Haible <bruno@clisp.org>. diff --git a/tests/test-lock.c b/tests/test-lock.c index a992f64..fb18dee 100644 --- a/tests/test-lock.c +++ b/tests/test-lock.c @@ -1,5 +1,5 @@ /* Test of locking in multithreaded situations. - Copyright (C) 2005, 2008-2015 Free Software Foundation, Inc. + Copyright (C) 2005, 2008-2017 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -50,6 +50,28 @@ Uncomment this to see if the operating system has a fair scheduler. */ #define EXPLICIT_YIELD 1 +/* Whether to use 'volatile' on some variables that communicate information + between threads. If set to 0, a semaphore or a lock is used to protect + these variables. If set to 1, 'volatile' is used; this is theoretically + equivalent but can lead to much slower execution (e.g. 30x slower total + run time on a 40-core machine), because 'volatile' does not imply any + synchronization/communication between different CPUs. */ +#define USE_VOLATILE 0 + +#if USE_POSIX_THREADS && HAVE_SEMAPHORE_H +/* Whether to use a semaphore to communicate information between threads. + If set to 0, a lock is used. If set to 1, a semaphore is used. + Uncomment this to reduce the dependencies of this test. */ +# define USE_SEMAPHORE 1 +/* Mac OS X provides only named semaphores (sem_open); its facility for + unnamed semaphores (sem_init) does not work. */ +# if defined __APPLE__ && defined __MACH__ +# define USE_NAMED_SEMAPHORE 1 +# else +# define USE_UNNAMED_SEMAPHORE 1 +# endif +#endif + /* Whether to print debugging messages. */ #define ENABLE_DEBUGGING 0 @@ -90,6 +112,12 @@ #include "glthread/thread.h" #include "glthread/yield.h" +#if USE_SEMAPHORE +# include <errno.h> +# include <fcntl.h> +# include <semaphore.h> +# include <unistd.h> +#endif #if ENABLE_DEBUGGING # define dbgprintf printf @@ -103,6 +131,132 @@ # define yield() #endif +#if USE_VOLATILE +struct atomic_int { + volatile int value; +}; +static void +init_atomic_int (struct atomic_int *ai) +{ +} +static int +get_atomic_int_value (struct atomic_int *ai) +{ + return ai->value; +} +static void +set_atomic_int_value (struct atomic_int *ai, int new_value) +{ + ai->value = new_value; +} +#elif USE_SEMAPHORE +/* This atomic_int implementation can only support the values 0 and 1. + It is initially 0 and can be set to 1 only once. */ +# if USE_UNNAMED_SEMAPHORE +struct atomic_int { + sem_t semaphore; +}; +#define atomic_int_semaphore(ai) (&(ai)->semaphore) +static void +init_atomic_int (struct atomic_int *ai) +{ + sem_init (&ai->semaphore, 0, 0); +} +# endif +# if USE_NAMED_SEMAPHORE +struct atomic_int { + sem_t *semaphore; +}; +#define atomic_int_semaphore(ai) ((ai)->semaphore) +static void +init_atomic_int (struct atomic_int *ai) +{ + sem_t *s; + unsigned int count; + for (count = 0; ; count++) + { + char name[80]; + /* Use getpid() in the name, so that different processes running at the + same time will not interfere. Use ai in the name, so that different + atomic_int in the same process will not interfere. Use a count in + the name, so that even in the (unlikely) case that a semaphore with + the specified name already exists, we can try a different name. */ + sprintf (name, "test-lock-%lu-%p-%u", + (unsigned long) getpid (), ai, count); + s = sem_open (name, O_CREAT | O_EXCL, 0600, 0); + if (s == SEM_FAILED) + { + if (errno == EEXIST) + /* Retry with a different name. */ + continue; + else + { + perror ("sem_open failed"); + abort (); + } + } + else + { + /* Try not to leave a semaphore hanging around on the file system + eternally, if we can avoid it. */ + sem_unlink (name); + break; + } + } + ai->semaphore = s; +} +# endif +static int +get_atomic_int_value (struct atomic_int *ai) +{ + if (sem_trywait (atomic_int_semaphore (ai)) == 0) + { + if (sem_post (atomic_int_semaphore (ai))) + abort (); + return 1; + } + else if (errno == EAGAIN) + return 0; + else + abort (); +} +static void +set_atomic_int_value (struct atomic_int *ai, int new_value) +{ + if (new_value == 0) + /* It's already initialized with 0. */ + return; + /* To set the value 1: */ + if (sem_post (atomic_int_semaphore (ai))) + abort (); +} +#else +struct atomic_int { + gl_lock_define (, lock) + int value; +}; +static void +init_atomic_int (struct atomic_int *ai) +{ + gl_lock_init (ai->lock); +} +static int +get_atomic_int_value (struct atomic_int *ai) +{ + gl_lock_lock (ai->lock); + int ret = ai->value; + gl_lock_unlock (ai->lock); + return ret; +} +static void +set_atomic_int_value (struct atomic_int *ai, int new_value) +{ + gl_lock_lock (ai->lock); + ai->value = new_value; + gl_lock_unlock (ai->lock); +} +#endif + #define ACCOUNT_COUNT 4 static int account[ACCOUNT_COUNT]; @@ -170,12 +324,12 @@ lock_mutator_thread (void *arg) return NULL; } -static volatile int lock_checker_done; +static struct atomic_int lock_checker_done; static void * lock_checker_thread (void *arg) { - while (!lock_checker_done) + while (get_atomic_int_value (&lock_checker_done) == 0) { dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); gl_lock_lock (my_lock); @@ -200,7 +354,8 @@ test_lock (void) /* Initialization. */ for (i = 0; i < ACCOUNT_COUNT; i++) account[i] = 1000; - lock_checker_done = 0; + init_atomic_int (&lock_checker_done); + set_atomic_int_value (&lock_checker_done, 0); /* Spawn the threads. */ checkerthread = gl_thread_create (lock_checker_thread, NULL); @@ -210,7 +365,7 @@ test_lock (void) /* Wait for the threads to terminate. */ for (i = 0; i < THREAD_COUNT; i++) gl_thread_join (threads[i], NULL); - lock_checker_done = 1; + set_atomic_int_value (&lock_checker_done, 1); gl_thread_join (checkerthread, NULL); check_accounts (); } @@ -254,12 +409,12 @@ rwlock_mutator_thread (void *arg) return NULL; } -static volatile int rwlock_checker_done; +static struct atomic_int rwlock_checker_done; static void * rwlock_checker_thread (void *arg) { - while (!rwlock_checker_done) + while (get_atomic_int_value (&rwlock_checker_done) == 0) { dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ()); gl_rwlock_rdlock (my_rwlock); @@ -284,7 +439,8 @@ test_rwlock (void) /* Initialization. */ for (i = 0; i < ACCOUNT_COUNT; i++) account[i] = 1000; - rwlock_checker_done = 0; + init_atomic_int (&rwlock_checker_done); + set_atomic_int_value (&rwlock_checker_done, 0); /* Spawn the threads. */ for (i = 0; i < THREAD_COUNT; i++) @@ -295,7 +451,7 @@ test_rwlock (void) /* Wait for the threads to terminate. */ for (i = 0; i < THREAD_COUNT; i++) gl_thread_join (threads[i], NULL); - rwlock_checker_done = 1; + set_atomic_int_value (&rwlock_checker_done, 1); for (i = 0; i < THREAD_COUNT; i++) gl_thread_join (checkerthreads[i], NULL); check_accounts (); @@ -356,12 +512,12 @@ reclock_mutator_thread (void *arg) return NULL; } -static volatile int reclock_checker_done; +static struct atomic_int reclock_checker_done; static void * reclock_checker_thread (void *arg) { - while (!reclock_checker_done) + while (get_atomic_int_value (&reclock_checker_done) == 0) { dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); gl_recursive_lock_lock (my_reclock); @@ -386,7 +542,8 @@ test_recursive_lock (void) /* Initialization. */ for (i = 0; i < ACCOUNT_COUNT; i++) account[i] = 1000; - reclock_checker_done = 0; + init_atomic_int (&reclock_checker_done); + set_atomic_int_value (&reclock_checker_done, 0); /* Spawn the threads. */ checkerthread = gl_thread_create (reclock_checker_thread, NULL); @@ -396,7 +553,7 @@ test_recursive_lock (void) /* Wait for the threads to terminate. */ for (i = 0; i < THREAD_COUNT; i++) gl_thread_join (threads[i], NULL); - reclock_checker_done = 1; + set_atomic_int_value (&reclock_checker_done, 1); gl_thread_join (checkerthread, NULL); check_accounts (); }