clone: open a shortcut for connectivity check

In order to make sure the cloned repository is good, we run "rev-list --objects --not --all $new_refs" on the repository. This is expensive on large repositories. This patch attempts to mitigate the impact in this special case. In the "good" clone case, we only have one pack. If all of the following are met, we can be sure that all objects reachable from the new refs exist, which is the intention of running "rev-list ...": - all refs point to an object in the pack - there are no dangling pointers in any object in the pack - no objects in the pack point to objects outside the pack The second and third checks can be done with the help of index-pack as a slight variation of --strict check (which introduces a new condition for the shortcut: pack transfer must be used and the number of objects large enough to call index-pack). The first is checked in check_everything_connected after we get an "ok" from index-pack. "index-pack + new checks" is still faster than the current "index-pack + rev-list", which is the whole point of this patch. If any of the conditions fail, we fall back to the good old but expensive "rev-list ..". In that case it's even more expensive because we have to pay for the new checks in index-pack. But that should only happen when the other side is either buggy or malicious. Cloning linux-2.6 over file:// before after real 3m25.693s 2m53.050s user 5m2.037s 4m42.396s sys 0m13.750s 0m16.574s A more realistic test with ssh:// over wireless before after real 11m26.629s 10m4.213s user 5m43.196s 5m19.444s sys 0m35.812s 0m37.630s This shortcut is not applied to shallow clones, partly because shallow clones should have no more objects than a usual fetch and the cost of rev-list is acceptable, partly to avoid dealing with corner cases when grafting is involved. This shortcut does not apply to unpack-objects code path either because the number of objects must be small in order to trigger that code path. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed May 26, 2013 at 08:16 UTC c6807a40dcd29f7e5ad1e2f4fc44f1729c9afa11
9 files changed +94 -15
Documentation/git-index-pack.txt
+3
@@ -74,6 +74,9 @@ OPTIONS
74 --strict::
75 Die, if the pack contains broken objects or links.
76
77 +--check-self-contained-and-connected::
78 + Die if the pack contains broken links. For internal use only.
79 +
80 --threads=<n>::
81 Specifies the number of threads to spawn when resolving
82 deltas. This requires that index-pack be compiled with
builtin/clone.c
+8 -3
@@ -542,13 +542,15 @@ static void update_remote_refs(const struct ref *refs,
542 const struct ref *mapped_refs,
543 const struct ref *remote_head_points_at,
544 const char *branch_top,
545 - const char *msg)
545 + const char *msg,
546 + struct transport *transport)
547 {
548 const struct ref *rm = mapped_refs;
549
550 if (0 <= option_verbosity)
551 printf(_("Checking connectivity... "));
551 - if (check_everything_connected(iterate_ref_map, 0, &rm))
552 + if (check_everything_connected_with_transport(iterate_ref_map,
553 + 0, &rm, transport))
554 die(_("remote did not send all necessary objects"));
555 if (0 <= option_verbosity)
556 printf(_("done\n"));
@@ -893,6 +895,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
895 if (option_upload_pack)
896 transport_set_option(transport, TRANS_OPT_UPLOADPACK,
897 option_upload_pack);
898 +
899 + if (transport->smart_options && !option_depth)
900 + transport->smart_options->check_self_contained_and_connected = 1;
901 }
902
903 refs = transport_get_remote_refs(transport);
@@ -954,7 +959,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
959 transport_fetch_refs(transport, mapped_refs);
960
961 update_remote_refs(refs, mapped_refs, remote_head_points_at,
957 - branch_top.buf, reflog_msg.buf);
962 + branch_top.buf, reflog_msg.buf, transport);
963
964 update_head(our_head_points_at, remote_head, reflog_msg.buf);
965
builtin/index-pack.c
+26 -9
@@ -77,8 +77,10 @@ static int nr_threads;
77
78 static int from_stdin;
79 static int strict;
80 +static int do_fsck_object;
81 static int verbose;
82 static int show_stat;
83 +static int check_self_contained_and_connected;
84
85 static struct progress *progress;
86
@@ -187,13 +189,13 @@ static int mark_link(struct object *obj, int type, void *data)
189
190 /* The content of each linked object must have been checked
191 or it must be already present in the object database */
190 -static void check_object(struct object *obj)
192 +static unsigned check_object(struct object *obj)
193 {
194 if (!obj)
193 - return;
195 + return 0;
196
197 if (!(obj->flags & FLAG_LINK))
196 - return;
198 + return 0;
199
200 if (!(obj->flags & FLAG_CHECKED)) {
201 unsigned long size;
@@ -201,17 +203,20 @@ static void check_object(struct object *obj)
203 if (type != obj->type || type <= 0)
204 die(_("object of unexpected type"));
205 obj->flags |= FLAG_CHECKED;
204 - return;
206 + return 1;
207 }
208 +
209 + return 0;
210 }
211
208 -static void check_objects(void)
212 +static unsigned check_objects(void)
213 {
210 - unsigned i, max;
214 + unsigned i, max, foreign_nr = 0;
215
216 max = get_max_object_index();
217 for (i = 0; i < max; i++)
214 - check_object(get_indexed_object(i));
218 + foreign_nr += check_object(get_indexed_object(i));
219 + return foreign_nr;
220 }
221
222
@@ -756,7 +761,8 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
761 obj = parse_object_buffer(sha1, type, size, buf, &eaten);
762 if (!obj)
763 die(_("invalid %s"), typename(type));
759 - if (fsck_object(obj, 1, fsck_error_function))
764 + if (do_fsck_object &&
765 + fsck_object(obj, 1, fsck_error_function))
766 die(_("Error in object"));
767 if (fsck_walk(obj, mark_link, NULL))
768 die(_("Not all child objects of %s are reachable"), sha1_to_hex(obj->sha1));
@@ -1490,6 +1496,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
1496 struct pack_idx_entry **idx_objects;
1497 struct pack_idx_option opts;
1498 unsigned char pack_sha1[20];
1499 + unsigned foreign_nr = 1; /* zero is a "good" value, assume bad */
1500
1501 if (argc == 2 && !strcmp(argv[1], "-h"))
1502 usage(index_pack_usage);
@@ -1511,6 +1518,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
1518 fix_thin_pack = 1;
1519 } else if (!strcmp(arg, "--strict")) {
1520 strict = 1;
1521 + do_fsck_object = 1;
1522 + } else if (!strcmp(arg, "--check-self-contained-and-connected")) {
1523 + strict = 1;
1524 + check_self_contained_and_connected = 1;
1525 } else if (!strcmp(arg, "--verify")) {
1526 verify = 1;
1527 } else if (!strcmp(arg, "--verify-stat")) {
@@ -1624,7 +1635,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
1635 conclude_pack(fix_thin_pack, curr_pack, pack_sha1);
1636 free(deltas);
1637 if (strict)
1627 - check_objects();
1638 + foreign_nr = check_objects();
1639
1640 if (show_stat)
1641 show_pack_info(stat_only);
@@ -1650,5 +1661,11 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
1661 if (index_name == NULL)
1662 free((void *) curr_index);
1663
1664 + /*
1665 + * Let the caller know this pack is not self contained
1666 + */
1667 + if (check_self_contained_and_connected && foreign_nr)
1668 + return 1;
1669 +
1670 return 0;
1671 }
connected.c
+33 -1
@@ -2,7 +2,12 @@
2 #include "run-command.h"
3 #include "sigchain.h"
4 #include "connected.h"
5 +#include "transport.h"
6
7 +int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
8 +{
9 + return check_everything_connected_with_transport(fn, quiet, cb_data, NULL);
10 +}
11 /*
12 * If we feed all the commits we want to verify to this command
13 *
@@ -14,7 +19,10 @@
19 *
20 * Returns 0 if everything is connected, non-zero otherwise.
21 */
17 -int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
22 +int check_everything_connected_with_transport(sha1_iterate_fn fn,
23 + int quiet,
24 + void *cb_data,
25 + struct transport *transport)
26 {
27 struct child_process rev_list;
28 const char *argv[] = {"rev-list", "--objects",
@@ -22,10 +30,23 @@ int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
30 char commit[41];
31 unsigned char sha1[20];
32 int err = 0;
33 + struct packed_git *new_pack = NULL;
34
35 if (fn(cb_data, sha1))
36 return err;
37
38 + if (transport && transport->smart_options &&
39 + transport->smart_options->self_contained_and_connected &&
40 + transport->pack_lockfile &&
41 + !suffixcmp(transport->pack_lockfile, ".keep")) {
42 + struct strbuf idx_file = STRBUF_INIT;
43 + strbuf_addstr(&idx_file, transport->pack_lockfile);
44 + strbuf_setlen(&idx_file, idx_file.len - 5); /* ".keep" */
45 + strbuf_addstr(&idx_file, ".idx");
46 + new_pack = add_packed_git(idx_file.buf, idx_file.len, 1);
47 + strbuf_release(&idx_file);
48 + }
49 +
50 if (quiet)
51 argv[5] = "--quiet";
52
@@ -42,6 +63,17 @@ int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
63
64 commit[40] = '\n';
65 do {
66 + /*
67 + * If index-pack already checked that:
68 + * - there are no dangling pointers in the new pack
69 + * - the pack is self contained
70 + * Then if the updated ref is in the new pack, then we
71 + * are sure the ref is good and not sending it to
72 + * rev-list for verification.
73 + */
74 + if (new_pack && find_pack_entry_one(sha1, new_pack))
75 + continue;
76 +
77 memcpy(commit, sha1_to_hex(sha1), 40);
78 if (write_in_full(rev_list.in, commit, 41) < 0) {
79 if (errno != EPIPE && errno != EINVAL)
connected.h
+5
@@ -1,6 +1,8 @@
1 #ifndef CONNECTED_H
2 #define CONNECTED_H
3
4 +struct transport;
5 +
6 /*
7 * Take callback data, and return next object name in the buffer.
8 * When called after returning the name for the last object, return -1
@@ -16,5 +18,8 @@ typedef int (*sha1_iterate_fn)(void *, unsigned char [20]);
18 * Return 0 if Ok, non zero otherwise (i.e. some missing objects)
19 */
20 extern int check_everything_connected(sha1_iterate_fn, int quiet, void *cb_data);
21 +extern int check_everything_connected_with_transport(sha1_iterate_fn, int quiet,
22 + void *cb_data,
23 + struct transport *transport);
24
25 #endif /* CONNECTED_H */
fetch-pack.c
+10 -1
@@ -691,6 +691,7 @@ static int get_pack(struct fetch_pack_args *args,
691 const char **av;
692 int do_keep = args->keep_pack;
693 struct child_process cmd;
694 + int ret;
695
696 memset(&demux, 0, sizeof(demux));
697 if (use_sideband) {
@@ -747,11 +748,14 @@ static int get_pack(struct fetch_pack_args *args,
748 strcpy(keep_arg + s, "localhost");
749 *av++ = keep_arg;
750 }
751 + if (args->check_self_contained_and_connected)
752 + *av++ = "--check-self-contained-and-connected";
753 }
754 else {
755 *av++ = "unpack-objects";
756 if (args->quiet || args->no_progress)
757 *av++ = "-q";
758 + args->check_self_contained_and_connected = 0;
759 }
760 if (*hdr_arg)
761 *av++ = hdr_arg;
@@ -772,7 +776,12 @@ static int get_pack(struct fetch_pack_args *args,
776 close(cmd.out);
777 }
778
775 - if (finish_command(&cmd))
779 + ret = finish_command(&cmd);
780 + if (!ret || (args->check_self_contained_and_connected && ret == 1))
781 + args->self_contained_and_connected =
782 + args->check_self_contained_and_connected &&
783 + ret == 0;
784 + else
785 die("%s failed", argv[0]);
786 if (use_sideband && finish_async(&demux))
787 die("error in sideband demultiplexer");
fetch-pack.h
+3 -1
@@ -16,7 +16,9 @@ struct fetch_pack_args {
16 verbose:1,
17 no_progress:1,
18 include_tag:1,
19 - stateless_rpc:1;
19 + stateless_rpc:1,
20 + check_self_contained_and_connected:1,
21 + self_contained_and_connected:1;
22 };
23
24 /*
transport.c
+4
@@ -534,6 +534,8 @@ static int fetch_refs_via_pack(struct transport *transport,
534 args.quiet = (transport->verbose < 0);
535 args.no_progress = !transport->progress;
536 args.depth = data->options.depth;
537 + args.check_self_contained_and_connected =
538 + data->options.check_self_contained_and_connected;
539
540 if (!data->got_remote_heads) {
541 connect_setup(transport, 0, 0);
@@ -551,6 +553,8 @@ static int fetch_refs_via_pack(struct transport *transport,
553 refs = NULL;
554 data->conn = NULL;
555 data->got_remote_heads = 0;
556 + data->options.self_contained_and_connected =
557 + args.self_contained_and_connected;
558
559 free_refs(refs_tmp);
560
transport.h
+2
@@ -8,6 +8,8 @@ struct git_transport_options {
8 unsigned thin : 1;
9 unsigned keep : 1;
10 unsigned followtags : 1;
11 + unsigned check_self_contained_and_connected : 1;
12 + unsigned self_contained_and_connected : 1;
13 int depth;
14 const char *uploadpack;
15 const char *receivepack;