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>

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index cb8e412c7b9db6ae589393813735c7e90b3b2d96..8d96074414f2e940da006b127d25ba96437bae3d 100644 (file)
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -22,9 +22,9 @@
 
 #include "xinclude.h"
 
-static unsigned long get_hash(xdfile_t *xdf, long index)
+static size_t get_hash(xdfile_t *xdf, long index)
 {
-       return xdf->recs[xdf->rindex[index]].ha;
+       return xdf->recs[xdf->rindex[index]].minimal_perfect_hash;
 }
 
 #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,
 
 static int recs_match(xrecord_t *rec1, xrecord_t *rec2)
 {
-       return (rec1->ha == rec2->ha);
+       return rec1->minimal_perfect_hash == rec2->minimal_perfect_hash;
 }
 
 /*

diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
index 6dc450b1fe1dfc2c85d7acf98c948581b48e6f28..5ae1282c27568cb1c9622dbf28bf5273868cebc5 100644 (file)
--- a/xdiff/xhistogram.c
+++ b/xdiff/xhistogram.c
@@ -90,7 +90,7 @@ struct region {
 
 static int cmp_recs(xrecord_t *r1, xrecord_t *r2)
 {
-       return r1->ha == r2->ha;
+       return r1->minimal_perfect_hash == r2->minimal_perfect_hash;
 
 }
 
@@ -98,7 +98,7 @@ static int cmp_recs(xrecord_t *r1, xrecord_t *r2)
        (cmp_recs(REC(i->env, s1, l1), REC(i->env, s2, l2)))
 
 #define TABLE_HASH(index, side, line) \
-       XDL_HASHLONG((REC(index->env, side, line))->ha, index->table_bits)
+       XDL_HASHLONG((REC(index->env, side, line))->minimal_perfect_hash, index->table_bits)
 
 static int scanA(struct histindex *index, int line1, int count1)
 {

diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c
index bb61354f22a17761d7bd54172309cc98ef0ecb0f..cc53266f3b8302f9b778cb4719cf023e3e3f37b6 100644 (file)
--- a/xdiff/xpatience.c
+++ b/xdiff/xpatience.c
@@ -48,7 +48,7 @@
 struct hashmap {
        int nr, alloc;
        struct entry {
-               unsigned long hash;
+               size_t minimal_perfect_hash;
                /*
                 * 0 = unused entry, 1 = first line, 2 = second, etc.
                 * 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,
         * So we multiply ha by 2 in the hope that the hashing was
         * "unique enough".
         */
-       int index = (int)((record->ha << 1) % map->alloc);
+       int index = (int)((record->minimal_perfect_hash << 1) % map->alloc);
 
        while (map->entries[index].line1) {
-               if (map->entries[index].hash != record->ha) {
+               if (map->entries[index].minimal_perfect_hash != record->minimal_perfect_hash) {
                        if (++index >= map->alloc)
                                index = 0;
                        continue;
@@ -120,7 +120,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
        if (pass == 2)
                return;
        map->entries[index].line1 = line;
-       map->entries[index].hash = record->ha;
+       map->entries[index].minimal_perfect_hash = record->minimal_perfect_hash;
        map->entries[index].anchor = is_anchor(xpp, (const char *)map->env->xdf1.recs[line - 1].ptr);
        if (!map->first)
                map->first = map->entries + index;
@@ -248,7 +248,7 @@ static int match(struct hashmap *map, int line1, int line2)
 {
        xrecord_t *record1 = &map->env->xdf1.recs[line1 - 1];
        xrecord_t *record2 = &map->env->xdf2.recs[line2 - 1];
-       return record1->ha == record2->ha;
+       return record1->minimal_perfect_hash == record2->minimal_perfect_hash;
 }
 
 static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,

diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 85e56021daf9e28613f91a6c4aa6896896cb66f5..bea0992b5e4a33486719caaa4b1305ae52a1fdbf 100644 (file)
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -93,12 +93,12 @@ static void xdl_free_classifier(xdlclassifier_t *cf) {
 
 
 static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t *rec) {
-       long hi;
+       size_t hi;
        xdlclass_t *rcrec;
 
-       hi = (long) XDL_HASHLONG(rec->ha, cf->hbits);
+       hi = XDL_HASHLONG(rec->line_hash, cf->hbits);
        for (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)
-               if (rcrec->rec.ha == rec->ha &&
+               if (rcrec->rec.line_hash == rec->line_hash &&
                                xdl_recmatch((const char *)rcrec->rec.ptr, (long)rcrec->rec.size,
                                        (const char *)rec->ptr, (long)rec->size, cf->flags))
                        break;
@@ -120,7 +120,7 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t
 
        (pass == 1) ? rcrec->len1++ : rcrec->len2++;
 
-       rec->ha = (unsigned long) rcrec->idx;
+       rec->minimal_perfect_hash = (size_t)rcrec->idx;
 
        return 0;
 }
@@ -158,7 +158,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
                        crec = &xdf->recs[xdf->nrec++];
                        crec->ptr = prev;
                        crec->size = cur - prev;
-                       crec->ha = hav;
+                       crec->line_hash = hav;
                        if (xdl_classify_record(pass, cf, crec) < 0)
                                goto abort;
                }
@@ -290,7 +290,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
        if ((mlim = xdl_bogosqrt(xdf1->nrec)) > XDL_MAX_EQLIMIT)
                mlim = XDL_MAX_EQLIMIT;
        for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {
-               rcrec = cf->rcrecs[recs->ha];
+               rcrec = cf->rcrecs[recs->minimal_perfect_hash];
                nm = rcrec ? rcrec->len2 : 0;
                action1[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP;
        }
@@ -298,7 +298,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
        if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT)
                mlim = XDL_MAX_EQLIMIT;
        for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {
-               rcrec = cf->rcrecs[recs->ha];
+               rcrec = cf->rcrecs[recs->minimal_perfect_hash];
                nm = rcrec ? rcrec->len1 : 0;
                action2[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP;
        }
@@ -350,7 +350,7 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {
        recs2 = xdf2->recs;
        for (i = 0, lim = XDL_MIN(xdf1->nrec, xdf2->nrec); i < lim;
             i++, recs1++, recs2++)
-               if (recs1->ha != recs2->ha)
+               if (recs1->minimal_perfect_hash != recs2->minimal_perfect_hash)
                        break;
 
        xdf1->dstart = xdf2->dstart = i;
@@ -358,7 +358,7 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {
        recs1 = xdf1->recs + xdf1->nrec - 1;
        recs2 = xdf2->recs + xdf2->nrec - 1;
        for (lim -= i, i = 0; i < lim; i++, recs1--, recs2--)
-               if (recs1->ha != recs2->ha)
+               if (recs1->minimal_perfect_hash != recs2->minimal_perfect_hash)
                        break;
 
        xdf1->dend = xdf1->nrec - i - 1;

diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h
index 354349b523fbac77704f1c5506fadb2df842dac8..d4e9cd2e7636166edca36313b55a9d0db0279a6a 100644 (file)
--- a/xdiff/xtypes.h
+++ b/xdiff/xtypes.h
@@ -41,7 +41,8 @@ typedef struct s_chastore {
 typedef struct s_xrecord {
        uint8_t const *ptr;
        size_t size;
-       unsigned long ha;
+       uint64_t line_hash;
+       size_t minimal_perfect_hash;
 } xrecord_t;
 
 typedef struct s_xdfile {