Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Konstantin Tokarev]

Hello,

Is it a known thing that addition of --filter=blob:none to workflow with shalow clone (e.g. --depth=1)
and following sparse checkout may significantly slow down process and result in much larger
.git repository?

In case anyone is interested, I've posted my measurements at [1].

I understand this may have something to do with GitHub's server side implementation, but AFAIK
there are some GitHub folks here as well.

[1] https://gist.github.com/annulen/835ac561e22bedd7138d13392a7a53be

-- 
Regards,
Konstantin

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Jeff King]

On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:

> Is it a known thing that addition of --filter=blob:none to workflow
> with shalow clone (e.g. --depth=1) and following sparse checkout may
> significantly slow down process and result in much larger .git
> repository?
> 
> In case anyone is interested, I've posted my measurements at [1].
> 
> I understand this may have something to do with GitHub's server side
> implementation, but AFAIK there are some GitHub folks here as well.

I think the problem is on the client side. Just with a local git.git
clone, try this:

  $ git config uploadpack.allowfilter true
  $ git clone --no-local --bare --depth=1 --filter=blob:none . both.git
  Cloning into bare repository 'both.git'...
  remote: Enumerating objects: 197, done.
  remote: Counting objects: 100% (197/197), done.
  remote: Compressing objects: 100% (153/153), done.
  remote: Total 197 (delta 3), reused 171 (delta 3), pack-reused 0
  Receiving objects: 100% (197/197), 113.63 KiB | 28.41 MiB/s, done.
  Resolving deltas: 100% (3/3), done.
  remote: Enumerating objects: 1871, done.
  remote: Counting objects: 100% (1871/1871), done.
  remote: Compressing objects: 100% (870/870), done.
  remote: Total 1871 (delta 1001), reused 1855 (delta 994), pack-reused 0
  Receiving objects: 100% (1871/1871), 384.93 KiB | 38.49 MiB/s, done.
  Resolving deltas: 100% (1001/1001), done.
  remote: Enumerating objects: 1878, done.
  remote: Counting objects: 100% (1878/1878), done.
  remote: Compressing objects: 100% (872/872), done.
  remote: Total 1878 (delta 1004), reused 1864 (delta 999), pack-reused 0
  Receiving objects: 100% (1878/1878), 386.41 KiB | 25.76 MiB/s, done.
  Resolving deltas: 100% (1004/1004), done.
  remote: Enumerating objects: 1903, done.
  remote: Counting objects: 100% (1903/1903), done.
  remote: Compressing objects: 100% (882/882), done.
  remote: Total 1903 (delta 1020), reused 1890 (delta 1014), pack-reused 0
  Receiving objects: 100% (1903/1903), 391.05 KiB | 16.29 MiB/s, done.
  Resolving deltas: 100% (1020/1020), done.
  remote: Enumerating objects: 1975, done.
  remote: Counting objects: 100% (1975/1975), done.
  remote: Compressing objects: 100% (915/915), done.
  remote: Total 1975 (delta 1059), reused 1959 (delta 1052), pack-reused 0
  Receiving objects: 100% (1975/1975), 405.58 KiB | 16.90 MiB/s, done.
  Resolving deltas: 100% (1059/1059), done.
  [...and so on...]

Oops. The backtrace for the clone during this process looks like:

  [...]
  #11 0x000055b980be01dc in fetch_objects (remote_name=0x55b981607620 "origin", oids=0x55b9816217a8, oid_nr=1)
      at promisor-remote.c:47
  #12 0x000055b980be0812 in promisor_remote_get_direct (repo=0x55b980dcab00 <the_repo>, oids=0x55b9816217a8, oid_nr=1)
      at promisor-remote.c:247
  #13 0x000055b980c3e475 in do_oid_object_info_extended (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, 
      oi=0x55b980dcaec0 <blank_oi>, flags=0) at sha1-file.c:1511
  #14 0x000055b980c3e579 in oid_object_info_extended (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, oi=0x0, flags=0)
      at sha1-file.c:1544
  #15 0x000055b980c3f7bc in repo_has_object_file_with_flags (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, flags=0)
      at sha1-file.c:1980
  #16 0x000055b980c3f7ee in repo_has_object_file (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8) at sha1-file.c:1986
  #17 0x000055b980a54533 in write_followtags (refs=0x55b981610900, 
      msg=0x55b981601230 "clone: from /home/peff/compile/git/.") at builtin/clone.c:646
  #18 0x000055b980a54723 in update_remote_refs (refs=0x55b981610900, mapped_refs=0x55b98160fe20, 
      remote_head_points_at=0x0, branch_top=0x55b981601130 "refs/heads/", 
      msg=0x55b981601230 "clone: from /home/peff/compile/git/.", transport=0x55b98160da90, check_connectivity=1, 
      check_refs_are_promisor_objects_only=1) at builtin/clone.c:699
  #19 0x000055b980a5625b in cmd_clone (argc=2, argv=0x7fff5e0a1e70, prefix=0x0) at builtin/clone.c:1280
  [...]

