[PATCH 1/3] grep: move grep_source_init outside critical section
- From
Rasmus Villemoes <rv@rasmusvillemoes.dk>
- Date
- Feb 15, 2018, 21:56 UTC
- Message-ID
- <20180215215615.21208-2-rv@rasmusvillemoes.dk>
- In-Reply-To
- <20180215215615.21208-1-rv@rasmusvillemoes.dk>
grep_source_init typically does three strdup()s, and in the threaded case, the call from add_work() happens while holding grep_mutex.
We can thus reduce the time we hold grep_mutex by moving the grep_source_init() call out of add_work(), and simply have add_work() copy the initialized structure to the available slot in the todo array.
This also simplifies the prototype of add_work(), since it no longer needs to duplicate all the parameters of grep_source_init(). In the callers of add_work(), we get to reduce the amount of code duplicated in the threaded and non-threaded cases slightly (avoiding repeating the "GREP_SOURCE_OID, pathbuf.buf, path, oid" argument list); a subsequent cleanup patch will make that even more so.
Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk> --- builtin/grep.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c index 3ca4ac80d..4a4f15172 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -92,8 +92,7 @@ static pthread_cond_t cond_result; static int skip_first_line; -static void add_work(struct grep_opt *opt, enum grep_source_type type, - const char *name, const char *path, const void *id) +static void add_work(struct grep_opt *opt, const struct grep_source *gs) { grep_lock(); @@ -101,7 +100,7 @@ static void add_work(struct grep_opt *opt, enum grep_source_type type, pthread_cond_wait(&cond_write, &grep_mutex); } - grep_source_init(&todo[todo_end].source, type, name, path, id); + todo[todo_end].source = *gs; if (opt->binary != GREP_BINARY_TEXT) grep_source_load_driver(&todo[todo_end].source); todo[todo_end].done = 0; @@ -317,6 +316,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid, const char *path) { struct strbuf pathbuf = STRBUF_INIT; + struct grep_source gs; if (opt->relative && opt->prefix_length) { quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf); @@ -325,18 +325,18 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid, strbuf_addstr(&pathbuf, filename); } + grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid); + #ifndef NO_PTHREADS if (num_threads) { - add_work(opt, GREP_SOURCE_OID, pathbuf.buf, path, oid); + add_work(opt, &gs); strbuf_release(&pathbuf); return 0; } else #endif { - struct grep_source gs; int hit; - grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid); strbuf_release(&pathbuf); hit = grep_source(opt, &gs); @@ -348,24 +348,25 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid, static int grep_file(struct grep_opt *opt, const char *filename) { struct strbuf buf = STRBUF_INIT; + struct grep_source gs; if (opt->relative && opt->prefix_length) quote_path_relative(filename, opt->prefix, &buf); else strbuf_addstr(&buf, filename); + grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename); + #ifndef NO_PTHREADS if (num_threads) { - add_work(opt, GREP_SOURCE_FILE, buf.buf, filename, filename); + add_work(opt, &gs); strbuf_release(&buf); return 0; } else #endif { - struct grep_source gs; int hit; - grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename); strbuf_release(&buf); hit = grep_source(opt, &gs);
-- 2.15.1