From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 15 Jun 2018 18:56:43 GMT
Subject: Re: [PATCH v2 0/7] Delete unused methods in EWAH bitmap
Message-ID: <ee8a1b8f-d14e-218c-3bdf-82908fefbec0@gmail.com>
In-Reply-To: <xmqq1sd7288z.fsf@gitster-ct.c.googlers.com>

On 6/15/2018 2:51 PM, Junio C Hamano wrote:
> Derrick Stolee <dstolee@microsoft.com> writes:
>
>> The EWAH bitmap code includes several logical operations that are
>> important for a general-purpose bitmap library. However, Git only
>> uses the XOR operation for storing deltas between reachability
>> bitmaps. This means that we can delete the following unused methods:
>>
>> * ewah_and()
>> * ewah_and_not()
>> * ewah_not()
>> * ewah_or()
>> * ewah_serialize()
>>
>> We can also delete the unused methods bitmap_clear() and
>> bitmap_each_bit().
>>
>> Derrick Stolee (7):
>>    ewah/bitmap.c: delete unused 'bitmap_clear()'
>>    ewah/bitmap.c: delete unused 'bitmap_each_bit()'
>>    ewah_bitmap: delete unused 'ewah_and()'
>>    ewah_bitmap: delete unused 'ewah_and_not()'
>>    ewah_bitmap: delete unused 'ewah_not()'
>>    ewah_bitmap: delete unused 'ewah_or()'
>>    ewah_io: delete unused 'ewah_serialize()'
>>
>>   ewah/bitmap.c      |  32 -------
>>   ewah/ewah_bitmap.c | 229 ---------------------------------------------
>>   ewah/ewah_io.c     |  10 --
>>   ewah/ewok.h        |  25 -----
>>   4 files changed, 296 deletions(-)
>>
>>
>> base-commit: fc54c1af3ec09bab8b8ea09768c2da4069b7f53e
> Thanks.
>
> ewah_clear() can become file-scope static, and
> rlwit_discharge_empty() can be eliminated.  I do not know if either
> is worth doing, though.

With Peff's patches, this is true. When I applied your diff to my patch 
alone we could not do that.

If you want to create a commit with the below, I'm happy to add my 
"Reviewed-by: Derrick Stolee <dstolee@microsoft.com>"

Good team effort today!

>
>   ewah/ewah_bitmap.c | 20 ++++++++++++--------
>   ewah/ewah_rlw.c    |  8 --------
>   ewah/ewok.h        |  6 ------
>   ewah/ewok_rlw.h    |  1 -
>   4 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
> index 017c677f98..d59b1afe3d 100644
> --- a/ewah/ewah_bitmap.c
> +++ b/ewah/ewah_bitmap.c
> @@ -276,6 +276,18 @@ void ewah_each_bit(struct ewah_bitmap *self, void (*callback)(size_t, void*), vo
>   	}
>   }
>   
> +/**
> + * Clear all the bits in the bitmap. Does not free or resize
> + * memory.
> + */
> +static void ewah_clear(struct ewah_bitmap *self)
> +{
> +	self->buffer_size = 1;
> +	self->buffer[0] = 0;
> +	self->bit_size = 0;
> +	self->rlw = self->buffer;
> +}
> +
>   struct ewah_bitmap *ewah_new(void)
>   {
>   	struct ewah_bitmap *self;
> @@ -288,14 +300,6 @@ struct ewah_bitmap *ewah_new(void)
>   	return self;
>   }
>   
> -void ewah_clear(struct ewah_bitmap *self)
> -{
> -	self->buffer_size = 1;
> -	self->buffer[0] = 0;
> -	self->bit_size = 0;
> -	self->rlw = self->buffer;
> -}
> -
>   void ewah_free(struct ewah_bitmap *self)
>   {
>   	if (!self)
> diff --git a/ewah/ewah_rlw.c b/ewah/ewah_rlw.c
> index b9643b7d0f..5093d43e2f 100644
> --- a/ewah/ewah_rlw.c
> +++ b/ewah/ewah_rlw.c
> @@ -104,11 +104,3 @@ size_t rlwit_discharge(
>   
>   	return index;
>   }
> -
> -void rlwit_discharge_empty(struct rlw_iterator *it, struct ewah_bitmap *out)
> -{
> -	while (rlwit_word_size(it) > 0) {
> -		ewah_add_empty_words(out, 0, rlwit_word_size(it));
> -		rlwit_discard_first_words(it, rlwit_word_size(it));
> -	}
> -}
> diff --git a/ewah/ewok.h b/ewah/ewok.h
> index 0c504f28e2..84b2a29faa 100644
> --- a/ewah/ewok.h
> +++ b/ewah/ewok.h
> @@ -72,12 +72,6 @@ void ewah_pool_free(struct ewah_bitmap *self);
>    */
>   struct ewah_bitmap *ewah_new(void);
>   
> -/**
> - * Clear all the bits in the bitmap. Does not free or resize
> - * memory.
> - */
> -void ewah_clear(struct ewah_bitmap *self);
> -
>   /**
>    * Free all the memory of the bitmap
>    */
> diff --git a/ewah/ewok_rlw.h b/ewah/ewok_rlw.h
> index bb3c6ff7e0..7cdfdd0c02 100644
> --- a/ewah/ewok_rlw.h
> +++ b/ewah/ewok_rlw.h
> @@ -98,7 +98,6 @@ void rlwit_init(struct rlw_iterator *it, struct ewah_bitmap *bitmap);
>   void rlwit_discard_first_words(struct rlw_iterator *it, size_t x);
>   size_t rlwit_discharge(
>   	struct rlw_iterator *it, struct ewah_bitmap *out, size_t max, int negate);
> -void rlwit_discharge_empty(struct rlw_iterator *it, struct ewah_bitmap *out);
>   
>   static inline size_t rlwit_word_size(struct rlw_iterator *it)
>   {
>
>