So I guess the problem is not with shallow clones specifically, but they
lead us to not having fetched the commits pointed to by tags, which
leads to us trying to fault in those commits (and their trees) rather
than realizing that we weren't meant to have them. And the size of the
local repo balloons because you're fetching all those commits one by
one, and not getting the benefit of the deltas you would when you do a
single --filter=blob:none fetch.

I guess we need something like this:

diff --git a/builtin/clone.c b/builtin/clone.c
index 488bdb0741..a1879994f5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -643,7 +643,8 @@ static void write_followtags(const struct ref *refs, const char *msg)
 			continue;
 		if (ends_with(ref->name, "^{}"))
 			continue;
-		if (!has_object_file(&ref->old_oid))
+		if (!has_object_file_with_flags(&ref->old_oid,
+						OBJECT_INFO_SKIP_FETCH_OBJECT))
 			continue;
 		update_ref(msg, ref->name, &ref->old_oid, NULL, 0,
 			   UPDATE_REFS_DIE_ON_ERR);

which seems to produce the desired result.

-Peff

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Derrick Stolee]

On 3/28/2020 10:40 AM, Jeff King wrote:
> On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:
> 
>> Is it a known thing that addition of --filter=blob:none to workflow
>> with shalow clone (e.g. --depth=1) and following sparse checkout may
>> significantly slow down process and result in much larger .git
>> repository?

In general, I would recommend not using shallow clones in conjunction
with partial clone. The blob:none filter will get you what you really
want from shallow clone without any of the downsides of shallow clone.

You do point out a bug that happens when these features are combined,
which is helpful. I'm just recommending that you do not combine these
features as you'll have a better experience (in my opinion).

>> In case anyone is interested, I've posted my measurements at [1].
>>
>> I understand this may have something to do with GitHub's server side
>> implementation, but AFAIK there are some GitHub folks here as well.
> 
> I think the problem is on the client side. Just with a local git.git
> clone, try this:
> 
>   $ git config uploadpack.allowfilter true
>   $ git clone --no-local --bare --depth=1 --filter=blob:none . both.git
>   Cloning into bare repository 'both.git'...
>   remote: Enumerating objects: 197, done.
>   remote: Counting objects: 100% (197/197), done.
>   remote: Compressing objects: 100% (153/153), done.
>   remote: Total 197 (delta 3), reused 171 (delta 3), pack-reused 0
>   Receiving objects: 100% (197/197), 113.63 KiB | 28.41 MiB/s, done.
>   Resolving deltas: 100% (3/3), done.
>   remote: Enumerating objects: 1871, done.
>   remote: Counting objects: 100% (1871/1871), done.
>   remote: Compressing objects: 100% (870/870), done.
>   remote: Total 1871 (delta 1001), reused 1855 (delta 994), pack-reused 0
>   Receiving objects: 100% (1871/1871), 384.93 KiB | 38.49 MiB/s, done.
>   Resolving deltas: 100% (1001/1001), done.
>   remote: Enumerating objects: 1878, done.
>   remote: Counting objects: 100% (1878/1878), done.
>   remote: Compressing objects: 100% (872/872), done.
>   remote: Total 1878 (delta 1004), reused 1864 (delta 999), pack-reused 0
>   Receiving objects: 100% (1878/1878), 386.41 KiB | 25.76 MiB/s, done.
>   Resolving deltas: 100% (1004/1004), done.
>   remote: Enumerating objects: 1903, done.
>   remote: Counting objects: 100% (1903/1903), done.
>   remote: Compressing objects: 100% (882/882), done.
>   remote: Total 1903 (delta 1020), reused 1890 (delta 1014), pack-reused 0
>   Receiving objects: 100% (1903/1903), 391.05 KiB | 16.29 MiB/s, done.
>   Resolving deltas: 100% (1020/1020), done.
>   remote: Enumerating objects: 1975, done.
>   remote: Counting objects: 100% (1975/1975), done.
>   remote: Compressing objects: 100% (915/915), done.
>   remote: Total 1975 (delta 1059), reused 1959 (delta 1052), pack-reused 0
>   Receiving objects: 100% (1975/1975), 405.58 KiB | 16.90 MiB/s, done.
>   Resolving deltas: 100% (1059/1059), done.
>   [...and so on...]
> 
> Oops. The backtrace for the clone during this process looks like:
> 
>   [...]
>   #11 0x000055b980be01dc in fetch_objects (remote_name=0x55b981607620 "origin", oids=0x55b9816217a8, oid_nr=1)
>       at promisor-remote.c:47
>   #12 0x000055b980be0812 in promisor_remote_get_direct (repo=0x55b980dcab00 <the_repo>, oids=0x55b9816217a8, oid_nr=1)
>       at promisor-remote.c:247
>   #13 0x000055b980c3e475 in do_oid_object_info_extended (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, 
>       oi=0x55b980dcaec0 <blank_oi>, flags=0) at sha1-file.c:1511
>   #14 0x000055b980c3e579 in oid_object_info_extended (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, oi=0x0, flags=0)
>       at sha1-file.c:1544
>   #15 0x000055b980c3f7bc in repo_has_object_file_with_flags (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, flags=0)
>       at sha1-file.c:1980
>   #16 0x000055b980c3f7ee in repo_has_object_file (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8) at sha1-file.c:1986
>   #17 0x000055b980a54533 in write_followtags (refs=0x55b981610900, 
>       msg=0x55b981601230 "clone: from /home/peff/compile/git/.") at builtin/clone.c:646
>   #18 0x000055b980a54723 in update_remote_refs (refs=0x55b981610900, mapped_refs=0x55b98160fe20, 
>       remote_head_points_at=0x0, branch_top=0x55b981601130 "refs/heads/", 
>       msg=0x55b981601230 "clone: from /home/peff/compile/git/.", transport=0x55b98160da90, check_connectivity=1, 
>       check_refs_are_promisor_objects_only=1) at builtin/clone.c:699
>   #19 0x000055b980a5625b in cmd_clone (argc=2, argv=0x7fff5e0a1e70, prefix=0x0) at builtin/clone.c:1280
>   [...]
> 
> So I guess the problem is not with shallow clones specifically, but they
> lead us to not having fetched the commits pointed to by tags, which
> leads to us trying to fault in those commits (and their trees) rather
> than realizing that we weren't meant to have them. And the size of the
> local repo balloons because you're fetching all those commits one by
> one, and not getting the benefit of the deltas you would when you do a
> single --filter=blob:none fetch.
> 
> I guess we need something like this:
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 488bdb0741..a1879994f5 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -643,7 +643,8 @@ static void write_followtags(const struct ref *refs, const char *msg)
>  			continue;
>  		if (ends_with(ref->name, "^{}"))
>  			continue;
> -		if (!has_object_file(&ref->old_oid))
> +		if (!has_object_file_with_flags(&ref->old_oid,
> +						OBJECT_INFO_SKIP_FETCH_OBJECT))
>  			continue;
>  		update_ref(msg, ref->name, &ref->old_oid, NULL, 0,
>  			   UPDATE_REFS_DIE_ON_ERR);
> 
> which seems to produce the desired result.

