Re: [PATCH v2 2/2] fetch: send "refs/tags/" prefix upon CLI refspecs

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2018, 20:35 UTC
Message-ID: <xmqqd0vwcfkr.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180709183328.GD81741@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 24 quoted lines

> On 07/09, Junio C Hamano wrote:
>> Brandon Williams <bmwill@google.com> writes:
>> 
>> > I agree with this observation, though I'm a bit sad about it.  I think
>> > that having tag auto-following the default is a little confusing (and
>> > hurts perf[1] when using proto v2) but since thats the way its always been
>> > we'll have to live with it for now.  I think exploring changing the
>> > defaults might be a good thing to do in the future.  But for now we've
>> > had enough people comment on this lacking functionality that we should
>> > fix it.
>> >
>> > [1] Thankfully it doesn't completely undo what protocol v2 did, as we
>> > still are able to eliminate refs/changes or refs/pull or other various
>> > refs which significantly add to the number of refs advertised during
>> > fetch.
>> 
>> I thought JTan's <20180618231642.174650-1-jonathantanmy@google.com>
>> showed us a way forward to reduce the overhead even further without
>> having to be sad ;-).  Am I mistaken?
>
> That's true, what Jonathan mentioned there would avoid having to send
> "refs/tags/*" when requesting the refs.  The question is do we wait on
> implementing that functionality (as another feature to fetch) or do we
> fix this now?

It's not like the earlier v2 protocol used to be super efficient and correct, whose performance benefit is destroyed with this "fix" irreversibly. It was a fast but sometimes incorrect implementation, and we'd protect users of the still-under-development feature with these two patches while updating the protocol further in order to become truly efficient and correct, so I do not see it a wrong move to take a hit like this patch does in the meantime.

What I would see a wrong move would be to leave it very long as-is after we apply this fix, and declare to flip the default not to follow tags, using the performance hit as an excuse.

So I do not care too deeply either way, whether we wait to regain efficiency or taking this safe fix as the first step, as long as it is fixed in the longer term. I had an impression that the sentiment in the thread was that it was OK to accept the inefficiency for now and fix it later, and I am personally fine with that approach.