Skip to content

Commit deec8aa

Browse files
gitstergitster
committed
Merge branch 'ps/fetch-optim'
Optimize code that handles large number of refs in the "git fetch" code path. * ps/fetch-optim: fetch: avoid second connectivity check if we already have all objects fetch: merge fetching and consuming refs fetch: refactor fetch refs to be more extendable fetch-pack: optimize loading of refs via commit graph connected: refactor iterator to return next object ID directly fetch: avoid unpacking headers in object existence check fetch: speed up lookup of want refs via commit-graph
2 parents 4c71930 + caff8b7 commit deec8aa

File tree

6 files changed

+67
-61
lines changed

6 files changed

+67
-61
lines changed

‎builtin/clone.c‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ static void write_followtags(const struct ref *refs, const char *msg)
657657
}
658658
}
659659

660-
static int iterate_ref_map(void *cb_data, struct object_id *oid)
660+
static const struct object_id *iterate_ref_map(void *cb_data)
661661
{
662662
struct ref **rm = cb_data;
663663
struct ref *ref = *rm;
@@ -668,13 +668,11 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid)
668668
*/
669669
while (ref && !ref->peer_ref)
670670
ref = ref->next;
671-
/* Returning -1 notes "end of list" to the caller. */
672671
if (!ref)
673-
return -1;
672+
return NULL;
674673

675-
oidcpy(oid, &ref->old_oid);
676674
*rm = ref->next;
677-
return 0;
675+
return &ref->old_oid;
678676
}
679677

