From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Sat, 17 Jan 2026 16:34:06 GMT
Subject: Re: [PATCH 09/10] xdiff: remove dependence on xdlclassifier from xdl_cleanup_records()
Message-ID: <CAH=ZcbDw0_Od3+zuGLsy3Z=bLR-4ByH8Fguiuw_MyLTi=U7gcQ@mail.gmail.com>
In-Reply-To: <07ca298a-ad32-4998-88ff-d69c04418fdd@web.de>

On Fri, Jan 16, 2026 at 1:19 PM René Scharfe <l.s.r@web.de> wrote:
>
> On 1/2/26 7:52 PM, Ezekiel Newren via GitGitGadget wrote:
> > @@ -253,22 +250,44 @@ static bool xdl_clean_mmatch(uint8_t const *action, long i, long s, long e) {
> >       return rpdis1 * XDL_KPDIS_RUN < (rpdis1 + rdis1);
> >  }
> >
> > +struct xoccurrence
> > +{
> > +     size_t file1, file2;
> > +};
> > +
> > +
> > +DEFINE_IVEC_TYPE(struct xoccurrence, xoccurrence);
> > +
> >
> >  /*
> >   * 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 appear in a run of discardable.
> >   */
> > -static int xdl_cleanup_records(xdlclassifier_t *cf, xdfenv_t *xe) {
> > -     long i, nm, mlim;
> > +static int xdl_cleanup_records(xdfenv_t *xe, uint64_t flags) {
> > +     long i;
> > +     size_t nm, mlim;
> >       xrecord_t *recs;
> > -     xdlclass_t *rcrec;
> >       uint8_t *action1 = NULL, *action2 = NULL;
> > -     bool need_min = !!(cf->flags & XDF_NEED_MINIMAL);
> > +     struct IVec_xoccurrence occ;
> > +     bool need_min = !!(flags & XDF_NEED_MINIMAL);
> >       int ret = 0;
> >       ptrdiff_t dend1 = xe->xdf1.nrec - 1 - xe->delta_end;
> >       ptrdiff_t dend2 = xe->xdf2.nrec - 1 - xe->delta_end;
> >
> > +     IVEC_INIT(occ);
> > +     ivec_zero(&occ, xe->mph_size);
>
> This array is presized here.  It is neither grown nor shrunken.
> CALLOC_ARRAY would work just as well, at least at this point, no?
>
> > +
> > +     for (size_t j = 0; j < xe->xdf1.nrec; j++) {
> > +             size_t mph1 = xe->xdf1.recs[j].minimal_perfect_hash;
> > +             occ.ptr[mph1].file1 += 1;
> > +     }
> > +
> > +     for (size_t j = 0; j < xe->xdf2.nrec; j++) {
> > +             size_t mph2 = xe->xdf2.recs[j].minimal_perfect_hash;
> > +             occ.ptr[mph2].file2 += 1;
> > +     }
> > +
> >       /*
> >        * Create temporary arrays that will help us decide if
> >        * changed[i] should remain false, or become true.
> > @@ -288,16 +307,14 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfenv_t *xe) {
> >       if ((mlim = xdl_bogosqrt((long)xe->xdf1.nrec)) > XDL_MAX_EQLIMIT)
> >               mlim = XDL_MAX_EQLIMIT;
> >       for (i = xe->delta_start, recs = &xe->xdf1.recs[xe->delta_start]; i <= dend1; i++, recs++) {
> > -             rcrec = cf->rcrecs[recs->minimal_perfect_hash];
> > -             nm = rcrec ? rcrec->len2 : 0;
> > +             nm = occ.ptr[recs->minimal_perfect_hash].file2;
> >               action1[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP;
> >       }
> >
> >       if ((mlim = xdl_bogosqrt((long)xe->xdf2.nrec)) > XDL_MAX_EQLIMIT)
> >               mlim = XDL_MAX_EQLIMIT;
> >       for (i = xe->delta_start, recs = &xe->xdf2.recs[xe->delta_start]; i <= dend2; i++, recs++) {
> > -             rcrec = cf->rcrecs[recs->minimal_perfect_hash];
> > -             nm = rcrec ? rcrec->len1 : 0;
> > +             nm = occ.ptr[recs->minimal_perfect_hash].file1;
> >               action2[i] = (nm == 0) ? DISCARD: (nm >= mlim && !need_min) ? INVESTIGATE: KEEP;
> >       }
> >
> > @@ -332,6 +349,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfenv_t *xe) {
> >  cleanup:
> >       xdl_free(action1);
> >       xdl_free(action2);
> > +     ivec_free(&occ);
> >
> >       return ret;
> >  }

In Rust the memory management macros defined in git-compat-util.h will
not be available. ivec was built expressly to bridge the gap between C
and Rust. I'm avoiding using those macros because I'm trying to get C
programmers familiar with how Rust's Vec operates without forcing them
to read and write in Rust. Also, it makes converting from IVec to Vec
super easy.

ivec_zero() also sets length and capacity. Also CALLOC_ARRAY needs to
know the type of the pointer which ivec_zero() does not have access
to. This is one of the few ivec functions that does not have a direct
equivalent in Rust's Vec, but is faster than what is logically
equivalent in Rust.

In Rust the closest safe equivalent would look like:

let size = 35;
let mut vec = Vec::<u64>::new();
vec.reserve_exact(size);
vec.fill(0);  // requires that T implements the `Copy` trait

The unsafe version would look like:
let size = 35;
let mut vec = Vec::<u64>::new();
vec.reserve_exact(size);
unsafe {
    std::ptr::write_bytes(vec.as_mut_ptr(), 0, size * size_of::<u64>());
}