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

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 19, 2016, 11:48 UTC
Message-ID: <20161219114807.GC24125@ash>
In-Reply-To: <20161217145547.11748-11-chriscool@tuxfamily.org>

On Sat, Dec 17, 2016 at 03:55:36PM +0100, Christian Couder wrote:

Show 16 quoted lines

> +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).

Perhaps it's 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.

Show 13 quoted lines

> +	default:
> +		; /* do nothing: just use the configured value */
> +	}
> +
> +	/* Count not shared entries */
> +	for (i = 0; i < istate->cache_nr; i++) {
> +		struct cache_entry *ce = istate->cache[i];
> +		if (!ce->index)
> +			not_shared++;
> +	}
> +
> +	return istate->cache_nr * max_split < not_shared * 100;
> +}