Merge branch 'jt/fix-fattening-promisor-fetch'
Fix performance regression of a recent "fatten promisor pack with local objects" protection against an unwanted gc. * jt/fix-fattening-promisor-fetch: index-pack --promisor: also check commits' trees index-pack --promisor: don't check blobs index-pack --promisor: dedup before checking links
This commit is contained in:
Junio C Hamano
@ -155,11 +155,11 @@ static int input_fd, output_fd;
|
||||
static const char *curr_pack;
|
||||
|
||||
/*
|
||||
* local_links is guarded by read_mutex, and record_local_links is read-only in
|
||||
* a thread.
|
||||
* outgoing_links is guarded by read_mutex, and record_outgoing_links is
|
||||
* read-only in a thread.
|
||||
*/
|
||||
static struct oidset local_links = OIDSET_INIT;
|
||||
static int record_local_links;
|
||||
static struct oidset outgoing_links = OIDSET_INIT;
|
||||
static int record_outgoing_links;
|
||||
|
||||
static struct thread_local_data *thread_data;
|
||||
static int nr_dispatched;
|
||||
@ -812,18 +812,41 @@ static int check_collison(struct object_entry *entry)
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void record_if_local_object(const struct object_id *oid)
|
||||
static void record_outgoing_link(const struct object_id *oid)
|
||||
{
|
||||
struct object_info info = OBJECT_INFO_INIT;
|
||||
if (oid_object_info_extended(the_repository, oid, &info, 0))
|
||||
/* Missing; assume it is a promisor object */
|
||||
return;
|
||||
if (info.whence == OI_PACKED && info.u.packed.pack->pack_promisor)
|
||||
return;
|
||||
oidset_insert(&local_links, oid);
|
||||
oidset_insert(&outgoing_links, oid);
|
||||
}
|
||||
|
||||
static void do_record_local_links(struct object *obj)
|
||||
static void maybe_record_name_entry(const struct name_entry *entry)
|
||||
{
|
||||
/*
|
||||
* Checking only trees here results in a significantly faster packfile
|
||||
* indexing, but the drawback is that if the packfile to be indexed
|
||||
* references a local blob only directly (that is, never through a
|
||||
* local tree), that local blob is in danger of being garbage
|
||||
* collected. Such a situation may arise if we push local commits,
|
||||
* including one with a change to a blob in the root tree, and then the
|
||||
* server incorporates them into its main branch through a "rebase" or
|
||||
* "squash" merge strategy, and then we fetch the new main branch from
|
||||
* the server.
|
||||
*
|
||||
* This situation has not been observed yet - we have only noticed
|
||||
* missing commits, not missing trees or blobs. (In fact, if it were
|
||||
* believed that only missing commits are problematic, one could argue
|
||||
* that we should also exclude trees during the outgoing link check;
|
||||
* but it is safer to include them.)
|
||||
*
|
||||
* Due to the rarity of the situation (it has not been observed to
|
||||
* happen in real life), and because the "penalty" in such a situation
|
||||
* is merely to refetch the missing blob when it's needed (and this
|
||||
* happens only once - when refetched, the blob goes into a promisor
|
||||
* pack, so it won't be GC-ed, the tradeoff seems worth it.
|
||||
*/
|
||||
if (S_ISDIR(entry->mode))
|
||||
record_outgoing_link(&entry->oid);
|
||||
}
|
||||
|
||||
static void do_record_outgoing_links(struct object *obj)
|
||||
{
|
||||
if (obj->type == OBJ_TREE) {
|
||||
struct tree *tree = (struct tree *)obj;
|
||||
@ -837,16 +860,17 @@ static void do_record_local_links(struct object *obj)
|
||||
*/
|
||||
return;
|
||||
while (tree_entry_gently(&desc, &entry))
|
||||
record_if_local_object(&entry.oid);
|
||||
maybe_record_name_entry(&entry);
|
||||
} else if (obj->type == OBJ_COMMIT) {
|
||||
struct commit *commit = (struct commit *) obj;
|
||||
struct commit_list *parents = commit->parents;
|
||||
|
||||
record_outgoing_link(get_commit_tree_oid(commit));
|
||||
for (; parents; parents = parents->next)
|
||||
record_if_local_object(&parents->item->object.oid);
|
||||
record_outgoing_link(&parents->item->object.oid);
|
||||
} else if (obj->type == OBJ_TAG) {
|
||||
struct tag *tag = (struct tag *) obj;
|
||||
record_if_local_object(get_tagged_oid(tag));
|
||||
record_outgoing_link(get_tagged_oid(tag));
|
||||
}
|
||||
}
|
||||
|
||||
@ -896,7 +920,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
|
||||
free(has_data);
|
||||
}
|
||||
|
||||
if (strict || do_fsck_object || record_local_links) {
|
||||
if (strict || do_fsck_object || record_outgoing_links) {
|
||||
read_lock();
|
||||
if (type == OBJ_BLOB) {
|
||||
struct blob *blob = lookup_blob(the_repository, oid);
|
||||
@ -928,8 +952,8 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
|
||||
die(_("fsck error in packed object"));
|
||||
if (strict && fsck_walk(obj, NULL, &fsck_options))
|
||||
die(_("Not all child objects of %s are reachable"), oid_to_hex(&obj->oid));
|
||||
if (record_local_links)
|
||||
do_record_local_links(obj);
|
||||
if (record_outgoing_links)
|
||||
do_record_outgoing_links(obj);
|
||||
|
||||
if (obj->type == OBJ_TREE) {
|
||||
struct tree *item = (struct tree *) obj;
|
||||
@ -1785,28 +1809,43 @@ static void repack_local_links(void)
|
||||
struct strbuf line = STRBUF_INIT;
|
||||
struct oidset_iter iter;
|
||||
struct object_id *oid;
|
||||
char *base_name;
|
||||
char *base_name = NULL;
|
||||
|
||||
if (!oidset_size(&local_links))
|
||||
if (!oidset_size(&outgoing_links))
|
||||
return;
|
||||
|
||||
base_name = mkpathdup("%s/pack/pack", repo_get_object_directory(the_repository));
|
||||
|
||||
strvec_push(&cmd.args, "pack-objects");
|
||||
strvec_push(&cmd.args, "--exclude-promisor-objects-best-effort");
|
||||
strvec_push(&cmd.args, base_name);
|
||||
cmd.git_cmd = 1;
|
||||
cmd.in = -1;
|
||||
cmd.out = -1;
|
||||
if (start_command(&cmd))
|
||||
die(_("could not start pack-objects to repack local links"));
|
||||
|
||||
oidset_iter_init(&local_links, &iter);
|
||||
oidset_iter_init(&outgoing_links, &iter);
|
||||
while ((oid = oidset_iter_next(&iter))) {
|
||||
struct object_info info = OBJECT_INFO_INIT;
|
||||
if (oid_object_info_extended(the_repository, oid, &info, 0))
|
||||
/* Missing; assume it is a promisor object */
|
||||
continue;
|
||||
if (info.whence == OI_PACKED && info.u.packed.pack->pack_promisor)
|
||||
continue;
|
||||
|
||||
if (!cmd.args.nr) {
|
||||
base_name = mkpathdup(
|
||||
"%s/pack/pack",
|
||||
repo_get_object_directory(the_repository));
|
||||
strvec_push(&cmd.args, "pack-objects");
|
||||
strvec_push(&cmd.args,
|
||||
"--exclude-promisor-objects-best-effort");
|
||||
strvec_push(&cmd.args, base_name);
|
||||
cmd.git_cmd = 1;
|
||||
cmd.in = -1;
|
||||
cmd.out = -1;
|
||||
if (start_command(&cmd))
|
||||
die(_("could not start pack-objects to repack local links"));
|
||||
}
|
||||
|
||||
if (write_in_full(cmd.in, oid_to_hex(oid), the_hash_algo->hexsz) < 0 ||
|
||||
write_in_full(cmd.in, "\n", 1) < 0)
|
||||
die(_("failed to feed local object to pack-objects"));
|
||||
}
|
||||
|
||||
if (!cmd.args.nr)
|
||||
return;
|
||||
|
||||
close(cmd.in);
|
||||
|
||||
out = xfdopen(cmd.out, "r");
|
||||
@ -1905,7 +1944,7 @@ int cmd_index_pack(int argc,
|
||||
} else if (skip_to_optional_arg(arg, "--keep", &keep_msg)) {
|
||||
; /* nothing to do */
|
||||
} else if (skip_to_optional_arg(arg, "--promisor", &promisor_msg)) {
|
||||
record_local_links = 1;
|
||||
record_outgoing_links = 1;
|
||||
} else if (starts_with(arg, "--threads=")) {
|
||||
char *end;
|
||||
nr_threads = strtoul(arg+10, &end, 0);
|
||||
|
||||
Reference in New Issue
Block a user