Changes since v2
- more cleanups in grep.c, read-cache.c and index-pack.c
- the send-pack.c changes are back, but this time I just add
async_with_fork() to move NO_PTHREADS back in run-command.c
For grep.c and read-cache.c, changes are split in two patches. The
first one is a dumb, mechanical conversion from #ifdef to if and is
straightforward. The second one makes "no thread" use "one thread"
code path and needs more careful review.
Diff against nd/pthreads
diff --git a/builtin/grep.c b/builtin/grep.c
index 6dd15dbaa2..de3f568cee 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -69,13 +69,11 @@ static pthread_mutex_t grep_mutex;
static inline void grep_lock(void)
{
- assert(num_threads);
pthread_mutex_lock(&grep_mutex);
}
static inline void grep_unlock(void)
{
- assert(num_threads);
pthread_mutex_unlock(&grep_mutex);
}
@@ -234,7 +232,7 @@ static int wait_all(void)
int i;
if (!HAVE_THREADS)
- return 0;
+ BUG("Never call this function unless you have started threads");
grep_lock();
all_work_added = 1;
@@ -279,14 +277,14 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
if (num_threads < 0)
die(_("invalid number of threads specified (%d) for %s"),
num_threads, var);
- else if (!HAVE_THREADS && num_threads && num_threads != 1) {
+ else if (!HAVE_THREADS && num_threads > 1) {
/*
* TRANSLATORS: %s is the configuration
* variable for tweaking threads, currently
* grep.threads
*/
warning(_("no threads support, ignoring %s"), var);
- num_threads = 0;
+ num_threads = 1;
}
}
@@ -323,7 +321,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);
strbuf_release(&pathbuf);
- if (HAVE_THREADS && num_threads) {
+ if (num_threads > 1) {
/*
* add_work() copies gs and thus assumes ownership of
* its fields, so do not call grep_source_clear()
@@ -353,7 +351,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);
strbuf_release(&buf);
- if (HAVE_THREADS && num_threads) {
+ if (num_threads > 1) {
/*
* add_work() copies gs and thus assumes ownership of
* its fields, so do not call grep_source_clear()
@@ -1025,36 +1023,34 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pathspec.recursive = 1;
pathspec.recurse_submodules = !!recurse_submodules;
- if (HAVE_THREADS) {
- if (list.nr || cached || show_in_pager)
- num_threads = 0;
- else if (num_threads == 0)
- num_threads = GREP_NUM_THREADS_DEFAULT;
- else if (num_threads < 0)
- die(_("invalid number of threads specified (%d)"), num_threads);
- if (num_threads == 1)
- num_threads = 0;
+ if (list.nr || cached || show_in_pager) {
+ if (num_threads > 1)
+ warning(_("invalid option combination, ignoring --threads"));
+ num_threads = 1;
+ } else if (!HAVE_THREADS && num_threads > 1) {
+ warning(_("no threads support, ignoring --threads"));
+ num_threads = 1;
+ } else if (num_threads < 0)
+ die(_("invalid number of threads specified (%d)"), num_threads);
+ else if (num_threads == 0)
+ num_threads = HAVE_THREADS ? GREP_NUM_THREADS_DEFAULT : 1;
+
+ if (num_threads > 1) {
+ if (!HAVE_THREADS)
+ BUG("Somebody got num_threads calculation wrong!");
+ if (!(opt.name_only || opt.unmatch_name_only || opt.count)
+ && (opt.pre_context || opt.post_context ||
+ opt.file_break || opt.funcbody))
+ skip_first_line = 1;
+ start_threads(&opt);
} else {
- if (num_threads)
- warning(_("no threads support, ignoring --threads"));
- num_threads = 0;
- }
-
- if (!num_threads)
/*
* The compiled patterns on the main path are only
* used when not using threading. Otherwise
- * start_threads() below calls compile_grep_patterns()
+ * start_threads() above calls compile_grep_patterns()
* for each thread.
*/
compile_grep_patterns(&opt);
-
- if (HAVE_THREADS && num_threads) {
- if (!(opt.name_only || opt.unmatch_name_only || opt.count)
- && (opt.pre_context || opt.post_context ||
- opt.file_break || opt.funcbody))
- skip_first_line = 1;
- start_threads(&opt);
}
if (show_in_pager && (cached || list.nr))
@@ -1106,7 +1102,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
hit = grep_objects(&opt, &pathspec, &list);
}
- if (num_threads)
+ if (num_threads > 1)
hit |= wait_all();
if (hit && show_in_pager)
run_pager(&opt, prefix);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index bbd66ca025..682042579b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1501,9 +1501,8 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)
if (nr_threads < 0)
die(_("invalid number of threads specified (%d)"),
nr_threads);
- if (!HAVE_THREADS) {
- if (nr_threads != 1)
- warning(_("no threads support, ignoring %s"), k);
+ if (!HAVE_THREADS && nr_threads != 1) {
+ warning(_("no threads support, ignoring %s"), k);
nr_threads = 1;
}
return 0;
@@ -1693,10 +1692,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
nr_threads = strtoul(arg+10, &end, 0);
if (!arg[10] || *end || nr_threads < 0)
usage(index_pack_usage);
- if (!HAVE_THREADS) {
- if (nr_threads != 1)
- warning(_("no threads support, "
- "ignoring %s"), arg);
+ if (!HAVE_THREADS && nr_threads != 1) {
+ warning(_("no threads support, ignoring %s"), arg);
nr_threads = 1;
}
} else if (starts_with(arg, "--pack_header=")) {
diff --git a/read-cache.c b/read-cache.c
index 4307b9a7bf..c510f598b1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2170,20 +2170,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
src_offset = sizeof(*hdr);
- if (HAVE_THREADS) {
- nr_threads = git_config_get_index_threads();
+ nr_threads = git_config_get_index_threads();
- /* TODO: does creating more threads than cores help? */
- if (!nr_threads) {
- nr_threads = istate->cache_nr / THREAD_COST;
- cpus = online_cpus();
- if (nr_threads > cpus)
- nr_threads = cpus;
- }
- } else {
- nr_threads = 1;
+ /* TODO: does creating more threads than cores help? */
+ if (!nr_threads) {
+ nr_threads = istate->cache_nr / THREAD_COST;
+ cpus = online_cpus();
+ if (nr_threads > cpus)
+ nr_threads = cpus;
}
+ if (!HAVE_THREADS)
+ nr_threads = 1;
+
if (nr_threads > 1) {
extension_offset = read_eoie_extension(mmap, mmap_size);
if (extension_offset) {
@@ -2216,12 +2215,11 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
istate->timestamp.nsec = ST_MTIME_NSEC(st);
/* if we created a thread, join it otherwise load the extensions on the primary thread */
- if (HAVE_THREADS && extension_offset) {
+ if (extension_offset) {
int ret = pthread_join(p.pthread, NULL);
if (ret)
die(_("unable to join load_index_extensions thread: %s"), strerror(ret));
- }
- if (!extension_offset) {
+ } else {
p.src_offset = src_offset;
load_index_extensions(&p);
}
@@ -2860,7 +2858,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
* strip_extensions parameter as we need it when loading the shared
* index.
*/
- if (HAVE_THREADS && ieot) {
+ if (ieot) {
struct strbuf sb = STRBUF_INIT;
write_ieot_extension(&sb, ieot);
diff --git a/run-command.c b/run-command.c
index 26154ba257..decf3239bd 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1246,6 +1246,15 @@ int finish_async(struct async *async)
#endif
}
+int async_with_fork(void)
+{
+#ifdef NO_PTHREADS
+ return 1;
+#else
+ return 0;
+#endif
+}
+
const char *find_hook(const char *name)
{
static struct strbuf path = STRBUF_INIT;
diff --git a/run-command.h b/run-command.h
index 3932420ec8..68f5369fc2 100644
--- a/run-command.h
+++ b/run-command.h
@@ -1,9 +1,7 @@
#ifndef RUN_COMMAND_H
#define RUN_COMMAND_H
-#ifndef NO_PTHREADS
-#include <pthread.h>
-#endif
+#include "thread-utils.h"
#include "argv-array.h"
@@ -143,6 +141,7 @@ struct async {
int start_async(struct async *async);
int finish_async(struct async *async);
int in_async(void);
+int async_with_fork(void);
void check_pipe(int err);
/**
diff --git a/send-pack.c b/send-pack.c
index e920ca57df..f692686770 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -203,9 +203,8 @@ static int receive_status(int in, struct ref *refs)
static int sideband_demux(int in, int out, void *data)
{
int *fd = data, ret;
-#ifdef NO_PTHREADS
- close(fd[1]);
-#endif
+ if (async_with_fork())
+ close(fd[1]);
ret = recv_sideband("send-pack", fd[0], out);
close(out);
return ret;
Nguyễn Thái Ngọc Duy (14):
thread-utils: macros to unconditionally compile pthreads API
run-command.h: include thread-utils.h instead of pthread.h
send-pack.c: move async's #ifdef NO_PTHREADS back to run-command.c
index-pack: remove #ifdef NO_PTHREADS
name-hash.c: remove #ifdef NO_PTHREADS
attr.c: remove #ifdef NO_PTHREADS
grep: remove #ifdef NO_PTHREADS
grep: clean up num_threads handling
preload-index.c: remove #ifdef NO_PTHREADS
pack-objects: remove #ifdef NO_PTHREADS
read-cache.c: remove #ifdef NO_PTHREADS
read-cache.c: reduce branching based on HAVE_THREADS
read-cache.c: initialize copy_len to shut up gcc 8
Clean up pthread_create() error handling
Makefile | 2 +-
attr.c | 14 --------
builtin/grep.c | 79 ++++++++++++++++--------------------------
builtin/index-pack.c | 63 ++++++++-------------------------
builtin/pack-objects.c | 26 ++------------
grep.c | 6 ----
grep.h | 6 ----
name-hash.c | 38 ++++++++------------
pack-objects.h | 6 ----
preload-index.c | 23 +++++-------
read-cache.c | 37 ++++++--------------
run-command.c | 11 +++++-
run-command.h | 5 ++-
send-pack.c | 5 ++-
thread-utils.c | 48 +++++++++++++++++++++++++
thread-utils.h | 48 +++++++++++++++++++++++--
16 files changed, 186 insertions(+), 231 deletions(-)
--
2.19.1.1005.gac84295441
