[PATCH v2 0/7] Delete unused methods in EWAH bitmap
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 -- 2.18.0.rc1
Re: [PATCH v2 0/7] Delete unused methods in EWAH bitmap
Derrick Stolee 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. 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) {
Re: [PATCH v2 0/7] Delete unused methods in EWAH bitmap
On 6/15/2018 2:51 PM, Junio C Hamano wrote: Derrick Stolee 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 " 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) {
Re: [PATCH v2 0/7] Delete unused methods in EWAH bitmap
Derrick Stolee writes: >> 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. Yeah, as I organized the patches on two topics in this order: jk/ewah-bounds-check (build on 'maint') Peff's 1/3 Peff's 4/3 ds/ewah-cleanup (build on top of the above) 7 patches in this series Peff's 2/3 Peff's 3/3 at the end, these two changes become possible. Again, I am not sure if these are worth doing, so I'll leave it out of these two series, at least for now. This is a tangent, but I too felt that roaring paper quite a pleasant read ;-)
Re: [PATCH v2 0/7] Delete unused methods in EWAH bitmap
On Fri, Jun 15, 2018 at 12:48:52PM -0700, Junio C Hamano wrote: > Derrick Stolee writes: > > >> 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. > > Yeah, as I organized the patches on two topics in this order: > > jk/ewah-bounds-check (build on 'maint') > Peff's 1/3 > Peff's 4/3 > > ds/ewah-cleanup (build on top of the above) > 7 patches in this series > Peff's 2/3 > Peff's 3/3 Thanks, I wasn't sure if I should resend mine on top, but it looks like you've got it all organized already. > at the end, these two changes become possible. Again, I am not sure > if these are worth doing, so I'll leave it out of these two series, > at least for now. IMHO the extra cleanups you showed are worth doing. -Peff