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
long "GREP_SOURCE_OID, pathbuf.buf, path, oid" argument list); a
subsequent cleanup patch will make that even more so.
Signed-off-by: Rasmus Villemoes <r...@rasmusvillemoes.dk>
---
builtin/grep.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 3ca4ac80d..aad422bb6 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,22 @@ 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() copies gs and thus assumes ownership of
+ * its fields, so do not call grep_source_clear()
+ */
+ 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 +352,29 @@ 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() copies gs and thus assumes ownership of
+ * its fields, so do not call grep_source_clear()
+ */
+ 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
