Re: [PATCH v2 13/24] pack-bitmap: read multi-pack bitmaps

> On Wed, Jul 21, 2021 at 07:01:16PM -0400, Taylor Blau wrote:
> > > > +			if (bitmap_is_midx(bitmap_git)) {
> > > > +				/*
> > > > +				 * Can't reuse from a non-preferred pack (see
> > > > +				 * above).
> > > > +				 */
> > > > +				if (pos + offset >= objects_nr)
> > > > +					continue;
> > > > +			}
> > > > +			try_partial_reuse(bitmap_git, pack, pos + offset, reuse, &w_curs);
> > >
> > > ...and this likewise makes sure we never go past that first pack. Good.
> > >
> > > I think this "continue" could actually be a "break", as the loop is
> > > iterating over "offset" (and "pos + offset" always gets larger). In
> > > fact, it could break out of the outer loop as well (which is
> > > incrementing "pos"). It's probably a pretty small efficiency in
> > > practice, though.
> >
> > Yeah; you're right. And we'll save up to BITS_IN_EWORD cycles of this
> > loop. (I wonder if smart-enough compilers will realize the same
> > optimization that you did and turn that `continue` into a `break`
> > automatically, but that's neither here nor there).
>
> If you break all the way out, then it saves iterating over all of those
> other words that are not in the first pack, too. I.e., if your bitmap
> has 10 million bits (for a 10-million object clone), but your first pack
> only has a million objects in it, we'll call try_partial_reuse() 9
> million extra times.
>
> Fortunately, each call is super cheap, because the first thing it does
> is check if the requested bit is past the end of the pack. Which kind of
> makes me wonder if we could simplify this further by just letting
> try_partial_reuse() tell us when there's no point going further:
>
> [snip suggested diff]