On Mon, Dec 19, 2016 at 12:48 PM, Duy Nguyen <pclouds@xxxxxxxxx> 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.