xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash

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: a straightforward hash of a line, independent of any external context. Its type is uint64_t, as it comes from a fixed width hash function. * 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. A size_t is used here because it's meant to be used to index an array. This also avoids ` as usize` casts on the Rust side when using it to index a slice. Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ezekiel Newren committed Nov 18, 2025 at 22:34 UTC 6a26019c81faa07ba811541b4cf35be9e8ee1ead
5 files changed +21 -20
xdiff/xdiffi.c
+3 -3
@@ -22,9 +22,9 @@
22
23 #include "xinclude.h"
24
25 -static unsigned long get_hash(xdfile_t *xdf, long index)
25 +static size_t get_hash(xdfile_t *xdf, long index)
26 {
27 - return xdf->recs[xdf->rindex[index]].ha;
27 + return xdf->recs[xdf->rindex[index]].minimal_perfect_hash;
28 }
29
30 #define XDL_MAX_COST_MIN 256
@@ -385,7 +385,7 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
385
386 static int recs_match(xrecord_t *rec1, xrecord_t *rec2)
387 {
388 - return (rec1->ha == rec2->ha);
388 + return rec1->minimal_perfect_hash == rec2->minimal_perfect_hash;
389 }
390
391 /*
xdiff/xhistogram.c
+2 -2
@@ -90,7 +90,7 @@ struct region {
90
91 static int cmp_recs(xrecord_t *r1, xrecord_t *r2)
92 {
93 - return r1->ha == r2->ha;
93 + return r1->minimal_perfect_hash == r2->minimal_perfect_hash;
94
95 }
96
@@ -98,7 +98,7 @@ static int cmp_recs(xrecord_t *r1, xrecord_t *r2)
98 (cmp_recs(REC(i->env, s1, l1), REC(i->env, s2, l2)))
99
100 #define TABLE_HASH(index, side, line) \
101 - XDL_HASHLONG((REC(index->env, side, line))->ha, index->table_bits)
101 + XDL_HASHLONG((REC(index->env, side, line))->minimal_perfect_hash, index->table_bits)
102
103 static int scanA(struct histindex *index, int line1, int count1)
104 {
xdiff/xpatience.c
+5 -5
@@ -48,7 +48,7 @@
48 struct hashmap {
49 int nr, alloc;
50 struct entry {
51 - unsigned long hash;
51 + size_t minimal_perfect_hash;
52 /*
53 * 0 = unused entry, 1 = first line, 2 = second, etc.
54 * line2 is NON_UNIQUE if the line is not unique
@@ -101,10 +101,10 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
101 * So we multiply ha by 2 in the hope that the hashing was
102 * "unique enough".
103 */
104 - int index = (int)((record->ha << 1) % map->alloc);
104 + int index = (int)((record->minimal_perfect_hash << 1) % map->alloc);
105
106 while (map->entries[index].line1) {
107 - if (map->entries[index].hash != record->ha) {
107 + if (map->entries[index].minimal_perfect_hash != record->minimal_perfect_hash) {
108 if (++index >= map->alloc)
109 index = 0;
110 continue;
@@ -120,7 +120,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
120 if (pass == 2)
121 return;
122 map->entries[index].line1 = line;
123 - map->entries[index].hash = record->ha;
123 + map->entries[index].minimal_perfect_hash = record->minimal_perfect_hash;
124 map->entries[index].anchor = is_anchor(xpp, (const char *)map->env->xdf1.recs[line - 1].ptr);
125 if (!map->first)
126 map->first = map->entries + index;
@@ -248,7 +248,7 @@ static int match(struct hashmap *map, int line1, int line2)
248 {
249 xrecord_t *record1 = &map->env->xdf1.recs[line1 - 1];
250 xrecord_t *record2 = &map->env->xdf2.recs[line2 - 1];
251 - return record1->ha == record2->ha;
251 + return record1->minimal_perfect_hash == record2->minimal_perfect_hash;
252 }
253
254 static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,
xdiff/xprepare.c
+9 -9
@@ -93,12 +93,12 @@ static void xdl_free_classifier(xdlclassifier_t *cf) {
93
94
95 static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t *rec) {
96 - long hi;
96 + size_t hi;
97 xdlclass_t *rcrec;
98
99 - hi = (long) XDL_HASHLONG(rec->ha, cf->hbits);
99 + hi = XDL_HASHLONG(rec->line_hash, cf->hbits);
100 for (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)
101 - if (rcrec->rec.ha == rec->ha &&
101 + if (rcrec->rec.line_hash == rec->line_hash &&
102 xdl_recmatch((const char *)rcrec->rec.ptr, (long)rcrec->rec.size,
103 (const char *)rec->ptr, (long)rec->size, cf->flags))
104 break;
@@ -120,7 +120,7 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t
120
121 (pass == 1) ? rcrec->len1++ : rcrec->len2++;
122
123 - rec->ha = (unsigned long) rcrec->idx;
123 + rec->minimal_perfect_hash = (size_t)rcrec->idx;
124
125 return 0;
126 }
@@ -158,7 +158,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
158 crec = &xdf->recs[xdf->nrec++];
159 crec->ptr = prev;
160 crec->size = cur - prev;
161 - crec->ha = hav;
161 + crec->line_hash = hav;
162 if (xdl_classify_record(pass, cf, crec) < 0)
163 goto abort;
164 }
@@ -290,7 +290,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
290 if ((mlim = xdl_bogosqrt(xdf1->nrec)) > XDL_MAX_EQLIMIT)
291 mlim = XDL_MAX_EQLIMIT;
292 for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {
293 - rcrec = cf->rcrecs[recs->ha];
293 + rcrec = cf->rcrecs[recs->minimal_perfect_hash];
294 nm = rcrec ? rcrec->len2 : 0;
295 action1[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP;
296 }
@@ -298,7 +298,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
298 if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT)
299 mlim = XDL_MAX_EQLIMIT;
300 for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {
301 - rcrec = cf->rcrecs[recs->ha];
301 + rcrec = cf->rcrecs[recs->minimal_perfect_hash];
302 nm = rcrec ? rcrec->len1 : 0;
303 action2[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP;
304 }
@@ -350,7 +350,7 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {
350 recs2 = xdf2->recs;
351 for (i = 0, lim = XDL_MIN(xdf1->nrec, xdf2->nrec); i < lim;
352 i++, recs1++, recs2++)
353 - if (recs1->ha != recs2->ha)
353 + if (recs1->minimal_perfect_hash != recs2->minimal_perfect_hash)
354 break;
355
356 xdf1->dstart = xdf2->dstart = i;
@@ -358,7 +358,7 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {
358 recs1 = xdf1->recs + xdf1->nrec - 1;
359 recs2 = xdf2->recs + xdf2->nrec - 1;
360 for (lim -= i, i = 0; i < lim; i++, recs1--, recs2--)
361 - if (recs1->ha != recs2->ha)
361 + if (recs1->minimal_perfect_hash != recs2->minimal_perfect_hash)
362 break;
363
364 xdf1->dend = xdf1->nrec - i - 1;
xdiff/xtypes.h
+2 -1
@@ -41,7 +41,8 @@ typedef struct s_chastore {
41 typedef struct s_xrecord {
42 uint8_t const *ptr;
43 size_t size;
44 - unsigned long ha;
44 + uint64_t line_hash;
45 + size_t minimal_perfect_hash;
46 } xrecord_t;
47
48 typedef struct s_xdfile {