This is a good find, and I expect we will find more "opportunities"
to insert OBJECT_INFO_SKIP_FETCH_OBJECT like this.

-Stolee

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Taylor Blau]

On Sat, Mar 28, 2020 at 12:58:41PM -0400, Derrick Stolee wrote:
> On 3/28/2020 10:40 AM, Jeff King wrote:
> > On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:
> >
> >> Is it a known thing that addition of --filter=blob:none to workflow
> >> with shalow clone (e.g. --depth=1) and following sparse checkout may
> >> significantly slow down process and result in much larger .git
> >> repository?
>
> In general, I would recommend not using shallow clones in conjunction
> with partial clone. The blob:none filter will get you what you really
> want from shallow clone without any of the downsides of shallow clone.
>
> You do point out a bug that happens when these features are combined,
> which is helpful. I'm just recommending that you do not combine these
> features as you'll have a better experience (in my opinion).
>
> >> In case anyone is interested, I've posted my measurements at [1].
> >>
> >> I understand this may have something to do with GitHub's server side
> >> implementation, but AFAIK there are some GitHub folks here as well.
> >
> > I think the problem is on the client side. Just with a local git.git
> > clone, try this:
> >
> >   $ git config uploadpack.allowfilter true
> >   $ git clone --no-local --bare --depth=1 --filter=blob:none . both.git
> >   Cloning into bare repository 'both.git'...
> >   remote: Enumerating objects: 197, done.
> >   remote: Counting objects: 100% (197/197), done.
> >   remote: Compressing objects: 100% (153/153), done.
> >   remote: Total 197 (delta 3), reused 171 (delta 3), pack-reused 0
> >   Receiving objects: 100% (197/197), 113.63 KiB | 28.41 MiB/s, done.
> >   Resolving deltas: 100% (3/3), done.
> >   remote: Enumerating objects: 1871, done.
> >   remote: Counting objects: 100% (1871/1871), done.
> >   remote: Compressing objects: 100% (870/870), done.
> >   remote: Total 1871 (delta 1001), reused 1855 (delta 994), pack-reused 0
> >   Receiving objects: 100% (1871/1871), 384.93 KiB | 38.49 MiB/s, done.
> >   Resolving deltas: 100% (1001/1001), done.
> >   remote: Enumerating objects: 1878, done.
> >   remote: Counting objects: 100% (1878/1878), done.
> >   remote: Compressing objects: 100% (872/872), done.
> >   remote: Total 1878 (delta 1004), reused 1864 (delta 999), pack-reused 0
> >   Receiving objects: 100% (1878/1878), 386.41 KiB | 25.76 MiB/s, done.
> >   Resolving deltas: 100% (1004/1004), done.
> >   remote: Enumerating objects: 1903, done.
> >   remote: Counting objects: 100% (1903/1903), done.
> >   remote: Compressing objects: 100% (882/882), done.
> >   remote: Total 1903 (delta 1020), reused 1890 (delta 1014), pack-reused 0
> >   Receiving objects: 100% (1903/1903), 391.05 KiB | 16.29 MiB/s, done.
> >   Resolving deltas: 100% (1020/1020), done.
> >   remote: Enumerating objects: 1975, done.
> >   remote: Counting objects: 100% (1975/1975), done.
> >   remote: Compressing objects: 100% (915/915), done.
> >   remote: Total 1975 (delta 1059), reused 1959 (delta 1052), pack-reused 0
> >   Receiving objects: 100% (1975/1975), 405.58 KiB | 16.90 MiB/s, done.
> >   Resolving deltas: 100% (1059/1059), done.
> >   [...and so on...]
> >
> > Oops. The backtrace for the clone during this process looks like:
> >
> >   [...]
> >   #11 0x000055b980be01dc in fetch_objects (remote_name=0x55b981607620 "origin", oids=0x55b9816217a8, oid_nr=1)
> >       at promisor-remote.c:47
> >   #12 0x000055b980be0812 in promisor_remote_get_direct (repo=0x55b980dcab00 <the_repo>, oids=0x55b9816217a8, oid_nr=1)
> >       at promisor-remote.c:247
> >   #13 0x000055b980c3e475 in do_oid_object_info_extended (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8,
> >       oi=0x55b980dcaec0 <blank_oi>, flags=0) at sha1-file.c:1511
> >   #14 0x000055b980c3e579 in oid_object_info_extended (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, oi=0x0, flags=0)
> >       at sha1-file.c:1544
> >   #15 0x000055b980c3f7bc in repo_has_object_file_with_flags (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8, flags=0)
> >       at sha1-file.c:1980
> >   #16 0x000055b980c3f7ee in repo_has_object_file (r=0x55b980dcab00 <the_repo>, oid=0x55b9816217a8) at sha1-file.c:1986
> >   #17 0x000055b980a54533 in write_followtags (refs=0x55b981610900,
> >       msg=0x55b981601230 "clone: from /home/peff/compile/git/.") at builtin/clone.c:646
> >   #18 0x000055b980a54723 in update_remote_refs (refs=0x55b981610900, mapped_refs=0x55b98160fe20,
> >       remote_head_points_at=0x0, branch_top=0x55b981601130 "refs/heads/",
> >       msg=0x55b981601230 "clone: from /home/peff/compile/git/.", transport=0x55b98160da90, check_connectivity=1,
> >       check_refs_are_promisor_objects_only=1) at builtin/clone.c:699
> >   #19 0x000055b980a5625b in cmd_clone (argc=2, argv=0x7fff5e0a1e70, prefix=0x0) at builtin/clone.c:1280
> >   [...]
> >
> > So I guess the problem is not with shallow clones specifically, but they
> > lead us to not having fetched the commits pointed to by tags, which
> > leads to us trying to fault in those commits (and their trees) rather
> > than realizing that we weren't meant to have them. And the size of the
> > local repo balloons because you're fetching all those commits one by
> > one, and not getting the benefit of the deltas you would when you do a
> > single --filter=blob:none fetch.
> >
> > I guess we need something like this:
> >
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 488bdb0741..a1879994f5 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -643,7 +643,8 @@ static void write_followtags(const struct ref *refs, const char *msg)
> >  			continue;
> >  		if (ends_with(ref->name, "^{}"))
> >  			continue;
> > -		if (!has_object_file(&ref->old_oid))
> > +		if (!has_object_file_with_flags(&ref->old_oid,
> > +						OBJECT_INFO_SKIP_FETCH_OBJECT))
> >  			continue;
> >  		update_ref(msg, ref->name, &ref->old_oid, NULL, 0,
> >  			   UPDATE_REFS_DIE_ON_ERR);
> >
> > which seems to produce the desired result.
>
> This is a good find, and I expect we will find more "opportunities"
> to insert OBJECT_INFO_SKIP_FETCH_OBJECT like this.

