Re: upload-pack is slow with lots of refs

2012-10-09 Thread Shawn Pearce
On Mon, Oct 8, 2012 at 8:05 AM, Johannes Sixt j...@kdbg.org wrote:
 Am 05.10.2012 18:57, schrieb Shawn Pearce:
 On Thu, Oct 4, 2012 at 11:24 PM, Johannes Sixt j.s...@viscovery.net wrote:
 Upload-pack can just start
 advertising refs in the v1 way and announce a v2 capability and listen
 for response in parallel. A v2 capable client can start sending wants or
 some other signal as soon as it sees the v2 capability. Upload-pack,
 which was listening for responses in parallel, can interrupt its
 advertisements and continue with v2 protocol from here.

 This sounds so simple (not the implementation, of course) - I must be
 missing something.

 Smart HTTP is not bidirectional. The client can't cut off the server.

 Smart HTTP does not need it: you already posted a better solution (I'm
 refering to v=2).

Yes but then it diverges even further from the native bidirectional protocol.

 Its also more complex to code the server to listen for a stop command
 from the client at the same time the server is blasting out useless
 references to the client.

 At least the server side does not seem to be that complex. See below.
 Of course, the server blasted out some refs, but I'm confident that in
 practice the client will be able to signal v2 capability after a few packets
 of advertisements. You can switch on TCP_NODELAY for the first line with
 the capabilities to ensure it goes out on the wire ASAP.
