| Message ID | 0d52628985bf4f499e0d5622ea40cdf499e73ebe.1626812121.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pack-bitmap: clarify comment in filter_bitmap_exclude_type() | expand |
On Tue, Jul 20, 2021 at 04:16:22PM -0400, Taylor Blau wrote: > The code that eventually became filter_bitmap_exclude_type() was > originally introduced in 4f3bd5606a (pack-bitmap: implement BLOB_NONE > filtering, 2020-02-14) to accelerate BLOB_NONE filters with bitmaps. > > In 856e12c18a (pack-bitmap.c: make object filtering functions generic, > 2020-05-04), it became filter_bitmap_exclude_type(). But not all of the > comments were updated to be agnostic to the provided type. > > Remove the remaining comments which should have been updated in > 856e12c18a to reflect the type-agnostic nature of the function. > > Signed-off-by: Taylor Blau <me@ttaylorr.com> > --- > Noticed while I was reading some code in this area to see if we had > accelerated the relatively new type filters. This trivial fix hopefully > shouldn't produce conflicts with other in-flight bitmap series. Yep, this looks good. I wrote a near-identical patch while working on [1], but that topic got stalled on "how is this part of partial clones even supposed to work?". But there's no reason not to make this trivial fix in isolation. -Peff
On Tue, Jul 20, 2021 at 05:02:24PM -0400, Jeff King wrote: > Yep, this looks good. I wrote a near-identical patch while working on > [1], but that topic got stalled on "how is this part of partial clones > even supposed to work?". But there's no reason not to make this trivial > fix in isolation. I'm not sure exactly which thread [1] is referring to, but I do recall the discussion now that you mention it. I'm happy to add an "Original-patch-by" trailer or similar, although I suspect that you do not care ;). Thanks, Taylor
On Tue, Jul 20, 2021 at 05:05:43PM -0400, Taylor Blau wrote: > On Tue, Jul 20, 2021 at 05:02:24PM -0400, Jeff King wrote: > > Yep, this looks good. I wrote a near-identical patch while working on > > [1], but that topic got stalled on "how is this part of partial clones > > even supposed to work?". But there's no reason not to make this trivial > > fix in isolation. > > I'm not sure exactly which thread [1] is referring to, but I do recall > the discussion now that you mention it. Oops. :) It was the one with: https://lore.kernel.org/git/YJ4mUJ+EEAnudI3G@coredump.intra.peff.net/ > I'm happy to add an "Original-patch-by" trailer or similar, although I > suspect that you do not care ;). Nah, mostly just thinking out loud since it seemed so familiar. :) And hopefully producing the same patch independently is a form of endorsement. -Peff
diff --git a/pack-bitmap.c b/pack-bitmap.c index bfc10148f5..c8cb9ef13b 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -809,8 +809,9 @@ static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git, tips = find_tip_objects(bitmap_git, tip_objects, type); /* - * We can use the blob type-bitmap to work in whole words - * for the objects that are actually in the bitmapped packfile. + * We can use the type-level bitmap for 'type' to work in whole + * words for the objects that are actually in the bitmapped + * packfile. */ for (i = 0, init_type_iterator(&it, bitmap_git, type); i < to_filter->word_alloc && ewah_iterator_next(&mask, &it); @@ -821,9 +822,9 @@ static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git, } /* - * Clear any blobs that weren't in the packfile (and so would not have - * been caught by the loop above. We'll have to check them - * individually. + * Clear any objects that weren't in the packfile (and so would + * not have been caught by the loop above. We'll have to check + * them individually. */ for (i = 0; i < eindex->count; i++) { uint32_t pos = i + bitmap_git->pack->num_objects;
The code that eventually became filter_bitmap_exclude_type() was originally introduced in 4f3bd5606a (pack-bitmap: implement BLOB_NONE filtering, 2020-02-14) to accelerate BLOB_NONE filters with bitmaps. In 856e12c18a (pack-bitmap.c: make object filtering functions generic, 2020-05-04), it became filter_bitmap_exclude_type(). But not all of the comments were updated to be agnostic to the provided type. Remove the remaining comments which should have been updated in 856e12c18a to reflect the type-agnostic nature of the function. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- Noticed while I was reading some code in this area to see if we had accelerated the relatively new type filters. This trivial fix hopefully shouldn't produce conflicts with other in-flight bitmap series. pack-bitmap.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) -- 2.31.1.163.ga65ce7f831