Re: [PATCH 3/3] ewah: drop ewah_serialize_native function

From: Jeff King <peff@peff.net>
Date: Jun 15, 2018, 17:51 UTC
Message-ID: <20180615175111.GD3067@sigill.intra.peff.net>
In-Reply-To: <ab6378a8-3728-7fa8-da08-b58ec768dbfe@gmail.com>

On Fri, Jun 15, 2018 at 10:15:50AM -0400, Derrick Stolee wrote:

Show 16 quoted lines

> > Hmm, if you are in the mood to drop ewah dead code, how about:
> > 
> >    ewah/bitmap.o   - bitmap_clear
> >    ewah/bitmap.o   - bitmap_each_bit
> >    ewah/ewah_bitmap.o      - ewah_and
> >    ewah/ewah_bitmap.o      - ewah_and_not
> >    ewah/ewah_bitmap.o      - ewah_not
> >    ewah/ewah_bitmap.o      - ewah_or
> > 
> > ... in addition to these *(de)serialize* functions. ;-)
> > 
> 
> Normally, I would say we should keep this folder as close to the "original"
> [1] as possible, so we could more easily take improvements to that library.
> However, it appears that code is not being updated. Perhaps this is in favor
> of other EWAH libraries [2] or other compressed bitmap formats [3].

The ewok library was written specifically for Git, as a port of Daniel Lemire's original EWAH work, since at the time there was no pure-C implementation of EWAH (that might even still be the case -- I don't know).

We pulled it out into a separate library with the thought that maybe other people would find it useful. But I don't specifically know of any other users of the library, nor do I think Vicent has spent any time on it since writing it for Git (I know I haven't).

So yeah, I think it is fine to just consider this "our" code and chop it up as we see fit (and you can see we've already tweaked it for things like allocating consistently with our other code).

One thing that gave me pause on ripping out more code is that I have some bitmap-related patches on my send-to-upstream list, and I wasn't sure if they used any of this code. But I checked against your patches, and no, this can all go (which makes sense -- my patches are about using .bitmap files in more places, so they build at a higher level).

So your patches look good to me, modulo the declarations that Ramsay noticed should be removed, too.

-Peff