Should we turn this into a proper patch and have it reviewed? It seems
to be helping the situation, and after thinking about it (only briefly,
but more than not ;-)), this seems like the right direction.

There's an argument to be had about bundling a number of these up
instead of having a slow drip of patches that sprinkle
'SKIP_FETCH_OBJECT' everywhere, but I don't think that we want perfect
to be the enemy of good here.

Peff, if you don't feel like doing this, or have a backlog that is too
long, I'd be happy to polish this for you.

> -Stolee

Thanks,
Taylor

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Jeff King]

On Tue, Mar 31, 2020 at 03:46:53PM -0600, Taylor Blau wrote:

> On Sat, Mar 28, 2020 at 12:58:41PM -0400, Derrick Stolee wrote:
>
> > This is a good find, and I expect we will find more "opportunities"
> > to insert OBJECT_INFO_SKIP_FETCH_OBJECT like this.

The worst part is that we _did_ find this in git-fetch, and fixed it in
3e96c66805 (partial-clone: avoid fetching when looking for objects,
2020-02-21). But as usual git-clone has its own kind-of-the-same
implementation of the same feature.

> Should we turn this into a proper patch and have it reviewed? It seems
> to be helping the situation, and after thinking about it (only briefly,
> but more than not ;-)), this seems like the right direction.

Yes, I wanted to simplify the test and double-check the addition of
QUICK. See the patch I just posted in:

  https://lore.kernel.org/git/20200401121537.GA1916590@coredump.intra.peff.net/

-Peff

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Konstantin Tokarev]

