Re: Shallow clone
From: Junio C Hamano <hidden>
Date: 2016-08-11 20:05:37
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-08-11 · Re: Shallow clone · Junio C Hamano <hidden>
- 2016-08-11 · Re: Shallow clone · Sergey Vlasov <hidden>
- 2016-08-11 · Re: Shallow clone · Alexandre Julliard <hidden>
- 2016-08-11 · Re: Shallow clone · Junio C Hamano <hidden>
- 2016-08-11 · Re: Shallow clone · Junio C Hamano <hidden>
Sergey Vlasov [off-list ref] writes:
This is due to optimization in builtin-pack-objects.c:try_delta(): /* * We do not bother to try a delta that we discarded * on an earlier try, but only when reusing delta data. */ if (!no_reuse_delta && trg_entry->in_pack && trg_entry->in_pack == src_entry->in_pack) return 0; After removing this part the shallow pack after clone is 2.6M, as it should be. The problem with this optimization is that it is only valid if we are repacking either the same set of objects as we did earlier, or its superset. But if we are packing a subset of objects, there will be some objects in that subset which were delta-compressed in the original pack, but base objects for that deltas are not included in our subset - therefore we will be unable to reuse existing deltas, and with that optimization we will never try to use delta compression for these objects. ... So any partial fetch (shallow or not) from a mostly packed repository currently results in a suboptimal pack.
That is correct. How about something like this? I think the determination of "repacking_superset" may need to be tweaked because existing packs may have overlaps, and the patch counts them once per pack.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 69e5dd3..fb25124 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c@@ -64,6 +64,7 @@ struct object_entry { static unsigned char object_list_sha1[20]; static int non_empty; static int no_reuse_delta; +static int repacking_superset; static int local; static int incremental; static int allow_ofs_delta;
@@ -1172,10 +1173,13 @@ static int try_delta(struct unpacked *tr return -1; /* - * We do not bother to try a delta that we discarded - * on an earlier try, but only when reusing delta data. + * When we are packing the superset of objects we have already + * packed, we do not bother to try a delta that we discarded + * on an earlier try. This heuristic of course should not + * kick in when we are not reusing delta, or we know we are + * sending a subset of objects from a repository. */ - if (!no_reuse_delta && trg_entry->in_pack && + if (!no_reuse_delta && repacking_superset && trg_entry->in_pack && trg_entry->in_pack == src_entry->in_pack) return 0;
@@ -1493,6 +1497,16 @@ static void get_object_list(int ac, cons traverse_commit_list(&revs, show_commit, show_object); } +static int count_packed_objects(void) +{ + struct packed_git *p; + int cnt = 0; + + for (p = packed_git; p; p = p->next) + cnt += num_packed_objects(p); + return cnt; +} + int cmd_pack_objects(int argc, const char **argv, const char *prefix) { SHA_CTX ctx;
@@ -1631,6 +1645,8 @@ int cmd_pack_objects(int argc, const cha if (non_empty && !nr_result) return 0; + repacking_superset = count_packed_objects() < nr_result; + SHA1_Init(&ctx); list = sorted_by_sha; for (i = 0; i < nr_result; i++) {