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: 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.

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 edd05466df5ca3d999fcdf5bb155e5e85c4440d2..436c34697deff10465cc29ad268af0c08fcf3307 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..16236bd0457bda2940ee2cb287b448cc054a7589 100644 (file)
--- 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);
        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 88b1fe464946142feef2476f91d5275458b0c6ca..742b81bf3b8fe29e5c3ec1f87585323ce43f3d6e 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 {