From: Johannes Schindelin Date: Thu, 20 Oct 2005 02:04:20 GMT Subject: Re: [PATCH] Do not send "want" lines for complete objects Message-ID: In-Reply-To: <7vwtk9vvhg.fsf@assigned-by-dhcp.cox.net> Hi, On Wed, 19 Oct 2005, Junio C Hamano wrote: > Junio C Hamano writes: > > > Let's draw a single strand of pearl case to illustrate. You > > have a commit chain A->B->C->D, and the other end says she has > > C. At this point, telling her that you have C is enough, and by > > not telling her about A and B, you would save her from depreting > > MAX_HAVE slots. Of course, if the other end has D as another > > head, then the above logic would give "^D^" to rev-list as well, > > telling it not to tell her about C, but that is what we want -- > > because she already knows you have C too when you tell her that > > you have D. I think I like this optimization. > > This was subtly wrong. ^D^ would barf if D is a tag that points > at a non commit (refs/tags/v2.6.11-tree). Also it would do a > suboptimal thing for a merge commit, since it will not cull the > second and later parents. Right, I keep forgetting about tags. And again right, like I said, it is suboptimal but helps the common case. > + if (has_sha1_file(remote) && > + ((o = parse_object(remote)) != NULL) && > + (o->flags & COMPLETE) && Why not split it here, and do a separate block here: > + ((commit = (struct commit *) deref_tag(o)) != NULL) && > + (commit->object.type = commit_type)) { > + struct commit_list *p = commit->parents; > + while (p && rev_command_len + 44 < sizeof(rev_command)) { > snprintf(rev_command + rev_command_len, 44, > - " ^%s^", sha1_to_hex(remote)); > + " ^%s", > + sha1_to_hex(p->item->object.sha1)); > rev_command_len += 43; > + p = p->next; > } And here I am at a loss: why only continue if p is empty? I mean, remote could be a tag, and still be complete, no? > - > - continue; > + if (!p) > + continue; > } > > packet_write(fd[1], "want %s\n", sha1_to_hex(remote)); > If the git-rev-list call goes, this probably gets prettier. I'll try to come up with a patch. Ciao, Dscho