680678
static void update_remote_refs(const struct ref *refs,

‎builtin/fetch.c‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -854,13 +854,11 @@ static int update_local_ref(struct ref *ref,
854854
int summary_width)
855855
{
856856
struct commit *current = NULL, *updated;
857-
enum object_type type;
858857
struct branch *current_branch = branch_get(NULL);
859858
const char *pretty_ref = prettify_refname(ref->name);
860859
int fast_forward = 0;
861860

862-
type = oid_object_info(the_repository, &ref->new_oid, NULL);
863-
if (type < 0)
861+
if (!repo_has_object_file(the_repository, &ref->new_oid))
864862
die(_("object %s not found"), oid_to_hex(&ref->new_oid));
865863

866864
if (oideq(&ref->old_oid, &ref->new_oid)) {
@@ -972,18 +970,17 @@ static int update_local_ref(struct ref *ref,
972970
}
973971
}
974972

975-
static int iterate_ref_map(void *cb_data, struct object_id *oid)
973+
static const struct object_id *iterate_ref_map(void *cb_data)
976974
{
977975
struct ref **rm = cb_data;
978976
struct ref *ref = *rm;
979977

980978
while (ref && ref->status == REF_STATUS_REJECT_SHALLOW)
981979
ref = ref->next;
982980
if (!ref)
983-
return -1; /* end of the list */
981+
return NULL;
984982
*rm = ref->next;
985-
oidcpy(oid, &ref->old_oid);
986-
return 0;
983+
return &ref->old_oid;
987984
}
988985

989986
struct fetch_head {
@@ -1082,7 +1079,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
10821079
int connectivity_checked, struct ref *ref_map)
10831080
{
10841081
struct fetch_head fetch_head;
1085-
struct commit *commit;
10861082
int url_len, i, rc = 0;
10871083
struct strbuf note = STRBUF_INIT, err = STRBUF_INIT;
10881084
struct ref_transaction *transaction = NULL;
@@ -1130,6 +1126,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
11301126
want_status <= FETCH_HEAD_IGNORE;
11311127
want_status++) {
11321128
for (rm = ref_map; rm; rm = rm->next) {
1129+
struct commit *commit = NULL;
11331130
struct ref *ref = NULL;
11341131

11351132
if (rm->status == REF_STATUS_REJECT_SHALLOW) {
@@ -1139,11 +1136,23 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
11391136
continue;
11401137
}
11411138

1142-
commit = lookup_commit_reference_gently(the_repository,
1143-
&rm->old_oid,
1144-
1);
1145-
if (!commit)
1146-
rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE;
1139+
/*
1140+
* References in "refs/tags/" are often going to point
1141+
* to annotated tags, which are not part of the
1142+
* commit-graph. We thus only try to look up refs in
1143+
* the graph which are not in that namespace to not
1144+
* regress performance in repositories with many
1145+
* annotated tags.
1146+
*/
1147+
if (!starts_with(rm->name, "refs/tags/"))
1148+
commit = lookup_commit_in_graph(the_repository, &rm->old_oid);
1149+
if (!commit) {
1150+
commit = lookup_commit_reference_gently(the_repository,
1151+
&rm->old_oid,
1152+
1);
1153+
if (!commit)
1154+
rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE;
1155+
}
11471156

11481157
if (rm->fetch_head_status != want_status)
11491158
continue;
@@ -1289,37 +1298,35 @@ static int check_exist_and_connected(struct ref *ref_map)
12891298
return check_connected(iterate_ref_map, &rm, &opt);
12901299
}
12911300

1292-
static int fetch_refs(struct transport *transport, struct ref *ref_map)
1301+
static int fetch_and_consume_refs(struct transport *transport, struct ref *ref_map)
12931302
{
1294-
int ret = check_exist_and_connected(ref_map);
1303+
int connectivity_checked = 1;
1304+
int ret;
1305+
1306+
/*
1307+
* We don't need to perform a fetch in case we can already satisfy all
1308+
* refs.
1309+
*/
1310+
ret = check_exist_and_connected(ref_map);
12951311
if (ret) {
12961312
trace2_region_enter("fetch", "fetch_refs", the_repository);
12971313
ret = transport_fetch_refs(transport, ref_map);
12981314
trace2_region_leave("fetch", "fetch_refs", the_repository);
1315+
if (ret)
1316+
goto out;
1317+
connectivity_checked = transport->smart_options ?
1318+
transport->smart_options->connectivity_checked : 0;
12991319
}
1300-
if (!ret)
1301-
/*
1302-
* Keep the new pack's ".keep" file around to allow the caller
1303-
* time to update refs to reference the new objects.
1304-
*/
1305-
return 0;
1306-
transport_unlock_pack(transport);
1307-
return ret;
1308-
}
13091320

1310-
/* Update local refs based on the ref values fetched from a remote */
1311-
static int consume_refs(struct transport *transport, struct ref *ref_map)
1312-
{
1313-
int connectivity_checked = transport->smart_options
1314-
? transport->smart_options->connectivity_checked : 0;
1315-
int ret;
13161321
trace2_region_enter("fetch", "consume_refs", the_repository);
13171322
ret = store_updated_refs(transport->url,
13181323
transport->remote->name,
13191324
connectivity_checked,
13201325
ref_map);
1321-
transport_unlock_pack(transport);
13221326
trace2_region_leave("fetch", "consume_refs", the_repository);
1327+
1328+
out:
1329+
transport_unlock_pack(transport);
13231330
return ret;
13241331
}
13251332

@@ -1508,8 +1515,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
15081515
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
15091516
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
15101517
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
1511-
if (!fetch_refs(transport, ref_map))
1512-
consume_refs(transport, ref_map);
1518+
fetch_and_consume_refs(transport, ref_map);
15131519

15141520
if (gsecondary) {
15151521
transport_disconnect(gsecondary);
@@ -1600,7 +1606,7 @@ static int do_fetch(struct transport *transport,
16001606
transport->url);
16011607
}
16021608
}
1603-
if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {
1609+
if (fetch_and_consume_refs(transport, ref_map)) {
16041610
free_refs(ref_map);
16051611
retcode = 1;
16061612
goto cleanup;

‎builtin/receive-pack.c‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,7 @@ static void refuse_unconfigured_deny_delete_current(void)
13061306
rp_error("%s", _(refuse_unconfigured_deny_delete_current_msg));
13071307
}
13081308

1309-
static int command_singleton_iterator(void *cb_data, struct object_id *oid);
1309+
static const struct object_id *command_singleton_iterator(void *cb_data);
13101310
static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
13111311
{
13121312
struct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;
@@ -1731,16 +1731,15 @@ static void check_aliased_updates(struct command *commands)
17311731
string_list_clear(&ref_list, 0);
17321732
}
17331733

1734-
static int command_singleton_iterator(void *cb_data, struct object_id *oid)
1734+
static const struct object_id *command_singleton_iterator(void *cb_data)
17351735
{
17361736
struct command **cmd_list = cb_data;
17371737
struct command *cmd = *cmd_list;
17381738

17391739
if (!cmd || is_null_oid(&cmd->new_oid))
1740-
return -1; /* end of list */
1740+
return NULL;
17411741
*cmd_list = NULL; /* this returns only one */
1742-
oidcpy(oid, &cmd->new_oid);
1743-
return 0;
1742+
return &cmd->new_oid;
17441743
}
17451744

17461745
static void set_connectivity_errors(struct command *commands,
@@ -1770,7 +1769,7 @@ struct iterate_data {
17701769
struct shallow_info *si;
17711770
};
17721771

1773-
static int iterate_receive_command_list(void *cb_data, struct object_id *oid)
1772+
static const struct object_id *iterate_receive_command_list(void *cb_data)
17741773
{
17751774
struct iterate_data *data = cb_data;
17761775
struct command **cmd_list = &data->cmds;
@@ -1781,13 +1780,11 @@ static int iterate_receive_command_list(void *cb_data, struct object_id *oid)
17811780
/* to be checked in update_shallow_ref() */
17821781
continue;
17831782
if (!is_null_oid(&cmd->new_oid) && !cmd->skip_update) {
1784-
oidcpy(oid, &cmd->new_oid);
17851783
*cmd_list = cmd->next;
1786-
return 0;
1784+
return &cmd->new_oid;
17871785
}
17881786
}
1789-
*cmd_list = NULL;
1790-
return -1; /* end of list */
1787+
return NULL;
17911788
}
17921789

17931790
static void reject_updates_to_hidden(struct command *commands)

‎connected.c‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
2424
struct child_process rev_list = CHILD_PROCESS_INIT;
2525
FILE *rev_list_in;
2626
struct check_connected_options defaults = CHECK_CONNECTED_INIT;
27-
struct object_id oid;
27+
const struct object_id *oid;
2828
int err = 0;
2929
struct packed_git *new_pack = NULL;
3030
struct transport *transport;
@@ -34,7 +34,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
3434
opt = &defaults;
3535
transport = opt->transport;
3636

37-
if (fn(cb_data, &oid)) {
37+
oid = fn(cb_data);
38+
if (!oid) {
3839
if (opt->err_fd)
3940
close(opt->err_fd);
4041
return err;
@@ -73,7 +74,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
7374
for (p = get_all_packs(the_repository); p; p = p->next) {
7475
if (!p->pack_promisor)
7576
continue;
76-
if (find_pack_entry_one(oid.hash, p))
77+
if (find_pack_entry_one(oid->hash, p))
7778
goto promisor_pack_found;
7879
}
7980
/*
@@ -83,7 +84,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
8384
goto no_promisor_pack_found;
8485
promisor_pack_found:
8586
;
86-
} while (!fn(cb_data, &oid));
87+
} while ((oid = fn(cb_data)) != NULL);
8788
return 0;
8889
}
8990

@@ -133,12 +134,12 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
133134
* are sure the ref is good and not sending it to
134135
* rev-list for verification.
135136
*/
136-
if (new_pack && find_pack_entry_one(oid.hash, new_pack))
137+
if (new_pack && find_pack_entry_one(oid->hash, new_pack))
137138
continue;
138139

139-
if (fprintf(rev_list_in, "%s\n", oid_to_hex(&oid)) < 0)
140+
if (fprintf(rev_list_in, "%s\n", oid_to_hex(oid)) < 0)
140141
break;
141-
} while (!fn(cb_data, &oid));
142+
} while ((oid = fn(cb_data)) != NULL);
142143

143144
if (ferror(rev_list_in) || fflush(rev_list_in)) {
144145
if (errno != EPIPE && errno != EINVAL)

‎connected.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ struct transport;
99
* When called after returning the name for the last object, return -1
1010
* to signal EOF, otherwise return 0.
1111
*/
12-
typedef int (*oid_iterate_fn)(void *, struct object_id *oid);
12+
typedef const struct object_id *(*oid_iterate_fn)(void *);
1313

1414
/*
1515
* Named-arguments struct for check_connected. All arguments are

‎fetch-pack.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ static struct commit *deref_without_lazy_fetch(const struct object_id *oid,
119119
{
120120
enum object_type type;
121121
struct object_info info = { .typep = &type };
122+
struct commit *commit;
123+
124+
commit = lookup_commit_in_graph(the_repository, oid);
125+
if (commit)
126+
return commit;
122127

123128
while (1) {
124129
if (oid_object_info_extended(the_repository, oid, &info,
@@ -1912,16 +1917,15 @@ static void update_shallow(struct fetch_pack_args *args,
19121917
oid_array_clear(&ref);
19131918
}
19141919

1915-
static int iterate_ref_map(void *cb_data, struct object_id *oid)
1920+
static const struct object_id *iterate_ref_map(void *cb_data)
19161921
{
19171922
struct ref **rm = cb_data;
19181923
struct ref *ref = *rm;
19191924

19201925
if (!ref)
1921-
return -1; /* end of the list */
1926+
return NULL;
19221927
*rm = ref->next;
1923-
oidcpy(oid, &ref->old_oid);
1924-
return 0;
1928+
return &ref->old_oid;
19251929
}
19261930

19271931
struct ref *fetch_pack(struct fetch_pack_args *args,

0 commit comments

Comments
 (0)