Re: [RFC PATCH 6/6] commit-graph: reuse existing Bloom filters where possible

From: Jonathan Tan <jonathantanmy@google.com>
Date: Aug 11, 2023, 22:06 UTC
Message-ID: <20230811220637.3330277-1-jonathantanmy@google.com>
In-Reply-To: <93f830ca61d17bb20f63c6a4254fe95816ae1cbe.1691426160.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 26 quoted lines

> @@ -292,9 +362,23 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
>  						     filter, graph_pos);
>  	}
>  
> -	if ((filter->data && filter->len) &&
> -	    (!settings || settings->hash_version == filter->version))
> -		return filter;
> +	if (filter->data && filter->len) {
> +		struct bloom_filter *upgrade;
> +		if (!settings || settings->hash_version == filter->version)
> +			return filter;
> +
> +		/* version mismatch, see if we can upgrade */
> +		if (compute_if_not_present &&
> +		    git_env_bool("GIT_TEST_UPGRADE_BLOOM_FILTERS", 1)) {
> +			upgrade = upgrade_filter(r, c, filter,
> +						 settings->hash_version);
> +			if (upgrade) {
> +				if (computed)
> +					*computed |= BLOOM_UPGRADED;
> +				return upgrade;
> +			}
> +		}
> +	}
>  	if (!compute_if_not_present)
>  		return NULL;

There's a part in this function that's untouched by the diff that computes and stores a new Bloom filter if it was not possible to upgrade the filter (look at the part where filter->data is calloc-ed). That part unconditionally overrides the existing filter->data, which may at that point contain genuine data (a Bloom filter that does not match the version we want). Right now this is fine because all overrides that we do involve overriding references to existing buffers, so we do not need to free anything.