On Mon, Aug 20, 2018 at 02:03:53PM -0700, Junio C Hamano wrote: > > So taking all of those options into account, what I ended up > > with is a separate list of "external bases" that are not > > part of the main packing list. Each delta entry that points > > to an external base has a single-bit flag to do so; we have a > > little breathing room in the bitfield section of > > object_entry. > > > > This lets us limit the change primarily to the oe_delta() > > and oe_set_delta_ext() functions. And as a bonus, most of > > the rest of the code does not consider these dummy entries > > at all, saving both runtime CPU and code complexity. > > Tricky ;-) > > I wonder if we can move the preferred base objects that we are not > going to send also off of the "main packing list" to this new > mechanism? That gets trickier. Obviously we don't need to actually send them, so they're ignored during the write phase. But we do put them into the delta-search window along with the other objects, and we care about things like their size and type (which our dummy objects do not even have; they really are just placeholders for the oids). So I'd guess that we'd end up with three arrays of entries: - objects we want to send, with full details including delta bases - preferred base objects with _some_ details - dummy thin objects for reused deltas The trick is that I don't know which details fall under the "some" in the second class. That could be extracted from a careful reading of the code, but it seems like it has a high chance of regression (and I'm not sure there's a huge upside, given that the code is already written as-is). I also worried about the same thing with these new reused dummy objects. but I think by now I'd have shaken out any problems in production use. > > +static struct bitmap_index *bitmap_git; > > ... > > +static int thin = 0; > > Please trust what BSS will do to your static vars. Heh. I copied the non-static declaration, but didn't think about shortening it. Looks like there were one or two minor comments, so I'll do a re-roll that addresses them. -Peff
