git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  Jeff King <peff@peff.net>,
	 Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH 2/2] upload-pack: don't ACK non-commits repeatedly in protocol v2
Date: Thu, 04 Sep 2025 09:37:58 -0700	[thread overview]
Message-ID: <xmqqqzwm9nrt.fsf@gitster.g> (raw)
In-Reply-To: <aLmJOdxUYiyHpiLA@pks.im> (Patrick Steinhardt's message of "Thu, 4 Sep 2025 14:42:33 +0200")

Patrick Steinhardt <ps@pks.im> writes:

> ... when the
> server ACKs an object there is no reason for the client to go deeper, so
> they stop advertising any of its parents.

Very true.  I did point out the difference in behaviour, but I
couldn't actually figure out what goodness we are deriving in this
code by marking grandparents (and possibly their ancestors) as
something they have.  Your change in behaviour could be seen as
stopping us from making unnecessary operations ;-)

If the other side is mischievous, they are not limited to feed us a
commit and then its parent in the naturally expected order.  They
could feed us A and then skip B and give us C that is a child of B.
Pre-painting B when we got A from them, in order to prepare for
seeing B (which we can return without doing anything) in the next
round, would not help us all that much, if they give us C after
giving us A, as we haven't even heard of C yet at that point.

But in the normal case against sane clients that do not skip the
probes, marking immediate parents (like B) when processing A might
be helping?  I dunno.  I also somehow thought that even normal case
we have an option to skip the probes in order to converge faster,
but I am misremembering.

cf. https://lore.kernel.org/git/?q=upload-pack%20fibonacci




  reply	other threads:[~2025-09-04 16:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-03  4:54 [PATCH 0/2] upload-pack: don't ACK non-commits repeatedly Patrick Steinhardt
2025-09-03  4:54 ` [PATCH 1/2] t5530: modernize tests Patrick Steinhardt
2025-09-04  4:10   ` Junio C Hamano
2025-09-03  4:54 ` [PATCH 2/2] upload-pack: don't ACK non-commits repeatedly in protocol v2 Patrick Steinhardt
2025-09-04  4:23   ` Junio C Hamano
2025-09-04 12:42     ` Patrick Steinhardt
2025-09-04 16:37       ` Junio C Hamano [this message]
2025-09-05  6:11         ` Patrick Steinhardt
2025-09-05  6:18 ` [PATCH v2 0/2] upload-pack: don't ACK non-commits repeatedly Patrick Steinhardt
2025-09-05  6:18   ` [PATCH v2 1/2] t5530: modernize tests Patrick Steinhardt
2025-09-05  6:18   ` [PATCH v2 2/2] upload-pack: don't ACK non-commits repeatedly in protocol v2 Patrick Steinhardt
2025-09-08 16:51     ` Jeff King
2025-09-05 21:39   ` [PATCH v2 0/2] upload-pack: don't ACK non-commits repeatedly Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqqzwm9nrt.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).