{"thread":{"id":"12359","subject":"[PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","startedAt":"2008-02-28T08:42:57Z","lastAt":"2008-03-02T14:13:16Z","messageCount":7,"participants":["Shawn O. Pearce","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"70223","messageId":"20080228084257.GC16870@spearce.org","threadId":"12359","inReplyTo":null,"subject":"[PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-02-28T08:42:57Z","receivedAt":"2008-02-28T08:42:57Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If autofollowing of tags is enabled, we see a new tag on the remote\nthat we don't have, and we already have the SHA-1 object that the\ntag is peeled to, then we can fetch the tag while we are fetching\nthe other objects on the first connection.\n\nThis is a slight optimization for projects that have a habit of\ntagging a release commit after most users have already seen and\ndownloaded that commit object through a prior fetch session. In\nsuch cases the users may still find new objects in branch heads,\nbut the new tag will now also be part of the first pack transfer\nand the subsequent connection to autofollow tags is not required.\n\nCurrently git.git does not benefit from this optimization as any\nrelease usually gets a new commit at the same time that it gets a\nnew release tag, however git-gui.git and many other projects are\nin the habit of tagging fairly old commits.\n\nUsers who did not already have the tagged commit still require\nopening a second connection to autofollow the tag, as we are unable\nto determine on the client side if $tag^{} will be sent to the\nclient during the first transfer or not. Such computation must be\nperformed on the remote side of the connection and is deferred to\nanother series of changes.\n\nSigned-off-by: Shawn O. Pearce \n---\n builtin-fetch.c | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 834fbc6..a80f95b 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -101,6 +101,10 @@ static void add_merge_config(struct ref **head,\n \t}\n }\n \n+static void find_non_local_tags(struct transport *transport,\n+\t\t\tstruct ref **head,\n+\t\t\tstruct ref ***tail);\n+\n static struct ref *get_ref_map(struct transport *transport,\n \t\t\t struct refspec *refs, int ref_count, int tags,\n \t\t\t int *autotags)\n@@ -159,6 +163,8 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\t\tref_map->merge = 1;\n \t\t}\n \t}\n+\tif (tags == TAGS_DEFAULT && autotags)\n+\t\tfind_non_local_tags(transport, &ref_map, &tail);\n \tref_remove_duplicates(ref_map);\n \n \treturn ref_map;\n-- \n1.5.4.3.393.g5540\n"},{"id":"70371","messageId":"7v3arcduil.fsf@gitster.siamese.dyndns.org","threadId":"12359","inReplyTo":"20080228084257.GC16870@spearce.org","subject":"Re: [PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-29T08:41:38Z","receivedAt":"2008-02-29T08:41:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is cute. Obviously some tests need to be adjusted for this\nchange, though.\n"},{"id":"70436","messageId":"20080229223431.GT8410@spearce.org","threadId":"12359","inReplyTo":"7v3arcduil.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-02-29T22:34:31Z","receivedAt":"2008-02-29T22:34:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> This is cute. Obviously some tests need to be adjusted for this\n> change, though.\n\nI'll take a look at the current tests this weekend and see what\nneeds to be adjusted, if anything. I'd also like to get a few\ntests written for this, so we are certain the optimizations are\nkicking in when they are supposed to be.\n\n-- \nShawn.\n"},{"id":"70437","messageId":"alpine.LNX.1.00.0802291736580.19665@iabervon.org","threadId":"12359","inReplyTo":"20080229223431.GT8410@spearce.org","subject":"Re: [PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-29T22:39:58Z","receivedAt":"2008-02-29T22:39:58Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 29 Feb 2008, Shawn O. Pearce wrote:\n\n> Junio C Hamano wrote:\n> > This is cute. Obviously some tests need to be adjusted for this\n> > change, though.\n> \n> I'll take a look at the current tests this weekend and see what\n> needs to be adjusted, if anything. I'd also like to get a few\n> tests written for this, so we are certain the optimizations are\n> kicking in when they are supposed to be.\n\nI'd be really grateful if you came up with a good general strategy for \ntesting that we're not doing too much work in fetching, because clone has \noptimizations that I need to test in a similar way, and I haven't been \nable to think of anything not horribly intrusive.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70462","messageId":"20080301042510.GW8410@spearce.org","threadId":"12359","inReplyTo":"alpine.LNX.1.00.0802291736580.19665@iabervon.org","subject":"Re: [PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-01T04:25:10Z","receivedAt":"2008-03-01T04:25:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> On Fri, 29 Feb 2008, Shawn O. Pearce wrote:\n> \n> > Junio C Hamano wrote:\n> > > This is cute. Obviously some tests need to be adjusted for this\n> > > change, though.\n> > \n> > I'll take a look at the current tests this weekend and see what\n> > needs to be adjusted, if anything. I'd also like to get a few\n> > tests written for this, so we are certain the optimizations are\n> > kicking in when they are supposed to be.\n> \n> I'd be really grateful if you came up with a good general strategy for \n> testing that we're not doing too much work in fetching, because clone has \n> optimizations that I need to test in a similar way, and I haven't been \n> able to think of anything not horribly intrusive.\n\nIs this \"horribly intrusive\" ?\n\nWe can test it with something like this:\n\n $ GIT_DEBUG_SEND_PACK=1 git fetch 3>UPLOAD_LOG\n $ cat UPLOAD_LOG\n #S\n want 8e10cf4e007ad7e003463c30c34b1050b039db78 multi_ack side-band-64k thin-pack ofs-delta\n want ddfa4a33562179aca1ace2bcc662244a17d0b503\n #E\n #S\n want 3253df4d1cf6fb138b52b1938473bcfec1483223 multi_ack side-band-64k thin-pack ofs-delta\n #E\n\nNotice we made two connections, wanting 2 things and then later\nonly wanting 1 thing. Tag auto-following. :-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b26d053..4e14020 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -444,7 +444,10 @@ static void receive_needs(void)\n \tstruct object_array shallows = {0, 0, NULL};\n \tstatic char line[1000];\n \tint len, depth = 0;\n+\tint debug_needs = !!getenv(\"GIT_DEBUG_SEND_PACK\");\n \n+\tif (debug_needs)\n+\t\twrite_in_full(3, \"#S\\n\", 3);\n \tfor (;;) {\n \t\tstruct object *o;\n \t\tunsigned char sha1_buf[20];\n@@ -452,6 +455,8 @@ static void receive_needs(void)\n \t\treset_timeout();\n \t\tif (!len)\n \t\t\tbreak;\n+\t\tif (debug_needs)\n+\t\t\twrite_in_full(3, line, len);\n \n \t\tif (!prefixcmp(line, \"shallow \")) {\n \t\t\tunsigned char sha1[20];\n@@ -507,6 +512,8 @@ static void receive_needs(void)\n \t\t\tadd_object_array(o, NULL, &want_obj);\n \t\t}\n \t}\n+\tif (debug_needs)\n+\t\twrite_in_full(3, \"#E\\n\", 3);\n \tif (depth == 0 && shallows.nr == 0)\n \t\treturn;\n \tif (depth > 0) {\n\n-- \nShawn.\n"},{"id":"70541","messageId":"alpine.LNX.1.00.0803011127490.19665@iabervon.org","threadId":"12359","inReplyTo":"20080301042510.GW8410@spearce.org","subject":"Re: [PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-01T16:33:21Z","receivedAt":"2008-03-01T16:33:21Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 29 Feb 2008, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > On Fri, 29 Feb 2008, Shawn O. Pearce wrote:\n> > \n> > > Junio C Hamano wrote:\n> > > > This is cute. Obviously some tests need to be adjusted for this\n> > > > change, though.\n> > > \n> > > I'll take a look at the current tests this weekend and see what\n> > > needs to be adjusted, if anything. I'd also like to get a few\n> > > tests written for this, so we are certain the optimizations are\n> > > kicking in when they are supposed to be.\n> > \n> > I'd be really grateful if you came up with a good general strategy for \n> > testing that we're not doing too much work in fetching, because clone has \n> > optimizations that I need to test in a similar way, and I haven't been \n> > able to think of anything not horribly intrusive.\n> \n> Is this \"horribly intrusive\" ?\n> \n> We can test it with something like this:\n\nThat looks suitable to me (although maybe the environment variable should \ncontain the fd to write to?). I also want to know the \"have\" info, so I \ncan verify that the client is listing things appropriately, but that's an \nobvious addition.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70637","messageId":"20080302141316.GB8410@spearce.org","threadId":"12359","inReplyTo":"alpine.LNX.1.00.0803011127490.19665@iabervon.org","subject":"Re: [PATCH 3/4] Make git-fetch follow tags we already have objects for sooner","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-02T14:13:16Z","receivedAt":"2008-03-02T14:13:16Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> On Fri, 29 Feb 2008, Shawn O. Pearce wrote:\n> > \n> > Is this \"horribly intrusive\" ?\n> > \n> > We can test it with something like this:\n> \n> That looks suitable to me (although maybe the environment variable should \n> contain the fd to write to?). I also want to know the \"have\" info, so I \n> can verify that the client is listing things appropriately, but that's an \n> obvious addition.\n\nYea, I was thinking about that last night myself. I think the patch\nis still in `pu` so I will work up something for Junio to squash into\nit that uses the variable to specify the fd, rather than assume 3.\n\n-- \nShawn.\n"}]}