> > > Right, REF_DELTA is definitely correctly handled currently, and I don't > > > think that would break with your patch. It's just that your patch would > > > introduce a bunch of extra traffic as we request bases separately that > > > are already in the pack. > > > > Ah...I see. For this problem, I think that it can be solved with the > > "if (objects[d->obj_no].real_type != OBJ_REF_DELTA)" check that the > > existing code uses before calling read_object(). I'll include this in > > the next reroll if any other issue comes up. > > I'm confused about this. Aren't we pre-fetching before we've actually > resolved deltas? The base could be in the pack as a true base, and we > might have seen it already then. But it could itself be a delta, and we > wouldn't know we have it until we resolve it (this gets into the > lucky/unlucky ordering thing). resolve_deltas(), invoked before any new code introduced in this patch, has this comment: > /* > * Second pass: > * - for all non-delta objects, look if it is used as a base for > * deltas; > * - if used as a base, uncompress the object and apply all deltas, > * recursively checking if the resulting object is used as a base > * for some more deltas. > */ I haven't seen any code that contradicts this comment. And looking at the code, for each non-delta object, I think that all deltas are checked - regardless of whether they appear before or after that non-delta object. (find_ref_delta() does a binary search from 0 to nr_ref_deltas, calculated in parse_pack_objects() which happens before any resolution of deltas.) And find_unresolved_deltas_1() (called from resolve_deltas() indirectly) sets the real_type when it resolves a delta, as far as I can tell. So there is more than one "resolve deltas" step - resolve_deltas() and then fix_unresolved_deltas(). The pre-fetching happens only during the latter.
