Re: [PATCH 01/12] grep: add ability to disable threading with --threads=0 or grep.threads=0
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Apr 11, 2017, 20:20 UTC
- Message-ID
- <CACBZZX7vEQ5jUzX3GsD6JXe50TnRUtGmSVi7zBxwOmAQGABQ4Q@mail.gmail.com>
- In-Reply-To
- <20170411100656.5bptxdaptc4zznan@sigill.intra.peff.net>
On Tue, Apr 11, 2017 at 12:06 PM, Jeff King <peff@peff.net> wrote:
Show 12 quoted lines
> 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.
Show 14 quoted lines
>> +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 })'