...
 +static int client_spoke(void)
 +{
 +   struct pollfd pfd;
 +   pfd.fd = 0;
 +   pfd.events = POLLIN;
 +   return poll(pfd, 1, 0)  0 
 +   (pfd.revents  (POLLIN|POLLHUP));

Except doing this in Java is harder on an arbitrary InputStream type.
I guess we really only care about basic TCP, in which case we can use
NIO to implement an emulation of poll, and SSH, where MINA SSHD
probably doesn't provide a way to see if the client has given us data
without blocking. That makes supporting v2 really hard in e.g. Gerrit
Code Review. You could argue that its improper to attempt to implement
a network protocol in a language whose standard libraries have gone
out of their way to prevent you from polling to see if data is
immediately available, but I prefer to ignore such arguments.

As it turns out we don't really have this problem with git://. Clients
can bury a v2 request in the extended headers where the host line
appears today. Its a bit tricky because of that \0 bug causing
infinite looping, but IIRC using \0\0 is safe even against ancient
servers. So git:// and http:// both have a way where the client can
ask for v2 support before the server speaks, and have it transparently
be ignored by ancient servers.


The only place we have a problem is SSH. That exec of the remote
binary is just super-strict. Its good to be paranoid, but its also
locked out any chance we have at doing the upgrade over SSH without
having to run two SSH commands in the worst case. I guess the best
approach is to try the v1 protocol by default, have the remote
advertise it supports v2, and remember this on a per-host basis in
~/.gitconfig for future requests. Users could always force a specific
preference with remote.NAME.uploadpack variable or --uploadpack
command line flag.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-09 Thread Johannes Sixt
Am 09.10.2012 08:46, schrieb Shawn Pearce:
 On Mon, Oct 8, 2012 at 8:05 AM, Johannes Sixt j...@kdbg.org wrote:
 Am 05.10.2012 18:57, schrieb Shawn Pearce:
 Smart HTTP is not bidirectional. The client can't cut off the server.

 Smart HTTP does not need it: you already posted a better solution (I'm
 refering to v=2).
 
 Yes but then it diverges even further from the native bidirectional protocol.

I won't argue here because I know next to nothing about Smart HTTP. But
it sounds like you either have compatibility, but a diverging protocol
or at least implementation, or no compatibility.

 +static int client_spoke(void)
 +{
 +   struct pollfd pfd;
 +   pfd.fd = 0;
 +   pfd.events = POLLIN;
 +   return poll(pfd, 1, 0)  0 
 +   (pfd.revents  (POLLIN|POLLHUP));
 
 Except doing this in Java is harder on an arbitrary InputStream type.
 I guess we really only care about basic TCP, in which case we can use
 NIO to implement an emulation of poll, and SSH, where MINA SSHD
 probably doesn't provide a way to see if the client has given us data
 without blocking. That makes supporting v2 really hard in e.g. Gerrit
 Code Review. You could argue that its improper to attempt to implement
 a network protocol in a language whose standard libraries have gone
 out of their way to prevent you from polling to see if data is
 immediately available, but I prefer to ignore such arguments.

Can't you read the inbound stream in a second thread while the first
thread writes the advertisements to the outbound stream? Then you don't
even need to poll; you can just read the 4-byte length header, stash it
away and set a flag. The implementation of client_spoke() would only
amount to check that flag.

 As it turns out we don't really have this problem with git://. Clients
 can bury a v2 request in the extended headers where the host line
 appears today.

I tried, but it seems that todays git-daemons are too strict and accept
only \0host=foo\0, nothing else :-(

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-09 Thread Johannes Sixt
Am 09.10.2012 22:30, schrieb Johannes Sixt:
 Am 09.10.2012 08:46, schrieb Shawn Pearce:
 As it turns out we don't really have this problem with git://. Clients
 can bury a v2 request in the extended headers where the host line
 appears today.
 
 I tried, but it seems that todays git-daemons are too strict and accept
 only \0host=foo\0, nothing else :-(

I take that back: Modern git-daemons accept \0host=foo\0\0version=2\0,
as you said.

It looks like SSH is the only stubborn protocol.

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-08 Thread Johannes Sixt
Am 05.10.2012 18:57, schrieb Shawn Pearce:
 On Thu, Oct 4, 2012 at 11:24 PM, Johannes Sixt j.s...@viscovery.net wrote:
 Upload-pack can just start
 advertising refs in the v1 way and announce a v2 capability and listen
 for response in parallel. A v2 capable client can start sending wants or
 some other signal as soon as it sees the v2 capability. Upload-pack,
 which was listening for responses in parallel, can interrupt its
 advertisements and continue with v2 protocol from here.

 This sounds so simple (not the implementation, of course) - I must be
 missing something.
 
 Smart HTTP is not bidirectional. The client can't cut off the server.

Smart HTTP does not need it: you already posted a better solution (I'm
refering to v=2).

 Its also more complex to code the server to listen for a stop command
 from the client at the same time the server is blasting out useless
 references to the client.

At least the server side does not seem to be that complex. See below.
Of course, the server blasted out some refs, but I'm confident that in
practice the client will be able to signal v2 capability after a few packets
of advertisements. You can switch on TCP_NODELAY for the first line with
the capabilities to ensure it goes out on the wire ASAP.

diff --git a/upload-pack.c b/upload-pack.c
index 2e90ccb..c29ae04 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -720,11 +720,20 @@ static void receive_needs(void)
free(shallows.objects);
 }
 
+static int client_spoke(void)
+{
+   struct pollfd pfd;
+   pfd.fd = 0;
+   pfd.events = POLLIN;
+   return poll(pfd, 1, 0)  0 
+   (pfd.revents  (POLLIN|POLLHUP));
+}
+
 static int send_ref(const char *refname, const unsigned char *sha1, int flag, 
void *cb_data)
 {
static const char *capabilities = multi_ack thin-pack side-band
 side-band-64k ofs-delta shallow no-progress
-include-tag multi_ack_detailed;
+include-tag multi_ack_detailed version2;
struct object *o = lookup_unknown_object(sha1);
const char *refname_nons = strip_namespace(refname);
 
@@ -752,7 +761,8 @@ static int send_ref(const char *refname, const unsigned 
char *sha1, int flag, vo
if (o)
packet_write(1, %s %s^{}\n, sha1_to_hex(o-sha1), 
refname_nons);
}
-   return 0;
+
+   return client_spoke();
 }
 
 static int mark_our_ref(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
@@ -771,8 +781,14 @@ static void upload_pack(void)
 {
if (advertise_refs || !stateless_rpc) {
reset_timeout();
-   head_ref_namespaced(send_ref, NULL);
-   for_each_namespaced_ref(send_ref, NULL);
+   if (head_ref_namespaced(send_ref, NULL) ||
+   for_each_namespaced_ref(send_ref, NULL)) {
+   /*
+* TODO: continue with protocol version 2
+* optimization: do not send refs
+* that were already sent
+*/
+   }
packet_flush(1);
} else {
head_ref_namespaced(mark_our_ref, NULL);

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-05 Thread Johannes Sixt
Am 10/3/2012 21:41, schrieb Shawn Pearce:
 On Wed, Oct 3, 2012 at 11:55 AM, Jeff King p...@peff.net wrote:
 On Wed, Oct 03, 2012 at 11:53:35AM -0700, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:

 Has there been any work on extending the protocol so that the client
 tells the server what refs it's interested in?

 I don't think so. It would be hard to do in a backwards-compatible way,
 because the advertisement is the first thing the server says, before it
 has negotiated any capabilities with the client at all.

 That is being discussed but hasn't surfaced on the list.

 Out of curiosity, how are you thinking about triggering such a new
 behavior in a backwards-compatible way? Invoke git-upload-pack2, and
 fall back to reconnecting to start git-upload-pack if it fails?
 
 Basically, yes. New clients connect for git-upload-pack2. Over git://
 the remote peer will just close the TCP socket with no messages. The
 client can fallback to git-upload-pack and try again. Over SSH a
 similar thing will happen in the sense there is no data output from
 the remote side, so the client can try again.

These connections are bidirectional. Upload-pack can just start
advertising refs in the v1 way and announce a v2 capability and listen
for response in parallel. A v2 capable client can start sending wants or
some other signal as soon as it sees the v2 capability. Upload-pack,
which was listening for responses in parallel, can interrupt its
advertisements and continue with v2 protocol from here.

This sounds so simple (not the implementation, of course) - I must be
missing something.

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-05 Thread Shawn Pearce
On Thu, Oct 4, 2012 at 11:24 PM, Johannes Sixt j.s...@viscovery.net wrote:
 Am 10/3/2012 21:41, schrieb Shawn Pearce:
 On Wed, Oct 3, 2012 at 11:55 AM, Jeff King p...@peff.net wrote:
 On Wed, Oct 03, 2012 at 11:53:35AM -0700, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:

 Has there been any work on extending the protocol so that the client
 tells the server what refs it's interested in?

 I don't think so. It would be hard to do in a backwards-compatible way,
 because the advertisement is the first thing the server says, before it
 has negotiated any capabilities with the client at all.

 That is being discussed but hasn't surfaced on the list.

 Out of curiosity, how are you thinking about triggering such a new
 behavior in a backwards-compatible way? Invoke git-upload-pack2, and
 fall back to reconnecting to start git-upload-pack if it fails?

 Basically, yes. New clients connect for git-upload-pack2. Over git://
 the remote peer will just close the TCP socket with no messages. The
 client can fallback to git-upload-pack and try again. Over SSH a
 similar thing will happen in the sense there is no data output from
 the remote side, so the client can try again.

 These connections are bidirectional.

Smart HTTP is not bidirectional.

 Upload-pack can just start
 advertising refs in the v1 way and announce a v2 capability and listen
 for response in parallel. A v2 capable client can start sending wants or
 some other signal as soon as it sees the v2 capability. Upload-pack,
 which was listening for responses in parallel, can interrupt its
 advertisements and continue with v2 protocol from here.

 This sounds so simple (not the implementation, of course) - I must be
 missing something.

Smart HTTP is not bidirectional. The client can't cut off the server.
Its also more complex to code the server to listen for a stop command
from the client at the same time the server is blasting out useless
references to the client.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-04 Thread Sascha Cunz
Am Mittwoch, 3. Oktober 2012, 16:13:16 schrieb Jeff King:
 On Wed, Oct 03, 2012 at 12:41:38PM -0700, Shawn O. Pearce wrote:
   Out of curiosity, how are you thinking about triggering such a new
   behavior in a backwards-compatible way? Invoke git-upload-pack2, and
   fall back to reconnecting to start git-upload-pack if it fails?
  
  Basically, yes. New clients connect for git-upload-pack2. Over git://
  the remote peer will just close the TCP socket with no messages. The
  client can fallback to git-upload-pack and try again. Over SSH a
  similar thing will happen in the sense there is no data output from
  the remote side, so the client can try again. This has the downside of
  authentication twice over SSH, which may prompt for a password twice.
  But the user can get out of this by setting remote.NAME.uploadpack =
  git-upload-pack and thus force the Git client to use the current
  protocol if they have a new client and must continue to work over SSH
  with an old server, and don't use an ssh-agent.
 
 It's a shame that we have to reestablish the TCP or ssh connection to do
 the retry. The password thing is annoying, but also it just wastes a
 round-trip. It means we'd probably want to default the v2 probe to off
 (and let the user turn it on for a specific remote) until v2 is much
 more common than v1. Otherwise everyone pays the price.

Would it be possible to use this workflow:

- Every client connects per default to v1

- If server is capable of v2, it sends a flag along with the usual response
  (A v1 server will obviously not send that flag)

- If client is also capable of v2 and gets the flag, it enables v2 for
  just that remote (probably unless the user said, i never want to)

- Next time the client connects to that remote it will use v2.

I'm not sure, if this is possible, since I think to remember that I have read 
in the Documentation folder something along the line: Capabilities announced 
from the server mean I want you to use exactly these flags.

Sascha

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-04 Thread Jeff King
On Thu, Oct 04, 2012 at 11:52:13PM +0200, Sascha Cunz wrote:

 Would it be possible to use this workflow:
 
 - Every client connects per default to v1
 
 - If server is capable of v2, it sends a flag along with the usual response
   (A v1 server will obviously not send that flag)

That is more or less the strategy we use for existing extensions (your
flag is a space-separated list of capability strings). But in this
case, the idea would be to change what the usual response is. Since a
v1 client would be expecting the response, we must send it, but at that
point it is too late to make the change. So we need to see some flag
from the client before the server says anything.

And the problem is that the client sending that flag will break v1
servers, and the client would need to waste time doing a retry when
connecting to the (initially more common) v1 servers.

 - If client is also capable of v2 and gets the flag, it enables v2 for
   just that remote (probably unless the user said, i never want to)
 
 - Next time the client connects to that remote it will use v2.

So yeah, that would work to help with the wasted time. We'd have
git-upload-pack2 to do the v2 protocol, but the v1 git-upload-pack for
the server would say by the way, next time you connect, try v2 first.
So the client would have to store a version number for each remote.
Which is not too onerous.

Another way to think of it is phasing it in like this:

  1. Add v2 support to client and server. Initially, clients try only
 v1.

  2. Add a remote.*.preferProtocol config option, defaulting to v1. This
 lets people turn on v2 for remotes they know support it. If v2
 fails, still fall back to v1.

  3. Add a server upload-pack capability that says by the way, try v2
 next time.  Have the client set the preferProtocol config option
 for a remote if we see that capability.

  4. Wait a while until v2 is very popular.

  5. Switch the default for preferProtocol to v2 (but still fall back to
 v1).

So always fall back and remain compatible, and let the config option
just be an optimization to avoid extra failed requests.

 I'm not sure, if this is possible, since I think to remember that I have read 
 in the Documentation folder something along the line: Capabilities announced 
 from the server mean I want you to use exactly these flags.

No, the server capability says I can do this, and the client should
respond with I want you to do this. Because the server might be
talking to an older client that does not know what this is, it must
handle the case that the capability does not come back.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Nguyen Thai Ngoc Duy
On Wed, Oct 3, 2012 at 7:36 PM, Ævar Arnfjörð Bjarmason
ava...@gmail.com wrote:
 I'm creating a system where a lot of remotes constantly fetch from a
 central repository for deployment purposes, but I've noticed that even
 with a remote.$name.fetch configuration to only get certain refs a
 git fetch will still call git-upload pack which will provide a list
 of all references.

 This is being done against a repository with tens of thousands of refs
 (it has a tag for each deployment), so it ends up burning a lot of CPU
 time on the uploader/receiver side.

If all refs are packed, will it still burn lots of CPU on server side?

 Has there been any work on extending the protocol so that the client
 tells the server what refs it's interested in?

It'll be a new protocol, not an extension for git protocol. Ref
advertising is step 1. Capababilities are advertised much later. The
client has to time to tell the server what protocol version it likes
to use for step 1. (I looked at this protcol extension from a
different angle. I wanted to compress the ref list for git protocol.
But git over http compresses well so I don't care much.)

On that git-over-http, I don't know, maybe git client can send
something as http headers, which are recognized by the server end, to
negotiate interested ref patterns?
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Jeff King
On Wed, Oct 03, 2012 at 02:36:00PM +0200, Ævar Arnfjörð Bjarmason wrote:

 I'm creating a system where a lot of remotes constantly fetch from a
 central repository for deployment purposes, but I've noticed that even
 with a remote.$name.fetch configuration to only get certain refs a
 git fetch will still call git-upload pack which will provide a list
 of all references.
 
 This is being done against a repository with tens of thousands of refs
 (it has a tag for each deployment), so it ends up burning a lot of CPU
 time on the uploader/receiver side.

Where is the CPU being burned? Are your refs packed (that's a huge
savings)? What are the refs like? Are they .have refs from an alternates
repository, or real refs? Are they pointing to commits or tag objects?

What version of git are you using?  In the past year or so, I've made
several tweaks to speed up large numbers of refs, including:

  - cff38a5 (receive-pack: eliminate duplicate .have refs, v1.7.6); note
that this only helps if they are being pulled in by an alternates
repo. And even then, it only helps if they are mostly duplicates;
distinct ones are still O(n^2).

  - 7db8d53 (fetch-pack: avoid quadratic behavior in remove_duplicates)
a0de288 (fetch-pack: avoid quadratic loop in filter_refs)
Both in v1.7.11. I think there is still a potential quadratic loop
in mark_complete()

  - 90108a2 (upload-pack: avoid parsing tag destinations)
926f1dd (upload-pack: avoid parsing objects during ref advertisement)
Both in v1.7.10. Note that tag objects are more expensive to
advertise than commits, because we have to load and peel them.

Even with those patches, though, I found that it was something like ~2s
to advertise 100,000 refs.

 Has there been any work on extending the protocol so that the client
 tells the server what refs it's interested in?

I don't think so. It would be hard to do in a backwards-compatible way,
because the advertisement is the first thing the server says, before it
has negotiated any capabilities with the client at all.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Has there been any work on extending the protocol so that the client
 tells the server what refs it's interested in?

 I don't think so. It would be hard to do in a backwards-compatible way,
 because the advertisement is the first thing the server says, before it
 has negotiated any capabilities with the client at all.

That is being discussed but hasn't surfaced on the list.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Jeff King
On Wed, Oct 03, 2012 at 11:53:35AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  Has there been any work on extending the protocol so that the client
  tells the server what refs it's interested in?
 
  I don't think so. It would be hard to do in a backwards-compatible way,
  because the advertisement is the first thing the server says, before it
  has negotiated any capabilities with the client at all.
 
 That is being discussed but hasn't surfaced on the list.

Out of curiosity, how are you thinking about triggering such a new
behavior in a backwards-compatible way? Invoke git-upload-pack2, and
fall back to reconnecting to start git-upload-pack if it fails?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 I'm creating a system where a lot of remotes constantly fetch from a
 central repository for deployment purposes, but I've noticed that even
 with a remote.$name.fetch configuration to only get certain refs a
 git fetch will still call git-upload pack which will provide a list
 of all references.

It has been observed that the sender has to advertise megabytes of
refs because it has to speak first before knowing what the receiver
wants, even when the receiver is interested in getting updates from
only one of them, or worse yet, when the receiver is only trying to
peek the ref it is interested has been updated.

I do not think upload-pack that runs on the sender side with
millions refs, when asked for a single want, feeds all the refs
that it has to the revision machinery, and if you observed it does,
I cannot explain why it happens.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Shawn Pearce
On Wed, Oct 3, 2012 at 11:55 AM, Jeff King p...@peff.net wrote:
 On Wed, Oct 03, 2012 at 11:53:35AM -0700, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:

  Has there been any work on extending the protocol so that the client
  tells the server what refs it's interested in?
 
  I don't think so. It would be hard to do in a backwards-compatible way,
  because the advertisement is the first thing the server says, before it
  has negotiated any capabilities with the client at all.

 That is being discussed but hasn't surfaced on the list.

 Out of curiosity, how are you thinking about triggering such a new
 behavior in a backwards-compatible way? Invoke git-upload-pack2, and
 fall back to reconnecting to start git-upload-pack if it fails?

Basically, yes. New clients connect for git-upload-pack2. Over git://
the remote peer will just close the TCP socket with no messages. The
client can fallback to git-upload-pack and try again. Over SSH a
similar thing will happen in the sense there is no data output from
the remote side, so the client can try again. This has the downside of
authentication twice over SSH, which may prompt for a password twice.
But the user can get out of this by setting remote.NAME.uploadpack =
git-upload-pack and thus force the Git client to use the current
protocol if they have a new client and must continue to work over SSH
with an old server, and don't use an ssh-agent.

Over HTTP we can request ?service=git-upload-pack2 and retry just like
git:// would, or be a bit smarter and say
?service=git-upload-packv=2, and determine the protocol support of
the remote peer based on the response we get. If we see an immediate
advertisement its still the v1 protocol, if we get back the yes I
speak v2 response like git:// would see, we can continue the
conversation from there.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Jeff King
On Wed, Oct 03, 2012 at 12:41:38PM -0700, Shawn O. Pearce wrote:

  Out of curiosity, how are you thinking about triggering such a new
  behavior in a backwards-compatible way? Invoke git-upload-pack2, and
  fall back to reconnecting to start git-upload-pack if it fails?
 
 Basically, yes. New clients connect for git-upload-pack2. Over git://
 the remote peer will just close the TCP socket with no messages. The
 client can fallback to git-upload-pack and try again. Over SSH a
 similar thing will happen in the sense there is no data output from
 the remote side, so the client can try again. This has the downside of
 authentication twice over SSH, which may prompt for a password twice.
 But the user can get out of this by setting remote.NAME.uploadpack =
 git-upload-pack and thus force the Git client to use the current
 protocol if they have a new client and must continue to work over SSH
 with an old server, and don't use an ssh-agent.

It's a shame that we have to reestablish the TCP or ssh connection to do
the retry. The password thing is annoying, but also it just wastes a
round-trip. It means we'd probably want to default the v2 probe to off
(and let the user turn it on for a specific remote) until v2 is much
more common than v1. Otherwise everyone pays the price.

It may also be worth designing v2 to handle more graceful capability
negotiation so this doesn't come up again.

Another alternative would be to tweak git-daemon to allow more graceful
fallback. That wouldn't help us now, but it would if we ever wanted a
v3. For stock ssh, you could send:

  sh -c 'git upload-pack2; test $? = 127  git-upload-pack'

which would work if you have an unrestricted shell on the other side.
But it would break for a restricted shell or other fake ssh
environment. It's probably too ugly to have restricted shells recognize
that as a magic token (well, I could maybe even live with the ugliness,
but it is not strictly backwards compatible).

I was hoping we could do something like git upload-pack --v2, but I'm
pretty sure current git-daemon would reject that.

 Over HTTP we can request ?service=git-upload-pack2 and retry just like
 git:// would, or be a bit smarter and say
 ?service=git-upload-packv=2, and determine the protocol support of
 the remote peer based on the response we get. If we see an immediate
 advertisement its still the v1 protocol, if we get back the yes I
 speak v2 response like git:// would see, we can continue the
 conversation from there.

Yeah, I would think v=2 would be better simply to avoid the
round-trip if we fail. It should be safe to turn the new protocol on by
default for http, then.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Ævar Arnfjörð Bjarmason
On Wed, Oct 3, 2012 at 8:03 PM, Jeff King p...@peff.net wrote:
 On Wed, Oct 03, 2012 at 02:36:00PM +0200, Ævar Arnfjörð Bjarmason wrote:

 I'm creating a system where a lot of remotes constantly fetch from a
 central repository for deployment purposes, but I've noticed that even
 with a remote.$name.fetch configuration to only get certain refs a
 git fetch will still call git-upload pack which will provide a list
 of all references.

 This is being done against a repository with tens of thousands of refs
 (it has a tag for each deployment), so it ends up burning a lot of CPU
 time on the uploader/receiver side.

 Where is the CPU being burned? Are your refs packed (that's a huge
 savings)? What are the refs like? Are they .have refs from an alternates
 repository, or real refs? Are they pointing to commits or tag objects?

 What version of git are you using?  In the past year or so, I've made
 several tweaks to speed up large numbers of refs, including:

   - cff38a5 (receive-pack: eliminate duplicate .have refs, v1.7.6); note
 that this only helps if they are being pulled in by an alternates
 repo. And even then, it only helps if they are mostly duplicates;
 distinct ones are still O(n^2).

   - 7db8d53 (fetch-pack: avoid quadratic behavior in remove_duplicates)
 a0de288 (fetch-pack: avoid quadratic loop in filter_refs)
 Both in v1.7.11. I think there is still a potential quadratic loop
 in mark_complete()

   - 90108a2 (upload-pack: avoid parsing tag destinations)
 926f1dd (upload-pack: avoid parsing objects during ref advertisement)
 Both in v1.7.10. Note that tag objects are more expensive to
 advertise than commits, because we have to load and peel them.

 Even with those patches, though, I found that it was something like ~2s
 to advertise 100,000 refs.

I can't provide all the details now (not with access to that machine
now), but briefly:

 * The git client/server version is 1.7.8

 * The repository has around 50k refs, they're real refs, almost all
   of them (say all but 0.5k-1k) are annotated tags, the rest are
   branches.

 * 99% of them are packed, there's a weekly cronjob that packs them
   all up, there were a few newly pushed branches and tags outside of
   the

 * I tried echo -n | git upload-pack repo on both that 50k
   repository and a repository with 100 refs, the former took around
   ~1-2s to run on a 24 core box and the latter ~500ms.

 * When I ran git-upload-pack with GNU parallel I managed around 20/s
   packs on the 24 core box on the 50k ref one, 40/s on the 100 ref
   one.

 * A co-worker who was working on this today tried it on 1.7.12 and
   claimed that it had the same performance characteristics.

 * I tried to profile it under gcc -pg  echo -n | ./git-upload-pack
   repo but it doesn't produce a profile like that, presumably
   because the process exits unsuccessfully.

   Maybe someone here knows offhand what mock data I could feed
   git-upload-pack to make it happy to just list the refs, or better
   yet do a bit more work which it would do if it were actually doing
   the fetch (I suppose I could just do a fetch, but I wanted to do
   this from a locally compiled checkout).

 Has there been any work on extending the protocol so that the client
 tells the server what refs it's interested in?

 I don't think so. It would be hard to do in a backwards-compatible way,
 because the advertisement is the first thing the server says, before it
 has negotiated any capabilities with the client at all.

I suppose at least for the ssh protocol we could just do:

ssh server (git upload-pack repo --refs=* || git upload-pack repo)

And something similar with HTTP headers, but that of course leaves the
git:// protocol.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Jeff King
On Wed, Oct 03, 2012 at 10:16:56PM +0200, Ævar Arnfjörð Bjarmason wrote:

 I can't provide all the details now (not with access to that machine
 now), but briefly:
 
  * The git client/server version is 1.7.8
 
  * The repository has around 50k refs, they're real refs, almost all
of them (say all but 0.5k-1k) are annotated tags, the rest are
branches.

I'd definitely try upgrading, then; I got measurable speedups from this
exact case using the patches in v1.7.10.

  * 99% of them are packed, there's a weekly cronjob that packs them
all up, there were a few newly pushed branches and tags outside of
the

A few strays shouldn't make a big difference. The killer is calling
open(2) 50,000 times, but having most of it packed should prevent that.
I suspect Michael Haggerty's work on the ref cache may help, too
(otherwise we have to try each packed ref in the filesystem to make sure
nobody has written it since we packed).

  * I tried echo -n | git upload-pack repo on both that 50k
repository and a repository with 100 refs, the former took around
~1-2s to run on a 24 core box and the latter ~500ms.

More cores won't help, of course, as dumping the refs is single-threaded.

With v1.7.12, my ~400K test repository takes about 0.8s to run (on my
2-year-old 1.8 GHz i7, though it is probably turbo-boosting to 3 GHz).
So I'm surprised it is so slow.

Your 100-ref case is slow, too. Upload-pack's initial advertisement on
my linux-2.6 repository (without about 900 refs) is more like 20ms. I'd

  * A co-worker who was working on this today tried it on 1.7.12 and
claimed that it had the same performance characteristics.

That's surprising to me. Can you try to verify those numbers?

  * I tried to profile it under gcc -pg  echo -n | ./git-upload-pack
repo but it doesn't produce a profile like that, presumably
because the process exits unsuccessfully.

If it's a recent version of Linux, you'll get much nicer results with
perf. Here's what my 400K-ref case looks like:

  $ time echo  | perf record git-upload-pack . /dev/null
  real0m0.808s
  user0m0.660s
  sys 0m0.136s

  $ perf report | grep -v ^# | head
  11.40%  git-upload-pack  libc-2.13.so[.] vfprintf
   9.70%  git-upload-pack  git-upload-pack [.] find_pack_entry_one
   7.64%  git-upload-pack  git-upload-pack [.] check_refname_format
   6.81%  git-upload-pack  libc-2.13.so[.] __memcmp_sse4_1
   5.79%  git-upload-pack  libc-2.13.so[.] getenv
   4.20%  git-upload-pack  libc-2.13.so[.] __strlen_sse42
   3.72%  git-upload-pack  git-upload-pack [.] ref_entry_cmp_sslice
   3.15%  git-upload-pack  git-upload-pack [.] read_packed_refs
   2.65%  git-upload-pack  git-upload-pack [.] sha1_to_hex
   2.44%  git-upload-pack  libc-2.13.so[.] _IO_default_xsputn

So nothing too surprising, though there is some room for improvement
(e.g., it looks like we are calling getenv in a tight loop, which could
be hoisted out to a single call).

Do note that this version of git was compiled with -O3. Compiling with
-O0 produces very different results (it's more like 1.3s, and the
hotspots are check_refname_component and sha1_to_hex).

Maybe someone here knows offhand what mock data I could feed
git-upload-pack to make it happy to just list the refs, or better
yet do a bit more work which it would do if it were actually doing
the fetch (I suppose I could just do a fetch, but I wanted to do
this from a locally compiled checkout).

If you feed  as I did above, that is the flush signal for I have
no more lines to send you, which means that we are not actually
fetching anything. I.e., this is the exact same conversation a no-op
git fetch would produce.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Ævar Arnfjörð Bjarmason
On Wed, Oct 3, 2012 at 11:20 PM, Jeff King p...@peff.net wrote:

Thanks for all that info, it's really useful.

  * A co-worker who was working on this today tried it on 1.7.12 and
claimed that it had the same performance characteristics.

 That's surprising to me. Can you try to verify those numbers?

I think he was wrong, I tested this on git.git by first creating a lot
of tags:

 parallel --eta git tag -a -m{} test-again-{} ::: $(git rev-list HEAD)

Then doing:

git pack-refs --all
git repack -A -d

And compiled with -g -O3 I get around 1.55 runs/s of git-upload-pack
on 1.7.8 and 2.59/s on the master branch.

  * I tried to profile it under gcc -pg  echo -n | ./git-upload-pack
repo but it doesn't produce a profile like that, presumably
because the process exits unsuccessfully.

 If it's a recent version of Linux, you'll get much nicer results with
 perf. Here's what my 400K-ref case looks like:

   $ time echo  | perf record git-upload-pack . /dev/null
   real0m0.808s
   user0m0.660s
   sys 0m0.136s

   $ perf report | grep -v ^# | head
   11.40%  git-upload-pack  libc-2.13.so[.] vfprintf
9.70%  git-upload-pack  git-upload-pack [.] find_pack_entry_one
7.64%  git-upload-pack  git-upload-pack [.] check_refname_format
6.81%  git-upload-pack  libc-2.13.so[.] __memcmp_sse4_1
5.79%  git-upload-pack  libc-2.13.so[.] getenv
4.20%  git-upload-pack  libc-2.13.so[.] __strlen_sse42
3.72%  git-upload-pack  git-upload-pack [.] ref_entry_cmp_sslice
3.15%  git-upload-pack  git-upload-pack [.] read_packed_refs
2.65%  git-upload-pack  git-upload-pack [.] sha1_to_hex
2.44%  git-upload-pack  libc-2.13.so[.] _IO_default_xsputn

FWIW here are my results on the above pathological git.git

$ uname -r; perf --version; echo  | perf record
./git-upload-pack ./dev/null; perf report | grep -v ^# | head
3.2.0-2-amd64
perf version 3.2.17
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.026 MB perf.data (~1131 samples) ]
29.08%  git-upload-pack  libz.so.1.2.7   [.] inflate
17.99%  git-upload-pack  libz.so.1.2.7   [.] 0xaec1
 6.21%  git-upload-pack  libc-2.13.so[.] 0x117503
 5.69%  git-upload-pack  libcrypto.so.1.0.0  [.] 0x82c3d
 4.87%  git-upload-pack  git-upload-pack [.] find_pack_entry_one
 3.18%  git-upload-pack  ld-2.13.so  [.] 0x886e
 2.96%  git-upload-pack  libc-2.13.so[.] vfprintf
 2.83%  git-upload-pack  git-upload-pack [.] search_for_subdir
 1.56%  git-upload-pack  [kernel.kallsyms]   [k] do_raw_spin_lock
 1.36%  git-upload-pack  libc-2.13.so[.] vsnprintf

I wonder why your report doesn't note any time in libz. This is on
Debian testing, maybe your OS uses different strip settings so it
doesn't show up?

$ ldd -r ./git-upload-pack
linux-vdso.so.1 =  (0x7fff621ff000)
libz.so.1 = /lib/x86_64-linux-gnu/libz.so.1 (0x7f768feee000)
libcrypto.so.1.0.0 =
/usr/lib/x86_64-linux-gnu/libcrypto.so.1.0.0 (0x7f768fb0a000)
libpthread.so.0 = /lib/x86_64-linux-gnu/libpthread.so.0
(0x7f768f8ed000)
libc.so.6 = /lib/x86_64-linux-gnu/libc.so.6 (0x7f768f566000)
libdl.so.2 = /lib/x86_64-linux-gnu/libdl.so.2 (0x7f768f362000)
/lib64/ld-linux-x86-64.so.2 (0x7f7690117000
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Ævar Arnfjörð Bjarmason
On Wed, Oct 3, 2012 at 8:03 PM, Jeff King p...@peff.net wrote:
 What version of git are you using?  In the past year or so, I've made
 several tweaks to speed up large numbers of refs, including:

   - cff38a5 (receive-pack: eliminate duplicate .have refs, v1.7.6); note
 that this only helps if they are being pulled in by an alternates
 repo. And even then, it only helps if they are mostly duplicates;
 distinct ones are still O(n^2).

   - 7db8d53 (fetch-pack: avoid quadratic behavior in remove_duplicates)
 a0de288 (fetch-pack: avoid quadratic loop in filter_refs)
 Both in v1.7.11. I think there is still a potential quadratic loop
 in mark_complete()

   - 90108a2 (upload-pack: avoid parsing tag destinations)
 926f1dd (upload-pack: avoid parsing objects during ref advertisement)
 Both in v1.7.10. Note that tag objects are more expensive to
 advertise than commits, because we have to load and peel them.

 Even with those patches, though, I found that it was something like ~2s
 to advertise 100,000 refs.

FWIW I bisected between 1.7.9 and 1.7.10 and found that the point at
which it went from 1.5/s to 2.5/s upload-pack runs on the pathological
git.git repository was none of those, but:

ccdc6037fe - parse_object: try internal cache before reading object db
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Jeff King
On Thu, Oct 04, 2012 at 12:15:47AM +0200, Ævar Arnfjörð Bjarmason wrote:

 I think he was wrong, I tested this on git.git by first creating a lot
 of tags:
 
  parallel --eta git tag -a -m{} test-again-{} ::: $(git rev-list HEAD)
 
 Then doing:
 
 git pack-refs --all
 git repack -A -d
 
 And compiled with -g -O3 I get around 1.55 runs/s of git-upload-pack
 on 1.7.8 and 2.59/s on the master branch.

Thanks for the update, that's more like what I expected.

 FWIW here are my results on the above pathological git.git
 
 $ uname -r; perf --version; echo  | perf record
 ./git-upload-pack ./dev/null; perf report | grep -v ^# | head
 3.2.0-2-amd64
 perf version 3.2.17
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.026 MB perf.data (~1131 samples) ]
 29.08%  git-upload-pack  libz.so.1.2.7   [.] inflate
 17.99%  git-upload-pack  libz.so.1.2.7   [.] 0xaec1
  6.21%  git-upload-pack  libc-2.13.so[.] 0x117503
  5.69%  git-upload-pack  libcrypto.so.1.0.0  [.] 0x82c3d
  4.87%  git-upload-pack  git-upload-pack [.] find_pack_entry_one
  3.18%  git-upload-pack  ld-2.13.so  [.] 0x886e
  2.96%  git-upload-pack  libc-2.13.so[.] vfprintf
  2.83%  git-upload-pack  git-upload-pack [.] search_for_subdir
  1.56%  git-upload-pack  [kernel.kallsyms]   [k] do_raw_spin_lock
  1.36%  git-upload-pack  libc-2.13.so[.] vsnprintf
 
 I wonder why your report doesn't note any time in libz. This is on
 Debian testing, maybe your OS uses different strip settings so it
 doesn't show up?

Mine was on Debian unstable. The difference is probably that I have 400K
refs, but only 12K unique ones (this is the master alternates repo
containing every ref from every fork of rails/rails on GitHub). So I
spend proportionally more time fiddling with refs and outputting than
I do actually inflating tag objects.

Hmm. It seems like we should not need to open the tags at all. The main
reason is to produce the peeled advertisement just after it. But for a
packed ref with a modern version of git that supports the peeled
extension, we should already have that information.

The hack-ish patch below tries to reuse that. The interface is terrible,
and we should probably just pass the peel information via for_each_ref
(peel_ref tries to do a similar thing, but it also has a bad interface;
if we don't have the information already, it will redo the ref lookup.
We could probably get away with a peel_sha1 which uses the same
optimization trick as peel_ref).

With this patch my 800ms upload-pack drops to 600ms. I suspect it will
have an even greater impact for you, since you are spending much more of
your time on object loading than I am.

And note of course that while these micro-optimizations are neat, we're
still going to end up shipping quite a lot of data over the wire. Moving
to a protocol where we are advertising fewer refs would solve a lot more
problems in the long run.

---
diff --git a/refs.c b/refs.c
index 551a0f9..68eca3a 100644
--- a/refs.c
+++ b/refs.c
@@ -510,6 +510,14 @@ static struct ref_entry *current_ref;
 
 static struct ref_entry *current_ref;
 
+/* XXX horrible interface due to implied argument. not for real use */
+const unsigned char *peel_current_ref(void)
+{
+   if (!current_ref || !(current_ref-flag  REF_KNOWS_PEELED))
+   return NULL;
+   return current_ref-u.value.peeled;
+}
+
 static int do_one_ref(const char *base, each_ref_fn fn, int trim,
  int flags, void *cb_data, struct ref_entry *entry)
 {
diff --git a/refs.h b/refs.h
index 9d14558..88c5445 100644
--- a/refs.h
+++ b/refs.h
@@ -14,6 +14,8 @@ struct ref_lock {
 #define REF_ISPACKED 0x02
 #define REF_ISBROKEN 0x04
 
+const unsigned char *peel_current_ref(void);
+
 /*
  * Calls the specified function for each ref file until it returns
  * nonzero, and returns the value.  Please note that it is not safe to
diff --git a/upload-pack.c b/upload-pack.c
index 8f4703b..cdf43b0 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -736,8 +736,9 @@ static int send_ref(const char *refname, const unsigned 
char *sha1, int flag, vo
 include-tag multi_ack_detailed;
struct object *o = lookup_unknown_object(sha1);
const char *refname_nons = strip_namespace(refname);
+   const unsigned char *peeled = peel_current_ref();
 
-   if (o-type == OBJ_NONE) {
+   if (!peeled  o-type == OBJ_NONE) {
o-type = sha1_object_info(sha1, NULL);
if (o-type  0)
die(git upload-pack: cannot find object %s:, 
sha1_to_hex(sha1));
@@ -756,11 +757,13 @@ static int send_ref(const char *refname, const unsigned 
char *sha1, int flag, vo
o-flags |= OUR_REF;
nr_our_refs++;
}
-   if (o-type == OBJ_TAG) {
+   if (!peeled  o-type == OBJ_TAG) {
o 

Re: upload-pack is slow with lots of refs

2012-10-03 Thread Jeff King
On Thu, Oct 04, 2012 at 12:32:35AM +0200, Ævar Arnfjörð Bjarmason wrote:

 On Wed, Oct 3, 2012 at 8:03 PM, Jeff King p...@peff.net wrote:
  What version of git are you using?  In the past year or so, I've made
  several tweaks to speed up large numbers of refs, including:
 
- cff38a5 (receive-pack: eliminate duplicate .have refs, v1.7.6); note
  that this only helps if they are being pulled in by an alternates
  repo. And even then, it only helps if they are mostly duplicates;
  distinct ones are still O(n^2).
 
- 7db8d53 (fetch-pack: avoid quadratic behavior in remove_duplicates)
  a0de288 (fetch-pack: avoid quadratic loop in filter_refs)
  Both in v1.7.11. I think there is still a potential quadratic loop
  in mark_complete()
 
- 90108a2 (upload-pack: avoid parsing tag destinations)
  926f1dd (upload-pack: avoid parsing objects during ref advertisement)
  Both in v1.7.10. Note that tag objects are more expensive to
  advertise than commits, because we have to load and peel them.
 
  Even with those patches, though, I found that it was something like ~2s
  to advertise 100,000 refs.
 
 FWIW I bisected between 1.7.9 and 1.7.10 and found that the point at
 which it went from 1.5/s to 2.5/s upload-pack runs on the pathological
 git.git repository was none of those, but:
 
 ccdc6037fe - parse_object: try internal cache before reading object db

Ah, yeah, I forgot about that one. That implies that you have a lot of
refs pointing to the same objects (since the benefit of that commit is
to avoid reading from disk when we have already seen it).

Out of curiosity, what does your repo contain? I saw a lot of speedup
with that commit because my repos are big object stores, where we have
the same duplicated tag refs for every fork of the repo.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Ævar Arnfjörð Bjarmason
On Thu, Oct 4, 2012 at 1:21 AM, Jeff King p...@peff.net wrote:
 On Thu, Oct 04, 2012 at 12:32:35AM +0200, Ævar Arnfjörð Bjarmason wrote:

 On Wed, Oct 3, 2012 at 8:03 PM, Jeff King p...@peff.net wrote:
  What version of git are you using?  In the past year or so, I've made
  several tweaks to speed up large numbers of refs, including:
 
- cff38a5 (receive-pack: eliminate duplicate .have refs, v1.7.6); note
  that this only helps if they are being pulled in by an alternates
  repo. And even then, it only helps if they are mostly duplicates;
  distinct ones are still O(n^2).
 
- 7db8d53 (fetch-pack: avoid quadratic behavior in remove_duplicates)
  a0de288 (fetch-pack: avoid quadratic loop in filter_refs)
  Both in v1.7.11. I think there is still a potential quadratic loop
  in mark_complete()
 
- 90108a2 (upload-pack: avoid parsing tag destinations)
  926f1dd (upload-pack: avoid parsing objects during ref advertisement)
  Both in v1.7.10. Note that tag objects are more expensive to
  advertise than commits, because we have to load and peel them.
 
  Even with those patches, though, I found that it was something like ~2s
  to advertise 100,000 refs.

 FWIW I bisected between 1.7.9 and 1.7.10 and found that the point at
 which it went from 1.5/s to 2.5/s upload-pack runs on the pathological
 git.git repository was none of those, but:

 ccdc6037fe - parse_object: try internal cache before reading object db

 Ah, yeah, I forgot about that one. That implies that you have a lot of
 refs pointing to the same objects (since the benefit of that commit is
 to avoid reading from disk when we have already seen it).

 Out of curiosity, what does your repo contain? I saw a lot of speedup
 with that commit because my repos are big object stores, where we have
 the same duplicated tag refs for every fork of the repo.

Things are much faster with your monkeypatch, got up to around 10
runs/s.

The repository mainly contains a lot of git-deploy[1] generated tags
which are added for every rollout to several subsystems.

Of the ~50k references in the repo 75% point to a commit that no other
reference points to. Around 98% of the references are annotated tags,
the rest are branches.

1. https://github.com/git-deploy/git-deploy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: upload-pack is slow with lots of refs

2012-10-03 Thread Ævar Arnfjörð Bjarmason
On Thu, Oct 4, 2012 at 1:15 AM, Jeff King p...@peff.net wrote:
 On Thu, Oct 04, 2012 at 12:15:47AM +0200, Ævar Arnfjörð Bjarmason wrote:

 I think he was wrong, I tested this on git.git by first creating a lot
 of tags:

  parallel --eta git tag -a -m{} test-again-{} ::: $(git rev-list 
 HEAD)

 Then doing:

 git pack-refs --all
 git repack -A -d

 And compiled with -g -O3 I get around 1.55 runs/s of git-upload-pack
 on 1.7.8 and 2.59/s on the master branch.

 Thanks for the update, that's more like what I expected.

 FWIW here are my results on the above pathological git.git

 $ uname -r; perf --version; echo  | perf record
 ./git-upload-pack ./dev/null; perf report | grep -v ^# | head
 3.2.0-2-amd64
 perf version 3.2.17
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.026 MB perf.data (~1131 samples) ]
 29.08%  git-upload-pack  libz.so.1.2.7   [.] inflate
 17.99%  git-upload-pack  libz.so.1.2.7   [.] 0xaec1
  6.21%  git-upload-pack  libc-2.13.so[.] 0x117503
  5.69%  git-upload-pack  libcrypto.so.1.0.0  [.] 0x82c3d
  4.87%  git-upload-pack  git-upload-pack [.] find_pack_entry_one
  3.18%  git-upload-pack  ld-2.13.so  [.] 0x886e
  2.96%  git-upload-pack  libc-2.13.so[.] vfprintf
  2.83%  git-upload-pack  git-upload-pack [.] search_for_subdir
  1.56%  git-upload-pack  [kernel.kallsyms]   [k] do_raw_spin_lock
  1.36%  git-upload-pack  libc-2.13.so[.] vsnprintf

 I wonder why your report doesn't note any time in libz. This is on
 Debian testing, maybe your OS uses different strip settings so it
 doesn't show up?

 Mine was on Debian unstable. The difference is probably that I have 400K
 refs, but only 12K unique ones (this is the master alternates repo
 containing every ref from every fork of rails/rails on GitHub). So I
 spend proportionally more time fiddling with refs and outputting than
 I do actually inflating tag objects.

An updated profile with your patch:

$ uname -r; perf --version; echo  | perf record
./git-upload-pack ./dev/null; perf report | grep -v ^# | head
3.2.0-2-amd64
perf version 3.2.17
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB perf.data (~662 samples) ]
14.45%  git-upload-pack  libc-2.13.so[.] 0x78140
12.13%  git-upload-pack  [kernel.kallsyms]   [k] walk_component
11.01%  git-upload-pack  libc-2.13.so[.] _IO_getline_info
10.74%  git-upload-pack  git-upload-pack [.] find_pack_entry_one
 8.96%  git-upload-pack  [kernel.kallsyms]   [k] __mmdrop
 8.64%  git-upload-pack  git-upload-pack [.] sha1_to_hex
 6.73%  git-upload-pack  libc-2.13.so[.] vfprintf
 4.07%  git-upload-pack  libc-2.13.so[.] strchrnul
 4.00%  git-upload-pack  libc-2.13.so[.] getenv
 3.37%  git-upload-pack  git-upload-pack [.] packet_write

 Hmm. It seems like we should not need to open the tags at all. The main
 reason is to produce the peeled advertisement just after it. But for a
 packed ref with a modern version of git that supports the peeled
 extension, we should already have that information.

B.t.w. do you plan to submit this as a non-hack, I'd like to have it
in git.git, so if you're not going to I could pick it up and clean it
up a bit. But I think it would be better coming from you.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html