On 2020-08-21 at 17:58:00, Jeff King wrote:
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index f865666db9..9721bf1ffe 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1798,9 +1798,22 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
>
> if (HAVE_THREADS && !nr_threads) {
> nr_threads = online_cpus();
> - /* An experiment showed that more threads does not mean faster */
> - if (nr_threads > 3)
> - nr_threads = 3;
> + /*
> + * Experiments show that going above 20 threads doesn't help,
> + * no matter how many cores you have. Below that, we tend to
> + * max at half the number of online_cpus(), presumably because
> + * half of those are hyperthreads rather than full cores. We'll
> + * never reduce the level below "3", though, to match a
> + * historical value that nobody complained about.
> + */
> + if (nr_threads < 4)
> + ; /* too few cores to consider capping */
> + else if (nr_threads < 6)
> + nr_threads = 3; /* historic cap */
> + else if (nr_threads < 40)
> + nr_threads /= 2;
I was going to ask if we could make the halving conditional on x86_64,
but it turns out POWER and UltraSPARC also have SMT, so that doesn't
make sense. I expect that most users who have more than 6 "cores" are
going to be on one of those systems or possibly ARM64, and since the
performance penalty of using half as many cores isn't that significant,
I'm not sure it's worth worrying about further. This will be an
improvement regardless.
Which is just a long way of saying, this patch seems fine to me.
--
brian m. carlson: Houston, Texas, US
Attachment:
signature.asc
Description: PGP signature
