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

[Takuto Ikuta]

Hi,

Thank you for merging!

Takuto

2017-11-28 20:27 GMT+09:00 Johannes Schindelin :
> 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)

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

[Johannes Schindelin]

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

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

[Takuto Ikuta]

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 :
> Hi Junio,
>
> On Tue, 28 Nov 2017, Junio C Hamano wrote:
>
>> Johannes Schindelin  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)

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

[Johannes Schindelin]

Hi Junio,

On Tue, 28 Nov 2017, Junio C Hamano wrote:

> Johannes Schindelin  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

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

[Junio C Hamano]

Johannes Schindelin  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.

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

[Johannes Schindelin]

Hi,

On Mon, 27 Nov 2017, Takuto Ikuta wrote:

> 2017-11-27 13:53 GMT+09:00 Junio C Hamano :
> > Jeff King  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

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

[Jeff King]

On Mon, Nov 27, 2017 at 01:53:34PM +0900, Junio C Hamano wrote:

> > 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

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

[Takuto Ikuta]

2017-11-27 13:53 GMT+09:00 Junio C Hamano :
> Jeff King  writes:
>
>>> cf. 
>>> https://public-inbox.org/git/20171120202920.7ppcwmzkxifyw...@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.

>>
>>> 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)

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

[Junio C Hamano]

Jeff King  writes:

>> cf. 
>> https://public-inbox.org/git/20171120202920.7ppcwmzkxifyw...@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.

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

[Jeff King]

On Mon, Nov 27, 2017 at 01:35:35PM +0900, Junio C Hamano wrote:

> Takuto Ikuta  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(>old_oid))
> > +   if (!has_object_file_with_flags(>old_oid, 
> > OBJECT_INFO_QUICK))
> > continue;
> >  
> 
> It appears that great minds think alike?
> 
> cf. 
> https://public-inbox.org/git/20171120202920.7ppcwmzkxifyw...@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

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

[Junio C Hamano]

Takuto Ikuta  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(>old_oid))
> + if (!has_object_file_with_flags(>old_oid, 
> OBJECT_INFO_QUICK))
>   continue;
>  

It appears that great minds think alike?

cf. 
https://public-inbox.org/git/20171120202920.7ppcwmzkxifyw...@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.

[PATCH] Use OBJECT_INFO_QUICK to speedup git fetch-pack

[Takuto Ikuta]

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 30 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 
---
 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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(>old_oid))
+   if (!has_object_file_with_flags(>old_oid, 
OBJECT_INFO_QUICK))
continue;
 
o = parse_object(>old_oid);

--
https://github.com/git/git/pull/437