@matheusbsbs / matheus-bagliano / commits / 56dfeb6263

pack-objects: compute local/ignore_pack_keep early

In want_object_in_pack(), we can exit early from our loop if neither "local" nor "ignore_pack_keep" are set. If they are, however, we must examine each pack to see if it has the object and is non-local or has a ".keep". It's quite common for there to be no non-local or .keep packs at all, in which case we know ahead of time that looking further will be pointless. We can pre-compute this by simply iterating over the list of packs ahead of time, and dropping the flags if there are no packs that could match. Another similar strategy would be to modify the loop in want_object_in_pack() to notice that we have already found the object once, and that we are looping only to check for "local" and "keep" attributes. If a pack has neither of those, we can skip the call to find_pack_entry_one(), which is the expensive part of the loop. This has two advantages: - it isn't all-or-nothing; we still get some improvement when there's a small number of kept or non-local packs, and a large number of non-kept local packs - it eliminates any possible race where we add new non-local or kept packs after our initial scan. In practice, I don't think this race matters; we already cache the packed_git information, so somebody who adds a new pack or .keep file after we've started will not be noticed at all, unless we happen to need to call reprepare_packed_git() because a lookup fails. In other words, we're already racy, and the race is not a big deal (losing the race means we might include an object in the pack that would not otherwise be, which is an acceptable outcome). However, it also has a disadvantage: we still loop over the rest of the packs for each object to check their flags. This is much less expensive than doing the object lookup, but still not free. So if we wanted to implement that strategy to cover the non-all-or-nothing cases, we could do so in addition to this one (so you get the most speedup in the all-or-nothing case, and the best we can do in the other cases). But given that the all-or-nothing case is likely the most common, it is probably not worth the trouble, and we can revisit this later if evidence points otherwise. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jul 29, 2016 at 00:11 UTC 56dfeb62638760fa78a442a97f19abf1af374d29

1 file changed +25 -1

builtin/pack-objects.c

+25 -1

@@ -46,6 +46,7 @@ static int keep_unreachable, unpack_unreachable, include_tag;

46 static unsigned long unpack_unreachable_expiration;

47 static int pack_loose_unreachable;

48 static int local;

49 +static int have_non_local_packs;

50 static int incremental;

51 static int ignore_packed_keep;

52 static int allow_ofs_delta;

@@ -990,7 +991,8 @@ static int want_object_in_pack(const unsigned char *sha1,

991 * we just found is going to be packed, so break

992 * out of the loop to return 1 now.

993 */

993 - if (!ignore_packed_keep && !local)

994 + if (!ignore_packed_keep &&

995 + (!local || !have_non_local_packs))

996 break;

997

998 if (local && !p->pack_local)

@@ -2799,6 +2801,28 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)

2801 progress = 2;

2802

2803 prepare_packed_git();

2804 + if (ignore_packed_keep) {

2805 + struct packed_git *p;

2806 + for (p = packed_git; p; p = p->next)

2807 + if (p->pack_local && p->pack_keep)

2808 + break;

2809 + if (!p) /* no keep-able packs found */

2810 + ignore_packed_keep = 0;

2811 + }

2812 + if (local) {

2813 + /*

2814 + * unlike ignore_packed_keep above, we do not want to

2815 + * unset "local" based on looking at packs, as it

2816 + * also covers non-local objects

2817 + */

2818 + struct packed_git *p;

2819 + for (p = packed_git; p; p = p->next) {

2820 + if (!p->pack_local) {

2821 + have_non_local_packs = 1;

2822 + break;

2823 + }

2824 + }

2825 + }

2826

2827 if (progress)

2828 progress_state = start_progress(_("Counting objects"), 0);