From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 2 Mar 2018 20:18:24 +0000 (-0800)
Subject: Merge branch 'rv/grep-cleanup' into next
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/4aafca15f9c67a336a7d648dd7b2a29286b1ec57?ds=inline;hp=-c

Merge branch 'rv/grep-cleanup' into next

Threaded "git grep" has been optimized to avoid allocation in code
section that is covered under a mutex.

* rv/grep-cleanup:
grep: simplify grep_oid and grep_file
grep: move grep_source_init outside critical section
---

4aafca15f9c67a336a7d648dd7b2a29286b1ec57
diff --combined builtin/grep.c
index 9a7ed449c2,9a8e4fadad..789a89133a
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@@ -92,8 -92,7 +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 +100,7 @@@
  		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 +316,7 @@@ static int grep_oid(struct grep_opt *op
  		     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,19 -325,22 +325,22 @@@
  		strbuf_addstr(&pathbuf, filename);
  	}
  
+ 	grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);
+ 	strbuf_release(&pathbuf);
+ 
  #ifndef NO_PTHREADS
  	if (num_threads) {
- 		add_work(opt, GREP_SOURCE_OID, pathbuf.buf, path, oid);
- 		strbuf_release(&pathbuf);
+ 		/*
+ 		 * add_work() copies gs and thus assumes ownership of
+ 		 * its fields, so do not call grep_source_clear()
+ 		 */
+ 		add_work(opt, &gs);
  		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);
  
  		grep_source_clear(&gs);
@@@ -348,25 -351,29 +351,29 @@@
  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);
+ 	strbuf_release(&buf);
+ 
  #ifndef NO_PTHREADS
  	if (num_threads) {
- 		add_work(opt, GREP_SOURCE_FILE, buf.buf, filename, filename);
- 		strbuf_release(&buf);
+ 		/*
+ 		 * add_work() copies gs and thus assumes ownership of
+ 		 * its fields, so do not call grep_source_clear()
+ 		 */
+ 		add_work(opt, &gs);
  		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);
  
  		grep_source_clear(&gs);
@@@ -627,7 -634,7 +634,7 @@@ static int grep_object(struct grep_opt 
  		free(data);
  		return hit;
  	}
 -	die(_("unable to grep from object of type %s"), typename(obj->type));
 +	die(_("unable to grep from object of type %s"), type_name(obj->type));
  }
  
  static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
@@@ -832,9 -839,8 +839,9 @@@ int cmd_grep(int argc, const char **arg
  		OPT_BOOL('L', "files-without-match",
  			&opt.unmatch_name_only,
  			N_("show only the names of files without match")),
 -		OPT_BOOL('z', "null", &opt.null_following_name,
 -			N_("print NUL after filenames")),
 +		OPT_BOOL_F('z', "null", &opt.null_following_name,
 +			   N_("print NUL after filenames"),
 +			   PARSE_OPT_NOCOMPLETE),
  		OPT_BOOL('c', "count", &opt.count,
  			N_("show the number of matches instead of matching lines")),
  		OPT__COLOR(&opt.color, N_("highlight matches")),
@@@ -885,11 -891,9 +892,11 @@@
  		OPT_GROUP(""),
  		{ OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager,
  			N_("pager"), N_("show matching files in the pager"),
 -			PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager },
 -		OPT_BOOL(0, "ext-grep", &external_grep_allowed__ignored,
 -			 N_("allow calling of grep(1) (ignored by this build)")),
 +			PARSE_OPT_OPTARG | PARSE_OPT_NOCOMPLETE,
 +			NULL, (intptr_t)default_pager },
 +		OPT_BOOL_F(0, "ext-grep", &external_grep_allowed__ignored,
 +			   N_("allow calling of grep(1) (ignored by this build)"),
 +			   PARSE_OPT_NOCOMPLETE),
  		OPT_END()
  	};