28.03.2020, 19:58, "Derrick Stolee" <stolee@gmail.com>:
> On 3/28/2020 10:40 AM, Jeff King wrote:
>>  On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:
>>
>>>  Is it a known thing that addition of --filter=blob:none to workflow
>>>  with shalow clone (e.g. --depth=1) and following sparse checkout may
>>>  significantly slow down process and result in much larger .git
>>>  repository?
>
> In general, I would recommend not using shallow clones in conjunction
> with partial clone. The blob:none filter will get you what you really
> want from shallow clone without any of the downsides of shallow clone.

Is it really so?

As you can see from my measurements [1], in my case simple shallow clone (1)
runs faster than simple partial clone (2) and produces slightly smaller .git,
from which I can infer that (2) downloads some data which is not downloaded
in (1).

To be clear, use case which I'm interested right now is checking out sources in
cloud CI system like GitHub Actions for one shot build. Right now checkout usually
takes 1-2 minutes and my hope was that someday in the future it would be possible\
to make it faster.

[1] https://gist.github.com/annulen/835ac561e22bedd7138d13392a7a53be

-- 
Regards,
Konstantin

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Konstantin Tokarev]

01.04.2020, 01:10, "Konstantin Tokarev" <annulen@yandex.ru>:
> 28.03.2020, 19:58, "Derrick Stolee" <stolee@gmail.com>:
>>  On 3/28/2020 10:40 AM, Jeff King wrote:
>>>   On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:
>>>
>>>>   Is it a known thing that addition of --filter=blob:none to workflow
>>>>   with shalow clone (e.g. --depth=1) and following sparse checkout may
>>>>   significantly slow down process and result in much larger .git
>>>>   repository?
>>
>>  In general, I would recommend not using shallow clones in conjunction
>>  with partial clone. The blob:none filter will get you what you really
>>  want from shallow clone without any of the downsides of shallow clone.
>
> Is it really so?
>
> As you can see from my measurements [1], in my case simple shallow clone (1)
> runs faster than simple partial clone (2) and produces slightly smaller .git,
> from which I can infer that (2) downloads some data which is not downloaded
> in (1).

Actually, as I have full git logs for all these cases, there is no need to be guessing:
    (1) downloads 295085 git objects of total size 1.00 GiB
    (2) downloads 1949129 git objects of total size 1.01 GiB
Total sizes are very close, but (2) downloads much more objects, and also it uses
3 passes to download them which leads to less efficient use of network bandwidth.

>
> To be clear, use case which I'm interested right now is checking out sources in
> cloud CI system like GitHub Actions for one shot build. Right now checkout usually
> takes 1-2 minutes and my hope was that someday in the future it would be possible\
> to make it faster.
>
> [1] https://gist.github.com/annulen/835ac561e22bedd7138d13392a7a53be
>
> --
> Regards,
> Konstantin

-- 
Regards,
Konstantin

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Derrick Stolee]

On 3/31/2020 6:23 PM, Konstantin Tokarev wrote:
> 01.04.2020, 01:10, "Konstantin Tokarev" <annulen@yandex.ru>:
>> 28.03.2020, 19:58, "Derrick Stolee" <stolee@gmail.com>:
>>>  On 3/28/2020 10:40 AM, Jeff King wrote:
>>>>   On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:
>>>>
>>>>>   Is it a known thing that addition of --filter=blob:none to workflow
>>>>>   with shalow clone (e.g. --depth=1) and following sparse checkout may
>>>>>   significantly slow down process and result in much larger .git
>>>>>   repository?
>>>
>>>  In general, I would recommend not using shallow clones in conjunction
>>>  with partial clone. The blob:none filter will get you what you really
>>>  want from shallow clone without any of the downsides of shallow clone.
>>
>> Is it really so?
>>
>> As you can see from my measurements [1], in my case simple shallow clone (1)
>> runs faster than simple partial clone (2) and produces slightly smaller .git,
>> from which I can infer that (2) downloads some data which is not downloaded
>> in (1).
> 
> Actually, as I have full git logs for all these cases, there is no need to be guessing:
>     (1) downloads 295085 git objects of total size 1.00 GiB
>     (2) downloads 1949129 git objects of total size 1.01 GiB

It is worth pointing out that these sizes are very close. The number of objects
may be part of why the timing is so different as the client needs to parse all
deltas to verify the object contents.

Re-running the test with GIT_TRACE2_PERF=1 might reveal some interesting info
about which regions are slower than others.

> Total sizes are very close, but (2) downloads much more objects, and also it uses
> 3 passes to download them which leads to less efficient use of network bandwidth.

Three passes, being:

1. Download commits and trees.
2. Initialize sparse-checkout with blobs at root.
3. Expand sparse-checkout.

Is that right? You could group 1 & 2 by setting your sparse-checkout patterns
before initializing a checkout (if you clone with --no-checkout). Your link
says you did this:

	git clone <mode> --no-checkout <url> <dir>
	git sparse-checkout init
	git sparse-checkout set '/*' '!LayoutTests'

Try doing it this way instead:

	git clone <mode> --no-checkout <url> <dir>
	git config core.sparseCheckout true
	git sparse-checkout set '/*' '!LayoutTests'

By doing it this way, you skip the step where the 'init' subcommand looks
for all blobs at root and does a network call for them. Should remove some
overhead.

