{"thread":{"id":"49697","subject":"[PATCH 00/10] Reduce #ifdef NO_PTHREADS","startedAt":"2018-10-27T07:10:15Z","lastAt":"2018-11-06T04:53:21Z","messageCount":66,"participants":["Nguyễn Thái Ngọc Duy","Jeff King","Duy Nguyen","Junio C Hamano","Ben Peart"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"361750","messageId":"20181027071003.1347-1-pclouds@gmail.com","threadId":"49697","inReplyTo":null,"subject":"[PATCH 00/10] Reduce #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:53Z","receivedAt":"2018-10-27T07:10:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"People seemed to support the idea of removing these #ifdef NO_PTHREADS [1]\nso this is a complete series. I left the #ifdef in run-command.c and\ntransport-helper.c because those code looked complicated so perhaps we\ncould clean them up later. Even these updated files could be updated\nmore, I think, to reduce some code duplication, but I tried to keep\nthe change here minimal.\n\n[1] https://public-inbox.org/git/20181018180522.17642-1-pclouds@gmail.com/\n\nNguyễn Thái Ngọc Duy (10):\n thread-utils: macros to unconditionally compile pthreads API\n index-pack: remove #ifdef NO_PTHREADS\n name-hash.c: remove #ifdef NO_PTHREADS\n attr.c: remove #ifdef NO_PTHREADS\n send-pack.c: remove #ifdef NO_PTHREADS\n grep: remove #ifdef NO_PTHREADS\n preload-index.c: remove #ifdef NO_PTHREADS\n pack-objects: remove #ifdef NO_PTHREADS\n read-cache.c: remove #ifdef NO_PTHREADS\n Clean up pthread_create() error handling\n\n Makefile | 2 +-\n attr.c | 14 ---------\n builtin/grep.c | 58 +++++++++++++----------------------\n builtin/index-pack.c | 68 +++++++++++-------------------------------\n builtin/pack-objects.c | 26 ++--------------\n grep.c | 6 ----\n grep.h | 6 ----\n name-hash.c | 38 +++++++++--------------\n pack-objects.h | 6 ----\n preload-index.c | 23 +++++---------\n read-cache.c | 49 +++++++++++-------------------\n run-command.c | 2 +-\n send-pack.c | 8 ++---\n thread-utils.c | 48 +++++++++++++++++++++++++++++\n thread-utils.h | 47 +++++++++++++++++++++++++++--\n 15 files changed, 178 insertions(+), 223 deletions(-)\n\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361751","messageId":"20181027071003.1347-2-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 01/10] thread-utils: macros to unconditionally compile pthreads API","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:54Z","receivedAt":"2018-10-27T07:10:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When built with NO_PTHREADS, the macros are used make the code build\neven though pthreads header and library may be missing. The code can\nstill have different code paths for no threads support with\nHAVE_THREADS variable.\n\nThere are of course impacts on no-pthreads builds:\n\n- data structure may get slightly bigger because all the mutexes and\n pthread_t are present (as an int)\n\n- code execution is not impacted much. Locking (in hot path) is\n no-op. Other wrapper function calls really should not matter much.\n\n- the binary size grows bigger because of threaded code. But at least\n on Linux this does not matter, if some code is not executed, it's\n not mapped in memory.\n\nThis is a preparation step to remove \"#ifdef NO_PTHREADS\" in the code\nmostly because of maintainability. As Jeff put it\n\n> it's probably OK to stop thinking of it as \"non-threaded platforms\n> are the default and must pay zero cost\" and more as \"threaded\n> platforms are the default, and non-threaded ones are OK to pay a\n> small cost as long as they still work\".\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Makefile | 2 +-\n thread-utils.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n thread-utils.h | 47 ++++++++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 93 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b08d5ea258..321540a736 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -991,6 +991,7 @@ LIB_OBJS += sub-process.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += tempfile.o\n+LIB_OBJS += thread-utils.o\n LIB_OBJS += tmp-objdir.o\n LIB_OBJS += trace.o\n LIB_OBJS += trailer.o\n@@ -1674,7 +1675,6 @@ ifdef NO_PTHREADS\n else\n \tBASIC_CFLAGS += $(PTHREAD_CFLAGS)\n \tEXTLIBS += $(PTHREAD_LIBS)\n-\tLIB_OBJS += thread-utils.o\n endif\n \n ifdef HAVE_PATHS_H\ndiff --git a/thread-utils.c b/thread-utils.c\nindex a2135e0743..5329845691 100644\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -20,6 +20,9 @@\n \n int online_cpus(void)\n {\n+#ifdef NO_PTHREADS\n+\treturn 1;\n+#else\n #ifdef _SC_NPROCESSORS_ONLN\n \tlong ncpus;\n #endif\n@@ -59,10 +62,12 @@ int online_cpus(void)\n #endif\n \n \treturn 1;\n+#endif\n }\n \n int init_recursive_mutex(pthread_mutex_t *m)\n {\n+#ifndef NO_PTHREADS\n \tpthread_mutexattr_t a;\n \tint ret;\n \n@@ -74,4 +79,47 @@ int init_recursive_mutex(pthread_mutex_t *m)\n \t\tpthread_mutexattr_destroy(&a);\n \t}\n \treturn ret;\n+#else\n+\treturn 0;\n+#endif\n+}\n+\n+#ifdef NO_PTHREADS\n+int dummy_pthread_create(pthread_t *pthread, const void *attr,\n+\t\t\t void *(*fn)(void *), void *data)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis and avoid -Wunused-variable false warnings.\n+\t */\n+\treturn ENOSYS;\n+}\n+\n+int dummy_pthread_init(void *data)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis or it may realize that functions like\n+\t * pthread_mutex_init() is no-op, which means the (static)\n+\t * variable is not used/initialized at all and trigger\n+\t * -Wunused-variable\n+\t */\n+\treturn ENOSYS;\n }\n+\n+int dummy_pthread_join(pthread_t pthread, void **retval)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis and avoid -Wunused-variable false warnings.\n+\t */\n+\treturn ENOSYS;\n+}\n+\n+#endif\ndiff --git a/thread-utils.h b/thread-utils.h\nindex d9a769d190..a12850e747 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -4,12 +4,53 @@\n #ifndef NO_PTHREADS\n #include \n \n-extern int online_cpus(void);\n-extern int init_recursive_mutex(pthread_mutex_t*);\n+#define HAVE_THREADS 1\n \n #else\n \n-#define online_cpus() 1\n+#define HAVE_THREADS 0\n+\n+/*\n+ * macros instead of typedefs because pthread definitions may have\n+ * been pulled in by some system dependencies even though the user\n+ * wants to disable pthread.\n+ */\n+#define pthread_t int\n+#define pthread_mutex_t int\n+#define pthread_cond_t int\n+\n+#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)\n+#define pthread_mutex_lock(mutex)\n+#define pthread_mutex_unlock(mutex)\n+#define pthread_mutex_destroy(mutex)\n+\n+#define pthread_cond_init(cond, attr) dummy_pthread_init(cond)\n+#define pthread_cond_wait(cond, mutex)\n+#define pthread_cond_signal(cond)\n+#define pthread_cond_broadcast(cond)\n+#define pthread_cond_destroy(cond)\n+\n+#define pthread_key_create(key, attr) dummy_pthread_init(key)\n+#define pthread_key_delete(key)\n+\n+#define pthread_create(thread, attr, fn, data) \\\n+\tdummy_pthread_create(thread, attr, fn, data)\n+#define pthread_join(thread, retval) \\\n+\tdummy_pthread_join(thread, retval)\n+\n+#define pthread_setspecific(key, data)\n+#define pthread_getspecific(key) NULL\n+\n+int dummy_pthread_create(pthread_t *pthread, const void *attr,\n+\t\t\t void *(*fn)(void *), void *data);\n+int dummy_pthread_join(pthread_t pthread, void **retval);\n+\n+int dummy_pthread_init(void *);\n \n #endif\n+\n+int online_cpus(void);\n+int init_recursive_mutex(pthread_mutex_t*);\n+\n+\n #endif /* THREAD_COMPAT_H */\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361752","messageId":"20181027071003.1347-4-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 03/10] name-hash.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:56Z","receivedAt":"2018-10-27T07:10:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n name-hash.c | 22 ++++------------------\n 1 file changed, 4 insertions(+), 18 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex 1fcda73cb3..b3c9ac791d 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -7,6 +7,7 @@\n */\n #define NO_THE_INDEX_COMPATIBILITY_MACROS\n #include \"cache.h\"\n+#include \"thread-utils.h\"\n \n struct dir_entry {\n \tstruct hashmap_entry ent;\n@@ -131,22 +132,6 @@ static int cache_entry_cmp(const void *unused_cmp_data,\n static int lazy_try_threaded = 1;\n static int lazy_nr_dir_threads;\n \n-#ifdef NO_PTHREADS\n-\n-static inline int lookup_lazy_params(struct index_state *istate)\n-{\n-\treturn 0;\n-}\n-\n-static inline void threaded_lazy_init_name_hash(\n-\tstruct index_state *istate)\n-{\n-}\n-\n-#else\n-\n-#include \"thread-utils.h\"\n-\n /*\n * Set a minimum number of cache_entries that we will handle per\n * thread and use that to decide how many threads to run (upto\n@@ -516,6 +501,9 @@ static void threaded_lazy_init_name_hash(\n \tstruct lazy_dir_thread_data *td_dir;\n \tstruct lazy_name_thread_data *td_name;\n \n+\tif (!HAVE_THREADS)\n+\t\treturn;\n+\n \tk_start = 0;\n \tnr_each = DIV_ROUND_UP(istate->cache_nr, lazy_nr_dir_threads);\n \n@@ -574,8 +562,6 @@ static void threaded_lazy_init_name_hash(\n \tfree(lazy_entries);\n }\n \n-#endif\n-\n static void lazy_init_name_hash(struct index_state *istate)\n {\n \n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361754","messageId":"20181027071003.1347-5-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 04/10] attr.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:57Z","receivedAt":"2018-10-27T07:10:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n attr.c | 14 --------------\n 1 file changed, 14 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 60d284796d..eaece6658d 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -41,23 +41,17 @@ const char *git_attr_name(const struct git_attr *attr)\n \n struct attr_hashmap {\n \tstruct hashmap map;\n-#ifndef NO_PTHREADS\n \tpthread_mutex_t mutex;\n-#endif\n };\n \n static inline void hashmap_lock(struct attr_hashmap *map)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&map->mutex);\n-#endif\n }\n \n static inline void hashmap_unlock(struct attr_hashmap *map)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&map->mutex);\n-#endif\n }\n \n /*\n@@ -498,23 +492,17 @@ static struct check_vector {\n \tsize_t nr;\n \tsize_t alloc;\n \tstruct attr_check **checks;\n-#ifndef NO_PTHREADS\n \tpthread_mutex_t mutex;\n-#endif\n } check_vector;\n \n static inline void vector_lock(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&check_vector.mutex);\n-#endif\n }\n \n static inline void vector_unlock(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&check_vector.mutex);\n-#endif\n }\n \n static void check_vector_add(struct attr_check *c)\n@@ -1181,8 +1169,6 @@ void git_all_attrs(const struct index_state *istate,\n \n void attr_start(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_init(&g_attr_hashmap.mutex, NULL);\n \tpthread_mutex_init(&check_vector.mutex, NULL);\n-#endif\n }\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361755","messageId":"20181027071003.1347-6-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 05/10] send-pack.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:58Z","receivedAt":"2018-10-27T07:10:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"While at there correct \"#include cache.h\" position. It must be one of\nthe first includes.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n send-pack.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex e920ca57df..fa7dc44b36 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -1,4 +1,5 @@\n #include \"builtin.h\"\n+#include \"cache.h\"\n #include \"config.h\"\n #include \"commit.h\"\n #include \"refs.h\"\n@@ -14,7 +15,7 @@\n #include \"version.h\"\n #include \"sha1-array.h\"\n #include \"gpg-interface.h\"\n-#include \"cache.h\"\n+#include \"thread-utils.h\"\n \n int option_parse_push_signed(const struct option *opt,\n \t\t\t const char *arg, int unset)\n@@ -203,9 +204,8 @@ static int receive_status(int in, struct ref *refs)\n static int sideband_demux(int in, int out, void *data)\n {\n \tint *fd = data, ret;\n-#ifdef NO_PTHREADS\n-\tclose(fd[1]);\n-#endif\n+\tif (!HAVE_THREADS)\n+\t\tclose(fd[1]);\n \tret = recv_sideband(\"send-pack\", fd[0], out);\n \tclose(out);\n \treturn ret;\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361753","messageId":"20181027071003.1347-7-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:59Z","receivedAt":"2018-10-27T07:10:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 58 +++++++++++++++++---------------------------------\n grep.c | 6 ------\n grep.h | 6 ------\n 3 files changed, 20 insertions(+), 50 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex d8508ddf79..29221e1003 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -34,7 +34,6 @@ static int recurse_submodules;\n #define GREP_NUM_THREADS_DEFAULT 8\n static int num_threads;\n \n-#ifndef NO_PTHREADS\n static pthread_t *threads;\n \n /* We use one producer thread and THREADS consumer\n@@ -265,13 +264,6 @@ static int wait_all(void)\n \n \treturn hit;\n }\n-#else /* !NO_PTHREADS */\n-\n-static int wait_all(void)\n-{\n-\treturn 0;\n-}\n-#endif\n \n static int grep_cmd_config(const char *var, const char *value, void *cb)\n {\n@@ -284,8 +276,7 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\tif (num_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d) for %s\"),\n \t\t\t num_threads, var);\n-#ifdef NO_PTHREADS\n-\t\telse if (num_threads && num_threads != 1) {\n+\t\telse if (!HAVE_THREADS && num_threads && num_threads != 1) {\n \t\t\t/*\n \t\t\t * TRANSLATORS: %s is the configuration\n \t\t\t * variable for tweaking threads, currently\n@@ -294,7 +285,6 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\t\twarning(_(\"no threads support, ignoring %s\"), var);\n \t\t\tnum_threads = 0;\n \t\t}\n-#endif\n \t}\n \n \tif (!strcmp(var, \"submodule.recurse\"))\n@@ -330,17 +320,14 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n \t\t */\n \t\tadd_work(opt, &gs);\n \t\treturn 0;\n-\t} else\n-#endif\n-\t{\n+\t} else {\n \t\tint hit;\n \n \t\thit = grep_source(opt, &gs);\n@@ -363,17 +350,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n \t\t */\n \t\tadd_work(opt, &gs);\n \t\treturn 0;\n-\t} else\n-#endif\n-\t{\n+\t} else {\n \t\tint hit;\n \n \t\thit = grep_source(opt, &gs);\n@@ -1038,20 +1022,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tpathspec.recursive = 1;\n \tpathspec.recurse_submodules = !!recurse_submodules;\n \n-#ifndef NO_PTHREADS\n-\tif (list.nr || cached || show_in_pager)\n-\t\tnum_threads = 0;\n-\telse if (num_threads == 0)\n-\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n-\telse if (num_threads < 0)\n-\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n-\tif (num_threads == 1)\n+\tif (HAVE_THREADS) {\n+\t\tif (list.nr || cached || show_in_pager)\n+\t\t\tnum_threads = 0;\n+\t\telse if (num_threads == 0)\n+\t\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n+\t\telse if (num_threads < 0)\n+\t\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n+\t\tif (num_threads == 1)\n+\t\t\tnum_threads = 0;\n+\t} else {\n+\t\tif (num_threads)\n+\t\t\twarning(_(\"no threads support, ignoring --threads\"));\n \t\tnum_threads = 0;\n-#else\n-\tif (num_threads)\n-\t\twarning(_(\"no threads support, ignoring --threads\"));\n-\tnum_threads = 0;\n-#endif\n+\t}\n \n \tif (!num_threads)\n \t\t/*\n@@ -1062,15 +1046,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\tcompile_grep_patterns(&opt);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n \t\t && (opt.pre_context || opt.post_context ||\n \t\t\topt.file_break || opt.funcbody))\n \t\t\tskip_first_line = 1;\n \t\tstart_threads(&opt);\n \t}\n-#endif\n \n \tif (show_in_pager && (cached || list.nr))\n \t\tdie(_(\"--open-files-in-pager only works on the worktree\"));\n@@ -1121,7 +1103,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\thit = grep_objects(&opt, &pathspec, &list);\n \t}\n \n-\tif (num_threads)\n+\tif (HAVE_THREADS && num_threads)\n \t\thit |= wait_all();\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\ndiff --git a/grep.c b/grep.c\nindex f6bd89e40b..4db1510d16 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1513,7 +1513,6 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t}\n }\n \n-#ifndef NO_PTHREADS\n int grep_use_locks;\n \n /*\n@@ -1539,11 +1538,6 @@ static inline void grep_attr_unlock(void)\n */\n pthread_mutex_t grep_read_mutex;\n \n-#else\n-#define grep_attr_lock()\n-#define grep_attr_unlock()\n-#endif\n-\n static int match_funcname(struct grep_opt *opt, struct grep_source *gs, char *bol, char *eol)\n {\n \txdemitconf_t *xecfg = opt->priv;\ndiff --git a/grep.h b/grep.h\nindex 1a57d12b90..fb04893721 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -229,7 +229,6 @@ int grep_source(struct grep_opt *opt, struct grep_source *gs);\n extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);\n extern int grep_threads_ok(const struct grep_opt *opt);\n \n-#ifndef NO_PTHREADS\n /*\n * Mutex used around access to the attributes machinery if\n * opt->use_threads. Must be initialized/destroyed by callers!\n@@ -250,9 +249,4 @@ static inline void grep_read_unlock(void)\n \t\tpthread_mutex_unlock(&grep_read_mutex);\n }\n \n-#else\n-#define grep_read_lock()\n-#define grep_read_unlock()\n-#endif\n-\n #endif\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361756","messageId":"20181027071003.1347-3-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 02/10] index-pack: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:09:55Z","receivedAt":"2018-10-27T07:10:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/index-pack.c | 68 ++++++++++++--------------------------------\n 1 file changed, 18 insertions(+), 50 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 2004e25da2..bbd66ca025 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -42,9 +42,7 @@ struct base_data {\n };\n \n struct thread_local {\n-#ifndef NO_PTHREADS\n \tpthread_t thread;\n-#endif\n \tstruct base_data *base_cache;\n \tsize_t base_cache_used;\n \tint pack_fd;\n@@ -98,8 +96,6 @@ static uint32_t input_crc32;\n static int input_fd, output_fd;\n static const char *curr_pack;\n \n-#ifndef NO_PTHREADS\n-\n static struct thread_local *thread_data;\n static int nr_dispatched;\n static int threads_active;\n@@ -179,26 +175,6 @@ static void cleanup_thread(void)\n \tfree(thread_data);\n }\n \n-#else\n-\n-#define read_lock()\n-#define read_unlock()\n-\n-#define counter_lock()\n-#define counter_unlock()\n-\n-#define work_lock()\n-#define work_unlock()\n-\n-#define deepest_delta_lock()\n-#define deepest_delta_unlock()\n-\n-#define type_cas_lock()\n-#define type_cas_unlock()\n-\n-#endif\n-\n-\n static int mark_link(struct object *obj, int type, void *data, struct fsck_options *options)\n {\n \tif (!obj)\n@@ -364,22 +340,20 @@ static NORETURN void bad_object(off_t offset, const char *format, ...)\n \n static inline struct thread_local *get_thread_data(void)\n {\n-#ifndef NO_PTHREADS\n-\tif (threads_active)\n-\t\treturn pthread_getspecific(key);\n-\tassert(!threads_active &&\n-\t \"This should only be reached when all threads are gone\");\n-#endif\n+\tif (HAVE_THREADS) {\n+\t\tif (threads_active)\n+\t\t\treturn pthread_getspecific(key);\n+\t\tassert(!threads_active &&\n+\t\t \"This should only be reached when all threads are gone\");\n+\t}\n \treturn ¬hread_data;\n }\n \n-#ifndef NO_PTHREADS\n static void set_thread_data(struct thread_local *data)\n {\n \tif (threads_active)\n \t\tpthread_setspecific(key, data);\n }\n-#endif\n \n static struct base_data *alloc_base_data(void)\n {\n@@ -1092,7 +1066,6 @@ static void resolve_base(struct object_entry *obj)\n \tfind_unresolved_deltas(base_obj);\n }\n \n-#ifndef NO_PTHREADS\n static void *threaded_second_pass(void *data)\n {\n \tset_thread_data(data);\n@@ -1116,7 +1089,6 @@ static void *threaded_second_pass(void *data)\n \t}\n \treturn NULL;\n }\n-#endif\n \n /*\n * First pass:\n@@ -1213,7 +1185,6 @@ static void resolve_deltas(void)\n \t\tprogress = start_progress(_(\"Resolving deltas\"),\n \t\t\t\t\t nr_ref_deltas + nr_ofs_deltas);\n \n-#ifndef NO_PTHREADS\n \tnr_dispatched = 0;\n \tif (nr_threads > 1 || getenv(\"GIT_FORCE_THREADS\")) {\n \t\tinit_thread();\n@@ -1229,7 +1200,6 @@ static void resolve_deltas(void)\n \t\tcleanup_thread();\n \t\treturn;\n \t}\n-#endif\n \n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *obj = &objects[i];\n@@ -1531,11 +1501,11 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\tif (nr_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t nr_threads);\n-#ifdef NO_PTHREADS\n-\t\tif (nr_threads != 1)\n-\t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n-\t\tnr_threads = 1;\n-#endif\n+\t\tif (!HAVE_THREADS) {\n+\t\t\tif (nr_threads != 1)\n+\t\t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n+\t\t\tnr_threads = 1;\n+\t\t}\n \t\treturn 0;\n \t}\n \treturn git_default_config(k, v, cb);\n@@ -1723,12 +1693,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n \t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n \t\t\t\t\tusage(index_pack_usage);\n-#ifdef NO_PTHREADS\n-\t\t\t\tif (nr_threads != 1)\n-\t\t\t\t\twarning(_(\"no threads support, \"\n-\t\t\t\t\t\t \"ignoring %s\"), arg);\n-\t\t\t\tnr_threads = 1;\n-#endif\n+\t\t\t\tif (!HAVE_THREADS) {\n+\t\t\t\t\tif (nr_threads != 1)\n+\t\t\t\t\t\twarning(_(\"no threads support, \"\n+\t\t\t\t\t\t\t \"ignoring %s\"), arg);\n+\t\t\t\t\tnr_threads = 1;\n+\t\t\t\t}\n \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n@@ -1791,14 +1761,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (strict)\n \t\topts.flags |= WRITE_IDX_STRICT;\n \n-#ifndef NO_PTHREADS\n-\tif (!nr_threads) {\n+\tif (HAVE_THREADS && !nr_threads) {\n \t\tnr_threads = online_cpus();\n \t\t/* An experiment showed that more threads does not mean faster */\n \t\tif (nr_threads > 3)\n \t\t\tnr_threads = 3;\n \t}\n-#endif\n \n \tcurr_pack = open_pack_file(pack_name);\n \tparse_pack_header();\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361757","messageId":"20181027071003.1347-8-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 07/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:10:00Z","receivedAt":"2018-10-27T07:10:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n preload-index.c | 15 ++-------------\n 1 file changed, 2 insertions(+), 13 deletions(-)\n\ndiff --git a/preload-index.c b/preload-index.c\nindex 9e7152ab14..0e24886aca 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -7,17 +7,7 @@\n #include \"fsmonitor.h\"\n #include \"config.h\"\n #include \"progress.h\"\n-\n-#ifdef NO_PTHREADS\n-static void preload_index(struct index_state *index,\n-\t\t\t const struct pathspec *pathspec,\n-\t\t\t unsigned int refresh_flags)\n-{\n-\t; /* nothing */\n-}\n-#else\n-\n-#include \n+#include \"thread-utils.h\"\n \n /*\n * Mostly randomly chosen maximum thread counts: we\n@@ -108,7 +98,7 @@ static void preload_index(struct index_state *index,\n \tstruct thread_data data[MAX_PARALLEL];\n \tstruct progress_data pd;\n \n-\tif (!core_preload_index)\n+\tif (!HAVE_THREADS || !core_preload_index)\n \t\treturn;\n \n \tthreads = index->cache_nr / THREAD_COST;\n@@ -151,7 +141,6 @@ static void preload_index(struct index_state *index,\n \n \ttrace_performance_leave(\"preload index\");\n }\n-#endif\n \n int read_index_preload(struct index_state *index,\n \t\t const struct pathspec *pathspec,\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361758","messageId":"20181027071003.1347-9-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 08/10] pack-objects: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:10:01Z","receivedAt":"2018-10-27T07:10:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 26 ++------------------------\n pack-objects.h | 6 ------\n 2 files changed, 2 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b059b86aee..12edd6da16 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1953,8 +1953,6 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \treturn 0;\n }\n \n-#ifndef NO_PTHREADS\n-\n /* Protect access to object database */\n static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n@@ -1979,16 +1977,6 @@ static pthread_mutex_t progress_mutex;\n * ahead in the list because they can be stolen and would need\n * progress_mutex for protection.\n */\n-#else\n-\n-#define read_lock()\t\t(void)0\n-#define read_unlock()\t\t(void)0\n-#define cache_lock()\t\t(void)0\n-#define cache_unlock()\t\t(void)0\n-#define progress_lock()\t\t(void)0\n-#define progress_unlock()\t(void)0\n-\n-#endif\n \n /*\n * Return the size of the object without doing any delta\n@@ -2347,8 +2335,6 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \tfree(array);\n }\n \n-#ifndef NO_PTHREADS\n-\n static void try_to_free_from_threads(size_t size)\n {\n \tread_lock();\n@@ -2578,10 +2564,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tfree(p);\n }\n \n-#else\n-#define ll_find_deltas(l, s, w, d, p)\tfind_deltas(l, &s, w, d, p)\n-#endif\n-\n static void add_tag_chain(const struct object_id *oid)\n {\n \tstruct tag *tag;\n@@ -2734,12 +2716,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n \t\tif (delta_search_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t delta_search_threads);\n-#ifdef NO_PTHREADS\n-\t\tif (delta_search_threads != 1) {\n+\t\tif (!HAVE_THREADS && delta_search_threads != 1) {\n \t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n \t\t\tdelta_search_threads = 0;\n \t\t}\n-#endif\n \t\treturn 0;\n \t}\n \tif (!strcmp(k, \"pack.indexversion\")) {\n@@ -3402,10 +3382,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n \t\tdelta_search_threads = online_cpus();\n \n-#ifdef NO_PTHREADS\n-\tif (delta_search_threads != 1)\n+\tif (!HAVE_THREADS && delta_search_threads != 1)\n \t\twarning(_(\"no threads support, ignoring --threads\"));\n-#endif\n \tif (!pack_to_stdout && !pack_size_limit)\n \t\tpack_size_limit = pack_size_limit_cfg;\n \tif (pack_to_stdout && pack_size_limit)\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 2ca39cfcfe..3a42727c7d 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -145,9 +145,7 @@ struct packing_data {\n \tstruct packed_git **in_pack_by_idx;\n \tstruct packed_git **in_pack;\n \n-#ifndef NO_PTHREADS\n \tpthread_mutex_t lock;\n-#endif\n \n \t/*\n \t * This list contains entries for bases which we know the other side\n@@ -169,15 +167,11 @@ void prepare_packing_data(struct packing_data *pdata);\n \n static inline void packing_data_lock(struct packing_data *pdata)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&pdata->lock);\n-#endif\n }\n static inline void packing_data_unlock(struct packing_data *pdata)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&pdata->lock);\n-#endif\n }\n \n struct object_entry *packlist_alloc(struct packing_data *pdata,\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361759","messageId":"20181027071003.1347-10-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 09/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:10:02Z","receivedAt":"2018-10-27T07:10:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n read-cache.c | 49 ++++++++++++++++++-------------------------------\n 1 file changed, 18 insertions(+), 31 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex d57958233e..ba870bc3fd 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1920,19 +1920,15 @@ struct index_entry_offset_table\n \tstruct index_entry_offset entries[FLEX_ARRAY];\n };\n \n-#ifndef NO_PTHREADS\n static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset);\n static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_table *ieot);\n-#endif\n \n static size_t read_eoie_extension(const char *mmap, size_t mmap_size);\n static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context, size_t offset);\n \n struct load_index_extensions\n {\n-#ifndef NO_PTHREADS\n \tpthread_t pthread;\n-#endif\n \tstruct index_state *istate;\n \tconst char *mmap;\n \tsize_t mmap_size;\n@@ -2010,8 +2006,6 @@ static unsigned long load_all_cache_entries(struct index_state *istate,\n \treturn consumed;\n }\n \n-#ifndef NO_PTHREADS\n-\n /*\n * Mostly randomly chosen maximum thread counts: we\n * cap the parallelism to online_cpus() threads, and we want\n@@ -2122,7 +2116,6 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con\n \n \treturn consumed;\n }\n-#endif\n \n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int must_exist)\n@@ -2135,10 +2128,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tsize_t mmap_size;\n \tstruct load_index_extensions p;\n \tsize_t extension_offset = 0;\n-#ifndef NO_PTHREADS\n \tint nr_threads, cpus;\n \tstruct index_entry_offset_table *ieot = NULL;\n-#endif\n \n \tif (istate->initialized)\n \t\treturn istate->cache_nr;\n@@ -2181,15 +2172,18 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tsrc_offset = sizeof(*hdr);\n \n-#ifndef NO_PTHREADS\n-\tnr_threads = git_config_get_index_threads();\n+\tif (HAVE_THREADS) {\n+\t\tnr_threads = git_config_get_index_threads();\n \n-\t/* TODO: does creating more threads than cores help? */\n-\tif (!nr_threads) {\n-\t\tnr_threads = istate->cache_nr / THREAD_COST;\n-\t\tcpus = online_cpus();\n-\t\tif (nr_threads > cpus)\n-\t\t\tnr_threads = cpus;\n+\t\t/* TODO: does creating more threads than cores help? */\n+\t\tif (!nr_threads) {\n+\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n+\t\t\tcpus = online_cpus();\n+\t\t\tif (nr_threads > cpus)\n+\t\t\t\tnr_threads = cpus;\n+\t\t}\n+\t} else {\n+\t\tnr_threads = 1;\n \t}\n \n \tif (nr_threads > 1) {\n@@ -2219,21 +2213,16 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t} else {\n \t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n \t}\n-#else\n-\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n-#endif\n \n \tistate->timestamp.sec = st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \n \t/* if we created a thread, join it otherwise load the extensions on the primary thread */\n-#ifndef NO_PTHREADS\n-\tif (extension_offset) {\n+\tif (HAVE_THREADS && extension_offset) {\n \t\tint ret = pthread_join(p.pthread, NULL);\n \t\tif (ret)\n \t\t\tdie(_(\"unable to join load_index_extensions thread: %s\"), strerror(ret));\n \t}\n-#endif\n \tif (!extension_offset) {\n \t\tp.src_offset = src_offset;\n \t\tload_index_extensions(&p);\n@@ -2756,8 +2745,11 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n \t\treturn -1;\n \n-#ifndef NO_PTHREADS\n-\tnr_threads = git_config_get_index_threads();\n+\tif (HAVE_THREADS)\n+\t\tnr_threads = git_config_get_index_threads();\n+\telse\n+\t\tnr_threads = 1;\n+\n \tif (nr_threads != 1) {\n \t\tint ieot_blocks, cpus;\n \n@@ -2787,7 +2779,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t\tieot_entries = DIV_ROUND_UP(entries, ieot_blocks);\n \t\t}\n \t}\n-#endif\n \n \toffset = lseek(newfd, 0, SEEK_CUR);\n \tif (offset < 0) {\n@@ -2871,8 +2862,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t * strip_extensions parameter as we need it when loading the shared\n \t * index.\n \t */\n-#ifndef NO_PTHREADS\n-\tif (ieot) {\n+\tif (HAVE_THREADS && ieot) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\twrite_ieot_extension(&sb, ieot);\n@@ -2883,7 +2873,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\tif (err)\n \t\t\treturn -1;\n \t}\n-#endif\n \n \tif (!strip_extensions && istate->split_index) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n@@ -3469,7 +3458,6 @@ static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context,\n \tstrbuf_add(sb, hash, the_hash_algo->rawsz);\n }\n \n-#ifndef NO_PTHREADS\n #define IEOT_VERSION\t(1)\n \n static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset)\n@@ -3542,4 +3530,3 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta\n \t strbuf_add(sb, &buffer, sizeof(uint32_t));\n }\n }\n-#endif\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361760","messageId":"20181027071003.1347-11-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH 10/10] Clean up pthread_create() error handling","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:10:03Z","receivedAt":"2018-10-27T07:10:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Normally pthread_create() rarely fails and sometimes error handling\ncan be neglected. But with new pthreads wrapper, pthread_create() will\nreturn ENOSYS on a system without thread support.\n\nThreaded code _is_ protected by HAVE_THREADS and pthread_create()\nshould never run in the first place. But the situation could change in\nthe future and bugs may sneak in. Make sure that all pthread_create()\nchecks and handles the result.\n\nWhile at there, mark these strings for translation if they aren't.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n name-hash.c | 16 ++++++++++------\n preload-index.c | 8 ++++++--\n run-command.c | 2 +-\n 3 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex b3c9ac791d..623ca6923a 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -494,6 +494,7 @@ static inline void lazy_update_dir_ref_counts(\n static void threaded_lazy_init_name_hash(\n \tstruct index_state *istate)\n {\n+\tint err;\n \tint nr_each;\n \tint k_start;\n \tint t;\n@@ -526,8 +527,9 @@ static void threaded_lazy_init_name_hash(\n \t\tif (k_start > istate->cache_nr)\n \t\t\tk_start = istate->cache_nr;\n \t\ttd_dir_t->k_end = k_start;\n-\t\tif (pthread_create(&td_dir_t->pthread, NULL, lazy_dir_thread_proc, td_dir_t))\n-\t\t\tdie(\"unable to create lazy_dir_thread\");\n+\t\terr = pthread_create(&td_dir_t->pthread, NULL, lazy_dir_thread_proc, td_dir_t);\n+\t\tif (err)\n+\t\t\tdie(_(\"unable to create lazy_dir thread: %s\"), strerror(err));\n \t}\n \tfor (t = 0; t < lazy_nr_dir_threads; t++) {\n \t\tstruct lazy_dir_thread_data *td_dir_t = td_dir + t;\n@@ -547,13 +549,15 @@ static void threaded_lazy_init_name_hash(\n \t */\n \ttd_name->istate = istate;\n \ttd_name->lazy_entries = lazy_entries;\n-\tif (pthread_create(&td_name->pthread, NULL, lazy_name_thread_proc, td_name))\n-\t\tdie(\"unable to create lazy_name_thread\");\n+\terr = pthread_create(&td_name->pthread, NULL, lazy_name_thread_proc, td_name);\n+\tif (err)\n+\t\tdie(_(\"unable to create lazy_name thread: %s\"), strerror(err));\n \n \tlazy_update_dir_ref_counts(istate, lazy_entries);\n \n-\tif (pthread_join(td_name->pthread, NULL))\n-\t\tdie(\"unable to join lazy_name_thread\");\n+\terr = pthread_join(td_name->pthread, NULL);\n+\tif (err)\n+\t\tdie(_(\"unable to join lazy_name thread: %s\"), strerror(err));\n \n \tcleanup_dir_mutex();\n \ndiff --git a/preload-index.c b/preload-index.c\nindex 0e24886aca..ddca1c216e 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -121,6 +121,8 @@ static void preload_index(struct index_state *index,\n \n \tfor (i = 0; i < threads; i++) {\n \t\tstruct thread_data *p = data+i;\n+\t\tint err;\n+\n \t\tp->index = index;\n \t\tif (pathspec)\n \t\t\tcopy_pathspec(&p->pathspec, pathspec);\n@@ -129,8 +131,10 @@ static void preload_index(struct index_state *index,\n \t\tif (pd.progress)\n \t\t\tp->progress = &pd;\n \t\toffset += work;\n-\t\tif (pthread_create(&p->pthread, NULL, preload_thread, p))\n-\t\t\tdie(\"unable to create threaded lstat\");\n+\t\terr = pthread_create(&p->pthread, NULL, preload_thread, p);\n+\n+\t\tif (err)\n+\t\t\tdie(_(\"unable to create threaded lstat: %s\"), strerror(err));\n \t}\n \tfor (i = 0; i < threads; i++) {\n \t\tstruct thread_data *p = data+i;\ndiff --git a/run-command.c b/run-command.c\nindex 84b883c213..26154ba257 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1213,7 +1213,7 @@ int start_async(struct async *async)\n \t{\n \t\tint err = pthread_create(&async->tid, NULL, run_thread, async);\n \t\tif (err) {\n-\t\t\terror_errno(\"cannot create thread\");\n+\t\t\terror(_(\"cannot create async thread: %s\"), strerror(err));\n \t\t\tgoto error;\n \t\t}\n \t}\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361764","messageId":"20181027072447.GA26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"Re: [PATCH 00/10] Reduce #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T07:24:47Z","receivedAt":"2018-10-27T07:24:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:09:53AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> People seemed to support the idea of removing these #ifdef NO_PTHREADS [1]\n> so this is a complete series. I left the #ifdef in run-command.c and\n> transport-helper.c because those code looked complicated so perhaps we\n> could clean them up later. Even these updated files could be updated\n> more, I think, to reduce some code duplication, but I tried to keep\n> the change here minimal.\n\nSome of the bits in run-command.c can probably be converted, but all of\nthe \"struct async\" bits can't ever be. Because there the fallback is not\n\"OK, let's just run a single thread\" but a whole separate forking\nmechanism.\n\n-Peff\n"},{"id":"361768","messageId":"20181027073125.GB26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027071003.1347-2-pclouds@gmail.com","subject":"Re: [PATCH 01/10] thread-utils: macros to unconditionally compile pthreads API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T07:31:26Z","receivedAt":"2018-10-27T07:31:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:09:54AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> +/*\n> + * macros instead of typedefs because pthread definitions may have\n> + * been pulled in by some system dependencies even though the user\n> + * wants to disable pthread.\n> + */\n> +#define pthread_t int\n> +#define pthread_mutex_t int\n> +#define pthread_cond_t int\n> +\n> +#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)\n> +#define pthread_mutex_lock(mutex)\n> +#define pthread_mutex_unlock(mutex)\n> +#define pthread_mutex_destroy(mutex)\n\nOK, we only need to do the dummy init to shut up the compiler, and the\nrest really can just become noops. Makes sense.\n\n> +#define pthread_cond_init(cond, attr) dummy_pthread_init(cond)\n> +#define pthread_cond_wait(cond, mutex)\n> +#define pthread_cond_signal(cond)\n> +#define pthread_cond_broadcast(cond)\n> +#define pthread_cond_destroy(cond)\n\nAnd this is the same.\n\n> +#define pthread_key_create(key, attr) dummy_pthread_init(key)\n> +#define pthread_key_delete(key)\n\nWe don't need keys because we know we have only one key: the main\nthread. Makes sense. But...\n\n> +#define pthread_setspecific(key, data)\n> +#define pthread_getspecific(key) NULL\n\nWe expect to be able to store a void pointer here and get it back, which\nshould work even for a single thread. Do we need something like:\n\n extern void *pthread_specific_data;\n\n #define pthread_setspecific(key, data) do { \\\n\tpthread_specific_data = data; \\\n } while(0)\n\n void pthread_getspecific(key) pthread_specific_data\n\n> +#define pthread_create(thread, attr, fn, data) \\\n> +\tdummy_pthread_create(thread, attr, fn, data)\n\nI wondered if this should actually run \"fn\", but I guess there is not\nmuch point. At this point the caller expects the main thread to keep\ngoing, so this is already an error, and reporting ENOSYS is probably the\nbest thing to do.\n\n-Peff\n"},{"id":"361769","messageId":"20181027073446.GC26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027071003.1347-3-pclouds@gmail.com","subject":"Re: [PATCH 02/10] index-pack: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T07:34:46Z","receivedAt":"2018-10-27T07:34:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:09:55AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> static inline struct thread_local *get_thread_data(void)\n> {\n> -#ifndef NO_PTHREADS\n> -\tif (threads_active)\n> -\t\treturn pthread_getspecific(key);\n> -\tassert(!threads_active &&\n> -\t \"This should only be reached when all threads are gone\");\n> -#endif\n> +\tif (HAVE_THREADS) {\n> +\t\tif (threads_active)\n> +\t\t\treturn pthread_getspecific(key);\n> +\t\tassert(!threads_active &&\n> +\t\t \"This should only be reached when all threads are gone\");\n> +\t}\n> \treturn ¬hread_data;\n> }\n> \n> -#ifndef NO_PTHREADS\n> static void set_thread_data(struct thread_local *data)\n> {\n> \tif (threads_active)\n> \t\tpthread_setspecific(key, data);\n> }\n> -#endif\n\nIf we do the getspecific/setspecific thing I mentioned for the previous\npatch, then I think helpers like this can just go away.\n\n-Peff\n"},{"id":"361770","messageId":"20181027073954.GD26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027071003.1347-6-pclouds@gmail.com","subject":"Re: [PATCH 05/10] send-pack.c: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T07:39:55Z","receivedAt":"2018-10-27T07:39:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:09:58AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> While at there correct \"#include cache.h\" position. It must be one of\n> the first includes.\n> [...]\n> @@ -1,4 +1,5 @@\n> #include \"builtin.h\"\n> +#include \"cache.h\"\n\nI think it's actually fine as it is. The rule is not about cache.h in\nparticular, but that the first include must be one of the special ones.\nAnd builtin.h is such a special one. After that, there are no ordering\nrules.\n\n> @@ -203,9 +204,8 @@ static int receive_status(int in, struct ref *refs)\n> static int sideband_demux(int in, int out, void *data)\n> {\n> \tint *fd = data, ret;\n> -#ifdef NO_PTHREADS\n> -\tclose(fd[1]);\n> -#endif\n> +\tif (!HAVE_THREADS)\n> +\t\tclose(fd[1]);\n> \tret = recv_sideband(\"send-pack\", fd[0], out);\n> \tclose(out);\n> \treturn ret;\n\nThis one is a very interesting case. Your conversion here isn't wrong,\nbut what we actually want to know is: is \"struct async\" implemented as a\nseparate process or not.\n\nAnd \"struct async\" will continue to switch on NO_PTHREADS, as it can\nnever use these dummy bits. I can't think of a way that HAVE_THREADS\nwould ever diverge from how \"struct async\" works, but my gut feeling is\nthat we probably ought to have a separate variable for the async code to\ntell how it works. Even if it ends up being the same as HAVE_THREADS,\nit helps untangle the two cases.\n\n-Peff\n"},{"id":"361771","messageId":"CACsJy8AA8ABmV5myBEETWP0uSNNFjvUP5mE705OV9=JtyHgTkg@mail.gmail.com","threadId":"49697","inReplyTo":"20181027073125.GB26685@sigill.intra.peff.net","subject":"Re: [PATCH 01/10] thread-utils: macros to unconditionally compile pthreads API","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T07:40:13Z","receivedAt":"2018-10-27T07:40:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Oct 27, 2018 at 9:31 AM Jeff King wrote:\n> > +#define pthread_setspecific(key, data)\n> > +#define pthread_getspecific(key) NULL\n>\n> We expect to be able to store a void pointer here and get it back, which\n> should work even for a single thread. Do we need something like:\n>\n> extern void *pthread_specific_data;\n>\n> #define pthread_setspecific(key, data) do { \\\n> pthread_specific_data = data; \\\n> } while(0)\n>\n> void pthread_getspecific(key) pthread_specific_data\n\nThe data is per key though so a correct implementation may involve a\nhashmap or a list. It does simplify index-pack which has to fall back\nto nothread_data when pthreads is not available. But with index-pack\nbeing the only call site that can take advantage of this\n(run-command.c probably will use real pthreads library anyway), I'm\nnot sure if it's worth really implementing these functions.\n-- \nDuy\n"},{"id":"361772","messageId":"20181027074443.GE26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027071003.1347-7-pclouds@gmail.com","subject":"Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T07:44:44Z","receivedAt":"2018-10-27T07:44:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:09:59AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index d8508ddf79..29221e1003 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -34,7 +34,6 @@ static int recurse_submodules;\n> #define GREP_NUM_THREADS_DEFAULT 8\n> static int num_threads;\n> \n> -#ifndef NO_PTHREADS\n> static pthread_t *threads;\n> \n> /* We use one producer thread and THREADS consumer\n> @@ -265,13 +264,6 @@ static int wait_all(void)\n> \n> \treturn hit;\n> }\n> -#else /* !NO_PTHREADS */\n> -\n> -static int wait_all(void)\n> -{\n> -\treturn 0;\n> -}\n> -#endif\n\nCases like this are kind of weird. I'd expect to see wait_all() return\nimmediately when !HAVE_THREADS. But we don't need to, because later we\ndo this:\n\n> -\tif (num_threads)\n> +\tif (HAVE_THREADS && num_threads)\n> \t\thit |= wait_all();\n\nWhich I think works, but it feels like we're introducing some subtle\ndependencies about which functions get called in which cases. I'd hoped\nin general that the non-threaded code paths could mostly just collapse\ndown into the same as the \"threads == 1\" cases, and we wouldn't have to\nask \"are threads even supported\" in a lot of places.\n\nI dunno. My comment isn't really an objection exactly, but mostly just\nan indication that I'm still thinking through what the best approach is,\nand what end state we want to end up in.\n\n-Peff\n"},{"id":"361775","messageId":"20181027081328.GF26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"Re: [PATCH 00/10] Reduce #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T08:13:28Z","receivedAt":"2018-10-27T08:13:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:09:53AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> People seemed to support the idea of removing these #ifdef NO_PTHREADS [1]\n> so this is a complete series. I left the #ifdef in run-command.c and\n> transport-helper.c because those code looked complicated so perhaps we\n> could clean them up later. Even these updated files could be updated\n> more, I think, to reduce some code duplication, but I tried to keep\n> the change here minimal.\n> \n> [1] https://public-inbox.org/git/20181018180522.17642-1-pclouds@gmail.com/\n> \n> Nguyễn Thái Ngọc Duy (10):\n> thread-utils: macros to unconditionally compile pthreads API\n> index-pack: remove #ifdef NO_PTHREADS\n> name-hash.c: remove #ifdef NO_PTHREADS\n> attr.c: remove #ifdef NO_PTHREADS\n> send-pack.c: remove #ifdef NO_PTHREADS\n> grep: remove #ifdef NO_PTHREADS\n> preload-index.c: remove #ifdef NO_PTHREADS\n> pack-objects: remove #ifdef NO_PTHREADS\n> read-cache.c: remove #ifdef NO_PTHREADS\n> Clean up pthread_create() error handling\n\nCompiling with NO_PTHREADS=1, I get (with gcc 8.2.0):\n\nread-cache.c: In function ‘do_read_index’:\nread-cache.c:1820:4: error: ‘copy_len’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n memcpy(ce->name, previous_ce->name, copy_len);\n ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nread-cache.c:1749:9: note: ‘copy_len’ was declared here\n size_t copy_len;\n\nwhich seems wrong to me. It's always tied to expand_name_field being\ntrue. And curiously an earlier use doesn't trigger the warning. I wonder\nif some kind of tricky pointer aliasing in the intervening code makes it\nthink that expand_name_field could change, but I sure don't see it.\n\nI ran the tests under ASan/UBSan, since this series seems like it has a\ngood chance of accidentally causing issues there, but didn't come up\nwith any problems (except for the ones already fixed on pu by 8628ace269\n(commit-reach: fix cast in compare_commits_by_gen(), 2018-10-01)).\n\n-Peff\n"},{"id":"361776","messageId":"20181027081538.GG26685@sigill.intra.peff.net","threadId":"49697","inReplyTo":"CACsJy8AA8ABmV5myBEETWP0uSNNFjvUP5mE705OV9=JtyHgTkg@mail.gmail.com","subject":"Re: [PATCH 01/10] thread-utils: macros to unconditionally compile pthreads API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-27T08:15:39Z","receivedAt":"2018-10-27T08:15:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 09:40:13AM +0200, Duy Nguyen wrote:\n\n> > We expect to be able to store a void pointer here and get it back, which\n> > should work even for a single thread. Do we need something like:\n> >\n> > extern void *pthread_specific_data;\n> >\n> > #define pthread_setspecific(key, data) do { \\\n> > pthread_specific_data = data; \\\n> > } while(0)\n> >\n> > void pthread_getspecific(key) pthread_specific_data\n> \n> The data is per key though so a correct implementation may involve a\n> hashmap or a list.\n\nAh, yeah, you're right, I was mixing up the thread id and the key in my\nhead. I think it would just be an array of void pointers, with\npthread_key_create() returning an static index.\n\n> It does simplify index-pack which has to fall back to nothread_data\n> when pthreads is not available. But with index-pack being the only\n> call site that can take advantage of this (run-command.c probably will\n> use real pthreads library anyway), I'm not sure if it's worth really\n> implementing these functions.\n\nYeah, you might be right.\n\n-Peff\n"},{"id":"361788","messageId":"CACsJy8DLW_smOJd6aCoRcJZxQ2Lzut5US=sPadj7=fhne0UHGg@mail.gmail.com","threadId":"49697","inReplyTo":"20181027081538.GG26685@sigill.intra.peff.net","subject":"Re: [PATCH 01/10] thread-utils: macros to unconditionally compile pthreads API","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T14:43:42Z","receivedAt":"2018-10-27T14:44:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Oct 27, 2018 at 10:15 AM Jeff King wrote:\n>\n> On Sat, Oct 27, 2018 at 09:40:13AM +0200, Duy Nguyen wrote:\n>\n> > > We expect to be able to store a void pointer here and get it back, which\n> > > should work even for a single thread. Do we need something like:\n> > >\n> > > extern void *pthread_specific_data;\n> > >\n> > > #define pthread_setspecific(key, data) do { \\\n> > > pthread_specific_data = data; \\\n> > > } while(0)\n> > >\n> > > void pthread_getspecific(key) pthread_specific_data\n> >\n> > The data is per key though so a correct implementation may involve a\n> > hashmap or a list.\n>\n> Ah, yeah, you're right, I was mixing up the thread id and the key in my\n> head. I think it would just be an array of void pointers, with\n> pthread_key_create() returning an static index.\n\nWe could redefine pthread_key_t as \"void *\" though, then\n_setspecific(key, data) is key = data; But there will be more changes\nin index-pack.c to take advantage of it. I took a stab but couldn't\nfigure out fast enough where _setspecific(.., ¬hread_data) should\nbe called in NO_PTHREADS mode (to simplify get_thread_data() to just a\nwrapper of _getspecific) and got bored. It could be a micro project\nfor someone really wants to learn about index-pack.c\n-- \nDuy\n"},{"id":"361790","messageId":"CACsJy8Ao2oJoEp8GpSU2DosQ1kf880uvZ0YKWveaehZsNdZhbQ@mail.gmail.com","threadId":"49697","inReplyTo":"20181027081328.GF26685@sigill.intra.peff.net","subject":"Re: [PATCH 00/10] Reduce #ifdef NO_PTHREADS","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:07:09Z","receivedAt":"2018-10-27T17:07:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Oct 27, 2018 at 10:13 AM Jeff King wrote:\n>\n> On Sat, Oct 27, 2018 at 09:09:53AM +0200, Nguyễn Thái Ngọc Duy wrote:\n>\n> > People seemed to support the idea of removing these #ifdef NO_PTHREADS [1]\n> > so this is a complete series. I left the #ifdef in run-command.c and\n> > transport-helper.c because those code looked complicated so perhaps we\n> > could clean them up later. Even these updated files could be updated\n> > more, I think, to reduce some code duplication, but I tried to keep\n> > the change here minimal.\n> >\n> > [1] https://public-inbox.org/git/20181018180522.17642-1-pclouds@gmail.com/\n> >\n> > Nguyễn Thái Ngọc Duy (10):\n> > thread-utils: macros to unconditionally compile pthreads API\n> > index-pack: remove #ifdef NO_PTHREADS\n> > name-hash.c: remove #ifdef NO_PTHREADS\n> > attr.c: remove #ifdef NO_PTHREADS\n> > send-pack.c: remove #ifdef NO_PTHREADS\n> > grep: remove #ifdef NO_PTHREADS\n> > preload-index.c: remove #ifdef NO_PTHREADS\n> > pack-objects: remove #ifdef NO_PTHREADS\n> > read-cache.c: remove #ifdef NO_PTHREADS\n> > Clean up pthread_create() error handling\n>\n> Compiling with NO_PTHREADS=1, I get (with gcc 8.2.0):\n>\n> read-cache.c: In function ‘do_read_index’:\n> read-cache.c:1820:4: error: ‘copy_len’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n> memcpy(ce->name, previous_ce->name, copy_len);\n> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> read-cache.c:1749:9: note: ‘copy_len’ was declared here\n> size_t copy_len;\n>\n> which seems wrong to me. It's always tied to expand_name_field being\n> true. And curiously an earlier use doesn't trigger the warning. I wonder\n> if some kind of tricky pointer aliasing in the intervening code makes it\n> think that expand_name_field could change, but I sure don't see it.\n\nI couldn't reproduce it with gcc 8.2.0 on gentoo (I tried all\noptimization levels just in case). I agree the warning is wrong. But I\nwill initialize copy_len anyway to avoid this.\n-- \nDuy\n"},{"id":"361791","messageId":"20181027173008.18852-1-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-1-pclouds@gmail.com","subject":"[PATCH v2 00/10] Reduce #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:29:58Z","receivedAt":"2018-10-27T17:30:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The send-pack.c patch is dropped since it's tied to async code and\nlike transport-helper.c (or more cleanups in index-pack.c) could be\nleft for later.\n\nI added one more patch to shut up -Wmaybe-uninitialized but since I\ncould not reproduce it, Jeff would need to verify if it works for him.\n\nNguyễn Thái Ngọc Duy (10):\n thread-utils: macros to unconditionally compile pthreads API\n index-pack: remove #ifdef NO_PTHREADS\n name-hash.c: remove #ifdef NO_PTHREADS\n attr.c: remove #ifdef NO_PTHREADS\n grep: remove #ifdef NO_PTHREADS\n preload-index.c: remove #ifdef NO_PTHREADS\n pack-objects: remove #ifdef NO_PTHREADS\n read-cache.c: remove #ifdef NO_PTHREADS\n Clean up pthread_create() error handling\n read-cache.c: initialize copy_len to shut up gcc 8\n\n Makefile | 2 +-\n attr.c | 14 ---------\n builtin/grep.c | 59 ++++++++++++++----------------------\n builtin/index-pack.c | 68 +++++++++++-------------------------------\n builtin/pack-objects.c | 26 ++--------------\n grep.c | 6 ----\n grep.h | 6 ----\n name-hash.c | 38 +++++++++--------------\n pack-objects.h | 6 ----\n preload-index.c | 23 +++++---------\n read-cache.c | 53 ++++++++++++--------------------\n run-command.c | 2 +-\n thread-utils.c | 48 +++++++++++++++++++++++++++++\n thread-utils.h | 48 +++++++++++++++++++++++++++--\n 14 files changed, 178 insertions(+), 221 deletions(-)\n\nRange-diff against v1:\n 1: fd4926a83a ! 1: 2791c06629 thread-utils: macros to unconditionally compile pthreads API\n @@ -146,6 +146,7 @@\n +#define pthread_t int\n +#define pthread_mutex_t int\n +#define pthread_cond_t int\n ++#define pthread_key_t int\n +\n +#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)\n +#define pthread_mutex_lock(mutex)\n 2: 47b1f63fdd = 2: 61b3efceee index-pack: remove #ifdef NO_PTHREADS\n 3: a842b27e1e = 3: 93aa33eaa6 name-hash.c: remove #ifdef NO_PTHREADS\n 4: c0939283c3 = 4: 5d9441d8e2 attr.c: remove #ifdef NO_PTHREADS\n 5: 6c6f3a0d28 < -: ---------- send-pack.c: remove #ifdef NO_PTHREADS\n 6: 9f6e259fb4 ! 5: 834b19d5c6 grep: remove #ifdef NO_PTHREADS\n @@ -13,6 +13,16 @@\n static pthread_t *threads;\n \n /* We use one producer thread and THREADS consumer\n +@@\n + \tint hit = 0;\n + \tint i;\n + \n ++\tif (!HAVE_THREADS)\n ++\t\treturn 0;\n ++\n + \tgrep_lock();\n + \tall_work_added = 1;\n + \n @@\n \n \treturn hit;\n @@ -136,15 +146,6 @@\n \n \tif (show_in_pager && (cached || list.nr))\n \t\tdie(_(\"--open-files-in-pager only works on the worktree\"));\n -@@\n - \t\thit = grep_objects(&opt, &pathspec, &list);\n - \t}\n - \n --\tif (num_threads)\n -+\tif (HAVE_THREADS && num_threads)\n - \t\thit |= wait_all();\n - \tif (hit && show_in_pager)\n - \t\trun_pager(&opt, prefix);\n \n diff --git a/grep.c b/grep.c\n --- a/grep.c\n 7: 97fb2fdba4 = 6: 10969f86ae preload-index.c: remove #ifdef NO_PTHREADS\n 8: b8aeb2776b = 7: 7e83190364 pack-objects: remove #ifdef NO_PTHREADS\n 9: 40a9bfb8f2 = 8: 404e9d9041 read-cache.c: remove #ifdef NO_PTHREADS\n10: 0bb0c667ff = 9: 046008b4ef Clean up pthread_create() error handling\n -: ---------- > 10: 2ad9554687 read-cache.c: initialize copy_len to shut up gcc 8\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361792","messageId":"20181027173008.18852-2-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 01/10] thread-utils: macros to unconditionally compile pthreads API","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:29:59Z","receivedAt":"2018-10-27T17:30:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When built with NO_PTHREADS, the macros are used make the code build\neven though pthreads header and library may be missing. The code can\nstill have different code paths for no threads support with\nHAVE_THREADS variable.\n\nThere are of course impacts on no-pthreads builds:\n\n- data structure may get slightly bigger because all the mutexes and\n pthread_t are present (as an int)\n\n- code execution is not impacted much. Locking (in hot path) is\n no-op. Other wrapper function calls really should not matter much.\n\n- the binary size grows bigger because of threaded code. But at least\n on Linux this does not matter, if some code is not executed, it's\n not mapped in memory.\n\nThis is a preparation step to remove \"#ifdef NO_PTHREADS\" in the code\nmostly because of maintainability. As Jeff put it\n\n> it's probably OK to stop thinking of it as \"non-threaded platforms\n> are the default and must pay zero cost\" and more as \"threaded\n> platforms are the default, and non-threaded ones are OK to pay a\n> small cost as long as they still work\".\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Makefile | 2 +-\n thread-utils.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n thread-utils.h | 48 +++++++++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 94 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b08d5ea258..321540a736 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -991,6 +991,7 @@ LIB_OBJS += sub-process.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += tempfile.o\n+LIB_OBJS += thread-utils.o\n LIB_OBJS += tmp-objdir.o\n LIB_OBJS += trace.o\n LIB_OBJS += trailer.o\n@@ -1674,7 +1675,6 @@ ifdef NO_PTHREADS\n else\n \tBASIC_CFLAGS += $(PTHREAD_CFLAGS)\n \tEXTLIBS += $(PTHREAD_LIBS)\n-\tLIB_OBJS += thread-utils.o\n endif\n \n ifdef HAVE_PATHS_H\ndiff --git a/thread-utils.c b/thread-utils.c\nindex a2135e0743..5329845691 100644\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -20,6 +20,9 @@\n \n int online_cpus(void)\n {\n+#ifdef NO_PTHREADS\n+\treturn 1;\n+#else\n #ifdef _SC_NPROCESSORS_ONLN\n \tlong ncpus;\n #endif\n@@ -59,10 +62,12 @@ int online_cpus(void)\n #endif\n \n \treturn 1;\n+#endif\n }\n \n int init_recursive_mutex(pthread_mutex_t *m)\n {\n+#ifndef NO_PTHREADS\n \tpthread_mutexattr_t a;\n \tint ret;\n \n@@ -74,4 +79,47 @@ int init_recursive_mutex(pthread_mutex_t *m)\n \t\tpthread_mutexattr_destroy(&a);\n \t}\n \treturn ret;\n+#else\n+\treturn 0;\n+#endif\n+}\n+\n+#ifdef NO_PTHREADS\n+int dummy_pthread_create(pthread_t *pthread, const void *attr,\n+\t\t\t void *(*fn)(void *), void *data)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis and avoid -Wunused-variable false warnings.\n+\t */\n+\treturn ENOSYS;\n+}\n+\n+int dummy_pthread_init(void *data)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis or it may realize that functions like\n+\t * pthread_mutex_init() is no-op, which means the (static)\n+\t * variable is not used/initialized at all and trigger\n+\t * -Wunused-variable\n+\t */\n+\treturn ENOSYS;\n }\n+\n+int dummy_pthread_join(pthread_t pthread, void **retval)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis and avoid -Wunused-variable false warnings.\n+\t */\n+\treturn ENOSYS;\n+}\n+\n+#endif\ndiff --git a/thread-utils.h b/thread-utils.h\nindex d9a769d190..4961487ed9 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -4,12 +4,54 @@\n #ifndef NO_PTHREADS\n #include \n \n-extern int online_cpus(void);\n-extern int init_recursive_mutex(pthread_mutex_t*);\n+#define HAVE_THREADS 1\n \n #else\n \n-#define online_cpus() 1\n+#define HAVE_THREADS 0\n+\n+/*\n+ * macros instead of typedefs because pthread definitions may have\n+ * been pulled in by some system dependencies even though the user\n+ * wants to disable pthread.\n+ */\n+#define pthread_t int\n+#define pthread_mutex_t int\n+#define pthread_cond_t int\n+#define pthread_key_t int\n+\n+#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)\n+#define pthread_mutex_lock(mutex)\n+#define pthread_mutex_unlock(mutex)\n+#define pthread_mutex_destroy(mutex)\n+\n+#define pthread_cond_init(cond, attr) dummy_pthread_init(cond)\n+#define pthread_cond_wait(cond, mutex)\n+#define pthread_cond_signal(cond)\n+#define pthread_cond_broadcast(cond)\n+#define pthread_cond_destroy(cond)\n+\n+#define pthread_key_create(key, attr) dummy_pthread_init(key)\n+#define pthread_key_delete(key)\n+\n+#define pthread_create(thread, attr, fn, data) \\\n+\tdummy_pthread_create(thread, attr, fn, data)\n+#define pthread_join(thread, retval) \\\n+\tdummy_pthread_join(thread, retval)\n+\n+#define pthread_setspecific(key, data)\n+#define pthread_getspecific(key) NULL\n+\n+int dummy_pthread_create(pthread_t *pthread, const void *attr,\n+\t\t\t void *(*fn)(void *), void *data);\n+int dummy_pthread_join(pthread_t pthread, void **retval);\n+\n+int dummy_pthread_init(void *);\n \n #endif\n+\n+int online_cpus(void);\n+int init_recursive_mutex(pthread_mutex_t*);\n+\n+\n #endif /* THREAD_COMPAT_H */\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361793","messageId":"20181027173008.18852-3-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 02/10] index-pack: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:00Z","receivedAt":"2018-10-27T17:30:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/index-pack.c | 68 ++++++++++++--------------------------------\n 1 file changed, 18 insertions(+), 50 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 2004e25da2..bbd66ca025 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -42,9 +42,7 @@ struct base_data {\n };\n \n struct thread_local {\n-#ifndef NO_PTHREADS\n \tpthread_t thread;\n-#endif\n \tstruct base_data *base_cache;\n \tsize_t base_cache_used;\n \tint pack_fd;\n@@ -98,8 +96,6 @@ static uint32_t input_crc32;\n static int input_fd, output_fd;\n static const char *curr_pack;\n \n-#ifndef NO_PTHREADS\n-\n static struct thread_local *thread_data;\n static int nr_dispatched;\n static int threads_active;\n@@ -179,26 +175,6 @@ static void cleanup_thread(void)\n \tfree(thread_data);\n }\n \n-#else\n-\n-#define read_lock()\n-#define read_unlock()\n-\n-#define counter_lock()\n-#define counter_unlock()\n-\n-#define work_lock()\n-#define work_unlock()\n-\n-#define deepest_delta_lock()\n-#define deepest_delta_unlock()\n-\n-#define type_cas_lock()\n-#define type_cas_unlock()\n-\n-#endif\n-\n-\n static int mark_link(struct object *obj, int type, void *data, struct fsck_options *options)\n {\n \tif (!obj)\n@@ -364,22 +340,20 @@ static NORETURN void bad_object(off_t offset, const char *format, ...)\n \n static inline struct thread_local *get_thread_data(void)\n {\n-#ifndef NO_PTHREADS\n-\tif (threads_active)\n-\t\treturn pthread_getspecific(key);\n-\tassert(!threads_active &&\n-\t \"This should only be reached when all threads are gone\");\n-#endif\n+\tif (HAVE_THREADS) {\n+\t\tif (threads_active)\n+\t\t\treturn pthread_getspecific(key);\n+\t\tassert(!threads_active &&\n+\t\t \"This should only be reached when all threads are gone\");\n+\t}\n \treturn ¬hread_data;\n }\n \n-#ifndef NO_PTHREADS\n static void set_thread_data(struct thread_local *data)\n {\n \tif (threads_active)\n \t\tpthread_setspecific(key, data);\n }\n-#endif\n \n static struct base_data *alloc_base_data(void)\n {\n@@ -1092,7 +1066,6 @@ static void resolve_base(struct object_entry *obj)\n \tfind_unresolved_deltas(base_obj);\n }\n \n-#ifndef NO_PTHREADS\n static void *threaded_second_pass(void *data)\n {\n \tset_thread_data(data);\n@@ -1116,7 +1089,6 @@ static void *threaded_second_pass(void *data)\n \t}\n \treturn NULL;\n }\n-#endif\n \n /*\n * First pass:\n@@ -1213,7 +1185,6 @@ static void resolve_deltas(void)\n \t\tprogress = start_progress(_(\"Resolving deltas\"),\n \t\t\t\t\t nr_ref_deltas + nr_ofs_deltas);\n \n-#ifndef NO_PTHREADS\n \tnr_dispatched = 0;\n \tif (nr_threads > 1 || getenv(\"GIT_FORCE_THREADS\")) {\n \t\tinit_thread();\n@@ -1229,7 +1200,6 @@ static void resolve_deltas(void)\n \t\tcleanup_thread();\n \t\treturn;\n \t}\n-#endif\n \n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *obj = &objects[i];\n@@ -1531,11 +1501,11 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\tif (nr_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t nr_threads);\n-#ifdef NO_PTHREADS\n-\t\tif (nr_threads != 1)\n-\t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n-\t\tnr_threads = 1;\n-#endif\n+\t\tif (!HAVE_THREADS) {\n+\t\t\tif (nr_threads != 1)\n+\t\t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n+\t\t\tnr_threads = 1;\n+\t\t}\n \t\treturn 0;\n \t}\n \treturn git_default_config(k, v, cb);\n@@ -1723,12 +1693,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n \t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n \t\t\t\t\tusage(index_pack_usage);\n-#ifdef NO_PTHREADS\n-\t\t\t\tif (nr_threads != 1)\n-\t\t\t\t\twarning(_(\"no threads support, \"\n-\t\t\t\t\t\t \"ignoring %s\"), arg);\n-\t\t\t\tnr_threads = 1;\n-#endif\n+\t\t\t\tif (!HAVE_THREADS) {\n+\t\t\t\t\tif (nr_threads != 1)\n+\t\t\t\t\t\twarning(_(\"no threads support, \"\n+\t\t\t\t\t\t\t \"ignoring %s\"), arg);\n+\t\t\t\t\tnr_threads = 1;\n+\t\t\t\t}\n \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n@@ -1791,14 +1761,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (strict)\n \t\topts.flags |= WRITE_IDX_STRICT;\n \n-#ifndef NO_PTHREADS\n-\tif (!nr_threads) {\n+\tif (HAVE_THREADS && !nr_threads) {\n \t\tnr_threads = online_cpus();\n \t\t/* An experiment showed that more threads does not mean faster */\n \t\tif (nr_threads > 3)\n \t\t\tnr_threads = 3;\n \t}\n-#endif\n \n \tcurr_pack = open_pack_file(pack_name);\n \tparse_pack_header();\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361794","messageId":"20181027173008.18852-5-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 04/10] attr.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:02Z","receivedAt":"2018-10-27T17:30:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n attr.c | 14 --------------\n 1 file changed, 14 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 60d284796d..eaece6658d 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -41,23 +41,17 @@ const char *git_attr_name(const struct git_attr *attr)\n \n struct attr_hashmap {\n \tstruct hashmap map;\n-#ifndef NO_PTHREADS\n \tpthread_mutex_t mutex;\n-#endif\n };\n \n static inline void hashmap_lock(struct attr_hashmap *map)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&map->mutex);\n-#endif\n }\n \n static inline void hashmap_unlock(struct attr_hashmap *map)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&map->mutex);\n-#endif\n }\n \n /*\n@@ -498,23 +492,17 @@ static struct check_vector {\n \tsize_t nr;\n \tsize_t alloc;\n \tstruct attr_check **checks;\n-#ifndef NO_PTHREADS\n \tpthread_mutex_t mutex;\n-#endif\n } check_vector;\n \n static inline void vector_lock(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&check_vector.mutex);\n-#endif\n }\n \n static inline void vector_unlock(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&check_vector.mutex);\n-#endif\n }\n \n static void check_vector_add(struct attr_check *c)\n@@ -1181,8 +1169,6 @@ void git_all_attrs(const struct index_state *istate,\n \n void attr_start(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_init(&g_attr_hashmap.mutex, NULL);\n \tpthread_mutex_init(&check_vector.mutex, NULL);\n-#endif\n }\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361795","messageId":"20181027173008.18852-4-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 03/10] name-hash.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:01Z","receivedAt":"2018-10-27T17:30:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n name-hash.c | 22 ++++------------------\n 1 file changed, 4 insertions(+), 18 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex 1fcda73cb3..b3c9ac791d 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -7,6 +7,7 @@\n */\n #define NO_THE_INDEX_COMPATIBILITY_MACROS\n #include \"cache.h\"\n+#include \"thread-utils.h\"\n \n struct dir_entry {\n \tstruct hashmap_entry ent;\n@@ -131,22 +132,6 @@ static int cache_entry_cmp(const void *unused_cmp_data,\n static int lazy_try_threaded = 1;\n static int lazy_nr_dir_threads;\n \n-#ifdef NO_PTHREADS\n-\n-static inline int lookup_lazy_params(struct index_state *istate)\n-{\n-\treturn 0;\n-}\n-\n-static inline void threaded_lazy_init_name_hash(\n-\tstruct index_state *istate)\n-{\n-}\n-\n-#else\n-\n-#include \"thread-utils.h\"\n-\n /*\n * Set a minimum number of cache_entries that we will handle per\n * thread and use that to decide how many threads to run (upto\n@@ -516,6 +501,9 @@ static void threaded_lazy_init_name_hash(\n \tstruct lazy_dir_thread_data *td_dir;\n \tstruct lazy_name_thread_data *td_name;\n \n+\tif (!HAVE_THREADS)\n+\t\treturn;\n+\n \tk_start = 0;\n \tnr_each = DIV_ROUND_UP(istate->cache_nr, lazy_nr_dir_threads);\n \n@@ -574,8 +562,6 @@ static void threaded_lazy_init_name_hash(\n \tfree(lazy_entries);\n }\n \n-#endif\n-\n static void lazy_init_name_hash(struct index_state *istate)\n {\n \n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361796","messageId":"20181027173008.18852-6-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 05/10] grep: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:03Z","receivedAt":"2018-10-27T17:30:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 59 +++++++++++++++++++-------------------------------\n grep.c | 6 -----\n grep.h | 6 -----\n 3 files changed, 22 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex d8508ddf79..6dd15dbaa2 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -34,7 +34,6 @@ static int recurse_submodules;\n #define GREP_NUM_THREADS_DEFAULT 8\n static int num_threads;\n \n-#ifndef NO_PTHREADS\n static pthread_t *threads;\n \n /* We use one producer thread and THREADS consumer\n@@ -234,6 +233,9 @@ static int wait_all(void)\n \tint hit = 0;\n \tint i;\n \n+\tif (!HAVE_THREADS)\n+\t\treturn 0;\n+\n \tgrep_lock();\n \tall_work_added = 1;\n \n@@ -265,13 +267,6 @@ static int wait_all(void)\n \n \treturn hit;\n }\n-#else /* !NO_PTHREADS */\n-\n-static int wait_all(void)\n-{\n-\treturn 0;\n-}\n-#endif\n \n static int grep_cmd_config(const char *var, const char *value, void *cb)\n {\n@@ -284,8 +279,7 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\tif (num_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d) for %s\"),\n \t\t\t num_threads, var);\n-#ifdef NO_PTHREADS\n-\t\telse if (num_threads && num_threads != 1) {\n+\t\telse if (!HAVE_THREADS && num_threads && num_threads != 1) {\n \t\t\t/*\n \t\t\t * TRANSLATORS: %s is the configuration\n \t\t\t * variable for tweaking threads, currently\n@@ -294,7 +288,6 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\t\twarning(_(\"no threads support, ignoring %s\"), var);\n \t\t\tnum_threads = 0;\n \t\t}\n-#endif\n \t}\n \n \tif (!strcmp(var, \"submodule.recurse\"))\n@@ -330,17 +323,14 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n \t\t */\n \t\tadd_work(opt, &gs);\n \t\treturn 0;\n-\t} else\n-#endif\n-\t{\n+\t} else {\n \t\tint hit;\n \n \t\thit = grep_source(opt, &gs);\n@@ -363,17 +353,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n \t\t */\n \t\tadd_work(opt, &gs);\n \t\treturn 0;\n-\t} else\n-#endif\n-\t{\n+\t} else {\n \t\tint hit;\n \n \t\thit = grep_source(opt, &gs);\n@@ -1038,20 +1025,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tpathspec.recursive = 1;\n \tpathspec.recurse_submodules = !!recurse_submodules;\n \n-#ifndef NO_PTHREADS\n-\tif (list.nr || cached || show_in_pager)\n-\t\tnum_threads = 0;\n-\telse if (num_threads == 0)\n-\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n-\telse if (num_threads < 0)\n-\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n-\tif (num_threads == 1)\n+\tif (HAVE_THREADS) {\n+\t\tif (list.nr || cached || show_in_pager)\n+\t\t\tnum_threads = 0;\n+\t\telse if (num_threads == 0)\n+\t\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n+\t\telse if (num_threads < 0)\n+\t\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n+\t\tif (num_threads == 1)\n+\t\t\tnum_threads = 0;\n+\t} else {\n+\t\tif (num_threads)\n+\t\t\twarning(_(\"no threads support, ignoring --threads\"));\n \t\tnum_threads = 0;\n-#else\n-\tif (num_threads)\n-\t\twarning(_(\"no threads support, ignoring --threads\"));\n-\tnum_threads = 0;\n-#endif\n+\t}\n \n \tif (!num_threads)\n \t\t/*\n@@ -1062,15 +1049,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\tcompile_grep_patterns(&opt);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n \t\t && (opt.pre_context || opt.post_context ||\n \t\t\topt.file_break || opt.funcbody))\n \t\t\tskip_first_line = 1;\n \t\tstart_threads(&opt);\n \t}\n-#endif\n \n \tif (show_in_pager && (cached || list.nr))\n \t\tdie(_(\"--open-files-in-pager only works on the worktree\"));\ndiff --git a/grep.c b/grep.c\nindex f6bd89e40b..4db1510d16 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1513,7 +1513,6 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t}\n }\n \n-#ifndef NO_PTHREADS\n int grep_use_locks;\n \n /*\n@@ -1539,11 +1538,6 @@ static inline void grep_attr_unlock(void)\n */\n pthread_mutex_t grep_read_mutex;\n \n-#else\n-#define grep_attr_lock()\n-#define grep_attr_unlock()\n-#endif\n-\n static int match_funcname(struct grep_opt *opt, struct grep_source *gs, char *bol, char *eol)\n {\n \txdemitconf_t *xecfg = opt->priv;\ndiff --git a/grep.h b/grep.h\nindex 1a57d12b90..fb04893721 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -229,7 +229,6 @@ int grep_source(struct grep_opt *opt, struct grep_source *gs);\n extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);\n extern int grep_threads_ok(const struct grep_opt *opt);\n \n-#ifndef NO_PTHREADS\n /*\n * Mutex used around access to the attributes machinery if\n * opt->use_threads. Must be initialized/destroyed by callers!\n@@ -250,9 +249,4 @@ static inline void grep_read_unlock(void)\n \t\tpthread_mutex_unlock(&grep_read_mutex);\n }\n \n-#else\n-#define grep_read_lock()\n-#define grep_read_unlock()\n-#endif\n-\n #endif\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361797","messageId":"20181027173008.18852-7-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 06/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:04Z","receivedAt":"2018-10-27T17:30:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n preload-index.c | 15 ++-------------\n 1 file changed, 2 insertions(+), 13 deletions(-)\n\ndiff --git a/preload-index.c b/preload-index.c\nindex 9e7152ab14..0e24886aca 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -7,17 +7,7 @@\n #include \"fsmonitor.h\"\n #include \"config.h\"\n #include \"progress.h\"\n-\n-#ifdef NO_PTHREADS\n-static void preload_index(struct index_state *index,\n-\t\t\t const struct pathspec *pathspec,\n-\t\t\t unsigned int refresh_flags)\n-{\n-\t; /* nothing */\n-}\n-#else\n-\n-#include \n+#include \"thread-utils.h\"\n \n /*\n * Mostly randomly chosen maximum thread counts: we\n@@ -108,7 +98,7 @@ static void preload_index(struct index_state *index,\n \tstruct thread_data data[MAX_PARALLEL];\n \tstruct progress_data pd;\n \n-\tif (!core_preload_index)\n+\tif (!HAVE_THREADS || !core_preload_index)\n \t\treturn;\n \n \tthreads = index->cache_nr / THREAD_COST;\n@@ -151,7 +141,6 @@ static void preload_index(struct index_state *index,\n \n \ttrace_performance_leave(\"preload index\");\n }\n-#endif\n \n int read_index_preload(struct index_state *index,\n \t\t const struct pathspec *pathspec,\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361799","messageId":"20181027173008.18852-8-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 07/10] pack-objects: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:05Z","receivedAt":"2018-10-27T17:30:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 26 ++------------------------\n pack-objects.h | 6 ------\n 2 files changed, 2 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b059b86aee..12edd6da16 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1953,8 +1953,6 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \treturn 0;\n }\n \n-#ifndef NO_PTHREADS\n-\n /* Protect access to object database */\n static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n@@ -1979,16 +1977,6 @@ static pthread_mutex_t progress_mutex;\n * ahead in the list because they can be stolen and would need\n * progress_mutex for protection.\n */\n-#else\n-\n-#define read_lock()\t\t(void)0\n-#define read_unlock()\t\t(void)0\n-#define cache_lock()\t\t(void)0\n-#define cache_unlock()\t\t(void)0\n-#define progress_lock()\t\t(void)0\n-#define progress_unlock()\t(void)0\n-\n-#endif\n \n /*\n * Return the size of the object without doing any delta\n@@ -2347,8 +2335,6 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \tfree(array);\n }\n \n-#ifndef NO_PTHREADS\n-\n static void try_to_free_from_threads(size_t size)\n {\n \tread_lock();\n@@ -2578,10 +2564,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tfree(p);\n }\n \n-#else\n-#define ll_find_deltas(l, s, w, d, p)\tfind_deltas(l, &s, w, d, p)\n-#endif\n-\n static void add_tag_chain(const struct object_id *oid)\n {\n \tstruct tag *tag;\n@@ -2734,12 +2716,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n \t\tif (delta_search_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t delta_search_threads);\n-#ifdef NO_PTHREADS\n-\t\tif (delta_search_threads != 1) {\n+\t\tif (!HAVE_THREADS && delta_search_threads != 1) {\n \t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n \t\t\tdelta_search_threads = 0;\n \t\t}\n-#endif\n \t\treturn 0;\n \t}\n \tif (!strcmp(k, \"pack.indexversion\")) {\n@@ -3402,10 +3382,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n \t\tdelta_search_threads = online_cpus();\n \n-#ifdef NO_PTHREADS\n-\tif (delta_search_threads != 1)\n+\tif (!HAVE_THREADS && delta_search_threads != 1)\n \t\twarning(_(\"no threads support, ignoring --threads\"));\n-#endif\n \tif (!pack_to_stdout && !pack_size_limit)\n \t\tpack_size_limit = pack_size_limit_cfg;\n \tif (pack_to_stdout && pack_size_limit)\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 2ca39cfcfe..3a42727c7d 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -145,9 +145,7 @@ struct packing_data {\n \tstruct packed_git **in_pack_by_idx;\n \tstruct packed_git **in_pack;\n \n-#ifndef NO_PTHREADS\n \tpthread_mutex_t lock;\n-#endif\n \n \t/*\n \t * This list contains entries for bases which we know the other side\n@@ -169,15 +167,11 @@ void prepare_packing_data(struct packing_data *pdata);\n \n static inline void packing_data_lock(struct packing_data *pdata)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&pdata->lock);\n-#endif\n }\n static inline void packing_data_unlock(struct packing_data *pdata)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&pdata->lock);\n-#endif\n }\n \n struct object_entry *packlist_alloc(struct packing_data *pdata,\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361798","messageId":"20181027173008.18852-9-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 08/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:06Z","receivedAt":"2018-10-27T17:30:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n read-cache.c | 49 ++++++++++++++++++-------------------------------\n 1 file changed, 18 insertions(+), 31 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex d57958233e..ba870bc3fd 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1920,19 +1920,15 @@ struct index_entry_offset_table\n \tstruct index_entry_offset entries[FLEX_ARRAY];\n };\n \n-#ifndef NO_PTHREADS\n static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset);\n static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_table *ieot);\n-#endif\n \n static size_t read_eoie_extension(const char *mmap, size_t mmap_size);\n static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context, size_t offset);\n \n struct load_index_extensions\n {\n-#ifndef NO_PTHREADS\n \tpthread_t pthread;\n-#endif\n \tstruct index_state *istate;\n \tconst char *mmap;\n \tsize_t mmap_size;\n@@ -2010,8 +2006,6 @@ static unsigned long load_all_cache_entries(struct index_state *istate,\n \treturn consumed;\n }\n \n-#ifndef NO_PTHREADS\n-\n /*\n * Mostly randomly chosen maximum thread counts: we\n * cap the parallelism to online_cpus() threads, and we want\n@@ -2122,7 +2116,6 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con\n \n \treturn consumed;\n }\n-#endif\n \n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int must_exist)\n@@ -2135,10 +2128,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tsize_t mmap_size;\n \tstruct load_index_extensions p;\n \tsize_t extension_offset = 0;\n-#ifndef NO_PTHREADS\n \tint nr_threads, cpus;\n \tstruct index_entry_offset_table *ieot = NULL;\n-#endif\n \n \tif (istate->initialized)\n \t\treturn istate->cache_nr;\n@@ -2181,15 +2172,18 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tsrc_offset = sizeof(*hdr);\n \n-#ifndef NO_PTHREADS\n-\tnr_threads = git_config_get_index_threads();\n+\tif (HAVE_THREADS) {\n+\t\tnr_threads = git_config_get_index_threads();\n \n-\t/* TODO: does creating more threads than cores help? */\n-\tif (!nr_threads) {\n-\t\tnr_threads = istate->cache_nr / THREAD_COST;\n-\t\tcpus = online_cpus();\n-\t\tif (nr_threads > cpus)\n-\t\t\tnr_threads = cpus;\n+\t\t/* TODO: does creating more threads than cores help? */\n+\t\tif (!nr_threads) {\n+\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n+\t\t\tcpus = online_cpus();\n+\t\t\tif (nr_threads > cpus)\n+\t\t\t\tnr_threads = cpus;\n+\t\t}\n+\t} else {\n+\t\tnr_threads = 1;\n \t}\n \n \tif (nr_threads > 1) {\n@@ -2219,21 +2213,16 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t} else {\n \t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n \t}\n-#else\n-\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n-#endif\n \n \tistate->timestamp.sec = st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \n \t/* if we created a thread, join it otherwise load the extensions on the primary thread */\n-#ifndef NO_PTHREADS\n-\tif (extension_offset) {\n+\tif (HAVE_THREADS && extension_offset) {\n \t\tint ret = pthread_join(p.pthread, NULL);\n \t\tif (ret)\n \t\t\tdie(_(\"unable to join load_index_extensions thread: %s\"), strerror(ret));\n \t}\n-#endif\n \tif (!extension_offset) {\n \t\tp.src_offset = src_offset;\n \t\tload_index_extensions(&p);\n@@ -2756,8 +2745,11 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n \t\treturn -1;\n \n-#ifndef NO_PTHREADS\n-\tnr_threads = git_config_get_index_threads();\n+\tif (HAVE_THREADS)\n+\t\tnr_threads = git_config_get_index_threads();\n+\telse\n+\t\tnr_threads = 1;\n+\n \tif (nr_threads != 1) {\n \t\tint ieot_blocks, cpus;\n \n@@ -2787,7 +2779,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t\tieot_entries = DIV_ROUND_UP(entries, ieot_blocks);\n \t\t}\n \t}\n-#endif\n \n \toffset = lseek(newfd, 0, SEEK_CUR);\n \tif (offset < 0) {\n@@ -2871,8 +2862,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t * strip_extensions parameter as we need it when loading the shared\n \t * index.\n \t */\n-#ifndef NO_PTHREADS\n-\tif (ieot) {\n+\tif (HAVE_THREADS && ieot) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\twrite_ieot_extension(&sb, ieot);\n@@ -2883,7 +2873,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\tif (err)\n \t\t\treturn -1;\n \t}\n-#endif\n \n \tif (!strip_extensions && istate->split_index) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n@@ -3469,7 +3458,6 @@ static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context,\n \tstrbuf_add(sb, hash, the_hash_algo->rawsz);\n }\n \n-#ifndef NO_PTHREADS\n #define IEOT_VERSION\t(1)\n \n static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset)\n@@ -3542,4 +3530,3 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta\n \t strbuf_add(sb, &buffer, sizeof(uint32_t));\n }\n }\n-#endif\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361800","messageId":"20181027173008.18852-10-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 09/10] Clean up pthread_create() error handling","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:07Z","receivedAt":"2018-10-27T17:30:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Normally pthread_create() rarely fails and sometimes error handling\ncan be neglected. But with new pthreads wrapper, pthread_create() will\nreturn ENOSYS on a system without thread support.\n\nThreaded code _is_ protected by HAVE_THREADS and pthread_create()\nshould never run in the first place. But the situation could change in\nthe future and bugs may sneak in. Make sure that all pthread_create()\nchecks and handles the result.\n\nWhile at there, mark these strings for translation if they aren't.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n name-hash.c | 16 ++++++++++------\n preload-index.c | 8 ++++++--\n run-command.c | 2 +-\n 3 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex b3c9ac791d..623ca6923a 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -494,6 +494,7 @@ static inline void lazy_update_dir_ref_counts(\n static void threaded_lazy_init_name_hash(\n \tstruct index_state *istate)\n {\n+\tint err;\n \tint nr_each;\n \tint k_start;\n \tint t;\n@@ -526,8 +527,9 @@ static void threaded_lazy_init_name_hash(\n \t\tif (k_start > istate->cache_nr)\n \t\t\tk_start = istate->cache_nr;\n \t\ttd_dir_t->k_end = k_start;\n-\t\tif (pthread_create(&td_dir_t->pthread, NULL, lazy_dir_thread_proc, td_dir_t))\n-\t\t\tdie(\"unable to create lazy_dir_thread\");\n+\t\terr = pthread_create(&td_dir_t->pthread, NULL, lazy_dir_thread_proc, td_dir_t);\n+\t\tif (err)\n+\t\t\tdie(_(\"unable to create lazy_dir thread: %s\"), strerror(err));\n \t}\n \tfor (t = 0; t < lazy_nr_dir_threads; t++) {\n \t\tstruct lazy_dir_thread_data *td_dir_t = td_dir + t;\n@@ -547,13 +549,15 @@ static void threaded_lazy_init_name_hash(\n \t */\n \ttd_name->istate = istate;\n \ttd_name->lazy_entries = lazy_entries;\n-\tif (pthread_create(&td_name->pthread, NULL, lazy_name_thread_proc, td_name))\n-\t\tdie(\"unable to create lazy_name_thread\");\n+\terr = pthread_create(&td_name->pthread, NULL, lazy_name_thread_proc, td_name);\n+\tif (err)\n+\t\tdie(_(\"unable to create lazy_name thread: %s\"), strerror(err));\n \n \tlazy_update_dir_ref_counts(istate, lazy_entries);\n \n-\tif (pthread_join(td_name->pthread, NULL))\n-\t\tdie(\"unable to join lazy_name_thread\");\n+\terr = pthread_join(td_name->pthread, NULL);\n+\tif (err)\n+\t\tdie(_(\"unable to join lazy_name thread: %s\"), strerror(err));\n \n \tcleanup_dir_mutex();\n \ndiff --git a/preload-index.c b/preload-index.c\nindex 0e24886aca..ddca1c216e 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -121,6 +121,8 @@ static void preload_index(struct index_state *index,\n \n \tfor (i = 0; i < threads; i++) {\n \t\tstruct thread_data *p = data+i;\n+\t\tint err;\n+\n \t\tp->index = index;\n \t\tif (pathspec)\n \t\t\tcopy_pathspec(&p->pathspec, pathspec);\n@@ -129,8 +131,10 @@ static void preload_index(struct index_state *index,\n \t\tif (pd.progress)\n \t\t\tp->progress = &pd;\n \t\toffset += work;\n-\t\tif (pthread_create(&p->pthread, NULL, preload_thread, p))\n-\t\t\tdie(\"unable to create threaded lstat\");\n+\t\terr = pthread_create(&p->pthread, NULL, preload_thread, p);\n+\n+\t\tif (err)\n+\t\t\tdie(_(\"unable to create threaded lstat: %s\"), strerror(err));\n \t}\n \tfor (i = 0; i < threads; i++) {\n \t\tstruct thread_data *p = data+i;\ndiff --git a/run-command.c b/run-command.c\nindex 84b883c213..26154ba257 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1213,7 +1213,7 @@ int start_async(struct async *async)\n \t{\n \t\tint err = pthread_create(&async->tid, NULL, run_thread, async);\n \t\tif (err) {\n-\t\t\terror_errno(\"cannot create thread\");\n+\t\t\terror(_(\"cannot create async thread: %s\"), strerror(err));\n \t\t\tgoto error;\n \t\t}\n \t}\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361801","messageId":"20181027173008.18852-11-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v2 10/10] read-cache.c: initialize copy_len to shut up gcc 8","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-27T17:30:08Z","receivedAt":"2018-10-27T17:30:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It was reported that when building with NO_PTHREADS=1,\n-Wmaybe-uninitialized is triggered. Just initialize the variable from\nthe beginning to shut the compiler up (because this warning is enabled\nin config.dev)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n read-cache.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex ba870bc3fd..4307b9a7bf 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1746,7 +1746,7 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \tsize_t len;\n \tconst char *name;\n \tunsigned int flags;\n-\tsize_t copy_len;\n+\tsize_t copy_len = 0;\n \t/*\n \t * Adjacent cache entries tend to share the leading paths, so it makes\n \t * sense to only store the differences in later entries. In the v4\n@@ -1786,8 +1786,6 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t\t\t\tdie(_(\"malformed name field in the index, near path '%s'\"),\n \t\t\t\t\tprevious_ce->name);\n \t\t\tcopy_len = previous_len - strip_len;\n-\t\t} else {\n-\t\t\tcopy_len = 0;\n \t\t}\n \t\tname = (const char *)cp;\n \t}\n-- \n2.19.1.647.g708186aaf9\n\n"},{"id":"361860","messageId":"xmqqo9bd8pko.fsf@gitster-ct.c.googlers.com","threadId":"49697","inReplyTo":"20181027074443.GE26685@sigill.intra.peff.net","subject":"Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T02:16:39Z","receivedAt":"2018-10-29T02:16:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Cases like this are kind of weird. I'd expect to see wait_all() return\n> immediately when !HAVE_THREADS. But we don't need to, because later we\n> do this:\n>\n>> -\tif (num_threads)\n>> +\tif (HAVE_THREADS && num_threads)\n>> \t\thit |= wait_all();\n>\n> Which I think works, but it feels like we're introducing some subtle\n> dependencies about which functions get called in which cases. I'd hoped\n> in general that the non-threaded code paths could mostly just collapse\n> down into the same as the \"threads == 1\" cases, and we wouldn't have to\n> ask \"are threads even supported\" in a lot of places.\n\nTrue, but the way \"grep\" code works with threads is already strange,\nand I suspect that the subtle strangeness you feel mostly comes from\nthat. The single threaded code signaled \"hit\" with return value of\nindividual functions like grep_file(), but the meaning of return\nvalue from them is changed to \"does not matter--we do not have\nmeaningful result yet at this point\" when threading is used.\n\nIn the new world order where \"threading is the norm but\nsingle-threaded is still supported\", I wonder if we would want to\ndrive the single threaded case the same way with the add_work(run)\ninterface and return the \"did we see hits?\" etc. from the same (or\nat lesat \"more similar than today's\") interface, so that the flow of\ndata smells the same in both cases.\n\n> I dunno. My comment isn't really an objection exactly, but mostly just\n> an indication that I'm still thinking through what the best approach is,\n> and what end state we want to end up in.\n>\n> -Peff\n"},{"id":"361901","messageId":"20181029142536.GE17668@sigill.intra.peff.net","threadId":"49697","inReplyTo":"xmqqo9bd8pko.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-29T14:25:37Z","receivedAt":"2018-10-29T14:25:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 29, 2018 at 11:16:39AM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Cases like this are kind of weird. I'd expect to see wait_all() return\n> > immediately when !HAVE_THREADS. But we don't need to, because later we\n> > do this:\n> >\n> >> -\tif (num_threads)\n> >> +\tif (HAVE_THREADS && num_threads)\n> >> \t\thit |= wait_all();\n> >\n> > Which I think works, but it feels like we're introducing some subtle\n> > dependencies about which functions get called in which cases. I'd hoped\n> > in general that the non-threaded code paths could mostly just collapse\n> > down into the same as the \"threads == 1\" cases, and we wouldn't have to\n> > ask \"are threads even supported\" in a lot of places.\n> \n> True, but the way \"grep\" code works with threads is already strange,\n> and I suspect that the subtle strangeness you feel mostly comes from\n> that. The single threaded code signaled \"hit\" with return value of\n> individual functions like grep_file(), but the meaning of return\n> value from them is changed to \"does not matter--we do not have\n> meaningful result yet at this point\" when threading is used.\n> \n> In the new world order where \"threading is the norm but\n> single-threaded is still supported\", I wonder if we would want to\n> drive the single threaded case the same way with the add_work(run)\n> interface and return the \"did we see hits?\" etc. from the same (or\n> at lesat \"more similar than today's\") interface, so that the flow of\n> data smells the same in both cases.\n\nRight, your second paragraph here is a better statement of what I was\ntrying to get at. ;)\n\nBut if the problem is simply that we are not quite there yet in the grep\ncode, I am OK with taking this as the first pass, and knowing that there\nis more cleanup to be done later (though that sort of thing is IMHO very\nuseful in a commit message).\n\n-Peff\n"},{"id":"361902","messageId":"20181029143033.GF17668@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027173008.18852-9-pclouds@gmail.com","subject":"Re: [PATCH v2 08/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-29T14:30:34Z","receivedAt":"2018-10-29T14:30:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 07:30:06PM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> -#ifndef NO_PTHREADS\n> -\tnr_threads = git_config_get_index_threads();\n> +\tif (HAVE_THREADS) {\n> +\t\tnr_threads = git_config_get_index_threads();\n> \n> -\t/* TODO: does creating more threads than cores help? */\n> -\tif (!nr_threads) {\n> -\t\tnr_threads = istate->cache_nr / THREAD_COST;\n> -\t\tcpus = online_cpus();\n> -\t\tif (nr_threads > cpus)\n> -\t\t\tnr_threads = cpus;\n> +\t\t/* TODO: does creating more threads than cores help? */\n> +\t\tif (!nr_threads) {\n> +\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n> +\t\t\tcpus = online_cpus();\n> +\t\t\tif (nr_threads > cpus)\n> +\t\t\t\tnr_threads = cpus;\n> +\t\t}\n> +\t} else {\n> +\t\tnr_threads = 1;\n> \t}\n\nI'd have thought we could just rely on online_cpus() returning 1 here to\navoid having to ask \"do we even have thread support?\". But I guess that\nTODO comment implies that we might one day two 2 threads on a single\nCPU.\n\n-Peff\n"},{"id":"361903","messageId":"20181029143140.GG17668@sigill.intra.peff.net","threadId":"49697","inReplyTo":"20181027173008.18852-11-pclouds@gmail.com","subject":"Re: [PATCH v2 10/10] read-cache.c: initialize copy_len to shut up gcc 8","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-29T14:31:41Z","receivedAt":"2018-10-29T14:31:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 27, 2018 at 07:30:08PM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> It was reported that when building with NO_PTHREADS=1,\n> -Wmaybe-uninitialized is triggered. Just initialize the variable from\n> the beginning to shut the compiler up (because this warning is enabled\n> in config.dev)\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> read-cache.c | 4 +---\n> 1 file changed, 1 insertion(+), 3 deletions(-)\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index ba870bc3fd..4307b9a7bf 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1746,7 +1746,7 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n> \tsize_t len;\n> \tconst char *name;\n> \tunsigned int flags;\n> -\tsize_t copy_len;\n> +\tsize_t copy_len = 0;\n> \t/*\n> \t * Adjacent cache entries tend to share the leading paths, so it makes\n> \t * sense to only store the differences in later entries. In the v4\n> @@ -1786,8 +1786,6 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n> \t\t\t\tdie(_(\"malformed name field in the index, near path '%s'\"),\n> \t\t\t\t\tprevious_ce->name);\n> \t\t\tcopy_len = previous_len - strip_len;\n> -\t\t} else {\n> -\t\t\tcopy_len = 0;\n> \t\t}\n> \t\tname = (const char *)cp;\n> \t}\n\nYes, this silences the compiler warning I saw (and is exactly the same\npatch I wrote to get past it the other day ;) ).\n\n-Peff\n"},{"id":"361914","messageId":"CACsJy8Bp_9qmGeNZuywtda1_TdQTZM1essUjBD9Q8e31175V=g@mail.gmail.com","threadId":"49697","inReplyTo":"20181029142536.GE17668@sigill.intra.peff.net","subject":"Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-29T16:01:41Z","receivedAt":"2018-10-29T16:02:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 29, 2018 at 3:25 PM Jeff King wrote:\n> But if the problem is simply that we are not quite there yet in the grep\n> code, I am OK with taking this as the first pass, and knowing that there\n> is more cleanup to be done later (though that sort of thing is IMHO very\n> useful in a commit message).\n\nSince the problem pops up now, I'm ok with updating/cleaning up all\nthis in this series, unless there's benefits in keeping this series\nsimple and merging it early (probably not?)\n-- \nDuy\n"},{"id":"361918","messageId":"20181029162007.GA31497@sigill.intra.peff.net","threadId":"49697","inReplyTo":"CACsJy8Bp_9qmGeNZuywtda1_TdQTZM1essUjBD9Q8e31175V=g@mail.gmail.com","subject":"Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-29T16:20:08Z","receivedAt":"2018-10-29T16:20:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 29, 2018 at 05:01:41PM +0100, Duy Nguyen wrote:\n\n> On Mon, Oct 29, 2018 at 3:25 PM Jeff King wrote:\n> > But if the problem is simply that we are not quite there yet in the grep\n> > code, I am OK with taking this as the first pass, and knowing that there\n> > is more cleanup to be done later (though that sort of thing is IMHO very\n> > useful in a commit message).\n> \n> Since the problem pops up now, I'm ok with updating/cleaning up all\n> this in this series, unless there's benefits in keeping this series\n> simple and merging it early (probably not?)\n\nMostly I did not want to tax you. I would rather have this series and\nsome cleanup left over, than to not have anything. But if you are\ninterested in moving it further, I will not say no. :)\n\n-Peff\n"},{"id":"361921","messageId":"1217c887-cb15-87c6-649f-8b927abe8180@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-8-pclouds@gmail.com","subject":"Re: [PATCH 07/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T16:52:56Z","receivedAt":"2018-10-29T16:53:04Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/27/2018 3:10 AM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> preload-index.c | 15 ++-------------\n> 1 file changed, 2 insertions(+), 13 deletions(-)\n> \n> diff --git a/preload-index.c b/preload-index.c\n> index 9e7152ab14..0e24886aca 100644\n> --- a/preload-index.c\n> +++ b/preload-index.c\n> @@ -7,17 +7,7 @@\n> #include \"fsmonitor.h\"\n> #include \"config.h\"\n> #include \"progress.h\"\n> -\n> -#ifdef NO_PTHREADS\n> -static void preload_index(struct index_state *index,\n> -\t\t\t const struct pathspec *pathspec,\n> -\t\t\t unsigned int refresh_flags)\n> -{\n> -\t; /* nothing */\n> -}\n> -#else\n> -\n> -#include \n> +#include \"thread-utils.h\"\n> \n> /*\n> * Mostly randomly chosen maximum thread counts: we\n> @@ -108,7 +98,7 @@ static void preload_index(struct index_state *index,\n> \tstruct thread_data data[MAX_PARALLEL];\n> \tstruct progress_data pd;\n> \n> -\tif (!core_preload_index)\n> +\tif (!HAVE_THREADS || !core_preload_index)\n\nI also \"hoped in general that the non-threaded code paths could mostly \njust collapse into the same as the \"threads == 1\" cases, and we wouldn't \nhave to \"are threads even supported\" in a lot of places.\"\n\nIn this case, if we cap the threads to online_cpus() the later test for \n'if (threads < 2)' would do that. I haven't measured this specific case \nbut in the other cases I have measured, having more threads than cpus \ndid not result in a performance win.\n\n> \t\treturn;\n> \n> \tthreads = index->cache_nr / THREAD_COST;\n> @@ -151,7 +141,6 @@ static void preload_index(struct index_state *index,\n> \n> \ttrace_performance_leave(\"preload index\");\n> }\n> -#endif\n> \n> int read_index_preload(struct index_state *index,\n> \t\t const struct pathspec *pathspec,\n> \n"},{"id":"361924","messageId":"9e489011-36b9-a8b3-838d-de1a097f4854@gmail.com","threadId":"49697","inReplyTo":"20181027071003.1347-10-pclouds@gmail.com","subject":"Re: [PATCH 09/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T17:05:40Z","receivedAt":"2018-10-29T17:05:48Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/27/2018 3:10 AM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> read-cache.c | 49 ++++++++++++++++++-------------------------------\n> 1 file changed, 18 insertions(+), 31 deletions(-)\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index d57958233e..ba870bc3fd 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1920,19 +1920,15 @@ struct index_entry_offset_table\n> \tstruct index_entry_offset entries[FLEX_ARRAY];\n> };\n> \n> -#ifndef NO_PTHREADS\n> static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset);\n> static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_table *ieot);\n> -#endif\n> \n> static size_t read_eoie_extension(const char *mmap, size_t mmap_size);\n> static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context, size_t offset);\n> \n> struct load_index_extensions\n> {\n> -#ifndef NO_PTHREADS\n> \tpthread_t pthread;\n> -#endif\n> \tstruct index_state *istate;\n> \tconst char *mmap;\n> \tsize_t mmap_size;\n> @@ -2010,8 +2006,6 @@ static unsigned long load_all_cache_entries(struct index_state *istate,\n> \treturn consumed;\n> }\n> \n> -#ifndef NO_PTHREADS\n> -\n> /*\n> * Mostly randomly chosen maximum thread counts: we\n> * cap the parallelism to online_cpus() threads, and we want\n> @@ -2122,7 +2116,6 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con\n> \n> \treturn consumed;\n> }\n> -#endif\n> \n> /* remember to discard_cache() before reading a different cache! */\n> int do_read_index(struct index_state *istate, const char *path, int must_exist)\n> @@ -2135,10 +2128,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n> \tsize_t mmap_size;\n> \tstruct load_index_extensions p;\n> \tsize_t extension_offset = 0;\n> -#ifndef NO_PTHREADS\n> \tint nr_threads, cpus;\n> \tstruct index_entry_offset_table *ieot = NULL;\n> -#endif\n> \n> \tif (istate->initialized)\n> \t\treturn istate->cache_nr;\n> @@ -2181,15 +2172,18 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n> \n> \tsrc_offset = sizeof(*hdr);\n> \n> -#ifndef NO_PTHREADS\n> -\tnr_threads = git_config_get_index_threads();\n> +\tif (HAVE_THREADS) {\n\nIn this case, nr_threads is already capped to online_cpus() below so \nthis HAVE_THREADS test isn't needed.\n\n> +\t\tnr_threads = git_config_get_index_threads();\n> \n> -\t/* TODO: does creating more threads than cores help? */\n> -\tif (!nr_threads) {\n> -\t\tnr_threads = istate->cache_nr / THREAD_COST;\n> -\t\tcpus = online_cpus();\n> -\t\tif (nr_threads > cpus)\n> -\t\t\tnr_threads = cpus;\n> +\t\t/* TODO: does creating more threads than cores help? */\n> +\t\tif (!nr_threads) {\n> +\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n> +\t\t\tcpus = online_cpus();\n> +\t\t\tif (nr_threads > cpus)\n> +\t\t\t\tnr_threads = cpus;\n> +\t\t}\n> +\t} else {\n> +\t\tnr_threads = 1;\n> \t}\n> \n> \tif (nr_threads > 1) {\n> @@ -2219,21 +2213,16 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n> \t} else {\n> \t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n> \t}\n> -#else\n> -\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n> -#endif\n> \n> \tistate->timestamp.sec = st.st_mtime;\n> \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n> \n> \t/* if we created a thread, join it otherwise load the extensions on the primary thread */\n> -#ifndef NO_PTHREADS\n> -\tif (extension_offset) {\n> +\tif (HAVE_THREADS && extension_offset) {\n\nHere extension_offset won't be set if there wasn't a thread created so \nthe HAVE_THREADS test is not needed.\n\n> \t\tint ret = pthread_join(p.pthread, NULL);\n> \t\tif (ret)\n> \t\t\tdie(_(\"unable to join load_index_extensions thread: %s\"), strerror(ret));\n> \t}\n> -#endif\n> \tif (!extension_offset) {\n> \t\tp.src_offset = src_offset;\n> \t\tload_index_extensions(&p);\n> @@ -2756,8 +2745,11 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n> \t\treturn -1;\n> \n> -#ifndef NO_PTHREADS\n> -\tnr_threads = git_config_get_index_threads();\n> +\tif (HAVE_THREADS)\n\nThis shouldn't be needed either. My assumption was that if someone \nexplicitly asked for multiple threads we're better off failing than \nsilently ignoring their request. The default/automatic setting will \ndetect the number of cpus and behave correctly.\n\n> +\t\tnr_threads = git_config_get_index_threads();\n> +\telse\n> +\t\tnr_threads = 1;\n> +\n> \tif (nr_threads != 1) {\n> \t\tint ieot_blocks, cpus;\n> \n> @@ -2787,7 +2779,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \t\t\tieot_entries = DIV_ROUND_UP(entries, ieot_blocks);\n> \t\t}\n> \t}\n> -#endif\n> \n> \toffset = lseek(newfd, 0, SEEK_CUR);\n> \tif (offset < 0) {\n> @@ -2871,8 +2862,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \t * strip_extensions parameter as we need it when loading the shared\n> \t * index.\n> \t */\n> -#ifndef NO_PTHREADS\n> -\tif (ieot) {\n> +\tif (HAVE_THREADS && ieot) {\n\nAgain, this one isn't needed. If there is only 1 thread, we don't read \nthe eoie or the ieot (see the code just above this) so the ieot test is \nsufficient.\n\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> \n> \t\twrite_ieot_extension(&sb, ieot);\n> @@ -2883,7 +2873,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \t\tif (err)\n> \t\t\treturn -1;\n> \t}\n> -#endif\n> \n> \tif (!strip_extensions && istate->split_index) {\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> @@ -3469,7 +3458,6 @@ static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context,\n> \tstrbuf_add(sb, hash, the_hash_algo->rawsz);\n> }\n> \n> -#ifndef NO_PTHREADS\n> #define IEOT_VERSION\t(1)\n> \n> static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset)\n> @@ -3542,4 +3530,3 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta\n> \t strbuf_add(sb, &buffer, sizeof(uint32_t));\n> }\n> }\n> -#endif\n> \n"},{"id":"361925","messageId":"4e3c20f1-580f-0a34-eb86-028bf8dd1b4b@gmail.com","threadId":"49697","inReplyTo":"20181029143033.GF17668@sigill.intra.peff.net","subject":"Re: [PATCH v2 08/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T17:07:25Z","receivedAt":"2018-10-29T17:07:33Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/29/2018 10:30 AM, Jeff King wrote:\n> On Sat, Oct 27, 2018 at 07:30:06PM +0200, Nguyễn Thái Ngọc Duy wrote:\n> \n>> -#ifndef NO_PTHREADS\n>> -\tnr_threads = git_config_get_index_threads();\n>> +\tif (HAVE_THREADS) {\n>> +\t\tnr_threads = git_config_get_index_threads();\n>> \n>> -\t/* TODO: does creating more threads than cores help? */\n>> -\tif (!nr_threads) {\n>> -\t\tnr_threads = istate->cache_nr / THREAD_COST;\n>> -\t\tcpus = online_cpus();\n>> -\t\tif (nr_threads > cpus)\n>> -\t\t\tnr_threads = cpus;\n>> +\t\t/* TODO: does creating more threads than cores help? */\n>> +\t\tif (!nr_threads) {\n>> +\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n>> +\t\t\tcpus = online_cpus();\n>> +\t\t\tif (nr_threads > cpus)\n>> +\t\t\t\tnr_threads = cpus;\n>> +\t\t}\n>> +\t} else {\n>> +\t\tnr_threads = 1;\n>> \t}\n> \n> I'd have thought we could just rely on online_cpus() returning 1 here to\n> avoid having to ask \"do we even have thread support?\". But I guess that\n> TODO comment implies that we might one day two 2 threads on a single\n> CPU.\n> \n> -Peff\n> \n\nSee my earlier response - the HAVE_THREADS tests are not needed.\n\nWe can remove the \"TODO\" comment - I tested it and it wasn't faster to \nhave more threads than CPUs.\n"},{"id":"361927","messageId":"0ee246db-d781-9a42-8b8c-3e98c132b0ae@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-7-pclouds@gmail.com","subject":"Re: [PATCH v2 06/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T17:21:07Z","receivedAt":"2018-10-29T17:21:14Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/27/2018 1:30 PM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> preload-index.c | 15 ++-------------\n> 1 file changed, 2 insertions(+), 13 deletions(-)\n> \n> diff --git a/preload-index.c b/preload-index.c\n> index 9e7152ab14..0e24886aca 100644\n> --- a/preload-index.c\n> +++ b/preload-index.c\n> @@ -7,17 +7,7 @@\n> #include \"fsmonitor.h\"\n> #include \"config.h\"\n> #include \"progress.h\"\n> -\n> -#ifdef NO_PTHREADS\n> -static void preload_index(struct index_state *index,\n> -\t\t\t const struct pathspec *pathspec,\n> -\t\t\t unsigned int refresh_flags)\n> -{\n> -\t; /* nothing */\n> -}\n> -#else\n> -\n> -#include \n> +#include \"thread-utils.h\"\n> \n> /*\n> * Mostly randomly chosen maximum thread counts: we\n> @@ -108,7 +98,7 @@ static void preload_index(struct index_state *index,\n> \tstruct thread_data data[MAX_PARALLEL];\n> \tstruct progress_data pd;\n> \n> -\tif (!core_preload_index)\n> +\tif (!HAVE_THREADS || !core_preload_index)\n> \t\treturn;\n> \n> \tthreads = index->cache_nr / THREAD_COST;\n> @@ -151,7 +141,6 @@ static void preload_index(struct index_state *index,\n> \n> \ttrace_performance_leave(\"preload index\");\n> }\n> -#endif\n> \n> int read_index_preload(struct index_state *index,\n> \t\t const struct pathspec *pathspec,\n> \n\n\nIn the theory that code speaks louder than comments, here is the patch I \nused when testing out the idea of getting rid of NO_PTHREADS:\n\n\n git diff head~1 preload-index.c\ndiff --git a/preload-index.c b/preload-index.c\nindex 9e7152ab14..266bc9d8d7 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -7,17 +7,7 @@\n #include \"fsmonitor.h\"\n #include \"config.h\"\n #include \"progress.h\"\n-\n-#ifdef NO_PTHREADS\n-static void preload_index(struct index_state *index,\n- const struct pathspec *pathspec,\n- unsigned int refresh_flags)\n-{\n- ; /* nothing */\n-}\n-#else\n-\n-#include \n+#include \"thread-utils.h\"\n\n /*\n * Mostly randomly chosen maximum thread counts: we\n@@ -104,7 +94,7 @@ static void preload_index(struct index_state *index,\n const struct pathspec *pathspec,\n unsigned int refresh_flags)\n {\n- int threads, i, work, offset;\n+ int cpus, threads, i, work, offset;\n struct thread_data data[MAX_PARALLEL];\n struct progress_data pd;\n\n@@ -114,6 +104,9 @@ static void preload_index(struct index_state *index,\n threads = index->cache_nr / THREAD_COST;\n if ((index->cache_nr > 1) && (threads < 2) && \ngit_env_bool(\"GIT_TEST_PRELOAD_INDEX\", 0))\n threads = 2;\n+ cpus = online_cpus();\n+ if (threads > cpus)\n+ threads = cpus;\n if (threads < 2)\n return;\n trace_performance_enter();\n@@ -151,7 +144,6 @@ static void preload_index(struct index_state *index,\n\n trace_performance_leave(\"preload index\");\n }\n-#endif\n\n int read_index_preload(struct index_state *index,\n const struct pathspec *pathspec,\n"},{"id":"361928","messageId":"CACsJy8BD8nUfJoNjWCnaYgUn5+LaunOx74AgwPf5okG0OhwEXw@mail.gmail.com","threadId":"49697","inReplyTo":"9e489011-36b9-a8b3-838d-de1a097f4854@gmail.com","subject":"Re: [PATCH 09/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-29T17:21:30Z","receivedAt":"2018-10-29T17:21:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 29, 2018 at 6:05 PM Ben Peart wrote:\n> > @@ -2756,8 +2745,11 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> > if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n> > return -1;\n> >\n> > -#ifndef NO_PTHREADS\n> > - nr_threads = git_config_get_index_threads();\n> > + if (HAVE_THREADS)\n>\n> This shouldn't be needed either. My assumption was that if someone\n> explicitly asked for multiple threads we're better off failing than\n> silently ignoring their request. The default/automatic setting will\n> detect the number of cpus and behave correctly.\n\nNo. I can have ~/.gitconfig shared between different machines. One\nwith multithread support and one without. I should not have to fall\nback to conditional includes in order to use the same config file on\nthe machine without multithread.\n-- \nDuy\n"},{"id":"361929","messageId":"61d354e9-dd40-14a8-29dc-d989dd5631af@gmail.com","threadId":"49697","inReplyTo":"20181029143033.GF17668@sigill.intra.peff.net","subject":"Re: [PATCH v2 08/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T17:23:36Z","receivedAt":"2018-10-29T17:23:44Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/29/2018 10:30 AM, Jeff King wrote:\n> On Sat, Oct 27, 2018 at 07:30:06PM +0200, Nguyễn Thái Ngọc Duy wrote:\n> \n>> -#ifndef NO_PTHREADS\n>> -\tnr_threads = git_config_get_index_threads();\n>> +\tif (HAVE_THREADS) {\n>> +\t\tnr_threads = git_config_get_index_threads();\n>> \n>> -\t/* TODO: does creating more threads than cores help? */\n>> -\tif (!nr_threads) {\n>> -\t\tnr_threads = istate->cache_nr / THREAD_COST;\n>> -\t\tcpus = online_cpus();\n>> -\t\tif (nr_threads > cpus)\n>> -\t\t\tnr_threads = cpus;\n>> +\t\t/* TODO: does creating more threads than cores help? */\n>> +\t\tif (!nr_threads) {\n>> +\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n>> +\t\t\tcpus = online_cpus();\n>> +\t\t\tif (nr_threads > cpus)\n>> +\t\t\t\tnr_threads = cpus;\n>> +\t\t}\n>> +\t} else {\n>> +\t\tnr_threads = 1;\n>> \t}\n> \n> I'd have thought we could just rely on online_cpus() returning 1 here to\n> avoid having to ask \"do we even have thread support?\". But I guess that\n> TODO comment implies that we might one day two 2 threads on a single\n> CPU.\n> \n> -Peff\n> \n\nAnd here is the patch I created when testing out the idea of removing \nNO_PTHREADS. It doesn't require any 'HAVE_THREADS' tests.\n\n\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 1df5c16dbc..1f088799fc 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1920,19 +1920,15 @@ struct index_entry_offset_table\n struct index_entry_offset entries[FLEX_ARRAY];\n };\n\n-#ifndef NO_PTHREADS\n static struct index_entry_offset_table *read_ieot_extension(const char \n*mmap, size_t mmap_size, size_t offset\n);\n static void write_ieot_extension(struct strbuf *sb, struct \nindex_entry_offset_table *ieot);\n-#endif\n\n static size_t read_eoie_extension(const char *mmap, size_t mmap_size);\n static void write_eoie_extension(struct strbuf *sb, git_hash_ctx \n*eoie_context, size_t offset);\n\n struct load_index_extensions\n {\n-#ifndef NO_PTHREADS\n pthread_t pthread;\n-#endif\n struct index_state *istate;\n const char *mmap;\n size_t mmap_size;\n@@ -2010,8 +2006,6 @@ static unsigned long load_all_cache_entries(struct \nindex_state *istate,\n return consumed;\n }\n\n-#ifndef NO_PTHREADS\n-\n /*\n * Mostly randomly chosen maximum thread counts: we\n * cap the parallelism to online_cpus() threads, and we want\n@@ -2122,7 +2116,6 @@ static unsigned long \nload_cache_entries_threaded(struct index_state *istate, con\n\n\n return consumed;\n }\n-#endif\n\n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int \nmust_exist)\n@@ -2135,10 +2128,8 @@ int do_read_index(struct index_state *istate, \nconst char *path, int must_exist)\n\n size_t mmap_size;\n struct load_index_extensions p;\n size_t extension_offset = 0;\n-#ifndef NO_PTHREADS\n int nr_threads, cpus;\n struct index_entry_offset_table *ieot = NULL;\n-#endif\n\n if (istate->initialized)\n return istate->cache_nr;\n@@ -2181,16 +2172,12 @@ int do_read_index(struct index_state *istate, \nconst char *path, int must_exist\n)\n\n src_offset = sizeof(*hdr);\n\n-#ifndef NO_PTHREADS\n nr_threads = git_config_get_index_threads();\n-\n- /* TODO: does creating more threads than cores help? */\n- if (!nr_threads) {\n+ if (!nr_threads)\n nr_threads = istate->cache_nr / THREAD_COST;\n- cpus = online_cpus();\n- if (nr_threads > cpus)\n- nr_threads = cpus;\n- }\n+ cpus = online_cpus();\n+ if (nr_threads > cpus)\n+ nr_threads = cpus;\n\n if (nr_threads > 1) {\n extension_offset = read_eoie_extension(mmap, mmap_size);\n@@ -2219,22 +2206,16 @@ int do_read_index(struct index_state *istate, \nconst char *path, int must_exist\n)\n } else {\n src_offset += load_all_cache_entries(istate, mmap, \nmmap_size, src_offset);\n }\n-#else\n- src_offset += load_all_cache_entries(istate, mmap, mmap_size, \nsrc_offset);\n-#endif\n\n istate->timestamp.sec = st.st_mtime;\n istate->timestamp.nsec = ST_MTIME_NSEC(st);\n\n /* if we created a thread, join it otherwise load the \nextensions on the primary thread */\n-#ifndef NO_PTHREADS\n if (extension_offset) {\n int ret = pthread_join(p.pthread, NULL);\n if (ret)\n die(_(\"unable to join load_index_extensions \nthread: %s\"), strerror(ret));\n- }\n-#endif\n- if (!extension_offset) {\n+ } else {\n p.src_offset = src_offset;\n load_index_extensions(&p);\n }\n@@ -2756,7 +2737,6 @@ static int do_write_index(struct index_state \n*istate, struct tempfile *tempfile,\n\n if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n return -1;\n\n-#ifndef NO_PTHREADS\n nr_threads = git_config_get_index_threads();\n if (nr_threads != 1) {\n int ieot_blocks, cpus;\n@@ -2787,7 +2767,6 @@ static int do_write_index(struct index_state \n*istate, struct tempfile *tempfile,\n\n ieot_entries = DIV_ROUND_UP(entries, ieot_blocks);\n }\n }\n-#endif\n\n offset = lseek(newfd, 0, SEEK_CUR);\n if (offset < 0) {\n@@ -2871,7 +2850,6 @@ static int do_write_index(struct index_state \n*istate, struct tempfile *tempfile,\n\n * strip_extensions parameter as we need it when loading the shared\n * index.\n */\n-#ifndef NO_PTHREADS\n if (ieot) {\n struct strbuf sb = STRBUF_INIT;\n\n@@ -2883,7 +2861,6 @@ static int do_write_index(struct index_state \n*istate, struct tempfile *tempfile,\n\n if (err)\n return -1;\n }\n-#endif\n\n if (!strip_extensions && istate->split_index) {\n struct strbuf sb = STRBUF_INIT;\n@@ -3469,7 +3446,6 @@ static void write_eoie_extension(struct strbuf \n*sb, git_hash_ctx *eoie_context,\n\n strbuf_add(sb, hash, the_hash_algo->rawsz);\n }\n\n-#ifndef NO_PTHREADS\n #define IEOT_VERSION (1)\n\n static struct index_entry_offset_table *read_ieot_extension(const char \n*mmap, size_t mmap_size, size_t offset\n)\n@@ -3542,4 +3518,3 @@ static void write_ieot_extension(struct strbuf \n*sb, struct index_entry_offset_ta\n\n strbuf_add(sb, &buffer, sizeof(uint32_t));\n }\n }\n-#endif\n"},{"id":"361930","messageId":"CACsJy8CYnngeA8JcFy83qYxKj3saB21Y32VV3R5o1PY_tkAq5Q@mail.gmail.com","threadId":"49697","inReplyTo":"0ee246db-d781-9a42-8b8c-3e98c132b0ae@gmail.com","subject":"Re: [PATCH v2 06/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-29T17:26:03Z","receivedAt":"2018-10-29T17:26:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 29, 2018 at 6:21 PM Ben Peart wrote:\n> @@ -114,6 +104,9 @@ static void preload_index(struct index_state *index,\n> threads = index->cache_nr / THREAD_COST;\n> if ((index->cache_nr > 1) && (threads < 2) &&\n> git_env_bool(\"GIT_TEST_PRELOAD_INDEX\", 0))\n> threads = 2;\n> + cpus = online_cpus();\n> + if (threads > cpus)\n> + threads = cpus;\n> if (threads < 2)\n> return;\n> trace_performance_enter();\n\nCapping the number of threads to online_cpus() does not always make\nsense. In this case (or at least the original use case where we stat()\nover nfs) we want to issue as many requests to nfs server as possible\nto reduce latency and it has nothing to do with how many cores we\nhave. Using more threads than cores might make sense since threads are\nblocked by nfs client, but we still want to send more to the server.\n-- \nDuy\n"},{"id":"361934","messageId":"c28a563d-853e-c976-8268-45e84ef6afc8@gmail.com","threadId":"49697","inReplyTo":"CACsJy8BD8nUfJoNjWCnaYgUn5+LaunOx74AgwPf5okG0OhwEXw@mail.gmail.com","subject":"Re: [PATCH 09/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T17:58:22Z","receivedAt":"2018-10-29T17:58:30Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/29/2018 1:21 PM, Duy Nguyen wrote:\n> On Mon, Oct 29, 2018 at 6:05 PM Ben Peart wrote:\n>>> @@ -2756,8 +2745,11 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n>>> if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n>>> return -1;\n>>>\n>>> -#ifndef NO_PTHREADS\n>>> - nr_threads = git_config_get_index_threads();\n>>> + if (HAVE_THREADS)\n>>\n>> This shouldn't be needed either. My assumption was that if someone\n>> explicitly asked for multiple threads we're better off failing than\n>> silently ignoring their request. The default/automatic setting will\n>> detect the number of cpus and behave correctly.\n> \n> No. I can have ~/.gitconfig shared between different machines. One\n> with multithread support and one without. I should not have to fall\n> back to conditional includes in order to use the same config file on\n> the machine without multithread.\n> \n\nIf you want to share the ~/.gitconfig across machines that have \ndifferent numbers of CPUs, it makes more sense to me to leave the \nsetting at the \"auto\" setting rather than specifically overriding it to \na number that won't work on at least one of your machines. Then it all \n\"just works\" without the need to conditional includes.\n"},{"id":"361935","messageId":"2935e3e8-c529-4292-baa3-fc3c9074028b@gmail.com","threadId":"49697","inReplyTo":"CACsJy8CYnngeA8JcFy83qYxKj3saB21Y32VV3R5o1PY_tkAq5Q@mail.gmail.com","subject":"Re: [PATCH v2 06/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-29T18:05:58Z","receivedAt":"2018-10-29T18:06:05Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/29/2018 1:26 PM, Duy Nguyen wrote:\n> On Mon, Oct 29, 2018 at 6:21 PM Ben Peart wrote:\n>> @@ -114,6 +104,9 @@ static void preload_index(struct index_state *index,\n>> threads = index->cache_nr / THREAD_COST;\n>> if ((index->cache_nr > 1) && (threads < 2) &&\n>> git_env_bool(\"GIT_TEST_PRELOAD_INDEX\", 0))\n>> threads = 2;\n>> + cpus = online_cpus();\n>> + if (threads > cpus)\n>> + threads = cpus;\n>> if (threads < 2)\n>> return;\n>> trace_performance_enter();\n> \n> Capping the number of threads to online_cpus() does not always make\n> sense. In this case (or at least the original use case where we stat()\n> over nfs) we want to issue as many requests to nfs server as possible\n> to reduce latency and it has nothing to do with how many cores we\n> have. Using more threads than cores might make sense since threads are\n> blocked by nfs client, but we still want to send more to the server.\n> \n\nThat makes sense. Some test will be necessary then.\n\nI guess HAVE_THREADS is functionally the same as online_cpus() == 1. \nFor some reason, I prefer the latter - probably because I'm typically \nalready calling it and so it doesn't feel like a 'special' value/test I \nhave to remember. I just know I need to make sure the logic works \ncorrectly with any number of cps greater than zero. :-)\n"},{"id":"361940","messageId":"20181029201151.GA32277@sigill.intra.peff.net","threadId":"49697","inReplyTo":"2935e3e8-c529-4292-baa3-fc3c9074028b@gmail.com","subject":"Re: [PATCH v2 06/10] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-29T20:11:51Z","receivedAt":"2018-10-29T20:11:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 29, 2018 at 02:05:58PM -0400, Ben Peart wrote:\n\n> On 10/29/2018 1:26 PM, Duy Nguyen wrote:\n> > On Mon, Oct 29, 2018 at 6:21 PM Ben Peart wrote:\n> > > @@ -114,6 +104,9 @@ static void preload_index(struct index_state *index,\n> > > threads = index->cache_nr / THREAD_COST;\n> > > if ((index->cache_nr > 1) && (threads < 2) &&\n> > > git_env_bool(\"GIT_TEST_PRELOAD_INDEX\", 0))\n> > > threads = 2;\n> > > + cpus = online_cpus();\n> > > + if (threads > cpus)\n> > > + threads = cpus;\n> > > if (threads < 2)\n> > > return;\n> > > trace_performance_enter();\n> > \n> > Capping the number of threads to online_cpus() does not always make\n> > sense. In this case (or at least the original use case where we stat()\n> > over nfs) we want to issue as many requests to nfs server as possible\n> > to reduce latency and it has nothing to do with how many cores we\n> > have. Using more threads than cores might make sense since threads are\n> > blocked by nfs client, but we still want to send more to the server.\n> > \n> \n> That makes sense. Some test will be necessary then.\n> \n> I guess HAVE_THREADS is functionally the same as online_cpus() == 1. For\n> some reason, I prefer the latter - probably because I'm typically already\n> calling it and so it doesn't feel like a 'special' value/test I have to\n> remember. I just know I need to make sure the logic works correctly with any\n> number of cps greater than zero. :-)\n\nI don't think they're functionally the same. If you see online_cpus()\n== 1, you are in one of two cases:\n\n - the system supports threads, but CPU-bound tasks should stick to a\n single thread\n\n - the system does not even support threading, and calling\n pthread_create() will give you ENOSYS\n\nIn the first case, if your task is _not_ CPU bound (i.e., the stat()\nthing), then you want to distinguish those cases.\n\nI'm not sure if I'm violently agreeing, but it just wasn't clear to me\nfrom the above discussion that we all agreed that HAVE_THREADS is still\nnecessary in some cases.\n\n(I do think in cases where it is not, that we would do just as well to\navoid it; if that is all you were saying, then I agree. ;) ).\n\n-Peff\n"},{"id":"361953","messageId":"xmqqo9bc2pgu.fsf@gitster-ct.c.googlers.com","threadId":"49697","inReplyTo":"20181029162007.GA31497@sigill.intra.peff.net","subject":"Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-30T01:27:45Z","receivedAt":"2018-10-30T01:27:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Oct 29, 2018 at 05:01:41PM +0100, Duy Nguyen wrote:\n>\n>> On Mon, Oct 29, 2018 at 3:25 PM Jeff King wrote:\n>> > But if the problem is simply that we are not quite there yet in the grep\n>> > code, I am OK with taking this as the first pass, and knowing that there\n>> > is more cleanup to be done later (though that sort of thing is IMHO very\n>> > useful in a commit message).\n>> \n>> Since the problem pops up now, I'm ok with updating/cleaning up all\n>> this in this series, unless there's benefits in keeping this series\n>> simple and merging it early (probably not?)\n>\n> Mostly I did not want to tax you. I would rather have this series and\n> some cleanup left over, than to not have anything. But if you are\n> interested in moving it further, I will not say no. :)\n\nLikewise.\n"},{"id":"361954","messageId":"xmqqk1m02op2.fsf@gitster-ct.c.googlers.com","threadId":"49697","inReplyTo":"CACsJy8BD8nUfJoNjWCnaYgUn5+LaunOx74AgwPf5okG0OhwEXw@mail.gmail.com","subject":"Re: [PATCH 09/10] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-30T01:44:25Z","receivedAt":"2018-10-30T01:44:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n>> This shouldn't be needed either. My assumption was that if someone\n>> explicitly asked for multiple threads we're better off failing than\n>> silently ignoring their request. The default/automatic setting will\n>> detect the number of cpus and behave correctly.\n>\n> No. I can have ~/.gitconfig shared between different machines. One\n> with multithread support and one without. I should not have to fall\n> back to conditional includes in order to use the same config file on\n> the machine without multithread.\n\nOur attitude has been to fail unsatisfyable requests from the\ncommand line loudly to let the users' know, and tolerate having\nelements from the future versions of Git in the configuration file\nby ignoring them or tweaking them (silently or with a warning).\n\nSo this hunk is in line with the current practice, and we should\nkeep it this way. It is a separate discussion if we want to rethink\nthat whole attitude, that discussion may be worth having, and I\nthink that discussion is wider than this single topic.\n"},{"id":"362339","messageId":"20181103084850.9584-1-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181027173008.18852-1-pclouds@gmail.com","subject":"[PATCH v3 00/14] Reduce #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:36Z","receivedAt":"2018-11-03T08:49:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Changes since v2\n\n- more cleanups in grep.c, read-cache.c and index-pack.c\n- the send-pack.c changes are back, but this time I just add\n async_with_fork() to move NO_PTHREADS back in run-command.c\n\nFor grep.c and read-cache.c, changes are split in two patches. The\nfirst one is a dumb, mechanical conversion from #ifdef to if and is\nstraightforward. The second one makes \"no thread\" use \"one thread\"\ncode path and needs more careful review.\n\nDiff against nd/pthreads\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 6dd15dbaa2..de3f568cee 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -69,13 +69,11 @@ static pthread_mutex_t grep_mutex;\n \n static inline void grep_lock(void)\n {\n-\tassert(num_threads);\n \tpthread_mutex_lock(&grep_mutex);\n }\n \n static inline void grep_unlock(void)\n {\n-\tassert(num_threads);\n \tpthread_mutex_unlock(&grep_mutex);\n }\n \n@@ -234,7 +232,7 @@ static int wait_all(void)\n \tint i;\n \n \tif (!HAVE_THREADS)\n-\t\treturn 0;\n+\t\tBUG(\"Never call this function unless you have started threads\");\n \n \tgrep_lock();\n \tall_work_added = 1;\n@@ -279,14 +277,14 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\tif (num_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d) for %s\"),\n \t\t\t num_threads, var);\n-\t\telse if (!HAVE_THREADS && num_threads && num_threads != 1) {\n+\t\telse if (!HAVE_THREADS && num_threads > 1) {\n \t\t\t/*\n \t\t\t * TRANSLATORS: %s is the configuration\n \t\t\t * variable for tweaking threads, currently\n \t\t\t * grep.threads\n \t\t\t */\n \t\t\twarning(_(\"no threads support, ignoring %s\"), var);\n-\t\t\tnum_threads = 0;\n+\t\t\tnum_threads = 1;\n \t\t}\n \t}\n \n@@ -323,7 +321,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n-\tif (HAVE_THREADS && num_threads) {\n+\tif (num_threads > 1) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n@@ -353,7 +351,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \n-\tif (HAVE_THREADS && num_threads) {\n+\tif (num_threads > 1) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n@@ -1025,36 +1023,34 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tpathspec.recursive = 1;\n \tpathspec.recurse_submodules = !!recurse_submodules;\n \n-\tif (HAVE_THREADS) {\n-\t\tif (list.nr || cached || show_in_pager)\n-\t\t\tnum_threads = 0;\n-\t\telse if (num_threads == 0)\n-\t\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n-\t\telse if (num_threads < 0)\n-\t\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n-\t\tif (num_threads == 1)\n-\t\t\tnum_threads = 0;\n+\tif (list.nr || cached || show_in_pager) {\n+\t\tif (num_threads > 1)\n+\t\t\twarning(_(\"invalid option combination, ignoring --threads\"));\n+\t\tnum_threads = 1;\n+\t} else if (!HAVE_THREADS && num_threads > 1) {\n+\t\twarning(_(\"no threads support, ignoring --threads\"));\n+\t\tnum_threads = 1;\n+\t} else if (num_threads < 0)\n+\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n+\telse if (num_threads == 0)\n+\t\tnum_threads = HAVE_THREADS ? GREP_NUM_THREADS_DEFAULT : 1;\n+\n+\tif (num_threads > 1) {\n+\t\tif (!HAVE_THREADS)\n+\t\t\tBUG(\"Somebody got num_threads calculation wrong!\");\n+\t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n+\t\t && (opt.pre_context || opt.post_context ||\n+\t\t\topt.file_break || opt.funcbody))\n+\t\t\tskip_first_line = 1;\n+\t\tstart_threads(&opt);\n \t} else {\n-\t\tif (num_threads)\n-\t\t\twarning(_(\"no threads support, ignoring --threads\"));\n-\t\tnum_threads = 0;\n-\t}\n-\n-\tif (!num_threads)\n \t\t/*\n \t\t * The compiled patterns on the main path are only\n \t\t * used when not using threading. Otherwise\n-\t\t * start_threads() below calls compile_grep_patterns()\n+\t\t * start_threads() above calls compile_grep_patterns()\n \t\t * for each thread.\n \t\t */\n \t\tcompile_grep_patterns(&opt);\n-\n-\tif (HAVE_THREADS && num_threads) {\n-\t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n-\t\t && (opt.pre_context || opt.post_context ||\n-\t\t\topt.file_break || opt.funcbody))\n-\t\t\tskip_first_line = 1;\n-\t\tstart_threads(&opt);\n \t}\n \n \tif (show_in_pager && (cached || list.nr))\n@@ -1106,7 +1102,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\thit = grep_objects(&opt, &pathspec, &list);\n \t}\n \n-\tif (num_threads)\n+\tif (num_threads > 1)\n \t\thit |= wait_all();\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex bbd66ca025..682042579b 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1501,9 +1501,8 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\tif (nr_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t nr_threads);\n-\t\tif (!HAVE_THREADS) {\n-\t\t\tif (nr_threads != 1)\n-\t\t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n+\t\tif (!HAVE_THREADS && nr_threads != 1) {\n+\t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n \t\t\tnr_threads = 1;\n \t\t}\n \t\treturn 0;\n@@ -1693,10 +1692,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n \t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n \t\t\t\t\tusage(index_pack_usage);\n-\t\t\t\tif (!HAVE_THREADS) {\n-\t\t\t\t\tif (nr_threads != 1)\n-\t\t\t\t\t\twarning(_(\"no threads support, \"\n-\t\t\t\t\t\t\t \"ignoring %s\"), arg);\n+\t\t\t\tif (!HAVE_THREADS && nr_threads != 1) {\n+\t\t\t\t\twarning(_(\"no threads support, ignoring %s\"), arg);\n \t\t\t\t\tnr_threads = 1;\n \t\t\t\t}\n \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\ndiff --git a/read-cache.c b/read-cache.c\nindex 4307b9a7bf..c510f598b1 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2170,20 +2170,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tsrc_offset = sizeof(*hdr);\n \n-\tif (HAVE_THREADS) {\n-\t\tnr_threads = git_config_get_index_threads();\n+\tnr_threads = git_config_get_index_threads();\n \n-\t\t/* TODO: does creating more threads than cores help? */\n-\t\tif (!nr_threads) {\n-\t\t\tnr_threads = istate->cache_nr / THREAD_COST;\n-\t\t\tcpus = online_cpus();\n-\t\t\tif (nr_threads > cpus)\n-\t\t\t\tnr_threads = cpus;\n-\t\t}\n-\t} else {\n-\t\tnr_threads = 1;\n+\t/* TODO: does creating more threads than cores help? */\n+\tif (!nr_threads) {\n+\t\tnr_threads = istate->cache_nr / THREAD_COST;\n+\t\tcpus = online_cpus();\n+\t\tif (nr_threads > cpus)\n+\t\t\tnr_threads = cpus;\n \t}\n \n+\tif (!HAVE_THREADS)\n+\t\tnr_threads = 1;\n+\n \tif (nr_threads > 1) {\n \t\textension_offset = read_eoie_extension(mmap, mmap_size);\n \t\tif (extension_offset) {\n@@ -2216,12 +2215,11 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \n \t/* if we created a thread, join it otherwise load the extensions on the primary thread */\n-\tif (HAVE_THREADS && extension_offset) {\n+\tif (extension_offset) {\n \t\tint ret = pthread_join(p.pthread, NULL);\n \t\tif (ret)\n \t\t\tdie(_(\"unable to join load_index_extensions thread: %s\"), strerror(ret));\n-\t}\n-\tif (!extension_offset) {\n+\t} else {\n \t\tp.src_offset = src_offset;\n \t\tload_index_extensions(&p);\n \t}\n@@ -2860,7 +2858,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t * strip_extensions parameter as we need it when loading the shared\n \t * index.\n \t */\n-\tif (HAVE_THREADS && ieot) {\n+\tif (ieot) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\twrite_ieot_extension(&sb, ieot);\ndiff --git a/run-command.c b/run-command.c\nindex 26154ba257..decf3239bd 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1246,6 +1246,15 @@ int finish_async(struct async *async)\n #endif\n }\n \n+int async_with_fork(void)\n+{\n+#ifdef NO_PTHREADS\n+\treturn 1;\n+#else\n+\treturn 0;\n+#endif\n+}\n+\n const char *find_hook(const char *name)\n {\n \tstatic struct strbuf path = STRBUF_INIT;\ndiff --git a/run-command.h b/run-command.h\nindex 3932420ec8..68f5369fc2 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -1,9 +1,7 @@\n #ifndef RUN_COMMAND_H\n #define RUN_COMMAND_H\n \n-#ifndef NO_PTHREADS\n-#include \n-#endif\n+#include \"thread-utils.h\"\n \n #include \"argv-array.h\"\n \n@@ -143,6 +141,7 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n int in_async(void);\n+int async_with_fork(void);\n void check_pipe(int err);\n \n /**\ndiff --git a/send-pack.c b/send-pack.c\nindex e920ca57df..f692686770 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -203,9 +203,8 @@ static int receive_status(int in, struct ref *refs)\n static int sideband_demux(int in, int out, void *data)\n {\n \tint *fd = data, ret;\n-#ifdef NO_PTHREADS\n-\tclose(fd[1]);\n-#endif\n+\tif (async_with_fork())\n+\t\tclose(fd[1]);\n \tret = recv_sideband(\"send-pack\", fd[0], out);\n \tclose(out);\n \treturn ret;\n\nNguyễn Thái Ngọc Duy (14):\n thread-utils: macros to unconditionally compile pthreads API\n run-command.h: include thread-utils.h instead of pthread.h\n send-pack.c: move async's #ifdef NO_PTHREADS back to run-command.c\n index-pack: remove #ifdef NO_PTHREADS\n name-hash.c: remove #ifdef NO_PTHREADS\n attr.c: remove #ifdef NO_PTHREADS\n grep: remove #ifdef NO_PTHREADS\n grep: clean up num_threads handling\n preload-index.c: remove #ifdef NO_PTHREADS\n pack-objects: remove #ifdef NO_PTHREADS\n read-cache.c: remove #ifdef NO_PTHREADS\n read-cache.c: reduce branching based on HAVE_THREADS\n read-cache.c: initialize copy_len to shut up gcc 8\n Clean up pthread_create() error handling\n\n Makefile | 2 +-\n attr.c | 14 --------\n builtin/grep.c | 79 ++++++++++++++++--------------------------\n builtin/index-pack.c | 63 ++++++++-------------------------\n builtin/pack-objects.c | 26 ++------------\n grep.c | 6 ----\n grep.h | 6 ----\n name-hash.c | 38 ++++++++------------\n pack-objects.h | 6 ----\n preload-index.c | 23 +++++-------\n read-cache.c | 37 ++++++--------------\n run-command.c | 11 +++++-\n run-command.h | 5 ++-\n send-pack.c | 5 ++-\n thread-utils.c | 48 +++++++++++++++++++++++++\n thread-utils.h | 48 +++++++++++++++++++++++--\n 16 files changed, 186 insertions(+), 231 deletions(-)\n\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362340","messageId":"20181103084850.9584-2-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 01/14] thread-utils: macros to unconditionally compile pthreads API","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:37Z","receivedAt":"2018-11-03T08:49:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When built with NO_PTHREADS, the macros are used make the code build\neven though pthreads header and library may be missing. The code can\nstill have different code paths for no threads support with\nHAVE_THREADS variable.\n\nThere are of course impacts on no-pthreads builds:\n\n- data structure may get slightly bigger because all the mutexes and\n pthread_t are present (as an int)\n\n- code execution is not impacted much. Locking (in hot path) is\n no-op. Other wrapper function calls really should not matter much.\n\n- the binary size grows bigger because of threaded code. But at least\n on Linux this does not matter, if some code is not executed, it's\n not mapped in memory.\n\nThis is a preparation step to remove \"#ifdef NO_PTHREADS\" in the code\nmostly because of maintainability. As Jeff put it\n\n> it's probably OK to stop thinking of it as \"non-threaded platforms\n> are the default and must pay zero cost\" and more as \"threaded\n> platforms are the default, and non-threaded ones are OK to pay a\n> small cost as long as they still work\".\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Makefile | 2 +-\n thread-utils.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n thread-utils.h | 48 +++++++++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 94 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b08d5ea258..321540a736 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -991,6 +991,7 @@ LIB_OBJS += sub-process.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += tempfile.o\n+LIB_OBJS += thread-utils.o\n LIB_OBJS += tmp-objdir.o\n LIB_OBJS += trace.o\n LIB_OBJS += trailer.o\n@@ -1674,7 +1675,6 @@ ifdef NO_PTHREADS\n else\n \tBASIC_CFLAGS += $(PTHREAD_CFLAGS)\n \tEXTLIBS += $(PTHREAD_LIBS)\n-\tLIB_OBJS += thread-utils.o\n endif\n \n ifdef HAVE_PATHS_H\ndiff --git a/thread-utils.c b/thread-utils.c\nindex a2135e0743..5329845691 100644\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -20,6 +20,9 @@\n \n int online_cpus(void)\n {\n+#ifdef NO_PTHREADS\n+\treturn 1;\n+#else\n #ifdef _SC_NPROCESSORS_ONLN\n \tlong ncpus;\n #endif\n@@ -59,10 +62,12 @@ int online_cpus(void)\n #endif\n \n \treturn 1;\n+#endif\n }\n \n int init_recursive_mutex(pthread_mutex_t *m)\n {\n+#ifndef NO_PTHREADS\n \tpthread_mutexattr_t a;\n \tint ret;\n \n@@ -74,4 +79,47 @@ int init_recursive_mutex(pthread_mutex_t *m)\n \t\tpthread_mutexattr_destroy(&a);\n \t}\n \treturn ret;\n+#else\n+\treturn 0;\n+#endif\n+}\n+\n+#ifdef NO_PTHREADS\n+int dummy_pthread_create(pthread_t *pthread, const void *attr,\n+\t\t\t void *(*fn)(void *), void *data)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis and avoid -Wunused-variable false warnings.\n+\t */\n+\treturn ENOSYS;\n+}\n+\n+int dummy_pthread_init(void *data)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis or it may realize that functions like\n+\t * pthread_mutex_init() is no-op, which means the (static)\n+\t * variable is not used/initialized at all and trigger\n+\t * -Wunused-variable\n+\t */\n+\treturn ENOSYS;\n }\n+\n+int dummy_pthread_join(pthread_t pthread, void **retval)\n+{\n+\t/*\n+\t * Do nothing.\n+\t *\n+\t * The main purpose of this function is to break compiler's\n+\t * flow analysis and avoid -Wunused-variable false warnings.\n+\t */\n+\treturn ENOSYS;\n+}\n+\n+#endif\ndiff --git a/thread-utils.h b/thread-utils.h\nindex d9a769d190..4961487ed9 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -4,12 +4,54 @@\n #ifndef NO_PTHREADS\n #include \n \n-extern int online_cpus(void);\n-extern int init_recursive_mutex(pthread_mutex_t*);\n+#define HAVE_THREADS 1\n \n #else\n \n-#define online_cpus() 1\n+#define HAVE_THREADS 0\n+\n+/*\n+ * macros instead of typedefs because pthread definitions may have\n+ * been pulled in by some system dependencies even though the user\n+ * wants to disable pthread.\n+ */\n+#define pthread_t int\n+#define pthread_mutex_t int\n+#define pthread_cond_t int\n+#define pthread_key_t int\n+\n+#define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex)\n+#define pthread_mutex_lock(mutex)\n+#define pthread_mutex_unlock(mutex)\n+#define pthread_mutex_destroy(mutex)\n+\n+#define pthread_cond_init(cond, attr) dummy_pthread_init(cond)\n+#define pthread_cond_wait(cond, mutex)\n+#define pthread_cond_signal(cond)\n+#define pthread_cond_broadcast(cond)\n+#define pthread_cond_destroy(cond)\n+\n+#define pthread_key_create(key, attr) dummy_pthread_init(key)\n+#define pthread_key_delete(key)\n+\n+#define pthread_create(thread, attr, fn, data) \\\n+\tdummy_pthread_create(thread, attr, fn, data)\n+#define pthread_join(thread, retval) \\\n+\tdummy_pthread_join(thread, retval)\n+\n+#define pthread_setspecific(key, data)\n+#define pthread_getspecific(key) NULL\n+\n+int dummy_pthread_create(pthread_t *pthread, const void *attr,\n+\t\t\t void *(*fn)(void *), void *data);\n+int dummy_pthread_join(pthread_t pthread, void **retval);\n+\n+int dummy_pthread_init(void *);\n \n #endif\n+\n+int online_cpus(void);\n+int init_recursive_mutex(pthread_mutex_t*);\n+\n+\n #endif /* THREAD_COMPAT_H */\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362341","messageId":"20181103084850.9584-3-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 02/14] run-command.h: include thread-utils.h instead of pthread.h","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:38Z","receivedAt":"2018-11-03T08:49:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"run-command.c may use threads for its async support. But instead of\nincluding pthread.h directly, let's include thread-utils.h.\n\nrun-command.c probably never needs the dummy bits in thread-utils.h\nwhen NO_PTHREADS is defined. But this makes sure we have consistent\nHAVE_THREADS behavior everywhere. From now on outside compat/,\nthread-utils.h is the only place that includes pthread.h\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n run-command.h | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/run-command.h b/run-command.h\nindex 3932420ec8..9b7f38202c 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -1,9 +1,7 @@\n #ifndef RUN_COMMAND_H\n #define RUN_COMMAND_H\n \n-#ifndef NO_PTHREADS\n-#include \n-#endif\n+#include \"thread-utils.h\"\n \n #include \"argv-array.h\"\n \n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362342","messageId":"20181103084850.9584-4-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 03/14] send-pack.c: move async's #ifdef NO_PTHREADS back to run-command.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:39Z","receivedAt":"2018-11-03T08:49:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On systems that do not support multithread, start_async() is\nimplemented with fork(). This implementation details unfortunately\nleak out at least in send-pack.c [1].\n\nTo keep the code base clean of NO_PTHREADS, move the this #ifdef back\nto run-command.c. The new wrapper function async_with_fork() at least\nhelps suggest that this special \"close()\" is related to async in fork\nmode.\n\n[1] 09c9957cf7 (send-pack: avoid deadlock when pack-object dies early\n - 2011-04-25)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n run-command.c | 9 +++++++++\n run-command.h | 1 +\n send-pack.c | 5 ++---\n 3 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 84b883c213..3c3b8814df 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1246,6 +1246,15 @@ int finish_async(struct async *async)\n #endif\n }\n \n+int async_with_fork(void)\n+{\n+#ifdef NO_PTHREADS\n+\treturn 1;\n+#else\n+\treturn 0;\n+#endif\n+}\n+\n const char *find_hook(const char *name)\n {\n \tstatic struct strbuf path = STRBUF_INIT;\ndiff --git a/run-command.h b/run-command.h\nindex 9b7f38202c..68f5369fc2 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -141,6 +141,7 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n int in_async(void);\n+int async_with_fork(void);\n void check_pipe(int err);\n \n /**\ndiff --git a/send-pack.c b/send-pack.c\nindex e920ca57df..f692686770 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -203,9 +203,8 @@ static int receive_status(int in, struct ref *refs)\n static int sideband_demux(int in, int out, void *data)\n {\n \tint *fd = data, ret;\n-#ifdef NO_PTHREADS\n-\tclose(fd[1]);\n-#endif\n+\tif (async_with_fork())\n+\t\tclose(fd[1]);\n \tret = recv_sideband(\"send-pack\", fd[0], out);\n \tclose(out);\n \treturn ret;\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362343","messageId":"20181103084850.9584-5-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 04/14] index-pack: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:40Z","receivedAt":"2018-11-03T08:49:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/index-pack.c | 63 ++++++++++----------------------------------\n 1 file changed, 14 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 2004e25da2..682042579b 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -42,9 +42,7 @@ struct base_data {\n };\n \n struct thread_local {\n-#ifndef NO_PTHREADS\n \tpthread_t thread;\n-#endif\n \tstruct base_data *base_cache;\n \tsize_t base_cache_used;\n \tint pack_fd;\n@@ -98,8 +96,6 @@ static uint32_t input_crc32;\n static int input_fd, output_fd;\n static const char *curr_pack;\n \n-#ifndef NO_PTHREADS\n-\n static struct thread_local *thread_data;\n static int nr_dispatched;\n static int threads_active;\n@@ -179,26 +175,6 @@ static void cleanup_thread(void)\n \tfree(thread_data);\n }\n \n-#else\n-\n-#define read_lock()\n-#define read_unlock()\n-\n-#define counter_lock()\n-#define counter_unlock()\n-\n-#define work_lock()\n-#define work_unlock()\n-\n-#define deepest_delta_lock()\n-#define deepest_delta_unlock()\n-\n-#define type_cas_lock()\n-#define type_cas_unlock()\n-\n-#endif\n-\n-\n static int mark_link(struct object *obj, int type, void *data, struct fsck_options *options)\n {\n \tif (!obj)\n@@ -364,22 +340,20 @@ static NORETURN void bad_object(off_t offset, const char *format, ...)\n \n static inline struct thread_local *get_thread_data(void)\n {\n-#ifndef NO_PTHREADS\n-\tif (threads_active)\n-\t\treturn pthread_getspecific(key);\n-\tassert(!threads_active &&\n-\t \"This should only be reached when all threads are gone\");\n-#endif\n+\tif (HAVE_THREADS) {\n+\t\tif (threads_active)\n+\t\t\treturn pthread_getspecific(key);\n+\t\tassert(!threads_active &&\n+\t\t \"This should only be reached when all threads are gone\");\n+\t}\n \treturn ¬hread_data;\n }\n \n-#ifndef NO_PTHREADS\n static void set_thread_data(struct thread_local *data)\n {\n \tif (threads_active)\n \t\tpthread_setspecific(key, data);\n }\n-#endif\n \n static struct base_data *alloc_base_data(void)\n {\n@@ -1092,7 +1066,6 @@ static void resolve_base(struct object_entry *obj)\n \tfind_unresolved_deltas(base_obj);\n }\n \n-#ifndef NO_PTHREADS\n static void *threaded_second_pass(void *data)\n {\n \tset_thread_data(data);\n@@ -1116,7 +1089,6 @@ static void *threaded_second_pass(void *data)\n \t}\n \treturn NULL;\n }\n-#endif\n \n /*\n * First pass:\n@@ -1213,7 +1185,6 @@ static void resolve_deltas(void)\n \t\tprogress = start_progress(_(\"Resolving deltas\"),\n \t\t\t\t\t nr_ref_deltas + nr_ofs_deltas);\n \n-#ifndef NO_PTHREADS\n \tnr_dispatched = 0;\n \tif (nr_threads > 1 || getenv(\"GIT_FORCE_THREADS\")) {\n \t\tinit_thread();\n@@ -1229,7 +1200,6 @@ static void resolve_deltas(void)\n \t\tcleanup_thread();\n \t\treturn;\n \t}\n-#endif\n \n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *obj = &objects[i];\n@@ -1531,11 +1501,10 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\tif (nr_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t nr_threads);\n-#ifdef NO_PTHREADS\n-\t\tif (nr_threads != 1)\n+\t\tif (!HAVE_THREADS && nr_threads != 1) {\n \t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n-\t\tnr_threads = 1;\n-#endif\n+\t\t\tnr_threads = 1;\n+\t\t}\n \t\treturn 0;\n \t}\n \treturn git_default_config(k, v, cb);\n@@ -1723,12 +1692,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n \t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n \t\t\t\t\tusage(index_pack_usage);\n-#ifdef NO_PTHREADS\n-\t\t\t\tif (nr_threads != 1)\n-\t\t\t\t\twarning(_(\"no threads support, \"\n-\t\t\t\t\t\t \"ignoring %s\"), arg);\n-\t\t\t\tnr_threads = 1;\n-#endif\n+\t\t\t\tif (!HAVE_THREADS && nr_threads != 1) {\n+\t\t\t\t\twarning(_(\"no threads support, ignoring %s\"), arg);\n+\t\t\t\t\tnr_threads = 1;\n+\t\t\t\t}\n \t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n@@ -1791,14 +1758,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (strict)\n \t\topts.flags |= WRITE_IDX_STRICT;\n \n-#ifndef NO_PTHREADS\n-\tif (!nr_threads) {\n+\tif (HAVE_THREADS && !nr_threads) {\n \t\tnr_threads = online_cpus();\n \t\t/* An experiment showed that more threads does not mean faster */\n \t\tif (nr_threads > 3)\n \t\t\tnr_threads = 3;\n \t}\n-#endif\n \n \tcurr_pack = open_pack_file(pack_name);\n \tparse_pack_header();\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362344","messageId":"20181103084850.9584-6-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 05/14] name-hash.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:41Z","receivedAt":"2018-11-03T08:49:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n name-hash.c | 22 ++++------------------\n 1 file changed, 4 insertions(+), 18 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex 1fcda73cb3..b3c9ac791d 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -7,6 +7,7 @@\n */\n #define NO_THE_INDEX_COMPATIBILITY_MACROS\n #include \"cache.h\"\n+#include \"thread-utils.h\"\n \n struct dir_entry {\n \tstruct hashmap_entry ent;\n@@ -131,22 +132,6 @@ static int cache_entry_cmp(const void *unused_cmp_data,\n static int lazy_try_threaded = 1;\n static int lazy_nr_dir_threads;\n \n-#ifdef NO_PTHREADS\n-\n-static inline int lookup_lazy_params(struct index_state *istate)\n-{\n-\treturn 0;\n-}\n-\n-static inline void threaded_lazy_init_name_hash(\n-\tstruct index_state *istate)\n-{\n-}\n-\n-#else\n-\n-#include \"thread-utils.h\"\n-\n /*\n * Set a minimum number of cache_entries that we will handle per\n * thread and use that to decide how many threads to run (upto\n@@ -516,6 +501,9 @@ static void threaded_lazy_init_name_hash(\n \tstruct lazy_dir_thread_data *td_dir;\n \tstruct lazy_name_thread_data *td_name;\n \n+\tif (!HAVE_THREADS)\n+\t\treturn;\n+\n \tk_start = 0;\n \tnr_each = DIV_ROUND_UP(istate->cache_nr, lazy_nr_dir_threads);\n \n@@ -574,8 +562,6 @@ static void threaded_lazy_init_name_hash(\n \tfree(lazy_entries);\n }\n \n-#endif\n-\n static void lazy_init_name_hash(struct index_state *istate)\n {\n \n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362345","messageId":"20181103084850.9584-7-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 06/14] attr.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:42Z","receivedAt":"2018-11-03T08:49:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n attr.c | 14 --------------\n 1 file changed, 14 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 60d284796d..eaece6658d 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -41,23 +41,17 @@ const char *git_attr_name(const struct git_attr *attr)\n \n struct attr_hashmap {\n \tstruct hashmap map;\n-#ifndef NO_PTHREADS\n \tpthread_mutex_t mutex;\n-#endif\n };\n \n static inline void hashmap_lock(struct attr_hashmap *map)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&map->mutex);\n-#endif\n }\n \n static inline void hashmap_unlock(struct attr_hashmap *map)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&map->mutex);\n-#endif\n }\n \n /*\n@@ -498,23 +492,17 @@ static struct check_vector {\n \tsize_t nr;\n \tsize_t alloc;\n \tstruct attr_check **checks;\n-#ifndef NO_PTHREADS\n \tpthread_mutex_t mutex;\n-#endif\n } check_vector;\n \n static inline void vector_lock(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&check_vector.mutex);\n-#endif\n }\n \n static inline void vector_unlock(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&check_vector.mutex);\n-#endif\n }\n \n static void check_vector_add(struct attr_check *c)\n@@ -1181,8 +1169,6 @@ void git_all_attrs(const struct index_state *istate,\n \n void attr_start(void)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_init(&g_attr_hashmap.mutex, NULL);\n \tpthread_mutex_init(&check_vector.mutex, NULL);\n-#endif\n }\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362346","messageId":"20181103084850.9584-8-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 07/14] grep: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:43Z","receivedAt":"2018-11-03T08:49:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a faithful conversion without attempting to improve\nanything. That comes later.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 59 +++++++++++++++++++-------------------------------\n grep.c | 6 -----\n grep.h | 6 -----\n 3 files changed, 22 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex d8508ddf79..6dd15dbaa2 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -34,7 +34,6 @@ static int recurse_submodules;\n #define GREP_NUM_THREADS_DEFAULT 8\n static int num_threads;\n \n-#ifndef NO_PTHREADS\n static pthread_t *threads;\n \n /* We use one producer thread and THREADS consumer\n@@ -234,6 +233,9 @@ static int wait_all(void)\n \tint hit = 0;\n \tint i;\n \n+\tif (!HAVE_THREADS)\n+\t\treturn 0;\n+\n \tgrep_lock();\n \tall_work_added = 1;\n \n@@ -265,13 +267,6 @@ static int wait_all(void)\n \n \treturn hit;\n }\n-#else /* !NO_PTHREADS */\n-\n-static int wait_all(void)\n-{\n-\treturn 0;\n-}\n-#endif\n \n static int grep_cmd_config(const char *var, const char *value, void *cb)\n {\n@@ -284,8 +279,7 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\tif (num_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d) for %s\"),\n \t\t\t num_threads, var);\n-#ifdef NO_PTHREADS\n-\t\telse if (num_threads && num_threads != 1) {\n+\t\telse if (!HAVE_THREADS && num_threads && num_threads != 1) {\n \t\t\t/*\n \t\t\t * TRANSLATORS: %s is the configuration\n \t\t\t * variable for tweaking threads, currently\n@@ -294,7 +288,6 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\t\twarning(_(\"no threads support, ignoring %s\"), var);\n \t\t\tnum_threads = 0;\n \t\t}\n-#endif\n \t}\n \n \tif (!strcmp(var, \"submodule.recurse\"))\n@@ -330,17 +323,14 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n \t\t */\n \t\tadd_work(opt, &gs);\n \t\treturn 0;\n-\t} else\n-#endif\n-\t{\n+\t} else {\n \t\tint hit;\n \n \t\thit = grep_source(opt, &gs);\n@@ -363,17 +353,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n \t\t */\n \t\tadd_work(opt, &gs);\n \t\treturn 0;\n-\t} else\n-#endif\n-\t{\n+\t} else {\n \t\tint hit;\n \n \t\thit = grep_source(opt, &gs);\n@@ -1038,20 +1025,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tpathspec.recursive = 1;\n \tpathspec.recurse_submodules = !!recurse_submodules;\n \n-#ifndef NO_PTHREADS\n-\tif (list.nr || cached || show_in_pager)\n-\t\tnum_threads = 0;\n-\telse if (num_threads == 0)\n-\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n-\telse if (num_threads < 0)\n-\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n-\tif (num_threads == 1)\n+\tif (HAVE_THREADS) {\n+\t\tif (list.nr || cached || show_in_pager)\n+\t\t\tnum_threads = 0;\n+\t\telse if (num_threads == 0)\n+\t\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n+\t\telse if (num_threads < 0)\n+\t\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n+\t\tif (num_threads == 1)\n+\t\t\tnum_threads = 0;\n+\t} else {\n+\t\tif (num_threads)\n+\t\t\twarning(_(\"no threads support, ignoring --threads\"));\n \t\tnum_threads = 0;\n-#else\n-\tif (num_threads)\n-\t\twarning(_(\"no threads support, ignoring --threads\"));\n-\tnum_threads = 0;\n-#endif\n+\t}\n \n \tif (!num_threads)\n \t\t/*\n@@ -1062,15 +1049,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\tcompile_grep_patterns(&opt);\n \n-#ifndef NO_PTHREADS\n-\tif (num_threads) {\n+\tif (HAVE_THREADS && num_threads) {\n \t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n \t\t && (opt.pre_context || opt.post_context ||\n \t\t\topt.file_break || opt.funcbody))\n \t\t\tskip_first_line = 1;\n \t\tstart_threads(&opt);\n \t}\n-#endif\n \n \tif (show_in_pager && (cached || list.nr))\n \t\tdie(_(\"--open-files-in-pager only works on the worktree\"));\ndiff --git a/grep.c b/grep.c\nindex f6bd89e40b..4db1510d16 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1513,7 +1513,6 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t}\n }\n \n-#ifndef NO_PTHREADS\n int grep_use_locks;\n \n /*\n@@ -1539,11 +1538,6 @@ static inline void grep_attr_unlock(void)\n */\n pthread_mutex_t grep_read_mutex;\n \n-#else\n-#define grep_attr_lock()\n-#define grep_attr_unlock()\n-#endif\n-\n static int match_funcname(struct grep_opt *opt, struct grep_source *gs, char *bol, char *eol)\n {\n \txdemitconf_t *xecfg = opt->priv;\ndiff --git a/grep.h b/grep.h\nindex 1a57d12b90..fb04893721 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -229,7 +229,6 @@ int grep_source(struct grep_opt *opt, struct grep_source *gs);\n extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);\n extern int grep_threads_ok(const struct grep_opt *opt);\n \n-#ifndef NO_PTHREADS\n /*\n * Mutex used around access to the attributes machinery if\n * opt->use_threads. Must be initialized/destroyed by callers!\n@@ -250,9 +249,4 @@ static inline void grep_read_unlock(void)\n \t\tpthread_mutex_unlock(&grep_read_mutex);\n }\n \n-#else\n-#define grep_read_lock()\n-#define grep_read_unlock()\n-#endif\n-\n #endif\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362347","messageId":"20181103084850.9584-10-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 09/14] preload-index.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:45Z","receivedAt":"2018-11-03T08:49:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n preload-index.c | 15 ++-------------\n 1 file changed, 2 insertions(+), 13 deletions(-)\n\ndiff --git a/preload-index.c b/preload-index.c\nindex 9e7152ab14..0e24886aca 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -7,17 +7,7 @@\n #include \"fsmonitor.h\"\n #include \"config.h\"\n #include \"progress.h\"\n-\n-#ifdef NO_PTHREADS\n-static void preload_index(struct index_state *index,\n-\t\t\t const struct pathspec *pathspec,\n-\t\t\t unsigned int refresh_flags)\n-{\n-\t; /* nothing */\n-}\n-#else\n-\n-#include \n+#include \"thread-utils.h\"\n \n /*\n * Mostly randomly chosen maximum thread counts: we\n@@ -108,7 +98,7 @@ static void preload_index(struct index_state *index,\n \tstruct thread_data data[MAX_PARALLEL];\n \tstruct progress_data pd;\n \n-\tif (!core_preload_index)\n+\tif (!HAVE_THREADS || !core_preload_index)\n \t\treturn;\n \n \tthreads = index->cache_nr / THREAD_COST;\n@@ -151,7 +141,6 @@ static void preload_index(struct index_state *index,\n \n \ttrace_performance_leave(\"preload index\");\n }\n-#endif\n \n int read_index_preload(struct index_state *index,\n \t\t const struct pathspec *pathspec,\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362348","messageId":"20181103084850.9584-9-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 08/14] grep: clean up num_threads handling","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:44Z","receivedAt":"2018-11-03T08:49:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When NO_PTHREADS is still used in this file, we have two separate code\npaths for thread and no thread support. The latter will always have\nnum_threads remain zero while the former uses num_threads zero as\n\"default number of threads\".\n\nWith recent changes blur the line between thread and no-thread\nsupport, this num_threads handling becomes a bit strange so let's\nredefine it like this:\n\n- num_threads == 0 means default number of threads and should become\n positive after all configuration and option parsing is done if\n multithread is supported.\n\n- num_threads <= 1 runs no threads. It does not matter if the platform\n supports threading or not.\n\n- num_threads > 1 will run multiple threads and is invalid if\n HAVE_THREADS is false. pthread API is only used in this case.\n\nPS. a new warning is also added when num_threads is forced back to one\nbecause a thread-incompatible option is specified.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 58 +++++++++++++++++++++++---------------------------\n 1 file changed, 27 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 6dd15dbaa2..de3f568cee 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -69,13 +69,11 @@ static pthread_mutex_t grep_mutex;\n \n static inline void grep_lock(void)\n {\n-\tassert(num_threads);\n \tpthread_mutex_lock(&grep_mutex);\n }\n \n static inline void grep_unlock(void)\n {\n-\tassert(num_threads);\n \tpthread_mutex_unlock(&grep_mutex);\n }\n \n@@ -234,7 +232,7 @@ static int wait_all(void)\n \tint i;\n \n \tif (!HAVE_THREADS)\n-\t\treturn 0;\n+\t\tBUG(\"Never call this function unless you have started threads\");\n \n \tgrep_lock();\n \tall_work_added = 1;\n@@ -279,14 +277,14 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \t\tif (num_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d) for %s\"),\n \t\t\t num_threads, var);\n-\t\telse if (!HAVE_THREADS && num_threads && num_threads != 1) {\n+\t\telse if (!HAVE_THREADS && num_threads > 1) {\n \t\t\t/*\n \t\t\t * TRANSLATORS: %s is the configuration\n \t\t\t * variable for tweaking threads, currently\n \t\t\t * grep.threads\n \t\t\t */\n \t\t\twarning(_(\"no threads support, ignoring %s\"), var);\n-\t\t\tnum_threads = 0;\n+\t\t\tnum_threads = 1;\n \t\t}\n \t}\n \n@@ -323,7 +321,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n-\tif (HAVE_THREADS && num_threads) {\n+\tif (num_threads > 1) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n@@ -353,7 +351,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \n-\tif (HAVE_THREADS && num_threads) {\n+\tif (num_threads > 1) {\n \t\t/*\n \t\t * add_work() copies gs and thus assumes ownership of\n \t\t * its fields, so do not call grep_source_clear()\n@@ -1025,36 +1023,34 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tpathspec.recursive = 1;\n \tpathspec.recurse_submodules = !!recurse_submodules;\n \n-\tif (HAVE_THREADS) {\n-\t\tif (list.nr || cached || show_in_pager)\n-\t\t\tnum_threads = 0;\n-\t\telse if (num_threads == 0)\n-\t\t\tnum_threads = GREP_NUM_THREADS_DEFAULT;\n-\t\telse if (num_threads < 0)\n-\t\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n-\t\tif (num_threads == 1)\n-\t\t\tnum_threads = 0;\n+\tif (list.nr || cached || show_in_pager) {\n+\t\tif (num_threads > 1)\n+\t\t\twarning(_(\"invalid option combination, ignoring --threads\"));\n+\t\tnum_threads = 1;\n+\t} else if (!HAVE_THREADS && num_threads > 1) {\n+\t\twarning(_(\"no threads support, ignoring --threads\"));\n+\t\tnum_threads = 1;\n+\t} else if (num_threads < 0)\n+\t\tdie(_(\"invalid number of threads specified (%d)\"), num_threads);\n+\telse if (num_threads == 0)\n+\t\tnum_threads = HAVE_THREADS ? GREP_NUM_THREADS_DEFAULT : 1;\n+\n+\tif (num_threads > 1) {\n+\t\tif (!HAVE_THREADS)\n+\t\t\tBUG(\"Somebody got num_threads calculation wrong!\");\n+\t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n+\t\t && (opt.pre_context || opt.post_context ||\n+\t\t\topt.file_break || opt.funcbody))\n+\t\t\tskip_first_line = 1;\n+\t\tstart_threads(&opt);\n \t} else {\n-\t\tif (num_threads)\n-\t\t\twarning(_(\"no threads support, ignoring --threads\"));\n-\t\tnum_threads = 0;\n-\t}\n-\n-\tif (!num_threads)\n \t\t/*\n \t\t * The compiled patterns on the main path are only\n \t\t * used when not using threading. Otherwise\n-\t\t * start_threads() below calls compile_grep_patterns()\n+\t\t * start_threads() above calls compile_grep_patterns()\n \t\t * for each thread.\n \t\t */\n \t\tcompile_grep_patterns(&opt);\n-\n-\tif (HAVE_THREADS && num_threads) {\n-\t\tif (!(opt.name_only || opt.unmatch_name_only || opt.count)\n-\t\t && (opt.pre_context || opt.post_context ||\n-\t\t\topt.file_break || opt.funcbody))\n-\t\t\tskip_first_line = 1;\n-\t\tstart_threads(&opt);\n \t}\n \n \tif (show_in_pager && (cached || list.nr))\n@@ -1106,7 +1102,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\thit = grep_objects(&opt, &pathspec, &list);\n \t}\n \n-\tif (num_threads)\n+\tif (num_threads > 1)\n \t\thit |= wait_all();\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362349","messageId":"20181103084850.9584-11-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 10/14] pack-objects: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:46Z","receivedAt":"2018-11-03T08:49:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/pack-objects.c | 26 ++------------------------\n pack-objects.h | 6 ------\n 2 files changed, 2 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b059b86aee..12edd6da16 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1953,8 +1953,6 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \treturn 0;\n }\n \n-#ifndef NO_PTHREADS\n-\n /* Protect access to object database */\n static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n@@ -1979,16 +1977,6 @@ static pthread_mutex_t progress_mutex;\n * ahead in the list because they can be stolen and would need\n * progress_mutex for protection.\n */\n-#else\n-\n-#define read_lock()\t\t(void)0\n-#define read_unlock()\t\t(void)0\n-#define cache_lock()\t\t(void)0\n-#define cache_unlock()\t\t(void)0\n-#define progress_lock()\t\t(void)0\n-#define progress_unlock()\t(void)0\n-\n-#endif\n \n /*\n * Return the size of the object without doing any delta\n@@ -2347,8 +2335,6 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \tfree(array);\n }\n \n-#ifndef NO_PTHREADS\n-\n static void try_to_free_from_threads(size_t size)\n {\n \tread_lock();\n@@ -2578,10 +2564,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tfree(p);\n }\n \n-#else\n-#define ll_find_deltas(l, s, w, d, p)\tfind_deltas(l, &s, w, d, p)\n-#endif\n-\n static void add_tag_chain(const struct object_id *oid)\n {\n \tstruct tag *tag;\n@@ -2734,12 +2716,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n \t\tif (delta_search_threads < 0)\n \t\t\tdie(_(\"invalid number of threads specified (%d)\"),\n \t\t\t delta_search_threads);\n-#ifdef NO_PTHREADS\n-\t\tif (delta_search_threads != 1) {\n+\t\tif (!HAVE_THREADS && delta_search_threads != 1) {\n \t\t\twarning(_(\"no threads support, ignoring %s\"), k);\n \t\t\tdelta_search_threads = 0;\n \t\t}\n-#endif\n \t\treturn 0;\n \t}\n \tif (!strcmp(k, \"pack.indexversion\")) {\n@@ -3402,10 +3382,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n \t\tdelta_search_threads = online_cpus();\n \n-#ifdef NO_PTHREADS\n-\tif (delta_search_threads != 1)\n+\tif (!HAVE_THREADS && delta_search_threads != 1)\n \t\twarning(_(\"no threads support, ignoring --threads\"));\n-#endif\n \tif (!pack_to_stdout && !pack_size_limit)\n \t\tpack_size_limit = pack_size_limit_cfg;\n \tif (pack_to_stdout && pack_size_limit)\ndiff --git a/pack-objects.h b/pack-objects.h\nindex 2ca39cfcfe..3a42727c7d 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -145,9 +145,7 @@ struct packing_data {\n \tstruct packed_git **in_pack_by_idx;\n \tstruct packed_git **in_pack;\n \n-#ifndef NO_PTHREADS\n \tpthread_mutex_t lock;\n-#endif\n \n \t/*\n \t * This list contains entries for bases which we know the other side\n@@ -169,15 +167,11 @@ void prepare_packing_data(struct packing_data *pdata);\n \n static inline void packing_data_lock(struct packing_data *pdata)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_lock(&pdata->lock);\n-#endif\n }\n static inline void packing_data_unlock(struct packing_data *pdata)\n {\n-#ifndef NO_PTHREADS\n \tpthread_mutex_unlock(&pdata->lock);\n-#endif\n }\n \n struct object_entry *packlist_alloc(struct packing_data *pdata,\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362350","messageId":"20181103084850.9584-12-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 11/14] read-cache.c: remove #ifdef NO_PTHREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:47Z","receivedAt":"2018-11-03T08:49:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a faithful conversion with no attempt to clean up whatsoever.\nCode indentation is left broken. There will be another commit to clean\nit up and un-indent if we just indent now. It's just more code noise.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n read-cache.c | 34 ++++++++++------------------------\n 1 file changed, 10 insertions(+), 24 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex d57958233e..40fc0cb65f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1920,19 +1920,15 @@ struct index_entry_offset_table\n \tstruct index_entry_offset entries[FLEX_ARRAY];\n };\n \n-#ifndef NO_PTHREADS\n static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset);\n static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_table *ieot);\n-#endif\n \n static size_t read_eoie_extension(const char *mmap, size_t mmap_size);\n static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context, size_t offset);\n \n struct load_index_extensions\n {\n-#ifndef NO_PTHREADS\n \tpthread_t pthread;\n-#endif\n \tstruct index_state *istate;\n \tconst char *mmap;\n \tsize_t mmap_size;\n@@ -2010,8 +2006,6 @@ static unsigned long load_all_cache_entries(struct index_state *istate,\n \treturn consumed;\n }\n \n-#ifndef NO_PTHREADS\n-\n /*\n * Mostly randomly chosen maximum thread counts: we\n * cap the parallelism to online_cpus() threads, and we want\n@@ -2122,7 +2116,6 @@ static unsigned long load_cache_entries_threaded(struct index_state *istate, con\n \n \treturn consumed;\n }\n-#endif\n \n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int must_exist)\n@@ -2135,10 +2128,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tsize_t mmap_size;\n \tstruct load_index_extensions p;\n \tsize_t extension_offset = 0;\n-#ifndef NO_PTHREADS\n \tint nr_threads, cpus;\n \tstruct index_entry_offset_table *ieot = NULL;\n-#endif\n \n \tif (istate->initialized)\n \t\treturn istate->cache_nr;\n@@ -2181,7 +2172,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tsrc_offset = sizeof(*hdr);\n \n-#ifndef NO_PTHREADS\n+\tif (HAVE_THREADS) {\n \tnr_threads = git_config_get_index_threads();\n \n \t/* TODO: does creating more threads than cores help? */\n@@ -2219,21 +2210,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t} else {\n \t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n \t}\n-#else\n-\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n-#endif\n+\t} else {\n+\t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n+\t}\n \n \tistate->timestamp.sec = st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \n \t/* if we created a thread, join it otherwise load the extensions on the primary thread */\n-#ifndef NO_PTHREADS\n-\tif (extension_offset) {\n+\tif (HAVE_THREADS && extension_offset) {\n \t\tint ret = pthread_join(p.pthread, NULL);\n \t\tif (ret)\n \t\t\tdie(_(\"unable to join load_index_extensions thread: %s\"), strerror(ret));\n \t}\n-#endif\n \tif (!extension_offset) {\n \t\tp.src_offset = src_offset;\n \t\tload_index_extensions(&p);\n@@ -2756,8 +2745,9 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n \t\treturn -1;\n \n-#ifndef NO_PTHREADS\n-\tnr_threads = git_config_get_index_threads();\n+\tif (HAVE_THREADS) {\n+\t\tnr_threads = git_config_get_index_threads();\n+\n \tif (nr_threads != 1) {\n \t\tint ieot_blocks, cpus;\n \n@@ -2787,7 +2777,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t\tieot_entries = DIV_ROUND_UP(entries, ieot_blocks);\n \t\t}\n \t}\n-#endif\n+\t}\n \n \toffset = lseek(newfd, 0, SEEK_CUR);\n \tif (offset < 0) {\n@@ -2871,8 +2861,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t * strip_extensions parameter as we need it when loading the shared\n \t * index.\n \t */\n-#ifndef NO_PTHREADS\n-\tif (ieot) {\n+\tif (HAVE_THREADS && ieot) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\twrite_ieot_extension(&sb, ieot);\n@@ -2883,7 +2872,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\tif (err)\n \t\t\treturn -1;\n \t}\n-#endif\n \n \tif (!strip_extensions && istate->split_index) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n@@ -3469,7 +3457,6 @@ static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context,\n \tstrbuf_add(sb, hash, the_hash_algo->rawsz);\n }\n \n-#ifndef NO_PTHREADS\n #define IEOT_VERSION\t(1)\n \n static struct index_entry_offset_table *read_ieot_extension(const char *mmap, size_t mmap_size, size_t offset)\n@@ -3542,4 +3529,3 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta\n \t strbuf_add(sb, &buffer, sizeof(uint32_t));\n }\n }\n-#endif\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362351","messageId":"20181103084850.9584-13-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 12/14] read-cache.c: reduce branching based on HAVE_THREADS","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:48Z","receivedAt":"2018-11-03T08:49:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n read-cache.c | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 40fc0cb65f..00cd416816 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2172,7 +2172,6 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tsrc_offset = sizeof(*hdr);\n \n-\tif (HAVE_THREADS) {\n \tnr_threads = git_config_get_index_threads();\n \n \t/* TODO: does creating more threads than cores help? */\n@@ -2183,6 +2182,9 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t\t\tnr_threads = cpus;\n \t}\n \n+\tif (!HAVE_THREADS)\n+\t\tnr_threads = 1;\n+\n \tif (nr_threads > 1) {\n \t\textension_offset = read_eoie_extension(mmap, mmap_size);\n \t\tif (extension_offset) {\n@@ -2210,20 +2212,16 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t} else {\n \t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n \t}\n-\t} else {\n-\t\tsrc_offset += load_all_cache_entries(istate, mmap, mmap_size, src_offset);\n-\t}\n \n \tistate->timestamp.sec = st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \n \t/* if we created a thread, join it otherwise load the extensions on the primary thread */\n-\tif (HAVE_THREADS && extension_offset) {\n+\tif (extension_offset) {\n \t\tint ret = pthread_join(p.pthread, NULL);\n \t\tif (ret)\n \t\t\tdie(_(\"unable to join load_index_extensions thread: %s\"), strerror(ret));\n-\t}\n-\tif (!extension_offset) {\n+\t} else {\n \t\tp.src_offset = src_offset;\n \t\tload_index_extensions(&p);\n \t}\n@@ -2745,8 +2743,10 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n \t\treturn -1;\n \n-\tif (HAVE_THREADS) {\n+\tif (HAVE_THREADS)\n \t\tnr_threads = git_config_get_index_threads();\n+\telse\n+\t\tnr_threads = 1;\n \n \tif (nr_threads != 1) {\n \t\tint ieot_blocks, cpus;\n@@ -2777,7 +2777,6 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t\tieot_entries = DIV_ROUND_UP(entries, ieot_blocks);\n \t\t}\n \t}\n-\t}\n \n \toffset = lseek(newfd, 0, SEEK_CUR);\n \tif (offset < 0) {\n@@ -2861,7 +2860,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t * strip_extensions parameter as we need it when loading the shared\n \t * index.\n \t */\n-\tif (HAVE_THREADS && ieot) {\n+\tif (ieot) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\twrite_ieot_extension(&sb, ieot);\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362352","messageId":"20181103084850.9584-14-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 13/14] read-cache.c: initialize copy_len to shut up gcc 8","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:49Z","receivedAt":"2018-11-03T08:49:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It was reported that when building with NO_PTHREADS=1,\n-Wmaybe-uninitialized is triggered. Just initialize the variable from\nthe beginning to shut the compiler up (because this warning is enabled\nin config.dev)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n read-cache.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 00cd416816..c510f598b1 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1746,7 +1746,7 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \tsize_t len;\n \tconst char *name;\n \tunsigned int flags;\n-\tsize_t copy_len;\n+\tsize_t copy_len = 0;\n \t/*\n \t * Adjacent cache entries tend to share the leading paths, so it makes\n \t * sense to only store the differences in later entries. In the v4\n@@ -1786,8 +1786,6 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n \t\t\t\tdie(_(\"malformed name field in the index, near path '%s'\"),\n \t\t\t\t\tprevious_ce->name);\n \t\t\tcopy_len = previous_len - strip_len;\n-\t\t} else {\n-\t\t\tcopy_len = 0;\n \t\t}\n \t\tname = (const char *)cp;\n \t}\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362353","messageId":"20181103084850.9584-15-pclouds@gmail.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"[PATCH v3 14/14] Clean up pthread_create() error handling","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T08:48:50Z","receivedAt":"2018-11-03T08:49:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Normally pthread_create() rarely fails. But with new pthreads wrapper,\npthread_create() will return ENOSYS on a system without thread support.\n\nThreaded code _is_ protected by HAVE_THREADS and pthread_create()\nshould never run in the first place. But the situation could change in\nthe future and bugs may sneak in. Make sure that all pthread_create()\nreports the error cause.\n\nWhile at there, mark these strings for translation if they aren't.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n name-hash.c | 16 ++++++++++------\n preload-index.c | 8 ++++++--\n run-command.c | 2 +-\n 3 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex b3c9ac791d..623ca6923a 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -494,6 +494,7 @@ static inline void lazy_update_dir_ref_counts(\n static void threaded_lazy_init_name_hash(\n \tstruct index_state *istate)\n {\n+\tint err;\n \tint nr_each;\n \tint k_start;\n \tint t;\n@@ -526,8 +527,9 @@ static void threaded_lazy_init_name_hash(\n \t\tif (k_start > istate->cache_nr)\n \t\t\tk_start = istate->cache_nr;\n \t\ttd_dir_t->k_end = k_start;\n-\t\tif (pthread_create(&td_dir_t->pthread, NULL, lazy_dir_thread_proc, td_dir_t))\n-\t\t\tdie(\"unable to create lazy_dir_thread\");\n+\t\terr = pthread_create(&td_dir_t->pthread, NULL, lazy_dir_thread_proc, td_dir_t);\n+\t\tif (err)\n+\t\t\tdie(_(\"unable to create lazy_dir thread: %s\"), strerror(err));\n \t}\n \tfor (t = 0; t < lazy_nr_dir_threads; t++) {\n \t\tstruct lazy_dir_thread_data *td_dir_t = td_dir + t;\n@@ -547,13 +549,15 @@ static void threaded_lazy_init_name_hash(\n \t */\n \ttd_name->istate = istate;\n \ttd_name->lazy_entries = lazy_entries;\n-\tif (pthread_create(&td_name->pthread, NULL, lazy_name_thread_proc, td_name))\n-\t\tdie(\"unable to create lazy_name_thread\");\n+\terr = pthread_create(&td_name->pthread, NULL, lazy_name_thread_proc, td_name);\n+\tif (err)\n+\t\tdie(_(\"unable to create lazy_name thread: %s\"), strerror(err));\n \n \tlazy_update_dir_ref_counts(istate, lazy_entries);\n \n-\tif (pthread_join(td_name->pthread, NULL))\n-\t\tdie(\"unable to join lazy_name_thread\");\n+\terr = pthread_join(td_name->pthread, NULL);\n+\tif (err)\n+\t\tdie(_(\"unable to join lazy_name thread: %s\"), strerror(err));\n \n \tcleanup_dir_mutex();\n \ndiff --git a/preload-index.c b/preload-index.c\nindex 0e24886aca..ddca1c216e 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -121,6 +121,8 @@ static void preload_index(struct index_state *index,\n \n \tfor (i = 0; i < threads; i++) {\n \t\tstruct thread_data *p = data+i;\n+\t\tint err;\n+\n \t\tp->index = index;\n \t\tif (pathspec)\n \t\t\tcopy_pathspec(&p->pathspec, pathspec);\n@@ -129,8 +131,10 @@ static void preload_index(struct index_state *index,\n \t\tif (pd.progress)\n \t\t\tp->progress = &pd;\n \t\toffset += work;\n-\t\tif (pthread_create(&p->pthread, NULL, preload_thread, p))\n-\t\t\tdie(\"unable to create threaded lstat\");\n+\t\terr = pthread_create(&p->pthread, NULL, preload_thread, p);\n+\n+\t\tif (err)\n+\t\t\tdie(_(\"unable to create threaded lstat: %s\"), strerror(err));\n \t}\n \tfor (i = 0; i < threads; i++) {\n \t\tstruct thread_data *p = data+i;\ndiff --git a/run-command.c b/run-command.c\nindex 3c3b8814df..decf3239bd 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1213,7 +1213,7 @@ int start_async(struct async *async)\n \t{\n \t\tint err = pthread_create(&async->tid, NULL, run_thread, async);\n \t\tif (err) {\n-\t\t\terror_errno(\"cannot create thread\");\n+\t\t\terror(_(\"cannot create async thread: %s\"), strerror(err));\n \t\t\tgoto error;\n \t\t}\n \t}\n-- \n2.19.1.1005.gac84295441\n\n"},{"id":"362556","messageId":"xmqqmuqmerkr.fsf@gitster-ct.c.googlers.com","threadId":"49697","inReplyTo":"20181103084850.9584-1-pclouds@gmail.com","subject":"Re: [PATCH v3 00/14] Reduce #ifdef NO_PTHREADS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-06T04:51:48Z","receivedAt":"2018-11-06T04:53:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Changes since v2\n>\n> - more cleanups in grep.c, read-cache.c and index-pack.c\n> - the send-pack.c changes are back, but this time I just add\n> async_with_fork() to move NO_PTHREADS back in run-command.c\n\nThe patches all looked sensible; I'll wait for a few more days\nto see if anybody else spots something.\n\nThanks.\n"}]}