Re: [PATCH 03/15] cache: add an algo member to struct object_id

From: Derrick Stolee <stolee@gmail.com>
Date: Apr 13, 2021, 12:12 UTC
Message-ID: <38f48634-077b-6351-5285-f8ecc6f552d0@gmail.com>
In-Reply-To: <20210410152140.3525040-4-sandals@crustytoothpaste.net>

On 4/10/2021 11:21 AM, brian m. carlson wrote:

Show 20 quoted lines

> Now that we're working with multiple hash algorithms in the same repo,
> it's best if we label each object ID with its algorithm so we can
> determine how to format a given object ID. Add a member called algo to
> struct object_id.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  hash.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hash.h b/hash.h
> index 3fb0c3d400..dafdcb3335 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -181,6 +181,7 @@ static inline int hash_algo_by_ptr(const struct git_hash_algo *p)
>  
>  struct object_id {
>  	unsigned char hash[GIT_MAX_RAWSZ];
> +	int algo;
>  };

What are the performance implications of adding this single bit (that actually costs us 4 to 8 bytes, based on alignment)? Later in the series you add longer hash comparisons, too. These seem like they will affect performance for existing SHA-1 repos, and it would be nice to know how much we are paying for this support.

I assume that we already checked what happened when GIT_MAX_RAWSZ increased, but that seemed worth the cost so we could have SHA-256 at all. I find the justification for this interoperability mode to be less significant, and potentially adding too much of a tax onto both SHA-1 repos that will never upgrade, and SHA-256 repos that upgrade all at once (or start as SHA-256).

Of course, if there truly is no serious performance implication to this change, then I support following the transition plan and allowing us to be flexible on timelines for interoperability. It just seems like we need to investigate what this will cost.

Thanks, -Stolee