Less efficient use of network bandwidth is one thing, but shallow clones are
also more CPU-intensive with the "counting objects" phase on the server. Your
link shares the following end-to-end timings:

* Shallow-clone: 234s
* Partial clone: 286s
* Both(???): 1023s

The data implies that by asking for both you actually got a full clone (4.1 GB).

The 234s to 286s difference is meaningful. Almost a minute.

>> To be clear, use case which I'm interested right now is checking out sources in
>> cloud CI system like GitHub Actions for one shot build. Right now checkout usually
>> takes 1-2 minutes and my hope was that someday in the future it would be possible\
>> to make it faster.

As long as you delete the shallow clone every time, then you also remove the
downsides of a shallow clone related to a later fetch or attempts to push.

If possible, a repo this size would benefit from persistent build agents that
you control. They can keep a copy of the repo around and do incremental fetches
that are much faster. It's a larger investment to run your own build lab, though.
But sometimes making builds faster is expensive. It depends on how "expensive" those
four minute clones per build are in terms of your team waiting.

Thanks,
-Stolee

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Konstantin Tokarev]

01.04.2020, 03:09, "Derrick Stolee" <stolee@gmail.com>:
> On 3/31/2020 6:23 PM, Konstantin Tokarev wrote:
>>  01.04.2020, 01:10, "Konstantin Tokarev" <annulen@yandex.ru>:
>>>  28.03.2020, 19:58, "Derrick Stolee" <stolee@gmail.com>:
>>>>   On 3/28/2020 10:40 AM, Jeff King wrote:
>>>>>    On Sat, Mar 28, 2020 at 12:08:17AM +0300, Konstantin Tokarev wrote:
>>>>>
>>>>>>    Is it a known thing that addition of --filter=blob:none to workflow
>>>>>>    with shalow clone (e.g. --depth=1) and following sparse checkout may
>>>>>>    significantly slow down process and result in much larger .git
>>>>>>    repository?
>>>>
>>>>   In general, I would recommend not using shallow clones in conjunction
>>>>   with partial clone. The blob:none filter will get you what you really
>>>>   want from shallow clone without any of the downsides of shallow clone.
>>>
>>>  Is it really so?
>>>
>>>  As you can see from my measurements [1], in my case simple shallow clone (1)
>>>  runs faster than simple partial clone (2) and produces slightly smaller .git,
>>>  from which I can infer that (2) downloads some data which is not downloaded
>>>  in (1).
>>
>>  Actually, as I have full git logs for all these cases, there is no need to be guessing:
>>      (1) downloads 295085 git objects of total size 1.00 GiB
>>      (2) downloads 1949129 git objects of total size 1.01 GiB
>
> It is worth pointing out that these sizes are very close. The number of objects
> may be part of why the timing is so different as the client needs to parse all
> deltas to verify the object contents.
>
> Re-running the test with GIT_TRACE2_PERF=1 might reveal some interesting info
> about which regions are slower than others.

Here are trace results for (1) with fix discussed below:
https://gist.github.com/annulen/58b868e35e992105e7028946a8370795

Here are trace results for (2) with fix discussed below:
https://gist.github.com/annulen/fa1ef1b5d1056e6dede815e9ebf85c03

>
>>  Total sizes are very close, but (2) downloads much more objects, and also it uses
>>  3 passes to download them which leads to less efficient use of network bandwidth.
>
> Three passes, being:
>
> 1. Download commits and trees.
> 2. Initialize sparse-checkout with blobs at root.
> 3. Expand sparse-checkout.
>
> Is that right? You could group 1 & 2 by setting your sparse-checkout patterns
> before initializing a checkout (if you clone with --no-checkout). Your link
> says you did this:
>
>         git clone <mode> --no-checkout <url> <dir>
>         git sparse-checkout init
>         git sparse-checkout set '/*' '!LayoutTests'
>
> Try doing it this way instead:
>
>         git clone <mode> --no-checkout <url> <dir>
>         git config core.sparseCheckout true
>         git sparse-checkout set '/*' '!LayoutTests'
>
> By doing it this way, you skip the step where the 'init' subcommand looks
> for all blobs at root and does a network call for them. Should remove some
> overhead.

Thanks, that helped. Now git downloads object only two times.

From reading man page I assumed that `git sparse-checkout init` should do
the same as `git config core.sparseCheckout true`, unless `--cone` argument
is specified.

>
> Less efficient use of network bandwidth is one thing, but shallow clones are
> also more CPU-intensive with the "counting objects" phase on the server. Your
> link shares the following end-to-end timings:
>
> * Shallow-clone: 234s
> * Partial clone: 286s
> * Both(???): 1023s
>
> The data implies that by asking for both you actually got a full clone (4.1 GB).

No, this is still a partial clone, full clone takes more than 6 GB

