Re: [PATCH v4 2/8] grep: skip pthreads overhead when using one thread

From: Stefan Beller <sbeller@google.com>
Date: Jun 1, 2017, 21:20 UTC
Message-ID: <CAGZ79kb6O7hraY4caY8tdFn1d0Fi+LRr9cHk2UuXf79LbnPdhw@mail.gmail.com>
In-Reply-To: <20170601182056.31142-3-avarab@gmail.com>

On Thu, Jun 1, 2017 at 11:20 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> +       if (num_threads == 1)
> +               num_threads = 0;

I would think that it is easier to maintain the code when keep the 1 hard coded, and apply the following diff instead. If we encounter a 0 later on, it is not clear what the original user input was. (Did the user ask for 0 as a proxy for GREP_NUM_THREADS_DEFAULT ? do they care about the number of threads?) It is less complexity in the decision logic here.

--8<-- (white space broken)

diff --git a/builtin/grep.c b/builtin/grep.c
index c6c26e9b9e..6ad9b3da20 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1231,7 +1231,7 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
 #endif

 #ifndef NO_PTHREADS
-       if (num_threads) {
+       if (num_threads > 1) {
                if (!(opt.name_only || opt.unmatch_name_only || opt.count)
                    && (opt.pre_context || opt.post_context ||
                        opt.file_break || opt.funcbody))
@@ -1295,7 +1295,7 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
                hit = grep_objects(&opt, &pathspec, &list);
        }

-       if (num_threads)
+       if (num_threads > 1)
                hit |= wait_all();
        if (hit && show_in_pager)
                run_pager(&opt, prefix);
--8<--