Subject: Re: [PATCH 09/12] receive-pack: use oidset to de-duplicate .have lines

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 Jan 2017 11:32:05 -0800

Jeff King <[email-masked]> writes:

> If you have an alternate object store with a very large
> number of refs, the peak memory usage of the sha1_array can
> grow high, even if most of them are duplicates that end up
> not being printed at all.
> ...
> Signed-off-by: Jeff King <[email-masked]>
> ---
>  builtin/receive-pack.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)

Nice.  

Incidentally, this also shows that the refnames in alternate ref
namespace will not matter.  We are to only show just one of many
anyway (and that name is masked and replaced with ".have").  Perhaps
we want to do 04/12 without refname?

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index b9f2c0cc5..27bb52988 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -21,6 +21,7 @@
>  #include "sigchain.h"
>  #include "fsck.h"
>  #include "tmp-objdir.h"
> +#include "oidset.h"
>  
>  static const char * const receive_pack_usage[] = {
>  	N_("git receive-pack <git-dir>"),
> @@ -271,27 +272,24 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid,
>  	return 0;
>  }
>  
> -static int show_one_alternate_sha1(const unsigned char sha1[20], void *unused)
> +static void show_one_alternate_ref(const char *refname,
> +				   const struct object_id *oid,
> +				   void *data)
>  {
> -	show_ref(".have", sha1);
> -	return 0;
> -}
> +	struct oidset *seen = data;
>  
> -static void collect_one_alternate_ref(const char *refname,
> -				      const struct object_id *oid,
> -				      void *data)
> -{
> -	struct sha1_array *sa = data;
> -	sha1_array_append(sa, oid->hash);
> +	if (oidset_insert(seen, oid))
> +		return;
> +
> +	show_ref(".have", oid->hash);
>  }
>  
>  static void write_head_info(void)
>  {
> -	struct sha1_array sa = SHA1_ARRAY_INIT;
> +	static struct oidset seen = OIDSET_INIT;
>  
> -	for_each_alternate_ref(collect_one_alternate_ref, &sa);
> -	sha1_array_for_each_unique(&sa, show_one_alternate_sha1, NULL);
> -	sha1_array_clear(&sa);
> +	for_each_alternate_ref(show_one_alternate_ref, &seen);
> +	oidset_clear(&seen);
>  	for_each_ref(show_ref_cb, NULL);
>  	if (!sent_capabilities)
>  		show_ref("capabilities^{}", null_sha1);