Re: [PATCH 1/2] upload-pack: avoid parsing objects during ref advertisement
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:55:49
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH 1/2] upload-pack: avoid parsing objects during ref advertisement · Shawn Pearce <hidden>
- 2016-06-15 · Re: [PATCH 1/2] upload-pack: avoid parsing objects during ref advertisement · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH 1/2] upload-pack: avoid parsing objects during ref advertisement · Junio C Hamano <hidden>
- 2016-06-15 · [PATCH 1/2] upload-pack: avoid parsing objects during ref advertisement · Jeff King <hidden>
Jeff King [off-list ref] writes:
When we advertise a ref, the first thing we do is parse the
pointed-to object. This gives us two things:
1. a "struct object" we can use to store flags
2. the type of the object, so we know whether we need to
dereference it as a tag
Instead, we can just use lookup_unknown_object to get an
object struct, and then fill in just the type field using
sha1_object_info (which, in the case of packed files, can
find the information without actually inflating the object
data).
This can save time if you have a large number of refs, and
the client isn't actually going to request those refs (e.g.,
because most of them are already up-to-date).
The downside is that we are no longer verifying objects that
we advertise by fully parsing them (however, we do still
know we actually have them, because sha1_object_info must
find them to get the type). While we might fail to detect a
corrupt object here, if the client actually fetches the
object, we will parse (and verify) it then.
...This is an old news, but do you recall why this patch did not update the code in mark_our_ref() that gets "struct object *o" from parse_object() the same way and mark them with OUR_REF flag? I was wondering about code consolidation like this. upload-pack.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index 95d8313..609cd6c 100644
--- a/upload-pack.c
+++ b/upload-pack.c@@ -722,15 +722,18 @@ static void receive_needs(void) free(shallows.objects); } +static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data); + static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { static const char *capabilities = "multi_ack thin-pack side-band" " side-band-64k ofs-delta shallow no-progress" " include-tag multi_ack_detailed"; - struct object *o = lookup_unknown_object(sha1); const char *refname_nons = strip_namespace(refname); unsigned char peeled[20]; + mark_our_ref(refname, sha1, flag, cb_data); + if (capabilities) packet_write(1, "%s %s%c%s%s agent=%s\n", sha1_to_hex(sha1), refname_nons,
@@ -740,10 +743,6 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo else packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname_nons); capabilities = NULL; - if (!(o->flags & OUR_REF)) { - o->flags |= OUR_REF; - nr_our_refs++; - } if (!peel_ref(refname, peeled)) packet_write(1, "%s %s^{}\n", sha1_to_hex(peeled), refname_nons); return 0;
@@ -751,7 +750,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { - struct object *o = parse_object(sha1); + struct object *o = parse_object(sha1); /* lookup-unknown??? */ if (!o) die("git upload-pack: cannot find object %s:", sha1_to_hex(sha1)); if (!(o->flags & OUR_REF)) {