Jeff King <p...@peff.net> writes:
> Here's the code to do the cycle-breaking. Aside from the "hacky" bit,
> it's quite simple. I added a new state enum to object_entry to handle
> the graph traversal. Since it only needs 2 bits, I _assume_ a compiler
> can fit it in with the bitfields above (or at the very least give it its
> own single byte so we just use what would otherwise be struct padding).
> But I didn't check; if it turns out not to be the case we can easily
> emulate it with two bitfields. The write_object() check abuses the
> "idx.offset" field to keep the same state, but we could convert it to
> use these flags if we care.
> @@ -1516,6 +1577,13 @@ static void get_object_details(void)
> entry->no_try_delta = 1;
> + /*
> + * This must happen in a second pass, since we rely on the delta
> + * information for the whole list being completed.
> + */
> + for (i = 0; i < to_pack.nr_objects; i++)
> + break_delta_cycles(&to_pack.objects[i]);
A potential cycle can only come from reusing deltas across packs in
an unstable order, that happens way before we do the find_delta()
thing, so this is a good place to have the new call. While reading
break_delta_cycles(), I was wondering if what it does is safe under
multi-threading but there is no need to worry.
The recursiveness of break-delta-cycles is not too bad for the same
reason why it is OK to recurse in check_delta_limit(), I would guess?
This is not new with this change, but I am not quite sure what in
the current code prevents us from busting the delta limit for reused
> I think my preference is to clean up the "hacky" bit of this patch, and
> then apply the earlier MRU patch on top of it (which takes my repack
> from 44 minutes to 5 minutes for this particular test set).
Yup, with something like this to break the delta chain _and_ allow
an object to go through the usual deltify machinery, I'd say the MRU
patch is a wonderful thing to have.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html