git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Ben Peart <peartben@gmail.com>
Subject: Re: [PATCH 06/10] grep: remove #ifdef NO_PTHREADS
Date: Sat, 27 Oct 2018 03:44:44 -0400	[thread overview]
Message-ID: <20181027074443.GE26685@sigill.intra.peff.net> (raw)
In-Reply-To: <20181027071003.1347-7-pclouds@gmail.com>

On Sat, Oct 27, 2018 at 09:09:59AM +0200, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/builtin/grep.c b/builtin/grep.c
> index d8508ddf79..29221e1003 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -34,7 +34,6 @@ static int recurse_submodules;
>  #define GREP_NUM_THREADS_DEFAULT 8
>  static int num_threads;
>  
> -#ifndef NO_PTHREADS
>  static pthread_t *threads;
>  
>  /* We use one producer thread and THREADS consumer
> @@ -265,13 +264,6 @@ static int wait_all(void)
>  
>  	return hit;
>  }
> -#else /* !NO_PTHREADS */
> -
> -static int wait_all(void)
> -{
> -	return 0;
> -}
> -#endif

Cases like this are kind of weird. I'd expect to see wait_all() return
immediately when !HAVE_THREADS. But we don't need to, because later we
do this:

> -	if (num_threads)
> +	if (HAVE_THREADS && num_threads)
>  		hit |= wait_all();

Which I think works, but it feels like we're introducing some subtle
dependencies about which functions get called in which cases. I'd hoped
in general that the non-threaded code paths could mostly just collapse
down into the same as the "threads == 1" cases, and we wouldn't have to
ask "are threads even supported" in a lot of places.

I dunno. My comment isn't really an objection exactly, but mostly just
an indication that I'm still thinking through what the best approach is,
and what end state we want to end up in.

-Peff

  reply	other threads:[~2018-10-27  7:44 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-27  7:09 [PATCH 00/10] Reduce #ifdef NO_PTHREADS Nguyễn Thái Ngọc Duy
2018-10-27  7:09 ` [PATCH 01/10] thread-utils: macros to unconditionally compile pthreads API Nguyễn Thái Ngọc Duy
2018-10-27  7:31   ` Jeff King
2018-10-27  7:40     ` Duy Nguyen
2018-10-27  8:15       ` Jeff King
2018-10-27 14:43         ` Duy Nguyen
2018-10-27  7:09 ` [PATCH 02/10] index-pack: remove #ifdef NO_PTHREADS Nguyễn Thái Ngọc Duy
2018-10-27  7:34   ` Jeff King
2018-10-27  7:09 ` [PATCH 03/10] name-hash.c: " Nguyễn Thái Ngọc Duy
2018-10-27  7:09 ` [PATCH 04/10] attr.c: " Nguyễn Thái Ngọc Duy
2018-10-27  7:09 ` [PATCH 05/10] send-pack.c: " Nguyễn Thái Ngọc Duy
2018-10-27  7:39   ` Jeff King
2018-10-27  7:09 ` [PATCH 06/10] grep: " Nguyễn Thái Ngọc Duy
2018-10-27  7:44   ` Jeff King [this message]
2018-10-29  2:16     ` Junio C Hamano
2018-10-29 14:25       ` Jeff King
2018-10-29 16:01         ` Duy Nguyen
2018-10-29 16:20           ` Jeff King
2018-10-30  1:27             ` Junio C Hamano
2018-10-27  7:10 ` [PATCH 07/10] preload-index.c: " Nguyễn Thái Ngọc Duy
2018-10-29 16:52   ` Ben Peart
2018-10-27  7:10 ` [PATCH 08/10] pack-objects: " Nguyễn Thái Ngọc Duy
2018-10-27  7:10 ` [PATCH 09/10] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-10-29 17:05   ` Ben Peart
2018-10-29 17:21     ` Duy Nguyen
2018-10-29 17:58       ` Ben Peart
2018-10-30  1:44       ` Junio C Hamano
2018-10-27  7:10 ` [PATCH 10/10] Clean up pthread_create() error handling Nguyễn Thái Ngọc Duy
2018-10-27  7:24 ` [PATCH 00/10] Reduce #ifdef NO_PTHREADS Jeff King
2018-10-27  8:13 ` Jeff King
2018-10-27 17:07   ` Duy Nguyen
2018-10-27 17:29 ` [PATCH v2 " Nguyễn Thái Ngọc Duy
2018-10-27 17:29   ` [PATCH v2 01/10] thread-utils: macros to unconditionally compile pthreads API Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 02/10] index-pack: remove #ifdef NO_PTHREADS Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 03/10] name-hash.c: " Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 04/10] attr.c: " Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 05/10] grep: " Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 06/10] preload-index.c: " Nguyễn Thái Ngọc Duy
2018-10-29 17:21     ` Ben Peart
2018-10-29 17:26       ` Duy Nguyen
2018-10-29 18:05         ` Ben Peart
2018-10-29 20:11           ` Jeff King
2018-10-27 17:30   ` [PATCH v2 07/10] pack-objects: " Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 08/10] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-10-29 14:30     ` Jeff King
2018-10-29 17:07       ` Ben Peart
2018-10-29 17:23       ` Ben Peart
2018-10-27 17:30   ` [PATCH v2 09/10] Clean up pthread_create() error handling Nguyễn Thái Ngọc Duy
2018-10-27 17:30   ` [PATCH v2 10/10] read-cache.c: initialize copy_len to shut up gcc 8 Nguyễn Thái Ngọc Duy
2018-10-29 14:31     ` Jeff King
2018-11-03  8:48   ` [PATCH v3 00/14] Reduce #ifdef NO_PTHREADS Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 01/14] thread-utils: macros to unconditionally compile pthreads API Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 02/14] run-command.h: include thread-utils.h instead of pthread.h Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 03/14] send-pack.c: move async's #ifdef NO_PTHREADS back to run-command.c Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 04/14] index-pack: remove #ifdef NO_PTHREADS Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 05/14] name-hash.c: " Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 06/14] attr.c: " Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 07/14] grep: " Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 08/14] grep: clean up num_threads handling Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 09/14] preload-index.c: remove #ifdef NO_PTHREADS Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 10/14] pack-objects: " Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 11/14] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 12/14] read-cache.c: reduce branching based on HAVE_THREADS Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 13/14] read-cache.c: initialize copy_len to shut up gcc 8 Nguyễn Thái Ngọc Duy
2018-11-03  8:48     ` [PATCH v3 14/14] Clean up pthread_create() error handling Nguyễn Thái Ngọc Duy
2018-11-06  4:51     ` [PATCH v3 00/14] Reduce #ifdef NO_PTHREADS Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20181027074443.GE26685@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peartben@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).