Re: [PATCH 1/2] Introduce grep threads param

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 15, 2015 at 3:06 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Victor Leschuk <vleschuk@xxxxxxxxx> writes:
>> Subject: Re: [PATCH 1/2] Introduce grep threads param
>
> I'll retitle this to something like
>
>     grep: add --threads=<num> option and grep.threads configuration
>
> while queuing (which I did for v7 earlier).
>
> I think [2/2] and also moving the code to disable threading when
> show-in-pager mode should be separate "preparatory clean-up" patches
> before this main patch.  I'll push out what I think this topic
> should be on 'pu' later today (with fixups suggested above squashed
> in); please check them and see what you think.

I read over what was pushed to 'pu' and noticed a couple problems.

First, the 'online_cpus() == 1' check, which was removed in patch 1/3,
accidentally creeps back in with patch 3/3.

>> +grep.threads::
>> +     Number of grep worker threads, use it to tune up performance on
>> +     your machines. Leave it unset (or set to 0) for default behavior,
>> +     which is using 8 threads for all systems.
>> +     Default behavior may change in future versions
>> +     to better suit hardware and circumstances.
>
> The last sentence is too noisy.  Perhaps drop it and phrase it like
> this instead?
>
>     grep.threads::
>             Number of grep worker threads to use.  If unset (or set to 0),
>             to 0), 8 threads are used by default (for now).

Second, the stray "to 0)," on the second line needs to be dropped.

Other than that, the series looks reasonable.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]