From 43d5f52ac4a3b9cb6c5717af30be42a363fedf20 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:48 +0000 Subject: [PATCH 01/12] xdiff: delete static forward declarations in xprepare Move xdl_prepare_env() later in the file to avoid the need for static forward declarations. Best-viewed-with: --color-moved Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 116 ++++++++++++++++++++--------------------------- 1 file changed, 50 insertions(+), 66 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index e1d4017b2d..249bfa678f 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -53,21 +53,6 @@ typedef struct s_xdlclassifier { -static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags); -static void xdl_free_classifier(xdlclassifier_t *cf); -static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t **rhash, - unsigned int hbits, xrecord_t *rec); -static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp, - xdlclassifier_t *cf, xdfile_t *xdf); -static void xdl_free_ctx(xdfile_t *xdf); -static int xdl_clean_mmatch(char const *dis, long i, long s, long e); -static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2); -static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2); -static int xdl_optimize_ctxs(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2); - - - - static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags) { cf->flags = flags; @@ -242,57 +227,6 @@ static void xdl_free_ctx(xdfile_t *xdf) { } -int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, - xdfenv_t *xe) { - long enl1, enl2, sample; - xdlclassifier_t cf; - - memset(&cf, 0, sizeof(cf)); - - /* - * For histogram diff, we can afford a smaller sample size and - * thus a poorer estimate of the number of lines, as the hash - * table (rhash) won't be filled up/grown. The number of lines - * (nrecs) will be updated correctly anyway by - * xdl_prepare_ctx(). - */ - sample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF - ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1); - - enl1 = xdl_guess_lines(mf1, sample) + 1; - enl2 = xdl_guess_lines(mf2, sample) + 1; - - if (xdl_init_classifier(&cf, enl1 + enl2 + 1, xpp->flags) < 0) - return -1; - - if (xdl_prepare_ctx(1, mf1, enl1, xpp, &cf, &xe->xdf1) < 0) { - - xdl_free_classifier(&cf); - return -1; - } - if (xdl_prepare_ctx(2, mf2, enl2, xpp, &cf, &xe->xdf2) < 0) { - - xdl_free_ctx(&xe->xdf1); - xdl_free_classifier(&cf); - return -1; - } - - if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && - (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) && - xdl_optimize_ctxs(&cf, &xe->xdf1, &xe->xdf2) < 0) { - - xdl_free_ctx(&xe->xdf2); - xdl_free_ctx(&xe->xdf1); - xdl_free_classifier(&cf); - return -1; - } - - xdl_free_classifier(&cf); - - return 0; -} - - void xdl_free_env(xdfenv_t *xe) { xdl_free_ctx(&xe->xdf2); @@ -460,3 +394,53 @@ static int xdl_optimize_ctxs(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2 return 0; } + +int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, + xdfenv_t *xe) { + long enl1, enl2, sample; + xdlclassifier_t cf; + + memset(&cf, 0, sizeof(cf)); + + /* + * For histogram diff, we can afford a smaller sample size and + * thus a poorer estimate of the number of lines, as the hash + * table (rhash) won't be filled up/grown. The number of lines + * (nrecs) will be updated correctly anyway by + * xdl_prepare_ctx(). + */ + sample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF + ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1); + + enl1 = xdl_guess_lines(mf1, sample) + 1; + enl2 = xdl_guess_lines(mf2, sample) + 1; + + if (xdl_init_classifier(&cf, enl1 + enl2 + 1, xpp->flags) < 0) + return -1; + + if (xdl_prepare_ctx(1, mf1, enl1, xpp, &cf, &xe->xdf1) < 0) { + + xdl_free_classifier(&cf); + return -1; + } + if (xdl_prepare_ctx(2, mf2, enl2, xpp, &cf, &xe->xdf2) < 0) { + + xdl_free_ctx(&xe->xdf1); + xdl_free_classifier(&cf); + return -1; + } + + if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && + (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) && + xdl_optimize_ctxs(&cf, &xe->xdf1, &xe->xdf2) < 0) { + + xdl_free_ctx(&xe->xdf2); + xdl_free_ctx(&xe->xdf1); + xdl_free_classifier(&cf); + return -1; + } + + xdl_free_classifier(&cf); + + return 0; +} From d1c028bdf75b9bcd380f85f74a34edcbaa060fee Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:49 +0000 Subject: [PATCH 02/12] xdiff: delete local variables and initialize/free xdfile_t directly These local variables are essentially a hand-rolled additional implementation of xdl_free_ctx() inlined into xdl_prepare_ctx(). Modify the code to use the existing xdl_free_ctx() function so there aren't two ways to free such variables. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 150 +++++++++++++++++++++-------------------------- 1 file changed, 66 insertions(+), 84 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 249bfa678f..96134c9fbf 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -134,90 +134,8 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t } -static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp, - xdlclassifier_t *cf, xdfile_t *xdf) { - unsigned int hbits; - long nrec, hsize, bsize; - unsigned long hav; - char const *blk, *cur, *top, *prev; - xrecord_t *crec; - xrecord_t **recs; - xrecord_t **rhash; - unsigned long *ha; - char *rchg; - long *rindex; - - ha = NULL; - rindex = NULL; - rchg = NULL; - rhash = NULL; - recs = NULL; - - if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) - goto abort; - if (!XDL_ALLOC_ARRAY(recs, narec)) - goto abort; - - hbits = xdl_hashbits((unsigned int) narec); - hsize = 1 << hbits; - if (!XDL_CALLOC_ARRAY(rhash, hsize)) - goto abort; - - nrec = 0; - if ((cur = blk = xdl_mmfile_first(mf, &bsize))) { - for (top = blk + bsize; cur < top; ) { - prev = cur; - hav = xdl_hash_record(&cur, top, xpp->flags); - if (XDL_ALLOC_GROW(recs, nrec + 1, narec)) - goto abort; - if (!(crec = xdl_cha_alloc(&xdf->rcha))) - goto abort; - crec->ptr = prev; - crec->size = (long) (cur - prev); - crec->ha = hav; - recs[nrec++] = crec; - if (xdl_classify_record(pass, cf, rhash, hbits, crec) < 0) - goto abort; - } - } - - if (!XDL_CALLOC_ARRAY(rchg, nrec + 2)) - goto abort; - - if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && - (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) { - if (!XDL_ALLOC_ARRAY(rindex, nrec + 1)) - goto abort; - if (!XDL_ALLOC_ARRAY(ha, nrec + 1)) - goto abort; - } - - xdf->nrec = nrec; - xdf->recs = recs; - xdf->hbits = hbits; - xdf->rhash = rhash; - xdf->rchg = rchg + 1; - xdf->rindex = rindex; - xdf->nreff = 0; - xdf->ha = ha; - xdf->dstart = 0; - xdf->dend = nrec - 1; - - return 0; - -abort: - xdl_free(ha); - xdl_free(rindex); - xdl_free(rchg); - xdl_free(rhash); - xdl_free(recs); - xdl_cha_free(&xdf->rcha); - return -1; -} - - -static void xdl_free_ctx(xdfile_t *xdf) { - +static void xdl_free_ctx(xdfile_t *xdf) +{ xdl_free(xdf->rhash); xdl_free(xdf->rindex); xdl_free(xdf->rchg - 1); @@ -227,6 +145,70 @@ static void xdl_free_ctx(xdfile_t *xdf) { } +static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp, + xdlclassifier_t *cf, xdfile_t *xdf) { + long bsize; + unsigned long hav; + char const *blk, *cur, *top, *prev; + xrecord_t *crec; + + xdf->ha = NULL; + xdf->rindex = NULL; + xdf->rchg = NULL; + xdf->rhash = NULL; + xdf->recs = NULL; + + if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) + goto abort; + if (!XDL_ALLOC_ARRAY(xdf->recs, narec)) + goto abort; + + xdf->hbits = xdl_hashbits((unsigned int) narec); + if (!XDL_CALLOC_ARRAY(xdf->rhash, 1 << xdf->hbits)) + goto abort; + + xdf->nrec = 0; + if ((cur = blk = xdl_mmfile_first(mf, &bsize))) { + for (top = blk + bsize; cur < top; ) { + prev = cur; + hav = xdl_hash_record(&cur, top, xpp->flags); + if (XDL_ALLOC_GROW(xdf->recs, xdf->nrec + 1, narec)) + goto abort; + if (!(crec = xdl_cha_alloc(&xdf->rcha))) + goto abort; + crec->ptr = prev; + crec->size = (long) (cur - prev); + crec->ha = hav; + xdf->recs[xdf->nrec++] = crec; + if (xdl_classify_record(pass, cf, xdf->rhash, xdf->hbits, crec) < 0) + goto abort; + } + } + + if (!XDL_CALLOC_ARRAY(xdf->rchg, xdf->nrec + 2)) + goto abort; + + if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && + (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) { + if (!XDL_ALLOC_ARRAY(xdf->rindex, xdf->nrec + 1)) + goto abort; + if (!XDL_ALLOC_ARRAY(xdf->ha, xdf->nrec + 1)) + goto abort; + } + + xdf->rchg += 1; + xdf->nreff = 0; + xdf->dstart = 0; + xdf->dend = xdf->nrec - 1; + + return 0; + +abort: + xdl_free_ctx(xdf); + return -1; +} + + void xdl_free_env(xdfenv_t *xe) { xdl_free_ctx(&xe->xdf2); From efaf553b1a4ea9cafcb9cab0697157091bc4825a Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:50 +0000 Subject: [PATCH 03/12] xdiff: delete unnecessary fields from xrecord_t and xdfile_t xrecord_t.next, xdfile_t.hbits, xdfile_t.rhash are initialized, but never used for anything by the code. Remove them. Best-viewed-with: --color-words Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 15 ++------------- xdiff/xtypes.h | 3 --- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 96134c9fbf..3576415c85 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -91,8 +91,7 @@ static void xdl_free_classifier(xdlclassifier_t *cf) { } -static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t **rhash, - unsigned int hbits, xrecord_t *rec) { +static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t *rec) { long hi; char const *line; xdlclass_t *rcrec; @@ -126,17 +125,12 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t rec->ha = (unsigned long) rcrec->idx; - hi = (long) XDL_HASHLONG(rec->ha, hbits); - rec->next = rhash[hi]; - rhash[hi] = rec; - return 0; } static void xdl_free_ctx(xdfile_t *xdf) { - xdl_free(xdf->rhash); xdl_free(xdf->rindex); xdl_free(xdf->rchg - 1); xdl_free(xdf->ha); @@ -155,7 +149,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xdf->ha = NULL; xdf->rindex = NULL; xdf->rchg = NULL; - xdf->rhash = NULL; xdf->recs = NULL; if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) @@ -163,10 +156,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ if (!XDL_ALLOC_ARRAY(xdf->recs, narec)) goto abort; - xdf->hbits = xdl_hashbits((unsigned int) narec); - if (!XDL_CALLOC_ARRAY(xdf->rhash, 1 << xdf->hbits)) - goto abort; - xdf->nrec = 0; if ((cur = blk = xdl_mmfile_first(mf, &bsize))) { for (top = blk + bsize; cur < top; ) { @@ -180,7 +169,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ crec->size = (long) (cur - prev); crec->ha = hav; xdf->recs[xdf->nrec++] = crec; - if (xdl_classify_record(pass, cf, xdf->rhash, xdf->hbits, crec) < 0) + if (xdl_classify_record(pass, cf, crec) < 0) goto abort; } } diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h index 8442bd436e..8b8467360e 100644 --- a/xdiff/xtypes.h +++ b/xdiff/xtypes.h @@ -39,7 +39,6 @@ typedef struct s_chastore { } chastore_t; typedef struct s_xrecord { - struct s_xrecord *next; char const *ptr; long size; unsigned long ha; @@ -48,8 +47,6 @@ typedef struct s_xrecord { typedef struct s_xdfile { chastore_t rcha; long nrec; - unsigned int hbits; - xrecord_t **rhash; long dstart, dend; xrecord_t **recs; char *rchg; From 7bdeb3afad908e52baab6e58397423aa2d2f3d29 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:51 +0000 Subject: [PATCH 04/12] xdiff: delete superfluous function xdl_get_rec() in xemit When xrecord_t was a linked list, and recs didn't exist, I assume this function walked the list until it found the right record. Accessing a contiguous array is so trivial that this function is now superfluous. Delete it. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xemit.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 1d40c9cb40..40fc8154f3 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -22,23 +22,14 @@ #include "xinclude.h" -static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec) { - - *rec = xdf->recs[ri]->ptr; - - return xdf->recs[ri]->size; -} - static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb) { long size, psize = strlen(pre); - char const *rec; - - size = xdl_get_rec(xdf, ri, &rec); - if (xdl_emit_diffrec(rec, size, pre, psize, ecb) < 0) { + char const *rec = xdf->recs[ri]->ptr; + size = xdf->recs[ri]->size; + if (xdl_emit_diffrec(rec, size, pre, psize, ecb) < 0) return -1; - } return 0; } @@ -120,8 +111,8 @@ static long def_ff(const char *rec, long len, char *buf, long sz) static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri, char *buf, long sz) { - const char *rec; - long len = xdl_get_rec(xdf, ri, &rec); + const char *rec = xdf->recs[ri]->ptr; + long len = xdf->recs[ri]->size; if (!xecfg->find_func) return def_ff(rec, len, buf, sz); return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv); @@ -160,8 +151,8 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg, static int is_empty_rec(xdfile_t *xdf, long ri) { - const char *rec; - long len = xdl_get_rec(xdf, ri, &rec); + const char *rec = xdf->recs[ri]->ptr; + long len = xdf->recs[ri]->size; while (len > 0 && XDL_ISSPACE(*rec)) { rec++; From 7c6ce2e47b274b299dd0a3b185e70f2ee5e3e07a Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:52 +0000 Subject: [PATCH 05/12] xdiff: delete local variables that alias fields in xrecord_t Use the type xrecord_t as the local variable for the functions in the file xdiff/xemit.c. Most places directly reference the fields inside of this struct, doing that here makes it more consistent with the rest of the code. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xemit.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 40fc8154f3..2161ac3cd0 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -23,12 +23,11 @@ #include "xinclude.h" -static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb) { - long size, psize = strlen(pre); - char const *rec = xdf->recs[ri]->ptr; +static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb) +{ + xrecord_t *rec = xdf->recs[ri]; - size = xdf->recs[ri]->size; - if (xdl_emit_diffrec(rec, size, pre, psize, ecb) < 0) + if (xdl_emit_diffrec(rec->ptr, rec->size, pre, strlen(pre), ecb) < 0) return -1; return 0; @@ -111,11 +110,11 @@ static long def_ff(const char *rec, long len, char *buf, long sz) static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri, char *buf, long sz) { - const char *rec = xdf->recs[ri]->ptr; - long len = xdf->recs[ri]->size; + xrecord_t *rec = xdf->recs[ri]; + if (!xecfg->find_func) - return def_ff(rec, len, buf, sz); - return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv); + return def_ff(rec->ptr, rec->size, buf, sz); + return xecfg->find_func(rec->ptr, rec->size, buf, sz, xecfg->find_func_priv); } static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri) @@ -151,14 +150,12 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg, static int is_empty_rec(xdfile_t *xdf, long ri) { - const char *rec = xdf->recs[ri]->ptr; - long len = xdf->recs[ri]->size; + xrecord_t *rec = xdf->recs[ri]; + long i = 0; - while (len > 0 && XDL_ISSPACE(*rec)) { - rec++; - len--; - } - return !len; + for (; i < rec->size && XDL_ISSPACE(rec->ptr[i]); i++); + + return i == rec->size; } int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, From f4ea812b2d930fb1825b99dc11ca186691dade99 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:53 +0000 Subject: [PATCH 06/12] xdiff: delete struct diffdata_t Every field in this struct is an alias for a certain field in xdfile_t. diffdata_t.nrec -> xdfile_t.nreff diffdata_t.ha -> xdfile_t.ha diffdata_t.rindex -> xdfile_t.rindex diffdata_t.rchg -> xdfile_t.rchg I think this struct existed before xdfile_t, and was kept for backward compatibility reasons. I think xdiffi should have been refactored to use the new (xdfile_t) struct, but was easier to alias it instead. The local variables rchg* and rindex* don't shorten the lines by much, nor do they really need to be there to make the code more readable. Delete them. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 32 ++++++++------------------------ xdiff/xdiffi.h | 11 ++--------- 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 5a96e36dfb..bbf0161f84 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -257,10 +257,10 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1, * sub-boxes by calling the box splitting function. Note that the real job * (marking changed lines) is done in the two boundary reaching checks. */ -int xdl_recs_cmp(diffdata_t *dd1, long off1, long lim1, - diffdata_t *dd2, long off2, long lim2, +int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1, + xdfile_t *xdf2, long off2, long lim2, long *kvdf, long *kvdb, int need_min, xdalgoenv_t *xenv) { - unsigned long const *ha1 = dd1->ha, *ha2 = dd2->ha; + unsigned long const *ha1 = xdf1->ha, *ha2 = xdf2->ha; /* * Shrink the box by walking through each diagonal snake (SW and NE). @@ -273,17 +273,11 @@ int xdl_recs_cmp(diffdata_t *dd1, long off1, long lim1, * be obviously changed. */ if (off1 == lim1) { - char *rchg2 = dd2->rchg; - long *rindex2 = dd2->rindex; - for (; off2 < lim2; off2++) - rchg2[rindex2[off2]] = 1; + xdf2->rchg[xdf2->rindex[off2]] = 1; } else if (off2 == lim2) { - char *rchg1 = dd1->rchg; - long *rindex1 = dd1->rindex; - for (; off1 < lim1; off1++) - rchg1[rindex1[off1]] = 1; + xdf1->rchg[xdf1->rindex[off1]] = 1; } else { xdpsplit_t spl; spl.i1 = spl.i2 = 0; @@ -300,9 +294,9 @@ int xdl_recs_cmp(diffdata_t *dd1, long off1, long lim1, /* * ... et Impera. */ - if (xdl_recs_cmp(dd1, off1, spl.i1, dd2, off2, spl.i2, + if (xdl_recs_cmp(xdf1, off1, spl.i1, xdf2, off2, spl.i2, kvdf, kvdb, spl.min_lo, xenv) < 0 || - xdl_recs_cmp(dd1, spl.i1, lim1, dd2, spl.i2, lim2, + xdl_recs_cmp(xdf1, spl.i1, lim1, xdf2, spl.i2, lim2, kvdf, kvdb, spl.min_hi, xenv) < 0) { return -1; @@ -318,7 +312,6 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, long ndiags; long *kvd, *kvdf, *kvdb; xdalgoenv_t xenv; - diffdata_t dd1, dd2; int res; if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) @@ -357,16 +350,7 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xenv.snake_cnt = XDL_SNAKE_CNT; xenv.heur_min = XDL_HEUR_MIN_COST; - dd1.nrec = xe->xdf1.nreff; - dd1.ha = xe->xdf1.ha; - dd1.rchg = xe->xdf1.rchg; - dd1.rindex = xe->xdf1.rindex; - dd2.nrec = xe->xdf2.nreff; - dd2.ha = xe->xdf2.ha; - dd2.rchg = xe->xdf2.rchg; - dd2.rindex = xe->xdf2.rindex; - - res = xdl_recs_cmp(&dd1, 0, dd1.nrec, &dd2, 0, dd2.nrec, + res = xdl_recs_cmp(&xe->xdf1, 0, xe->xdf1.nreff, &xe->xdf2, 0, xe->xdf2.nreff, kvdf, kvdb, (xpp->flags & XDF_NEED_MINIMAL) != 0, &xenv); xdl_free(kvd); diff --git a/xdiff/xdiffi.h b/xdiff/xdiffi.h index 126c9d8ff4..49e52c67f9 100644 --- a/xdiff/xdiffi.h +++ b/xdiff/xdiffi.h @@ -24,13 +24,6 @@ #define XDIFFI_H -typedef struct s_diffdata { - long nrec; - unsigned long const *ha; - long *rindex; - char *rchg; -} diffdata_t; - typedef struct s_xdalgoenv { long mxcost; long snake_cnt; @@ -46,8 +39,8 @@ typedef struct s_xdchange { -int xdl_recs_cmp(diffdata_t *dd1, long off1, long lim1, - diffdata_t *dd2, long off2, long lim2, +int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1, + xdfile_t *xdf2, long off2, long lim2, long *kvdf, long *kvdb, int need_min, xdalgoenv_t *xenv); int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdfenv_t *xe); From 5c294dceb23633a8bcced946ce3f65a06038cf52 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:54 +0000 Subject: [PATCH 07/12] xdiff: delete redundant array xdfile_t.ha When 0 <= i < xdfile_t.nreff the following is true: xdfile_t.ha[i] == xdfile_t.recs[xdfile_t.rindex[i]] This makes the code about 5% slower. The fields rindex and ha are specific to the classic diff (myers and minimal). I plan on creating a struct for classic diff, but there's a lot of cleanup that needs to be done before that can happen and leaving ha in would make those cleanups harder to follow. A subsequent commit will delete the chastore cha from xdfile_t. That later commit will investigate deleting ha and cha independently and together. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 24 ++++++++++++++---------- xdiff/xprepare.c | 12 ++---------- xdiff/xtypes.h | 1 - 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index bbf0161f84..11cd090b53 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -22,6 +22,11 @@ #include "xinclude.h" +static unsigned long get_hash(xdfile_t *xdf, long index) +{ + return xdf->recs[xdf->rindex[index]]->ha; +} + #define XDL_MAX_COST_MIN 256 #define XDL_HEUR_MIN_COST 256 #define XDL_LINE_MAX (long)((1UL << (CHAR_BIT * sizeof(long) - 1)) - 1) @@ -42,8 +47,8 @@ typedef struct s_xdpsplit { * using this algorithm, so a little bit of heuristic is needed to cut the * search and to return a suboptimal point. */ -static long xdl_split(unsigned long const *ha1, long off1, long lim1, - unsigned long const *ha2, long off2, long lim2, +static long xdl_split(xdfile_t *xdf1, long off1, long lim1, + xdfile_t *xdf2, long off2, long lim2, long *kvdf, long *kvdb, int need_min, xdpsplit_t *spl, xdalgoenv_t *xenv) { long dmin = off1 - lim2, dmax = lim1 - off2; @@ -87,7 +92,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1, i1 = kvdf[d + 1]; prev1 = i1; i2 = i1 - d; - for (; i1 < lim1 && i2 < lim2 && ha1[i1] == ha2[i2]; i1++, i2++); + for (; i1 < lim1 && i2 < lim2 && get_hash(xdf1, i1) == get_hash(xdf2, i2); i1++, i2++); if (i1 - prev1 > xenv->snake_cnt) got_snake = 1; kvdf[d] = i1; @@ -124,7 +129,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1, i1 = kvdb[d + 1] - 1; prev1 = i1; i2 = i1 - d; - for (; i1 > off1 && i2 > off2 && ha1[i1 - 1] == ha2[i2 - 1]; i1--, i2--); + for (; i1 > off1 && i2 > off2 && get_hash(xdf1, i1 - 1) == get_hash(xdf2, i2 - 1); i1--, i2--); if (prev1 - i1 > xenv->snake_cnt) got_snake = 1; kvdb[d] = i1; @@ -159,7 +164,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1, if (v > XDL_K_HEUR * ec && v > best && off1 + xenv->snake_cnt <= i1 && i1 < lim1 && off2 + xenv->snake_cnt <= i2 && i2 < lim2) { - for (k = 1; ha1[i1 - k] == ha2[i2 - k]; k++) + for (k = 1; get_hash(xdf1, i1 - k) == get_hash(xdf2, i2 - k); k++) if (k == xenv->snake_cnt) { best = v; spl->i1 = i1; @@ -183,7 +188,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1, if (v > XDL_K_HEUR * ec && v > best && off1 < i1 && i1 <= lim1 - xenv->snake_cnt && off2 < i2 && i2 <= lim2 - xenv->snake_cnt) { - for (k = 0; ha1[i1 + k] == ha2[i2 + k]; k++) + for (k = 0; get_hash(xdf1, i1 + k) == get_hash(xdf2, i2 + k); k++) if (k == xenv->snake_cnt - 1) { best = v; spl->i1 = i1; @@ -260,13 +265,12 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1, int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1, xdfile_t *xdf2, long off2, long lim2, long *kvdf, long *kvdb, int need_min, xdalgoenv_t *xenv) { - unsigned long const *ha1 = xdf1->ha, *ha2 = xdf2->ha; /* * Shrink the box by walking through each diagonal snake (SW and NE). */ - for (; off1 < lim1 && off2 < lim2 && ha1[off1] == ha2[off2]; off1++, off2++); - for (; off1 < lim1 && off2 < lim2 && ha1[lim1 - 1] == ha2[lim2 - 1]; lim1--, lim2--); + for (; off1 < lim1 && off2 < lim2 && get_hash(xdf1, off1) == get_hash(xdf2, off2); off1++, off2++); + for (; off1 < lim1 && off2 < lim2 && get_hash(xdf1, lim1 - 1) == get_hash(xdf2, lim2 - 1); lim1--, lim2--); /* * If one dimension is empty, then all records on the other one must @@ -285,7 +289,7 @@ int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1, /* * Divide ... */ - if (xdl_split(ha1, off1, lim1, ha2, off2, lim2, kvdf, kvdb, + if (xdl_split(xdf1, off1, lim1, xdf2, off2, lim2, kvdf, kvdb, need_min, &spl, xenv) < 0) { return -1; diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 3576415c85..22c44f0683 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -133,7 +133,6 @@ static void xdl_free_ctx(xdfile_t *xdf) { xdl_free(xdf->rindex); xdl_free(xdf->rchg - 1); - xdl_free(xdf->ha); xdl_free(xdf->recs); xdl_cha_free(&xdf->rcha); } @@ -146,7 +145,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ char const *blk, *cur, *top, *prev; xrecord_t *crec; - xdf->ha = NULL; xdf->rindex = NULL; xdf->rchg = NULL; xdf->recs = NULL; @@ -181,8 +179,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) { if (!XDL_ALLOC_ARRAY(xdf->rindex, xdf->nrec + 1)) goto abort; - if (!XDL_ALLOC_ARRAY(xdf->ha, xdf->nrec + 1)) - goto abort; } xdf->rchg += 1; @@ -300,9 +296,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd i <= xdf1->dend; i++, recs++) { if (dis1[i] == 1 || (dis1[i] == 2 && !xdl_clean_mmatch(dis1, i, xdf1->dstart, xdf1->dend))) { - xdf1->rindex[nreff] = i; - xdf1->ha[nreff] = (*recs)->ha; - nreff++; + xdf1->rindex[nreff++] = i; } else xdf1->rchg[i] = 1; } @@ -312,9 +306,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd i <= xdf2->dend; i++, recs++) { if (dis2[i] == 1 || (dis2[i] == 2 && !xdl_clean_mmatch(dis2, i, xdf2->dstart, xdf2->dend))) { - xdf2->rindex[nreff] = i; - xdf2->ha[nreff] = (*recs)->ha; - nreff++; + xdf2->rindex[nreff++] = i; } else xdf2->rchg[i] = 1; } diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h index 8b8467360e..85848f1685 100644 --- a/xdiff/xtypes.h +++ b/xdiff/xtypes.h @@ -52,7 +52,6 @@ typedef struct s_xdfile { char *rchg; long *rindex; long nreff; - unsigned long *ha; } xdfile_t; typedef struct s_xdfenv { From 6d507bd41a6f57f802a93a134cca0949a3d4370a Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:55 +0000 Subject: [PATCH 08/12] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t The fields from xdlclass_t are aliases of xrecord_t: xdlclass_t.line -> xrecord_t.ptr xdlclass_t.size -> xrecord_t.size xdlclass_t.ha -> xrecord_t.ha xdlclass_t carries a copy of the data in xrecord_t, but instead of embedding xrecord_t it duplicates the individual fields. A future commit will change the types used in xrecord_t so embed it in xdlclass_t first, so we don't have to remember to change the types here as well. Best-viewed-with: --color-words Helped-by: Phillip Wood Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 22c44f0683..e6e2c0e1c0 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -32,9 +32,7 @@ typedef struct s_xdlclass { struct s_xdlclass *next; - unsigned long ha; - char const *line; - long size; + xrecord_t rec; long idx; long len1, len2; } xdlclass_t; @@ -93,14 +91,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; - char const *line; xdlclass_t *rcrec; - line = rec->ptr; hi = (long) XDL_HASHLONG(rec->ha, cf->hbits); for (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next) - if (rcrec->ha == rec->ha && - xdl_recmatch(rcrec->line, rcrec->size, + if (rcrec->rec.ha == rec->ha && + xdl_recmatch(rcrec->rec.ptr, rcrec->rec.size, rec->ptr, rec->size, cf->flags)) break; @@ -113,9 +109,7 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t if (XDL_ALLOC_GROW(cf->rcrecs, cf->count, cf->alloc)) return -1; cf->rcrecs[rcrec->idx] = rcrec; - rcrec->line = line; - rcrec->size = rec->size; - rcrec->ha = rec->ha; + rcrec->rec = *rec; rcrec->len1 = rcrec->len2 = 0; rcrec->next = cf->rchash[hi]; cf->rchash[hi] = rcrec; From d43d591252cfac10433aac01cc3d9d906c2f72c3 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:56 +0000 Subject: [PATCH 09/12] xdiff: delete chastore from xdfile_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xdfile_t currently uses chastore_t which is an arena allocator. I think that xrecord_t used to be a linked list and recs didn't exist originally. When recs was added I think they forgot to remove xdfile_t.next, but was overlooked. This dual data structure setup makes the code somewhat confusing. Additionally the C type chastore_t isn't FFI friendly, and provides little to no performance benefit over using realloc to grow an array. Performance impact of deleting fields from xdfile_t: Deleting ha is about 5% slower. Deleting cha is about 5% faster. Delete ha, but keep cha time hyperfine --warmup 3 -L exe build_v2.51.0/git,build_delete_ha/git '{exe} log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null' Benchmark 1: build_v2.51.0/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Time (mean ± σ): 1.269 s ± 0.017 s [User: 1.135 s, System: 0.128 s] Range (min … max): 1.249 s … 1.286 s 10 runs Benchmark 2: build_delete_ha/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Time (mean ± σ): 1.339 s ± 0.017 s [User: 1.234 s, System: 0.099 s] Range (min … max): 1.320 s … 1.358 s 10 runs Summary build_v2.51.0/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null ran 1.06 ± 0.02 times faster than build_delete_ha/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Delete cha, but keep ha time hyperfine --warmup 3 -L exe build_v2.51.0/git,build_delete_chastore/git '{exe} log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null' Benchmark 1: build_v2.51.0/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Time (mean ± σ): 1.290 s ± 0.001 s [User: 1.154 s, System: 0.130 s] Range (min … max): 1.288 s … 1.292 s 10 runs Benchmark 2: build_delete_chastore/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Time (mean ± σ): 1.232 s ± 0.017 s [User: 1.105 s, System: 0.121 s] Range (min … max): 1.205 s … 1.249 s 10 runs Summary build_delete_chastore/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null ran 1.05 ± 0.01 times faster than build_v2.51.0/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Delete ha AND chastore time hyperfine --warmup 3 -L exe build_v2.51.0/git,build_delete_ha_and_chastore/git '{exe} log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null' Benchmark 1: build_v2.51.0/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Time (mean ± σ): 1.291 s ± 0.002 s [User: 1.156 s, System: 0.129 s] Range (min … max): 1.287 s … 1.295 s 10 runs Benchmark 2: build_delete_ha_and_chastore/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Time (mean ± σ): 1.306 s ± 0.001 s [User: 1.195 s, System: 0.105 s] Range (min … max): 1.305 s … 1.308 s 10 runs Summary build_v2.51.0/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null ran 1.01 ± 0.00 times faster than build_delete_ha_and_chastore/git log --oneline --shortstat --diff-algorithm=myers -3000 v2.39.1 >/dev/null Best-viewed-with: --color-words Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 24 ++++++++++---------- xdiff/xemit.c | 6 ++--- xdiff/xhistogram.c | 2 +- xdiff/xmerge.c | 56 +++++++++++++++++++++++----------------------- xdiff/xpatience.c | 10 ++++----- xdiff/xprepare.c | 19 ++++++---------- xdiff/xtypes.h | 3 +-- xdiff/xutils.c | 12 +++++----- 8 files changed, 63 insertions(+), 69 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 11cd090b53..a66125d44a 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -24,7 +24,7 @@ static unsigned long get_hash(xdfile_t *xdf, long index) { - return xdf->recs[xdf->rindex[index]]->ha; + return xdf->recs[xdf->rindex[index]].ha; } #define XDL_MAX_COST_MIN 256 @@ -489,13 +489,13 @@ static void measure_split(const xdfile_t *xdf, long split, m->indent = -1; } else { m->end_of_file = 0; - m->indent = get_indent(xdf->recs[split]); + m->indent = get_indent(&xdf->recs[split]); } m->pre_blank = 0; m->pre_indent = -1; for (i = split - 1; i >= 0; i--) { - m->pre_indent = get_indent(xdf->recs[i]); + m->pre_indent = get_indent(&xdf->recs[i]); if (m->pre_indent != -1) break; m->pre_blank += 1; @@ -508,7 +508,7 @@ static void measure_split(const xdfile_t *xdf, long split, m->post_blank = 0; m->post_indent = -1; for (i = split + 1; i < xdf->nrec; i++) { - m->post_indent = get_indent(xdf->recs[i]); + m->post_indent = get_indent(&xdf->recs[i]); if (m->post_indent != -1) break; m->post_blank += 1; @@ -752,7 +752,7 @@ static inline int group_previous(xdfile_t *xdf, struct xdlgroup *g) static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g) { if (g->end < xdf->nrec && - recs_match(xdf->recs[g->start], xdf->recs[g->end])) { + recs_match(&xdf->recs[g->start], &xdf->recs[g->end])) { xdf->rchg[g->start++] = 0; xdf->rchg[g->end++] = 1; @@ -773,7 +773,7 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g) static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g) { if (g->start > 0 && - recs_match(xdf->recs[g->start - 1], xdf->recs[g->end - 1])) { + recs_match(&xdf->recs[g->start - 1], &xdf->recs[g->end - 1])) { xdf->rchg[--g->start] = 1; xdf->rchg[--g->end] = 0; @@ -988,16 +988,16 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags) for (xch = xscr; xch; xch = xch->next) { int ignore = 1; - xrecord_t **rec; + xrecord_t *rec; long i; rec = &xe->xdf1.recs[xch->i1]; for (i = 0; i < xch->chg1 && ignore; i++) - ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags); + ignore = xdl_blankline(rec[i].ptr, rec[i].size, flags); rec = &xe->xdf2.recs[xch->i2]; for (i = 0; i < xch->chg2 && ignore; i++) - ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags); + ignore = xdl_blankline(rec[i].ptr, rec[i].size, flags); xch->ignore = ignore; } @@ -1021,7 +1021,7 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe, xdchange_t *xch; for (xch = xscr; xch; xch = xch->next) { - xrecord_t **rec; + xrecord_t *rec; int ignore = 1; long i; @@ -1033,11 +1033,11 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe, rec = &xe->xdf1.recs[xch->i1]; for (i = 0; i < xch->chg1 && ignore; i++) - ignore = record_matches_regex(rec[i], xpp); + ignore = record_matches_regex(&rec[i], xpp); rec = &xe->xdf2.recs[xch->i2]; for (i = 0; i < xch->chg2 && ignore; i++) - ignore = record_matches_regex(rec[i], xpp); + ignore = record_matches_regex(&rec[i], xpp); xch->ignore = ignore; } diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 2161ac3cd0..b2f1f30cd3 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -25,7 +25,7 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb) { - xrecord_t *rec = xdf->recs[ri]; + xrecord_t *rec = &xdf->recs[ri]; if (xdl_emit_diffrec(rec->ptr, rec->size, pre, strlen(pre), ecb) < 0) return -1; @@ -110,7 +110,7 @@ static long def_ff(const char *rec, long len, char *buf, long sz) static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri, char *buf, long sz) { - xrecord_t *rec = xdf->recs[ri]; + xrecord_t *rec = &xdf->recs[ri]; if (!xecfg->find_func) return def_ff(rec->ptr, rec->size, buf, sz); @@ -150,7 +150,7 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg, static int is_empty_rec(xdfile_t *xdf, long ri) { - xrecord_t *rec = xdf->recs[ri]; + xrecord_t *rec = &xdf->recs[ri]; long i = 0; for (; i < rec->size && XDL_ISSPACE(rec->ptr[i]); i++); diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index 040d81e0bc..4d857e8ae2 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -86,7 +86,7 @@ struct region { ((LINE_MAP(index, ptr))->cnt) #define REC(env, s, l) \ - (env->xdf##s.recs[l - 1]) + (&env->xdf##s.recs[l - 1]) static int cmp_recs(xrecord_t *r1, xrecord_t *r2) { diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c index af40c88a5b..fd600cbb5d 100644 --- a/xdiff/xmerge.c +++ b/xdiff/xmerge.c @@ -97,12 +97,12 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2, int line_count, long flags) { int i; - xrecord_t **rec1 = xe1->xdf2.recs + i1; - xrecord_t **rec2 = xe2->xdf2.recs + i2; + xrecord_t *rec1 = xe1->xdf2.recs + i1; + xrecord_t *rec2 = xe2->xdf2.recs + i2; for (i = 0; i < line_count; i++) { - int result = xdl_recmatch(rec1[i]->ptr, rec1[i]->size, - rec2[i]->ptr, rec2[i]->size, flags); + int result = xdl_recmatch(rec1[i].ptr, rec1[i].size, + rec2[i].ptr, rec2[i].size, flags); if (!result) return -1; } @@ -111,7 +111,7 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2, static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int needs_cr, int add_nl, char *dest) { - xrecord_t **recs; + xrecord_t *recs; int size = 0; recs = (use_orig ? xe->xdf1.recs : xe->xdf2.recs) + i; @@ -119,12 +119,12 @@ static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int nee if (count < 1) return 0; - for (i = 0; i < count; size += recs[i++]->size) + for (i = 0; i < count; size += recs[i++].size) if (dest) - memcpy(dest + size, recs[i]->ptr, recs[i]->size); + memcpy(dest + size, recs[i].ptr, recs[i].size); if (add_nl) { - i = recs[count - 1]->size; - if (i == 0 || recs[count - 1]->ptr[i - 1] != '\n') { + i = recs[count - 1].size; + if (i == 0 || recs[count - 1].ptr[i - 1] != '\n') { if (needs_cr) { if (dest) dest[size] = '\r'; @@ -160,22 +160,22 @@ static int is_eol_crlf(xdfile_t *file, int i) if (i < file->nrec - 1) /* All lines before the last *must* end in LF */ - return (size = file->recs[i]->size) > 1 && - file->recs[i]->ptr[size - 2] == '\r'; + return (size = file->recs[i].size) > 1 && + file->recs[i].ptr[size - 2] == '\r'; if (!file->nrec) /* Cannot determine eol style from empty file */ return -1; - if ((size = file->recs[i]->size) && - file->recs[i]->ptr[size - 1] == '\n') + if ((size = file->recs[i].size) && + file->recs[i].ptr[size - 1] == '\n') /* Last line; ends in LF; Is it CR/LF? */ return size > 1 && - file->recs[i]->ptr[size - 2] == '\r'; + file->recs[i].ptr[size - 2] == '\r'; if (!i) /* The only line has no eol */ return -1; /* Determine eol from second-to-last line */ - return (size = file->recs[i - 1]->size) > 1 && - file->recs[i - 1]->ptr[size - 2] == '\r'; + return (size = file->recs[i - 1].size) > 1 && + file->recs[i - 1].ptr[size - 2] == '\r'; } static int is_cr_needed(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m) @@ -334,22 +334,22 @@ static int recmatch(xrecord_t *rec1, xrecord_t *rec2, unsigned long flags) static void xdl_refine_zdiff3_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m, xpparam_t const *xpp) { - xrecord_t **rec1 = xe1->xdf2.recs, **rec2 = xe2->xdf2.recs; + xrecord_t *rec1 = xe1->xdf2.recs, *rec2 = xe2->xdf2.recs; for (; m; m = m->next) { /* let's handle just the conflicts */ if (m->mode) continue; while(m->chg1 && m->chg2 && - recmatch(rec1[m->i1], rec2[m->i2], xpp->flags)) { + recmatch(&rec1[m->i1], &rec2[m->i2], xpp->flags)) { m->chg1--; m->chg2--; m->i1++; m->i2++; } while (m->chg1 && m->chg2 && - recmatch(rec1[m->i1 + m->chg1 - 1], - rec2[m->i2 + m->chg2 - 1], xpp->flags)) { + recmatch(&rec1[m->i1 + m->chg1 - 1], + &rec2[m->i2 + m->chg2 - 1], xpp->flags)) { m->chg1--; m->chg2--; } @@ -381,12 +381,12 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m, * This probably does not work outside git, since * we have a very simple mmfile structure. */ - t1.ptr = (char *)xe1->xdf2.recs[m->i1]->ptr; - t1.size = xe1->xdf2.recs[m->i1 + m->chg1 - 1]->ptr - + xe1->xdf2.recs[m->i1 + m->chg1 - 1]->size - t1.ptr; - t2.ptr = (char *)xe2->xdf2.recs[m->i2]->ptr; - t2.size = xe2->xdf2.recs[m->i2 + m->chg2 - 1]->ptr - + xe2->xdf2.recs[m->i2 + m->chg2 - 1]->size - t2.ptr; + t1.ptr = (char *)xe1->xdf2.recs[m->i1].ptr; + t1.size = xe1->xdf2.recs[m->i1 + m->chg1 - 1].ptr + + xe1->xdf2.recs[m->i1 + m->chg1 - 1].size - t1.ptr; + t2.ptr = (char *)xe2->xdf2.recs[m->i2].ptr; + t2.size = xe2->xdf2.recs[m->i2 + m->chg2 - 1].ptr + + xe2->xdf2.recs[m->i2 + m->chg2 - 1].size - t2.ptr; if (xdl_do_diff(&t1, &t2, xpp, &xe) < 0) return -1; if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 || @@ -440,8 +440,8 @@ static int line_contains_alnum(const char *ptr, long size) static int lines_contain_alnum(xdfenv_t *xe, int i, int chg) { for (; chg; chg--, i++) - if (line_contains_alnum(xe->xdf2.recs[i]->ptr, - xe->xdf2.recs[i]->size)) + if (line_contains_alnum(xe->xdf2.recs[i].ptr, + xe->xdf2.recs[i].size)) return 1; return 0; } diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index 77dc411d19..bf69a58527 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -88,9 +88,9 @@ static int is_anchor(xpparam_t const *xpp, const char *line) static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map, int pass) { - xrecord_t **records = pass == 1 ? + xrecord_t *records = pass == 1 ? map->env->xdf1.recs : map->env->xdf2.recs; - xrecord_t *record = records[line - 1]; + xrecord_t *record = &records[line - 1]; /* * After xdl_prepare_env() (or more precisely, due to * xdl_classify_record()), the "ha" member of the records (AKA lines) @@ -121,7 +121,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map, return; map->entries[index].line1 = line; map->entries[index].hash = record->ha; - map->entries[index].anchor = is_anchor(xpp, map->env->xdf1.recs[line - 1]->ptr); + map->entries[index].anchor = is_anchor(xpp, map->env->xdf1.recs[line - 1].ptr); if (!map->first) map->first = map->entries + index; if (map->last) { @@ -246,8 +246,8 @@ static int find_longest_common_sequence(struct hashmap *map, struct entry **res) 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]; + xrecord_t *record1 = &map->env->xdf1.recs[line1 - 1]; + xrecord_t *record2 = &map->env->xdf2.recs[line2 - 1]; return record1->ha == record2->ha; } diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index e6e2c0e1c0..27c5a4d636 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -128,7 +128,6 @@ static void xdl_free_ctx(xdfile_t *xdf) xdl_free(xdf->rindex); xdl_free(xdf->rchg - 1); xdl_free(xdf->recs); - xdl_cha_free(&xdf->rcha); } @@ -143,8 +142,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xdf->rchg = NULL; xdf->recs = NULL; - if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) - goto abort; if (!XDL_ALLOC_ARRAY(xdf->recs, narec)) goto abort; @@ -155,12 +152,10 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ hav = xdl_hash_record(&cur, top, xpp->flags); if (XDL_ALLOC_GROW(xdf->recs, xdf->nrec + 1, narec)) goto abort; - if (!(crec = xdl_cha_alloc(&xdf->rcha))) - goto abort; + crec = &xdf->recs[xdf->nrec++]; crec->ptr = prev; crec->size = (long) (cur - prev); crec->ha = hav; - xdf->recs[xdf->nrec++] = crec; if (xdl_classify_record(pass, cf, crec) < 0) goto abort; } @@ -260,7 +255,7 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) { */ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2) { long i, nm, nreff, mlim; - xrecord_t **recs; + xrecord_t *recs; xdlclass_t *rcrec; char *dis, *dis1, *dis2; int need_min = !!(cf->flags & XDF_NEED_MINIMAL); @@ -273,7 +268,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->ha]; nm = rcrec ? rcrec->len2 : 0; dis1[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1; } @@ -281,7 +276,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->ha]; nm = rcrec ? rcrec->len1 : 0; dis2[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1; } @@ -317,13 +312,13 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd */ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) { long i, lim; - xrecord_t **recs1, **recs2; + xrecord_t *recs1, *recs2; recs1 = xdf1->recs; 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->ha != recs2->ha) break; xdf1->dstart = xdf2->dstart = i; @@ -331,7 +326,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->ha != recs2->ha) break; xdf1->dend = xdf1->nrec - i - 1; diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h index 85848f1685..3d26cbf1ec 100644 --- a/xdiff/xtypes.h +++ b/xdiff/xtypes.h @@ -45,10 +45,9 @@ typedef struct s_xrecord { } xrecord_t; typedef struct s_xdfile { - chastore_t rcha; + xrecord_t *recs; long nrec; long dstart, dend; - xrecord_t **recs; char *rchg; long *rindex; long nreff; diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 444a108f87..332982b509 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -416,12 +416,12 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp, mmfile_t subfile1, subfile2; xdfenv_t env; - subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1]->ptr; - subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2]->ptr + - diff_env->xdf1.recs[line1 + count1 - 2]->size - subfile1.ptr; - subfile2.ptr = (char *)diff_env->xdf2.recs[line2 - 1]->ptr; - subfile2.size = diff_env->xdf2.recs[line2 + count2 - 2]->ptr + - diff_env->xdf2.recs[line2 + count2 - 2]->size - subfile2.ptr; + subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1].ptr; + subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2].ptr + + diff_env->xdf1.recs[line1 + count1 - 2].size - subfile1.ptr; + subfile2.ptr = (char *)diff_env->xdf2.recs[line2 - 1].ptr; + subfile2.size = diff_env->xdf2.recs[line2 + count2 - 2].ptr + + diff_env->xdf2.recs[line2 + count2 - 2].size - subfile2.ptr; if (xdl_do_diff(&subfile1, &subfile2, xpp, &env) < 0) return -1; From b7de64a6d6f58953a0c1dc7ff34f7080b3e38b37 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:57 +0000 Subject: [PATCH 10/12] xdiff: rename rchg -> changed in xdfile_t The field rchg (now 'changed') declares if a line in a file is changed or not. A later commit will change it's type from 'char' to 'bool' to make its purpose even more clear. Best-viewed-with: --color-words Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 30 +++++++++++++++--------------- xdiff/xhistogram.c | 8 ++++---- xdiff/xpatience.c | 8 ++++---- xdiff/xprepare.c | 12 ++++++------ xdiff/xtypes.h | 2 +- xdiff/xutils.c | 4 ++-- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index a66125d44a..bd5b31c664 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -278,10 +278,10 @@ int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1, */ if (off1 == lim1) { for (; off2 < lim2; off2++) - xdf2->rchg[xdf2->rindex[off2]] = 1; + xdf2->changed[xdf2->rindex[off2]] = 1; } else if (off2 == lim2) { for (; off1 < lim1; off1++) - xdf1->rchg[xdf1->rindex[off1]] = 1; + xdf1->changed[xdf1->rindex[off1]] = 1; } else { xdpsplit_t spl; spl.i1 = spl.i2 = 0; @@ -708,7 +708,7 @@ struct xdlgroup { static void group_init(xdfile_t *xdf, struct xdlgroup *g) { g->start = g->end = 0; - while (xdf->rchg[g->end]) + while (xdf->changed[g->end]) g->end++; } @@ -722,7 +722,7 @@ static inline int group_next(xdfile_t *xdf, struct xdlgroup *g) return -1; g->start = g->end + 1; - for (g->end = g->start; xdf->rchg[g->end]; g->end++) + for (g->end = g->start; xdf->changed[g->end]; g->end++) ; return 0; @@ -738,7 +738,7 @@ static inline int group_previous(xdfile_t *xdf, struct xdlgroup *g) return -1; g->end = g->start - 1; - for (g->start = g->end; xdf->rchg[g->start - 1]; g->start--) + for (g->start = g->end; xdf->changed[g->start - 1]; g->start--) ; return 0; @@ -753,10 +753,10 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g) { if (g->end < xdf->nrec && recs_match(&xdf->recs[g->start], &xdf->recs[g->end])) { - xdf->rchg[g->start++] = 0; - xdf->rchg[g->end++] = 1; + xdf->changed[g->start++] = 0; + xdf->changed[g->end++] = 1; - while (xdf->rchg[g->end]) + while (xdf->changed[g->end]) g->end++; return 0; @@ -774,10 +774,10 @@ static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g) { if (g->start > 0 && recs_match(&xdf->recs[g->start - 1], &xdf->recs[g->end - 1])) { - xdf->rchg[--g->start] = 1; - xdf->rchg[--g->end] = 0; + xdf->changed[--g->start] = 1; + xdf->changed[--g->end] = 0; - while (xdf->rchg[g->start - 1]) + while (xdf->changed[g->start - 1]) g->start--; return 0; @@ -932,16 +932,16 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) { int xdl_build_script(xdfenv_t *xe, xdchange_t **xscr) { xdchange_t *cscr = NULL, *xch; - char *rchg1 = xe->xdf1.rchg, *rchg2 = xe->xdf2.rchg; + char *changed1 = xe->xdf1.changed, *changed2 = xe->xdf2.changed; long i1, i2, l1, l2; /* * Trivial. Collects "groups" of changes and creates an edit script. */ for (i1 = xe->xdf1.nrec, i2 = xe->xdf2.nrec; i1 >= 0 || i2 >= 0; i1--, i2--) - if (rchg1[i1 - 1] || rchg2[i2 - 1]) { - for (l1 = i1; rchg1[i1 - 1]; i1--); - for (l2 = i2; rchg2[i2 - 1]; i2--); + if (changed1[i1 - 1] || changed2[i2 - 1]) { + for (l1 = i1; changed1[i1 - 1]; i1--); + for (l2 = i2; changed2[i2 - 1]; i2--); if (!(xch = xdl_add_change(cscr, i1, i2, l1 - i1, l2 - i2))) { xdl_free_script(cscr); diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index 4d857e8ae2..15ca15f6b0 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -318,11 +318,11 @@ redo: if (!count1) { while(count2--) - env->xdf2.rchg[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = 1; return 0; } else if (!count2) { while(count1--) - env->xdf1.rchg[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = 1; return 0; } @@ -335,9 +335,9 @@ redo: else { if (lcs.begin1 == 0 && lcs.begin2 == 0) { while (count1--) - env->xdf1.rchg[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = 1; while (count2--) - env->xdf2.rchg[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = 1; result = 0; } else { result = histogram_diff(xpp, env, diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index bf69a58527..14092ffb86 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -331,11 +331,11 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env, /* trivial case: one side is empty */ if (!count1) { while(count2--) - env->xdf2.rchg[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = 1; return 0; } else if (!count2) { while(count1--) - env->xdf1.rchg[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = 1; return 0; } @@ -347,9 +347,9 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env, /* are there any matching lines at all? */ if (!map.has_matches) { while(count1--) - env->xdf1.rchg[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = 1; while(count2--) - env->xdf2.rchg[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = 1; xdl_free(map.entries); return 0; } diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 27c5a4d636..b9b19c36de 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -126,7 +126,7 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t static void xdl_free_ctx(xdfile_t *xdf) { xdl_free(xdf->rindex); - xdl_free(xdf->rchg - 1); + xdl_free(xdf->changed - 1); xdl_free(xdf->recs); } @@ -139,7 +139,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xrecord_t *crec; xdf->rindex = NULL; - xdf->rchg = NULL; + xdf->changed = NULL; xdf->recs = NULL; if (!XDL_ALLOC_ARRAY(xdf->recs, narec)) @@ -161,7 +161,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ } } - if (!XDL_CALLOC_ARRAY(xdf->rchg, xdf->nrec + 2)) + if (!XDL_CALLOC_ARRAY(xdf->changed, xdf->nrec + 2)) goto abort; if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && @@ -170,7 +170,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ goto abort; } - xdf->rchg += 1; + xdf->changed += 1; xdf->nreff = 0; xdf->dstart = 0; xdf->dend = xdf->nrec - 1; @@ -287,7 +287,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd (dis1[i] == 2 && !xdl_clean_mmatch(dis1, i, xdf1->dstart, xdf1->dend))) { xdf1->rindex[nreff++] = i; } else - xdf1->rchg[i] = 1; + xdf1->changed[i] = 1; } xdf1->nreff = nreff; @@ -297,7 +297,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd (dis2[i] == 2 && !xdl_clean_mmatch(dis2, i, xdf2->dstart, xdf2->dend))) { xdf2->rindex[nreff++] = i; } else - xdf2->rchg[i] = 1; + xdf2->changed[i] = 1; } xdf2->nreff = nreff; diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h index 3d26cbf1ec..c4b5d2d8fa 100644 --- a/xdiff/xtypes.h +++ b/xdiff/xtypes.h @@ -48,7 +48,7 @@ typedef struct s_xdfile { xrecord_t *recs; long nrec; long dstart, dend; - char *rchg; + char *changed; long *rindex; long nreff; } xdfile_t; diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 332982b509..ed65c222e6 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -425,8 +425,8 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp, if (xdl_do_diff(&subfile1, &subfile2, xpp, &env) < 0) return -1; - memcpy(diff_env->xdf1.rchg + line1 - 1, env.xdf1.rchg, count1); - memcpy(diff_env->xdf2.rchg + line2 - 1, env.xdf2.rchg, count2); + memcpy(diff_env->xdf1.changed + line1 - 1, env.xdf1.changed, count1); + memcpy(diff_env->xdf2.changed + line2 - 1, env.xdf2.changed, count2); xdl_free_env(&env); From e385e1b7d2d7f531a0006131e7f1d974de351df5 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:58 +0000 Subject: [PATCH 11/12] xdiff: add macros DISCARD(0), KEEP(1), INVESTIGATE(2) in xprepare.c This commit is refactor-only; no behavior is changed. A future commit will use bool literals for changed[i]. The functions xdl_clean_mmatch() and xdl_cleanup_records() will be cleaned up more in a future patch series. The changes to xdl_cleanup_records(), in this patch, are just to make it clear why `char rchg` is refactored to `bool changed`. Rename dis* to action* and replace literal numericals with macros. The old names came from when dis* (which I think was short for discard) was treated like a boolean, but over time it grew into a ternary state machine. The result was confusing because dis* and rchg* both used 0/1 values with different meanings. The new names and macros make the states explicit. nm is short for number of matches, and mlim is a heuristic limit: nm == 0 -> action[i] = DISCARD -> changed[i] = true 0 < nm < mlim -> action[i] = KEEP -> changed[i] = false nm >= mlim -> action[i] = INVESTIGATE -> changed[i] = xdl_clean_mmatch() When need_min is true, only DISCARD and KEEP occur because the limit is effectively infinite. Best-viewed-with: --color-words Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 106 ++++++++++++++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 37 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index b9b19c36de..55e3b50ce6 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -29,6 +29,9 @@ #define XDL_GUESS_NLINES1 256 #define XDL_GUESS_NLINES2 20 +#define DISCARD 0 +#define KEEP 1 +#define INVESTIGATE 2 typedef struct s_xdlclass { struct s_xdlclass *next; @@ -190,15 +193,15 @@ void xdl_free_env(xdfenv_t *xe) { } -static int xdl_clean_mmatch(char const *dis, long i, long s, long e) { +static bool xdl_clean_mmatch(uint8_t const *action, long i, long s, long e) { long r, rdis0, rpdis0, rdis1, rpdis1; /* - * Limits the window the is examined during the similar-lines - * scan. The loops below stops when dis[i - r] == 1 (line that - * has no match), but there are corner cases where the loop - * proceed all the way to the extremities by causing huge - * performance penalties in case of big files. + * Limits the window that is examined during the similar-lines + * scan. The loops below stops when action[i - r] == KEEP + * (line that has no match), but there are corner cases where + * the loop proceed all the way to the extremities by causing + * huge performance penalties in case of big files. */ if (i - s > XDL_SIMSCAN_WINDOW) s = i - XDL_SIMSCAN_WINDOW; @@ -207,40 +210,47 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) { /* * Scans the lines before 'i' to find a run of lines that either - * have no match (dis[j] == 0) or have multiple matches (dis[j] > 1). - * Note that we always call this function with dis[i] > 1, so the - * current line (i) is already a multimatch line. + * have no match (action[j] == DISCARD) or have multiple matches + * (action[j] == INVESTIGATE). Note that we always call this + * function with action[i] == INVESTIGATE, so the current line + * (i) is already a multimatch line. */ for (r = 1, rdis0 = 0, rpdis0 = 1; (i - r) >= s; r++) { - if (!dis[i - r]) + if (action[i - r] == DISCARD) rdis0++; - else if (dis[i - r] == 2) + else if (action[i - r] == INVESTIGATE) rpdis0++; - else + else if (action[i - r] == KEEP) break; + else + BUG("Illegal value for action[i - r]"); } /* - * If the run before the line 'i' found only multimatch lines, we - * return 0 and hence we don't make the current line (i) discarded. - * We want to discard multimatch lines only when they appear in the - * middle of runs with nomatch lines (dis[j] == 0). + * If the run before the line 'i' found only multimatch lines, + * we return false and hence we don't make the current line (i) + * discarded. We want to discard multimatch lines only when + * they appear in the middle of runs with nomatch lines + * (action[j] == DISCARD). */ if (rdis0 == 0) return 0; for (r = 1, rdis1 = 0, rpdis1 = 1; (i + r) <= e; r++) { - if (!dis[i + r]) + if (action[i + r] == DISCARD) rdis1++; - else if (dis[i + r] == 2) + else if (action[i + r] == INVESTIGATE) rpdis1++; - else + else if (action[i + r] == KEEP) break; + else + BUG("Illegal value for action[i + r]"); } /* - * If the run after the line 'i' found only multimatch lines, we - * return 0 and hence we don't make the current line (i) discarded. + * If the run after the line 'i' found only multimatch lines, + * we return false and hence we don't make the current line (i) + * discarded. */ if (rdis1 == 0) - return 0; + return false; rdis1 += rdis0; rpdis1 += rpdis0; @@ -251,26 +261,38 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) { /* * Try to reduce the problem complexity, discard records that have no * matches on the other file. Also, lines that have multiple matches - * might be potentially discarded if they happear in a run of discardable. + * might be potentially discarded if they appear in a run of discardable. */ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2) { long i, nm, nreff, mlim; xrecord_t *recs; xdlclass_t *rcrec; - char *dis, *dis1, *dis2; - int need_min = !!(cf->flags & XDF_NEED_MINIMAL); + uint8_t *action1 = NULL, *action2 = NULL; + bool need_min = !!(cf->flags & XDF_NEED_MINIMAL); + int ret = 0; - if (!XDL_CALLOC_ARRAY(dis, xdf1->nrec + xdf2->nrec + 2)) - return -1; - dis1 = dis; - dis2 = dis1 + xdf1->nrec + 1; + /* + * Create temporary arrays that will help us decide if + * changed[i] should remain 0 or become 1. + */ + if (!XDL_CALLOC_ARRAY(action1, xdf1->nrec + 1)) { + ret = -1; + goto cleanup; + } + if (!XDL_CALLOC_ARRAY(action2, xdf2->nrec + 1)) { + ret = -1; + goto cleanup; + } + /* + * Initialize temporary arrays with DISCARD, KEEP, or INVESTIGATE. + */ 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]; nm = rcrec ? rcrec->len2 : 0; - dis1[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1; + action1[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP; } if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT) @@ -278,32 +300,42 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) { rcrec = cf->rcrecs[recs->ha]; nm = rcrec ? rcrec->len1 : 0; - dis2[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1; + action2[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP; } + /* + * Use temporary arrays to decide if changed[i] should remain + * 0 or become 1. + */ for (nreff = 0, i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) { - if (dis1[i] == 1 || - (dis1[i] == 2 && !xdl_clean_mmatch(dis1, i, xdf1->dstart, xdf1->dend))) { + if (action1[i] == KEEP || + (action1[i] == INVESTIGATE && !xdl_clean_mmatch(action1, i, xdf1->dstart, xdf1->dend))) { xdf1->rindex[nreff++] = i; + /* changed[i] remains 0, i.e. keep */ } else xdf1->changed[i] = 1; + /* i.e. discard */ } xdf1->nreff = nreff; for (nreff = 0, i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) { - if (dis2[i] == 1 || - (dis2[i] == 2 && !xdl_clean_mmatch(dis2, i, xdf2->dstart, xdf2->dend))) { + if (action2[i] == KEEP || + (action2[i] == INVESTIGATE && !xdl_clean_mmatch(action2, i, xdf2->dstart, xdf2->dend))) { xdf2->rindex[nreff++] = i; + /* changed[i] remains 0, i.e. keep */ } else xdf2->changed[i] = 1; + /* i.e. discard */ } xdf2->nreff = nreff; - xdl_free(dis); +cleanup: + xdl_free(action1); + xdl_free(action2); - return 0; + return ret; } From 8b9c5d2e3a38b6e0c2278fe10fe2a4bf34496a9d Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:59 +0000 Subject: [PATCH 12/12] xdiff: change type of xdfile_t.changed from char to bool The only values possible for 'changed' is 1 and 0, which exactly maps to a bool type. It might not look like this because action1 and action2 (which use to be dis1, and dis2) were also of type char and were assigned numerical values within a few lines of 'changed' (what used to be rchg). Using DISCARD/KEEP/INVESTIGATE for action1[i]/action2[j], and true/false for changed[k] makes it clear to future readers that these are logically separate concepts. Best-viewed-with: --color-words Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 14 +++++++------- xdiff/xhistogram.c | 8 ++++---- xdiff/xpatience.c | 8 ++++---- xdiff/xprepare.c | 12 ++++++------ xdiff/xtypes.h | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index bd5b31c664..6f3998ee54 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -278,10 +278,10 @@ int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1, */ if (off1 == lim1) { for (; off2 < lim2; off2++) - xdf2->changed[xdf2->rindex[off2]] = 1; + xdf2->changed[xdf2->rindex[off2]] = true; } else if (off2 == lim2) { for (; off1 < lim1; off1++) - xdf1->changed[xdf1->rindex[off1]] = 1; + xdf1->changed[xdf1->rindex[off1]] = true; } else { xdpsplit_t spl; spl.i1 = spl.i2 = 0; @@ -753,8 +753,8 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g) { if (g->end < xdf->nrec && recs_match(&xdf->recs[g->start], &xdf->recs[g->end])) { - xdf->changed[g->start++] = 0; - xdf->changed[g->end++] = 1; + xdf->changed[g->start++] = false; + xdf->changed[g->end++] = true; while (xdf->changed[g->end]) g->end++; @@ -774,8 +774,8 @@ static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g) { if (g->start > 0 && recs_match(&xdf->recs[g->start - 1], &xdf->recs[g->end - 1])) { - xdf->changed[--g->start] = 1; - xdf->changed[--g->end] = 0; + xdf->changed[--g->start] = true; + xdf->changed[--g->end] = false; while (xdf->changed[g->start - 1]) g->start--; @@ -932,7 +932,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) { int xdl_build_script(xdfenv_t *xe, xdchange_t **xscr) { xdchange_t *cscr = NULL, *xch; - char *changed1 = xe->xdf1.changed, *changed2 = xe->xdf2.changed; + bool *changed1 = xe->xdf1.changed, *changed2 = xe->xdf2.changed; long i1, i2, l1, l2; /* diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index 15ca15f6b0..6dc450b1fe 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -318,11 +318,11 @@ redo: if (!count1) { while(count2--) - env->xdf2.changed[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = true; return 0; } else if (!count2) { while(count1--) - env->xdf1.changed[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = true; return 0; } @@ -335,9 +335,9 @@ redo: else { if (lcs.begin1 == 0 && lcs.begin2 == 0) { while (count1--) - env->xdf1.changed[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = true; while (count2--) - env->xdf2.changed[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = true; result = 0; } else { result = histogram_diff(xpp, env, diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index 14092ffb86..669b653580 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -331,11 +331,11 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env, /* trivial case: one side is empty */ if (!count1) { while(count2--) - env->xdf2.changed[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = true; return 0; } else if (!count2) { while(count1--) - env->xdf1.changed[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = true; return 0; } @@ -347,9 +347,9 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env, /* are there any matching lines at all? */ if (!map.has_matches) { while(count1--) - env->xdf1.changed[line1++ - 1] = 1; + env->xdf1.changed[line1++ - 1] = true; while(count2--) - env->xdf2.changed[line2++ - 1] = 1; + env->xdf2.changed[line2++ - 1] = true; xdl_free(map.entries); return 0; } diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 55e3b50ce6..192334f1b7 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -273,7 +273,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd /* * Create temporary arrays that will help us decide if - * changed[i] should remain 0 or become 1. + * changed[i] should remain false, or become true. */ if (!XDL_CALLOC_ARRAY(action1, xdf1->nrec + 1)) { ret = -1; @@ -305,16 +305,16 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd /* * Use temporary arrays to decide if changed[i] should remain - * 0 or become 1. + * false, or become true. */ for (nreff = 0, i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) { if (action1[i] == KEEP || (action1[i] == INVESTIGATE && !xdl_clean_mmatch(action1, i, xdf1->dstart, xdf1->dend))) { xdf1->rindex[nreff++] = i; - /* changed[i] remains 0, i.e. keep */ + /* changed[i] remains false, i.e. keep */ } else - xdf1->changed[i] = 1; + xdf1->changed[i] = true; /* i.e. discard */ } xdf1->nreff = nreff; @@ -324,9 +324,9 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd if (action2[i] == KEEP || (action2[i] == INVESTIGATE && !xdl_clean_mmatch(action2, i, xdf2->dstart, xdf2->dend))) { xdf2->rindex[nreff++] = i; - /* changed[i] remains 0, i.e. keep */ + /* changed[i] remains false, i.e. keep */ } else - xdf2->changed[i] = 1; + xdf2->changed[i] = true; /* i.e. discard */ } xdf2->nreff = nreff; diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h index c4b5d2d8fa..f145abba3e 100644 --- a/xdiff/xtypes.h +++ b/xdiff/xtypes.h @@ -48,7 +48,7 @@ typedef struct s_xdfile { xrecord_t *recs; long nrec; long dstart, dend; - char *changed; + bool *changed; long *rindex; long nreff; } xdfile_t;