threads / patch / 47326

patchUse OBJECT_INFO_QUICK to speedup git fetch-pack

Subject: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

## tl;dr

12 messages between Nov 27, 2017 and Nov 28, 2017. Diffs are folded; open one to read it.

replies: 11people: 4as markdown or json

Takuto Ikuta· Nov 27, 2017, 04:27 UTC · lore

Do not call prepare_packed_git for every refs. In fetch-pack, git list ups entries in .git/objects/pack directory for each refs. Such behavior makes git fetch-pack slow for the repository having the large number of refs, especially for windows.

For chromium repository, having more than 300000 remote refs, git fetch takes more than 3 minutes if fetch-pack runs on my windows workstation. This patch improves the time to around 17 seconds in the same machine.

Signed-off-by: Takuto Ikuta <tikuta@google.com>
---
 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to fetch-pack.c +1 −2
diff --git a/fetch-pack.c b/fetch-pack.c
index 008b25d3db087..0184584e80599 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -716,7 +716,7 @@ static int everything_local(struct fetch_pack_args *args,
 	for (ref = *refs; ref; ref = ref->next) {
 		struct object *o;
 
-		if (!has_object_file(&ref->old_oid))
+		if (!has_object_file_with_flags(&ref->old_oid, OBJECT_INFO_QUICK))
 			continue;
 
 		o = parse_object(&ref->old_oid);

--
https://github.com/git/git/pull/437
Junio C Hamano· Nov 27, 2017, 04:35 UTC · re: Takuto Ikuta · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Takuto Ikuta <tikuta@google.com> writes:
Show 12 quoted lines
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 008b25d3db087..0184584e80599 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -716,7 +716,7 @@ static int everything_local(struct fetch_pack_args *args,
>  	for (ref = *refs; ref; ref = ref->next) {
>  		struct object *o;
>  
> -		if (!has_object_file(&ref->old_oid))
> +		if (!has_object_file_with_flags(&ref->old_oid, OBJECT_INFO_QUICK))
>  			continue;
>  
It appears that great minds think alike?
cf. https://public-inbox.org/git/20171120202920.7ppcwmzkxifywtoj@sigill.intra.peff.net/

The 5-patch series that contains the same change as this one is cooking and will hopefully be in the released version before the end of the year.

Jeff King· Nov 27, 2017, 04:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

On Mon, Nov 27, 2017 at 01:35:35PM +0900, Junio C Hamano wrote:
Show 18 quoted lines
> Takuto Ikuta <tikuta@google.com> writes:
> 
> > diff --git a/fetch-pack.c b/fetch-pack.c
> > index 008b25d3db087..0184584e80599 100644
> > --- a/fetch-pack.c
> > +++ b/fetch-pack.c
> > @@ -716,7 +716,7 @@ static int everything_local(struct fetch_pack_args *args,
> >  	for (ref = *refs; ref; ref = ref->next) {
> >  		struct object *o;
> >  
> > -		if (!has_object_file(&ref->old_oid))
> > +		if (!has_object_file_with_flags(&ref->old_oid, OBJECT_INFO_QUICK))
> >  			continue;
> >  
> 
> It appears that great minds think alike?
> 
> cf. https://public-inbox.org/git/20171120202920.7ppcwmzkxifywtoj@sigill.intra.peff.net/

It's funny that we'd get two patches so close together. AFAIK the slowness here has been with us for years, and I just happened to investigate it recently.

> The 5-patch series that contains the same change as this one is
> cooking and will hopefully be in the released version before the end
> of the year.

I'd be curious if the 5th patch there provides an additional speedup for Takuto's case.

-Peff
Junio C Hamano· Nov 27, 2017, 04:53 UTC · re: Jeff King · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Jeff King <peff@peff.net> writes:
Show 12 quoted lines
>> cf. https://public-inbox.org/git/20171120202920.7ppcwmzkxifywtoj@sigill.intra.peff.net/
>
> It's funny that we'd get two patches so close together. AFAIK the
> slowness here has been with us for years, and I just happened to
> investigate it recently.
>
>> The 5-patch series that contains the same change as this one is
>> cooking and will hopefully be in the released version before the end
>> of the year.
>
> I'd be curious if the 5th patch there provides an additional speedup for
> Takuto's case.
Indeed, it is a very good point.

IIUC, the 5th one is about fetching tons of refs that you have never seen, right? If a repository that has trouble with everything-local is suffering because it right now has 300k remote-tracking branches, I'd imagine that these remote-tracking branches are being added at a considerable rate, so I'd not be surprised if these "new" refs benefits from that patch. And it would be nice to know how much a real life scenario actually does improve.

Thanks.
Takuto Ikuta· Nov 27, 2017, 05:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

2017-11-27 13:53 GMT+09:00 Junio C Hamano <gitster@pobox.com>:
Show 7 quoted lines
> Jeff King <peff@peff.net> writes:
>
>>> cf. https://public-inbox.org/git/20171120202920.7ppcwmzkxifywtoj@sigill.intra.peff.net/
>>
>> It's funny that we'd get two patches so close together. AFAIK the
>> slowness here has been with us for years, and I just happened to
>> investigate it recently.

Yes, thank you for let me know. Please ignore my patch, sorry.

Show 19 quoted lines
>>
>>> The 5-patch series that contains the same change as this one is
>>> cooking and will hopefully be in the released version before the end
>>> of the year.
>>
>> I'd be curious if the 5th patch there provides an additional speedup for
>> Takuto's case.
>
> Indeed, it is a very good point.
>
> IIUC, the 5th one is about fetching tons of refs that you have never
> seen, right?  If a repository that has trouble with everything-local
> is suffering because it right now has 300k remote-tracking branches,
> I'd imagine that these remote-tracking branches are being added at a
> considerable rate, so I'd not be surprised if these "new" refs
> benefits from that patch.  And it would be nice to know how much a
> real life scenario actually does improve.
>
> Thanks.

In chromium repository, your 5th patch does not improve performance, took more than 5 minutes to run fetch on windows. 4th patch is very important for the repository in daily fetch. I hope your 4th patch will be merged.

Thanks.
-- 
Takuto Ikuta
Software Engineer in Tokyo
Chrome Infrastructure (goma team)
Johannes Schindelin· Nov 27, 2017, 21:24 UTC · re: Takuto Ikuta · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Hi,
On Mon, 27 Nov 2017, Takuto Ikuta wrote:
Show 26 quoted lines
> 2017-11-27 13:53 GMT+09:00 Junio C Hamano <gitster@pobox.com>:
> > Jeff King <peff@peff.net> writes:
> >
> >>> The 5-patch series that contains the same change as this one is
> >>> cooking and will hopefully be in the released version before the end
> >>> of the year.
> >>
> >> I'd be curious if the 5th patch there provides an additional speedup
> >> for Takuto's case.
> >
> > Indeed, it is a very good point.
> >
> > IIUC, the 5th one is about fetching tons of refs that you have never
> > seen, right?  If a repository that has trouble with everything-local
> > is suffering because it right now has 300k remote-tracking branches,
> > I'd imagine that these remote-tracking branches are being added at a
> > considerable rate, so I'd not be surprised if these "new" refs
> > benefits from that patch.  And it would be nice to know how much a
> > real life scenario actually does improve.
> >
> > Thanks.
> 
> In chromium repository,  your 5th patch does not improve performance,
> took more than 5 minutes to run fetch on windows.
> 4th patch is very important for the repository in daily fetch.
> I hope your 4th patch will be merged.

If you want it in Git for Windows early (where performance is traditionally worse than on Linux because Git is really optimized for Linux), I would not mind a Pull Request at https://github.com/git-for-windows/git.

My current plan is to release a new Git for Windows version on Wednesday, to allow for a new cURL version to be bundled.

Ciao, Johannes

Junio C Hamano· Nov 27, 2017, 23:52 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> My current plan is to release a new Git for Windows version on Wednesday,
> to allow for a new cURL version to be bundled.

Is this an updated 2.15.0 or are you planning to package 2.15.1? I am asking because you earlier asked me to hold 2.15.1 while you were away last week and there are accumulated changes on 'maint'.

As I won't be online later this week, unless I cut 2.15.1 today or early tomorrow, it will have to be done early next week, and I'd prefer not to do 2.15.1 _immediately_ after a platform announces an updated release of 2.15.0 for obvious reasons.

Johannes Schindelin· Nov 28, 2017, 00:33 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Hi Junio,
On Tue, 28 Nov 2017, Junio C Hamano wrote:
Show 6 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > My current plan is to release a new Git for Windows version on Wednesday,
> > to allow for a new cURL version to be bundled.
> 
> Is this an updated 2.15.0 or are you planning to package 2.15.1?

If there is a 2.15.1 to pick up for me, I'll take it. Otherwise it'll be Git for Windows v2.15.0(2).

Ciao, Dscho

Takuto Ikuta· Nov 28, 2017, 01:58 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Hi Johannes,

As long as this PR is included in next Git for Windows release, I won't suffer from slow git fetch. https://github.com/git-for-windows/git/pull/1372

But I sent you 2 PRs to follow right way. https://github.com/git-for-windows/git/pull/1379 https://github.com/git-for-windows/git/pull/1380 Feel free to merge these PRs.

Thanks.
2017-11-28 9:33 GMT+09:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
Show 16 quoted lines
> Hi Junio,
>
> On Tue, 28 Nov 2017, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > My current plan is to release a new Git for Windows version on Wednesday,
>> > to allow for a new cURL version to be bundled.
>>
>> Is this an updated 2.15.0 or are you planning to package 2.15.1?
>
> If there is a 2.15.1 to pick up for me, I'll take it. Otherwise it'll be
> Git for Windows v2.15.0(2).
>
> Ciao,
> Dscho
-- 
Takuto Ikuta
Software Engineer in Tokyo
Chrome Infrastructure (goma team)
Johannes Schindelin· Nov 28, 2017, 11:27 UTC · re: Takuto Ikuta · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Hi,
On Tue, 28 Nov 2017, Takuto Ikuta wrote:
Show 8 quoted lines
> As long as this PR is included in next Git for Windows release, I
> won't suffer from slow git fetch.
> https://github.com/git-for-windows/git/pull/1372
> 
> But I sent you 2 PRs to follow right way.
> https://github.com/git-for-windows/git/pull/1379
> https://github.com/git-for-windows/git/pull/1380
> Feel free to merge these PRs.
I amended them slightly, and merged them.

Thank you, Dscho

Takuto Ikuta· Nov 28, 2017, 11:35 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

Hi,
Thank you for merging!
Takuto
2017-11-28 20:27 GMT+09:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
Show 17 quoted lines
> Hi,
>
> On Tue, 28 Nov 2017, Takuto Ikuta wrote:
>
>> As long as this PR is included in next Git for Windows release, I
>> won't suffer from slow git fetch.
>> https://github.com/git-for-windows/git/pull/1372
>>
>> But I sent you 2 PRs to follow right way.
>> https://github.com/git-for-windows/git/pull/1379
>> https://github.com/git-for-windows/git/pull/1380
>> Feel free to merge these PRs.
>
> I amended them slightly, and merged them.
>
> Thank you,
> Dscho
-- 
Takuto Ikuta
Software Engineer in Tokyo
Chrome Infrastructure (goma team)
Jeff King· Nov 27, 2017, 05:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

On Mon, Nov 27, 2017 at 01:53:34PM +0900, Junio C Hamano wrote:
Show 12 quoted lines
> > I'd be curious if the 5th patch there provides an additional speedup for
> > Takuto's case.
> 
> Indeed, it is a very good point.
> 
> IIUC, the 5th one is about fetching tons of refs that you have never
> seen, right?  If a repository that has trouble with everything-local
> is suffering because it right now has 300k remote-tracking branches,
> I'd imagine that these remote-tracking branches are being added at a
> considerable rate, so I'd not be surprised if these "new" refs
> benefits from that patch.  And it would be nice to know how much a
> real life scenario actually does improve.

Right, I think it will only kick in for new refs (and maybe deleted ones, but I didn't check).

I actually did have a real life scenario that was improved, but it's not very typical. As I've mentioned before, we keep a big shared-object repository for forks of a single repo, so a fairly common operation is

  git -C network.git fetch ../$fork.git +refs/*:refs/remotes/$fork/*
The first time that's run on a fork, everything looks like a new ref.
So "real life", but thankfully not life for most git users. :)
-Peff

← back to recent threads