Jeff King <peff@xxxxxxxx> 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]); > + > free(sorted_by_offset); > } 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 ones, though. > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html