Re: [PATCH v2 10/21] read-cache: regenerate shared index if necessary

[Christian Couder <christian.couder@gmail.com>]

On Mon, Dec 19, 2016 at 12:48 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sat, Dec 17, 2016 at 03:55:36PM +0100, Christian Couder wrote:
>> +static const int default_max_percent_split_change = 20;
>> +
>> +static int too_many_not_shared_entries(struct index_state *istate)
>> +{
>> +     int i, not_shared = 0;
>> +     int max_split = git_config_get_max_percent_split_change();
>> +
>> +     switch (max_split) {
>> +     case -1:
>> +             /* not or badly configured: use the default value */
>> +             max_split = default_max_percent_split_change;
>> +             break;
>> +     case 0:
>> +             return 1; /* 0% means always write a new shared index */
>> +     case 100:
>> +             return 0; /* 100% means never write a new shared index */
>
> I wonder if we really need to special case these here. If I read it
> correctly, the expression at the end of this function will return 1
> when max_split is 0, and 0 when max_split is 100 (not counting the
> case when cache_nr is zero).

It's better for performance if we can avoid computing the number of
unshared entries, which we can in case of 0 or 100.

> Perhaps it's good for documentation purpose.

Yeah, I think it's also good for documentation purpose.

> Though I find it hard to
> see a use case for max_split == 0. Always creating a new shared index
> sounds crazy.

Yeah, but perhaps to test shared index writing performance people
might want to use it. And I don't see any good way or any good reason
to disallow it.