>
> The 234s to 286s difference is meaningful. Almost a minute.
>
>>>  To be clear, use case which I'm interested right now is checking out sources in
>>>  cloud CI system like GitHub Actions for one shot build. Right now checkout usually
>>>  takes 1-2 minutes and my hope was that someday in the future it would be possible\
>>>  to make it faster.
>
> As long as you delete the shallow clone every time, then you also remove the
> downsides of a shallow clone related to a later fetch or attempts to push.
>
> If possible, a repo this size would benefit from persistent build agents that
> you control. They can keep a copy of the repo around and do incremental fetches
> that are much faster. It's a larger investment to run your own build lab, though.
> But sometimes making builds faster is expensive. It depends on how "expensive" those
> four minute clones per build are in terms of your team waiting.

No, current checkout times for shallow clone + sparseCheckout are quite acceptable.
(FWIW, initially I used shallow clone without sparseCheckout, as the latter is not supported
by GitHub Actions out of the box, and those times were NOT acceptable, as depending on
server load checkout could take 16 minutes or even more)

For now it's just my curioisty and desire to provide info which could make Git better.
It just seemed logical to me initially that if we limit both required paths in worktree and history
depth on object download stage, it should be more efficient than limiting only  history depth
(or, at least, have the same efficiency).

BTW, I did more measurements and results seem to be highly dependent on server side.
Once partial clone (2) even worked faster than shallow clone + sparseCheckout (1). Still, most of
the time (1) is faster than (2).

-- 
Regards,
Konstantin

Re: Inefficiency of partial shallow clone vs shallow clone + "old-style" sparse checkout

[Jeff King]

On Wed, Apr 01, 2020 at 04:49:20AM +0300, Konstantin Tokarev wrote:

> > Less efficient use of network bandwidth is one thing, but shallow clones are
> > also more CPU-intensive with the "counting objects" phase on the server. Your
> > link shares the following end-to-end timings:
> >
> > * Shallow-clone: 234s
> > * Partial clone: 286s
> > * Both(???): 1023s
> >
> > The data implies that by asking for both you actually got a full clone (4.1 GB).
> 
> No, this is still a partial clone, full clone takes more than 6 GB

I think that 4GB number is just because of the bug, though. With the fix
I showed earlier, doing clones of linux.git from a local repo yields:

  type       objects (in passes)      bytes  time
  ----       -----------------------  -----  ----
  shallow      71447 (  71447+  n/a)  188MB   23s
  blob:none  5260567 (5193557+67010)  870MB   99s
  both         71447 (   4437+67010)  188MB   37s

The object counts and sizes make sense. blob:none is still going to get
the whole history of commits and trees, which are substantial. The sizes
for "shallow" and "both" are the same, because the checkout is going to
grab all of the blobs from the tip commit, which were included in the
original "shallow" anyway. It does take longer, because they come in a
second followup fetch (though I'm surprised it's so _much_ slower).

So to me that implies that shallow is strictly better than partial if
you're just going to check out the full tip commit. But doing both
together opens up the possibility of narrowing the sparse checkout.
Doing:

  $ git clone --no-local --no-checkout --filter=blob:none --depth=1 \
      /path/to/linux sparse
  $ cd sparse
  $ git sparse-checkout set arch

fetches 20795 objects (4437+16357+1), consuming only 27MB.

-Peff

[PATCH] clone: use "quick" lookup while following tags

[Jeff King]

On Sat, Mar 28, 2020 at 10:40:23AM -0400, Jeff King wrote:

> So I guess the problem is not with shallow clones specifically, but they
> lead us to not having fetched the commits pointed to by tags, which
> leads to us trying to fault in those commits (and their trees) rather
> than realizing that we weren't meant to have them. And the size of the
> local repo balloons because you're fetching all those commits one by
> one, and not getting the benefit of the deltas you would when you do a
> single --filter=blob:none fetch.
> 
> I guess we need something like this:

The issue is actually with --single-branch, which is implied by --depth.
But the fix is the same either way.

Here it is with a commit message and test.

-- >8 --
Subject: [PATCH] clone: use "quick" lookup while following tags

When cloning with --single-branch, we implement git-fetch's usual
tag-following behavior, grabbing any tag objects that point to objects
we have locally.

When we're a partial clone, though, our has_object_file() check will
actually lazy-fetch each tag. That not only defeats the purpose of
--single-branch, but it does it incredibly slowly, potentially kicking
off a new fetch for each tag. This is even worse for a shallow clone,
which implies --single-branch, because even tags which are supersets of
each other will be fetched individually.

We can fix this by passing OBJECT_INFO_SKIP_FETCH_OBJECT to the call,
which is what git-fetch does in this case.

Likewise, let's include OBJECT_INFO_QUICK, as that's what git-fetch
does. The rationale is discussed in 5827a03545 (fetch: use "quick"
has_sha1_file for tag following, 2016-10-13), but here the tradeoff
would apply even more so because clone is very unlikely to be racing
with another process repacking our newly-created repository.

This may provide a very small speedup even in the non-partial case case,
as we'd avoid calling reprepare_packed_git() for each tag (though in
practice, we'd only have a single packfile, so that reprepare should be
quite cheap).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/clone.c          | 4 +++-
 t/t5616-partial-clone.sh | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index d8b1f413aa..9da6459f1d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -643,7 +643,9 @@ static void write_followtags(const struct ref *refs, const char *msg)
 			continue;
 		if (ends_with(ref->name, "^{}"))
 			continue;
