git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Eric Herman" <eric@freesa.org>,
	"Sverre Rabbelier" <srabbelier@gmail.com>,
	"Fernando Vezzosi" <buccia@repnz.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] grep: detect number of CPUs for thread spawning
Date: Sat,  5 Nov 2011 14:16:08 +0000	[thread overview]
Message-ID: <1320502568-14085-1-git-send-email-avarab@gmail.com> (raw)

From: Eric Herman <eric@freesa.org>

Change the number of threads that we spawn from a hardcoded value of
"8" to what online_cpus() returns.

Back in v1.7.0-rc1~19^2 when threaded grep was introduced the number
of threads was hardcoded at compile time to 8, but this value was only
used if online_cpus() returned greater than 1.

However just using 8 threads regardless of the actual number of CPUs
is inefficient if we have more than 8 CPUs, and potentially wasteful
if we have fewer than 8 CPUs.

The array holding the threads is now being allocated at runtime
instead of being allocated statically. We free the array further down
the line in the wait_all() function; this is OK since the allocation
and freeing is guarded by the global "use_threads" variable, which is
based on the return value of online_cpus().

Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
Signed-off-by: Eric Herman <eric@freesa.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/grep.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 3d7329d..307a253 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -27,8 +27,8 @@ static char const * const grep_usage[] = {
 static int use_threads = 1;
 
 #ifndef NO_PTHREADS
-#define THREADS 8
-static pthread_t threads[THREADS];
+static int nthreads;
+static pthread_t *threads;
 
 static void *load_sha1(const unsigned char *sha1, unsigned long *size,
 		       const char *name);
@@ -36,7 +36,7 @@ static void *load_file(const char *filename, size_t *sz);
 
 enum work_type {WORK_SHA1, WORK_FILE};
 
-/* We use one producer thread and THREADS consumer
+/* We use one producer thread and online_cpus() consumer
  * threads. The producer adds struct work_items to 'todo' and the
  * consumers pick work items from the same array.
  */
@@ -254,6 +254,8 @@ static void start_threads(struct grep_opt *opt)
 {
 	int i;
 
+	threads = xmalloc(nthreads * sizeof(pthread_t));
+
 	pthread_mutex_init(&grep_mutex, NULL);
 	pthread_mutex_init(&read_sha1_mutex, NULL);
 	pthread_cond_init(&cond_add, NULL);
@@ -264,7 +266,7 @@ static void start_threads(struct grep_opt *opt)
 		strbuf_init(&todo[i].out, 0);
 	}
 
-	for (i = 0; i < ARRAY_SIZE(threads); i++) {
+	for (i = 0; i < nthreads; i++) {
 		int err;
 		struct grep_opt *o = grep_opt_dup(opt);
 		o->output = strbuf_out;
@@ -295,7 +297,7 @@ static int wait_all(void)
 	pthread_cond_broadcast(&cond_add);
 	grep_unlock();
 
-	for (i = 0; i < ARRAY_SIZE(threads); i++) {
+	for (i = 0; i < nthreads; i++) {
 		void *h;
 		pthread_join(threads[i], &h);
 		hit |= (int) (intptr_t) h;
@@ -307,6 +309,8 @@ static int wait_all(void)
 	pthread_cond_destroy(&cond_write);
 	pthread_cond_destroy(&cond_result);
 
+	free(threads);
+
 	return hit;
 }
 #else /* !NO_PTHREADS */
@@ -1001,7 +1005,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		opt.regflags |= REG_ICASE;
 
 #ifndef NO_PTHREADS
-	if (online_cpus() == 1 || !grep_threads_ok(&opt))
+	nthreads = online_cpus();
+	if (nthreads == 1 || !grep_threads_ok(&opt))
 		use_threads = 0;
 
 	if (use_threads) {
-- 
1.7.7

             reply	other threads:[~2011-11-05 14:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-05 14:16 Ævar Arnfjörð Bjarmason [this message]
2011-11-06 14:50 ` [PATCH] grep: detect number of CPUs for thread spawning Pete Wyckoff
2011-11-06 18:00   ` Eric Herman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1320502568-14085-1-git-send-email-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=buccia@repnz.net \
    --cc=eric@freesa.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=srabbelier@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).