Subject: Re: [PATCH v2 06/10] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash

From: Phillip Wood <[email-masked]>
Date: Thu, 6 Nov 2025 11:00:34 +0000

Hi Ezekiel

On 29/10/2025 22:19, Ezekiel Newren via GitGitGadget wrote:
> From: Ezekiel Newren <[email-masked]>
> 
> The ha field is serving two different purposes, which makes the code
> harder to read. At first glance it looks like many places assume
> there could never be hash collisions between lines of the two input
> files. In reality, line_hash is used together with xdl_recmatch() to
> ensure correct comparisons of lines, even when collisions occur.
> 
> To make this clearer, the old ha field has been split:
>    * line_hash: The straightforward hash of a line, requiring no
>      additional context.
>    * minimal_perfect_hash: Not a new concept, but now a separate
>      field. It comes from the classifier's general-purpose hash table,
>      which assigns each line a unique and minimal hash across the two
>      files.

It would be nice to explain the differing types for the two fields in 
the commit message.
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index 85e56021da..16236bd045 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -96,9 +96,9 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t
>   	long hi;
>   	xdlclass_t *rcrec;
>   
> -	hi = (long) XDL_HASHLONG(rec->ha, cf->hbits);
> +	hi = (long) XDL_HASHLONG(rec->line_hash, cf->hbits);

"hi" is only used as an array index so it might be nicer to change it to 
size_t and avoid this cast instead.

Thanks

Phillip