-		if (!has_object_file(&ref->old_oid))
+		if (!has_object_file_with_flags(&ref->old_oid,
+						OBJECT_INFO_QUICK |
+						OBJECT_INFO_SKIP_FETCH_OBJECT))
 			continue;
 		update_ref(msg, ref->name, &ref->old_oid, NULL, 0,
 			   UPDATE_REFS_DIE_ON_ERR);
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 77bb91e976..8f0d81a27e 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -415,6 +415,14 @@ test_expect_success 'verify fetch downloads only one pack when updating refs' '
 	test_line_count = 3 pack-list
 '
 
+test_expect_success 'single-branch tag following respects partial clone' '
+	git clone --single-branch -b B --filter=blob:none \
+		"file://$(pwd)/srv.bare" single &&
+	git -C single rev-parse --verify refs/tags/B &&
+	git -C single rev-parse --verify refs/tags/A &&
+	test_must_fail git -C single rev-parse --verify refs/tags/C
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
2.26.0.408.gebd8a4413c

Re: [PATCH] clone: use "quick" lookup while following tags

[Konstantin Tokarev]

01.04.2020, 15:15, "Jeff King" <peff@peff.net>:
> On Sat, Mar 28, 2020 at 10:40:23AM -0400, Jeff King wrote:
>
>>  So I guess the problem is not with shallow clones specifically, but they
>>  lead us to not having fetched the commits pointed to by tags, which
>>  leads to us trying to fault in those commits (and their trees) rather
>>  than realizing that we weren't meant to have them. And the size of the
>>  local repo balloons because you're fetching all those commits one by
>>  one, and not getting the benefit of the deltas you would when you do a
>>  single --filter=blob:none fetch.
>>
>>  I guess we need something like this:
>
> The issue is actually with --single-branch, which is implied by --depth.
> But the fix is the same either way.
>
> Here it is with a commit message and test.
>
> -- >8 --
> Subject: [PATCH] clone: use "quick" lookup while following tags
>
> When cloning with --single-branch, we implement git-fetch's usual
> tag-following behavior, grabbing any tag objects that point to objects
> we have locally.
>
> When we're a partial clone, though, our has_object_file() check will
> actually lazy-fetch each tag. That not only defeats the purpose of
> --single-branch, but it does it incredibly slowly, potentially kicking
> off a new fetch for each tag. This is even worse for a shallow clone,
> which implies --single-branch, because even tags which are supersets of
> each other will be fetched individually.
>
> We can fix this by passing OBJECT_INFO_SKIP_FETCH_OBJECT to the call,
> which is what git-fetch does in this case.
>
> Likewise, let's include OBJECT_INFO_QUICK, as that's what git-fetch
> does. The rationale is discussed in 5827a03545 (fetch: use "quick"
> has_sha1_file for tag following, 2016-10-13), but here the tradeoff
> would apply even more so because clone is very unlikely to be racing
> with another process repacking our newly-created repository.
>
> This may provide a very small speedup even in the non-partial case case,
> as we'd avoid calling reprepare_packed_git() for each tag (though in
> practice, we'd only have a single packfile, so that reprepare should be
> quite cheap).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/clone.c | 4 +++-
>  t/t5616-partial-clone.sh | 8 ++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index d8b1f413aa..9da6459f1d 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -643,7 +643,9 @@ static void write_followtags(const struct ref *refs, const char *msg)
>                          continue;
>                  if (ends_with(ref->name, "^{}"))
>                          continue;
> - if (!has_object_file(&ref->old_oid))
> + if (!has_object_file_with_flags(&ref->old_oid,
> + OBJECT_INFO_QUICK |
> + OBJECT_INFO_SKIP_FETCH_OBJECT))
>                          continue;
>                  update_ref(msg, ref->name, &ref->old_oid, NULL, 0,
>                             UPDATE_REFS_DIE_ON_ERR);
> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> index 77bb91e976..8f0d81a27e 100755
> --- a/t/t5616-partial-clone.sh
> +++ b/t/t5616-partial-clone.sh
> @@ -415,6 +415,14 @@ test_expect_success 'verify fetch downloads only one pack when updating refs' '
>          test_line_count = 3 pack-list
>  '
>
> +test_expect_success 'single-branch tag following respects partial clone' '
> + git clone --single-branch -b B --filter=blob:none \
> + "file://$(pwd)/srv.bare" single &&
> + git -C single rev-parse --verify refs/tags/B &&
> + git -C single rev-parse --verify refs/tags/A &&
> + test_must_fail git -C single rev-parse --verify refs/tags/C
> +'
> +
>  . "$TEST_DIRECTORY"/lib-httpd.sh
>  start_httpd
>
> --
> 2.26.0.408.gebd8a4413c

Would you recommend using this patch in production, or is it better to wait for reviews?

-- 
Regards,
Konstantin

Re: [PATCH] clone: use "quick" lookup while following tags

[Jeff King]

On Wed, Apr 01, 2020 at 10:12:30PM +0300, Konstantin Tokarev wrote:

> Would you recommend using this patch in production, or is it better to wait for reviews?

Only you know the risk tolerance of your production systems, so I make
no guarantees.  But I would personally think it is quite safe.

-Peff