git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH 1/2] fetch-pack: Finish negotation if remote replies "ACK %s ready"
Date: Mon, 14 Mar 2011 16:48:38 -0700	[thread overview]
Message-ID: <1300146519-26508-1-git-send-email-spearce@spearce.org> (raw)

If multi_ack_detailed was selected in the protocol capabilities
(both client and server are >= Git 1.6.6) the upload-pack side will
send "ACK %s ready" when it knows how to safely cut the graph and
produce a reasonable pack for the want list that was already sent
on the connection.

Upon receiving "ACK %s ready" there is no point in looking at
the remaining commits inside of rev_list.  Sending additional
"have %s" lines to the remote will not construct a smaller pack.
It is unlikely a commit older than the current cut point will have
a better delta base than the cut point itself has.

The original design of this code had fetch-pack empty rev_list by
marking a commit and its transitive ancestors COMMON whenever the
remote side said "ACK %s {continue,common}" and skipping over any
already COMMON commits during get_rev().  This approach does not
work when most of rev_list is actually COMMON_REF, commits that
are pointed to by a reference on the remote, which exist locally,
and which have not yet been sent to the remote as a "have %s" line.

Most of the common references are tags in the ref/tags namespace,
using points in the commit graph that are more than 1 commit apart.
In git.git itself, this is currently 340 tags, 339 of which point to
commits in the commit graph.  fetch-pack pushes all of these into
rev_list, but is unable to mark them COMMON and discard during a
remote's "ACK %s {continue,common}" because it does not parse through
the entire parent chain.  Not parsing the entire parent chain is
an optimization to avoid walking back to the roots of the repository.

Assuming the client is only following the remote (and does not make
its own local commits), the client needs 11 rounds to spin through
the entire list of tags (32 commits per round, ceil(339/32) == 11).
Unfortunately the server knows on the first "have %s" line that
it can produce a good pack, and does not need to see the remaining
320 tags in the other 10 rounds.

Over git:// and ssh:// this isn't as bad as it sounds, the client is
only transmitting an extra 16,000 bytes that it doesn't need to send.

Over smart HTTP, the client must do an additional 10 HTTP POST
requests, each of which incurs round-trip latency, and must upload
the entire state vector of all known common objects.  On the final
POST request, this is 16 KiB worth of data.

Fix all of this by clearing rev_list as soon as the remote side
says it can construct a pack.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin/fetch-pack.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index b999413..dfacfdf 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -379,6 +379,8 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 					retval = 0;
 					in_vain = 0;
 					got_continue = 1;
+					if (ack == ACK_ready)
+					  rev_list = NULL;
 					break;
 					}
 				}
-- 
1.7.0.9.5.g6bd7.dirty

             reply	other threads:[~2011-03-14 23:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-14 23:48 Shawn O. Pearce [this message]
2011-03-14 23:48 ` [PATCH 2/2] upload-pack: More aggressively send 'ACK %s ready' Shawn O. Pearce
2011-03-17  7:15 ` [PATCH 1/2] fetch-pack: Finish negotation if remote replies "ACK %s ready" Jeff King
2011-03-17  7:16   ` Jeff King
2011-03-17 15:51   ` Shawn Pearce

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=1300146519-26508-1-git-send-email-spearce@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).