Re: [PATCH v2 0/5] packfile: use oidset for bad objects

On Sat, Sep 11, 2021 at 11:51:36PM -0400, Derrick Stolee wrote:

On 9/11/21 10:29 PM, Taylor Blau wrote:

quoted

On Sat, Sep 11, 2021 at 09:51:04PM -0400, Taylor Blau wrote:

quoted

On Sat, Sep 11, 2021 at 07:59:40PM -0400, Derrick Stolee wrote:

quoted

I initially hesitated to support the drop of
nth_midxed_pack_entry(), since it was designed with things
like midx bitmaps in mind (specifically, to also support
lex-order-to-stable-order conversions).

I didn't know that nth_midxed_pack_entry was designed with either
purpose in mind, since it predates midx bitmaps by quite a bit.

Thinking on it more, I can imagine that you wrote this function
aspirationally envisioning something like MIDX bitmaps. And since you
and I discussed the design together quite a bit, I imagine that that's
the case ;-).

But I agree that after reading this series again, that the inline-ing
suggested makes sense (and doesn't conflict with any series I have in
flight which don't add any new callers).

I'm thinking more to my original design of the multi-pack-index.
At that time, I was thinking about the possible integration
with bitmaps based on my experience in other systems which used
a stable object order to allow writing bitmaps asynchronously
with respect to the multi-pack-index write and object packing.

Makes sense, and thank you for clarifying. After re-reading my first
email, I figured that this is what you must have been talking about,
which is why I felt like I should rephrase (hence the follow-up email).

One thing that you did when first considering bitmaps over the
multi-pack-index was to demonstrate that a stable object order
is not required, which surprised and delighted me. It greatly
reduced the complexity of the problem, and being able to inline
this method is only one small fallout from that simplicity.

This was definitely a consequence of what I had observed from seeing
what was "slow" when running bitmaps in production at GitHub. There,
repacking a repository's objects all into one pack each time we ran our
automated background jobs far outpaced the amount of time we spent
generating bitmaps.

And with your and Peff's work on improving bitmap generation itself,
things are in a pretty good place. I do have some potential ideas for
future improvement, like a mode where bitmaps are only "fast forwarded",
meaning that new bitmaps are only added between the commits selected for
bitmapping in the previous round and the current reference tips.

I think things like that end up getting you pretty far, but it may be
interesting to come back eventually and revisit adding a stable object
order. In the meantime, though... ;)

Thanks,
Taylor