Re: [PATCH 01/12] grep: add ability to disable threading with --threads=0 or grep.threads=0

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Apr 11, 2017 at 12:06 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Apr 08, 2017 at 01:24:55PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> Add the ability to entirely disable threading by having grep.threads=0
>> in the config or --threads=0 on the command-line.
>
> In pack-objects and index-pack, --threads=0 means "auto-detect". It
> seems like we should try to keep this consistent.
>
> Wouldn't --threads=1 be a better way to disable threading? Pack-objects
> is smart enough to just use the non-threaded code path entirely in that
> case (rather than wasting time spawning a single worker thread). Grep
> should probably do the same.

I'm struggling to find a use-case where threading makes sense at all.
The example in the initial introduction in 5b594f457a is always slower
with >0 for me, and since then in 0579f91dd7 it got disabled entirely
for non-worktree cases.

But assuming it works for someone out there, then 0 threads is clearly
not the same as 1. On linux.git with pcre2 grepping for [q]werty for
example[1]

        Rate P_12  P_8  P_4  P_1  P_2  P_0
P_12 0.861/s   --  -7% -22% -27% -30% -43%
P_8  0.924/s   7%   -- -16% -22% -25% -39%
P_4   1.10/s  28%  19%   --  -7% -10% -27%
P_1   1.19/s  38%  29%   8%   --  -3% -21%
P_2   1.23/s  43%  33%  12%   4%   -- -18%
P_0   1.51/s  75%  63%  37%  27%  22%   --

And for [a]var on git.git:

       Rate P_12  P_8  P_4  P_2  P_1  P_0
P_12 15.6/s   --  -5% -15% -17% -21% -42%
P_8  16.4/s   5%   -- -11% -12% -17% -39%
P_4  18.4/s  18%  13%   --  -1%  -7% -32%
P_2  18.7/s  20%  14%   1%   --  -6% -31%
P_1  19.8/s  27%  21%   8%   6%   -- -27%
P_0  27.0/s  73%  65%  46%  44%  36%   --

Tthere's a >20% performance difference between 0 and 1 threads. The
more threads I add the slower it gets, but if there's some case where
we have the inverse of that and you have e.g. 2 cores, then presumably
you really want 1 thread, and not 0, or 2.

>> +static int thread_callback(const struct option *opt,
>> +                        const char *arg, int unset)
>> +{
>> +     int *threads = (int*)opt->value;
>> +     char *end;
>> +
>> +     if (unset) {
>> +             *threads = GREP_NUM_THREADS_DEFAULT;
>> +             return 0;
>> +     }
>
> This means that "--no-threads" will use 8 threads. Which _kind of_ makes
> sense in that it cancels any previous "--threads", but I wonder if it
> should be the same as "--threads=1".

Dear lazyweb, how do you distinguish --no-foo from no --foo=X being
provided in this API?

> This isn't really a change in behavior from the existing code, though
> (OPT_INTEGER will set it to 0 in that case, too, and we'd later pick up
> the default value).

1. =~/g/git/ perl -MBenchmark=cmpthese -wE 'cmpthese(10, { map { my $t
= $_; +("P_$t" => sub { system "$ENV{PF}git -c grep.patternType=pcre2
grep --threads=$t [q]werty >/dev/null" }) } 0,1,2,4,8,12 })'