From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Sat, 13 Aug 2011 12:54:38 GMT
Subject: Re: [PATCH 6/6] Retain caches of submodule refs
Message-ID: <20110813125438.GC4783@book.hvoigt.net>
In-Reply-To: <1313188589-2330-7-git-send-email-mhagger@alum.mit.edu>

Hi,

On Sat, Aug 13, 2011 at 12:36:29AM +0200, Michael Haggerty wrote:
> diff --git a/refs.c b/refs.c
> index 8d1055d..f02cf94 100644
> --- a/refs.c
> +++ b/refs.c

...

> @@ -205,23 +208,28 @@ struct cached_refs *create_cached_refs(const char *submodule)
>   */
>  static struct cached_refs *get_cached_refs(const char *submodule)
>  {
> -	if (! submodule) {
> -		if (!cached_refs)
> -			cached_refs = create_cached_refs(submodule);
> -		return cached_refs;
> -	} else {
> -		if (!submodule_refs)
> -			submodule_refs = create_cached_refs(submodule);
> -		else
> -			/* For now, don't reuse the refs cache for submodules. */
> -			clear_cached_refs(submodule_refs);
> -		return submodule_refs;
> +	struct cached_refs *refs = cached_refs;
> +	if (! submodule)
> +		submodule = "";

Maybe instead of searching for the main refs store a pointer to them
locally so you can immediately return here. That will keep the
performance when requesting the main refs the same.

If I see it correctly you are always prepending to the linked list and
in case many submodules get cached this could slow down the iteration
over the refs of the main repository.

> +	while (refs) {
> +		if (!strcmp(submodule, refs->name))
> +			return refs;
> +		refs = refs->next;
>  	}
> +
> +	refs = create_cached_refs(submodule);
> +	refs->next = cached_refs;
> +	cached_refs = refs;
> +	return refs;
>  }

...

Cheers Heiko