| Message ID | 20181027071003.1347-2-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reduce #ifdef NO_PTHREADS | expand |
On Sat, Oct 27, 2018 at 09:09:54AM +0200, Nguyễn Thái Ngọc Duy wrote: > +/* > + * macros instead of typedefs because pthread definitions may have > + * been pulled in by some system dependencies even though the user > + * wants to disable pthread. > + */ > +#define pthread_t int > +#define pthread_mutex_t int > +#define pthread_cond_t int > + > +#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex) > +#define pthread_mutex_lock(mutex) > +#define pthread_mutex_unlock(mutex) > +#define pthread_mutex_destroy(mutex) OK, we only need to do the dummy init to shut up the compiler, and the rest really can just become noops. Makes sense. > +#define pthread_cond_init(cond, attr) dummy_pthread_init(cond) > +#define pthread_cond_wait(cond, mutex) > +#define pthread_cond_signal(cond) > +#define pthread_cond_broadcast(cond) > +#define pthread_cond_destroy(cond) And this is the same. > +#define pthread_key_create(key, attr) dummy_pthread_init(key) > +#define pthread_key_delete(key) We don't need keys because we know we have only one key: the main thread. Makes sense. But... > +#define pthread_setspecific(key, data) > +#define pthread_getspecific(key) NULL We expect to be able to store a void pointer here and get it back, which should work even for a single thread. Do we need something like: extern void *pthread_specific_data; #define pthread_setspecific(key, data) do { \ pthread_specific_data = data; \ } while(0) void pthread_getspecific(key) pthread_specific_data > +#define pthread_create(thread, attr, fn, data) \ > + dummy_pthread_create(thread, attr, fn, data) I wondered if this should actually run "fn", but I guess there is not much point. At this point the caller expects the main thread to keep going, so this is already an error, and reporting ENOSYS is probably the best thing to do. -Peff
On Sat, Oct 27, 2018 at 9:31 AM Jeff King <peff@peff.net> wrote: > > +#define pthread_setspecific(key, data) > > +#define pthread_getspecific(key) NULL > > We expect to be able to store a void pointer here and get it back, which > should work even for a single thread. Do we need something like: > > extern void *pthread_specific_data; > > #define pthread_setspecific(key, data) do { \ > pthread_specific_data = data; \ > } while(0) > > void pthread_getspecific(key) pthread_specific_data The data is per key though so a correct implementation may involve a hashmap or a list. It does simplify index-pack which has to fall back to nothread_data when pthreads is not available. But with index-pack being the only call site that can take advantage of this (run-command.c probably will use real pthreads library anyway), I'm not sure if it's worth really implementing these functions.
On Sat, Oct 27, 2018 at 09:40:13AM +0200, Duy Nguyen wrote: > > We expect to be able to store a void pointer here and get it back, which > > should work even for a single thread. Do we need something like: > > > > extern void *pthread_specific_data; > > > > #define pthread_setspecific(key, data) do { \ > > pthread_specific_data = data; \ > > } while(0) > > > > void pthread_getspecific(key) pthread_specific_data > > The data is per key though so a correct implementation may involve a > hashmap or a list. Ah, yeah, you're right, I was mixing up the thread id and the key in my head. I think it would just be an array of void pointers, with pthread_key_create() returning an static index. > It does simplify index-pack which has to fall back to nothread_data > when pthreads is not available. But with index-pack being the only > call site that can take advantage of this (run-command.c probably will > use real pthreads library anyway), I'm not sure if it's worth really > implementing these functions. Yeah, you might be right. -Peff
On Sat, Oct 27, 2018 at 10:15 AM Jeff King <peff@peff.net> wrote: > > On Sat, Oct 27, 2018 at 09:40:13AM +0200, Duy Nguyen wrote: > > > > We expect to be able to store a void pointer here and get it back, which > > > should work even for a single thread. Do we need something like: > > > > > > extern void *pthread_specific_data; > > > > > > #define pthread_setspecific(key, data) do { \ > > > pthread_specific_data = data; \ > > > } while(0) > > > > > > void pthread_getspecific(key) pthread_specific_data > > > > The data is per key though so a correct implementation may involve a > > hashmap or a list. > > Ah, yeah, you're right, I was mixing up the thread id and the key in my > head. I think it would just be an array of void pointers, with > pthread_key_create() returning an static index. We could redefine pthread_key_t as "void *" though, then _setspecific(key, data) is key = data; But there will be more changes in index-pack.c to take advantage of it. I took a stab but couldn't figure out fast enough where _setspecific(.., ¬hread_data) should be called in NO_PTHREADS mode (to simplify get_thread_data() to just a wrapper of _getspecific) and got bored. It could be a micro project for someone really wants to learn about index-pack.c
diff --git a/Makefile b/Makefile index b08d5ea258..321540a736 100644 --- a/Makefile +++ b/Makefile @@ -991,6 +991,7 @@ LIB_OBJS += sub-process.o LIB_OBJS += symlinks.o LIB_OBJS += tag.o LIB_OBJS += tempfile.o +LIB_OBJS += thread-utils.o LIB_OBJS += tmp-objdir.o LIB_OBJS += trace.o LIB_OBJS += trailer.o @@ -1674,7 +1675,6 @@ ifdef NO_PTHREADS else BASIC_CFLAGS += $(PTHREAD_CFLAGS) EXTLIBS += $(PTHREAD_LIBS) - LIB_OBJS += thread-utils.o endif ifdef HAVE_PATHS_H diff --git a/thread-utils.c b/thread-utils.c index a2135e0743..5329845691 100644 --- a/thread-utils.c +++ b/thread-utils.c @@ -20,6 +20,9 @@ int online_cpus(void) { +#ifdef NO_PTHREADS + return 1; +#else #ifdef _SC_NPROCESSORS_ONLN long ncpus; #endif @@ -59,10 +62,12 @@ int online_cpus(void) #endif return 1; +#endif } int init_recursive_mutex(pthread_mutex_t *m) { +#ifndef NO_PTHREADS pthread_mutexattr_t a; int ret; @@ -74,4 +79,47 @@ int init_recursive_mutex(pthread_mutex_t *m) pthread_mutexattr_destroy(&a); } return ret; +#else + return 0; +#endif +} + +#ifdef NO_PTHREADS +int dummy_pthread_create(pthread_t *pthread, const void *attr, + void *(*fn)(void *), void *data) +{ + /* + * Do nothing. + * + * The main purpose of this function is to break compiler's + * flow analysis and avoid -Wunused-variable false warnings. + */ + return ENOSYS; +} + +int dummy_pthread_init(void *data) +{ + /* + * Do nothing. + * + * The main purpose of this function is to break compiler's + * flow analysis or it may realize that functions like + * pthread_mutex_init() is no-op, which means the (static) + * variable is not used/initialized at all and trigger + * -Wunused-variable + */ + return ENOSYS; } + +int dummy_pthread_join(pthread_t pthread, void **retval) +{ + /* + * Do nothing. + * + * The main purpose of this function is to break compiler's + * flow analysis and avoid -Wunused-variable false warnings. + */ + return ENOSYS; +} + +#endif diff --git a/thread-utils.h b/thread-utils.h index d9a769d190..a12850e747 100644 --- a/thread-utils.h +++ b/thread-utils.h @@ -4,12 +4,53 @@ #ifndef NO_PTHREADS #include <pthread.h> -extern int online_cpus(void); -extern int init_recursive_mutex(pthread_mutex_t*); +#define HAVE_THREADS 1 #else -#define online_cpus() 1 +#define HAVE_THREADS 0 + +/* + * macros instead of typedefs because pthread definitions may have + * been pulled in by some system dependencies even though the user + * wants to disable pthread. + */ +#define pthread_t int +#define pthread_mutex_t int +#define pthread_cond_t int + +#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex) +#define pthread_mutex_lock(mutex) +#define pthread_mutex_unlock(mutex) +#define pthread_mutex_destroy(mutex) + +#define pthread_cond_init(cond, attr) dummy_pthread_init(cond) +#define pthread_cond_wait(cond, mutex) +#define pthread_cond_signal(cond) +#define pthread_cond_broadcast(cond) +#define pthread_cond_destroy(cond) + +#define pthread_key_create(key, attr) dummy_pthread_init(key) +#define pthread_key_delete(key) + +#define pthread_create(thread, attr, fn, data) \ + dummy_pthread_create(thread, attr, fn, data) +#define pthread_join(thread, retval) \ + dummy_pthread_join(thread, retval) + +#define pthread_setspecific(key, data) +#define pthread_getspecific(key) NULL + +int dummy_pthread_create(pthread_t *pthread, const void *attr, + void *(*fn)(void *), void *data); +int dummy_pthread_join(pthread_t pthread, void **retval); + +int dummy_pthread_init(void *); #endif + +int online_cpus(void); +int init_recursive_mutex(pthread_mutex_t*); + + #endif /* THREAD_COMPAT_H */