Re: [PATCH 10/40] external-odb: implement external_odb_get_direct

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Jan 4, 2018, 17:44 UTC
Message-ID: <4ce872c0-8f4b-602a-9ee3-cf429cc6a146@jeffhostetler.com>
In-Reply-To: <20180103163403.11303-11-chriscool@tuxfamily.org>

On 1/3/2018 11:33 AM, Christian Couder wrote:

Show 30 quoted lines

> This is implemented only in the promisor remote mode
> for now by calling fetch_object().
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>   external-odb.c | 15 +++++++++++++++
>   external-odb.h |  1 +
>   odb-helper.c   | 13 +++++++++++++
>   odb-helper.h   |  3 ++-
>   4 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/external-odb.c b/external-odb.c
> index d26e63d8b1..5d0afb9762 100644
> --- a/external-odb.c
> +++ b/external-odb.c
> @@ -76,3 +76,18 @@ int external_odb_has_object(const unsigned char *sha1)
>   			return 1;
>   	return 0;
>   }
> +
> +int external_odb_get_direct(const unsigned char *sha1)
> +{
> +	struct odb_helper *o;
> +
> +	external_odb_init();
> +
> +	for (o = helpers; o; o = o->next) {
> +		if (odb_helper_get_direct(o, sha1) < 0)
> +			continue;
> +		return 0;

 > +	}

Would this be simpler said as:
	for (o = ...)
		if (!odb_helper_get_direct(...))
			return 0;

Show 41 quoted lines

> +
> +	return -1;
> +}
> diff --git a/external-odb.h b/external-odb.h
> index 9a3c2f01b3..fd6708163e 100644
> --- a/external-odb.h
> +++ b/external-odb.h
> @@ -4,5 +4,6 @@
>   extern int has_external_odb(void);
>   extern const char *external_odb_root(void);
>   extern int external_odb_has_object(const unsigned char *sha1);
> +extern int external_odb_get_direct(const unsigned char *sha1);
>   
>   #endif /* EXTERNAL_ODB_H */
> diff --git a/odb-helper.c b/odb-helper.c
> index 1404393807..4b70b287af 100644
> --- a/odb-helper.c
> +++ b/odb-helper.c
> @@ -4,6 +4,7 @@
>   #include "odb-helper.h"
>   #include "run-command.h"
>   #include "sha1-lookup.h"
> +#include "fetch-object.h"
>   
>   struct odb_helper *odb_helper_new(const char *name, int namelen)
>   {
> @@ -52,3 +53,15 @@ int odb_helper_has_object(struct odb_helper *o, const unsigned char *sha1)
>   	return !!odb_helper_lookup(o, sha1);
>   }
>   
> +int odb_helper_get_direct(struct odb_helper *o,
> +			  const unsigned char *sha1)
> +{
> +	int res = 0;
> +	uint64_t start = getnanotime();
> +
> +	fetch_object(o->dealer, sha1);
> +
> +	trace_performance_since(start, "odb_helper_get_direct");
> +
> +	return res;

'res' will always be 0, so the external_odb_get_direct() will only do the first helper. i haven't looked at the rest of the series yet, so maybe you've already addressed this.

Also, I put a TODO comment in the fetch_object() header to consider returning an error/success, so maybe that could help here too.

Show 14 quoted lines

> +}
> diff --git a/odb-helper.h b/odb-helper.h
> index 9395e606ce..f4bc66b0ef 100644
> --- a/odb-helper.h
> +++ b/odb-helper.h
> @@ -20,5 +20,6 @@ struct odb_helper {
>   extern struct odb_helper *odb_helper_new(const char *name, int namelen);
>   extern int odb_helper_has_object(struct odb_helper *o,
>   				 const unsigned char *sha1);
> -
> +extern int odb_helper_get_direct(struct odb_helper *o,
> +				 const unsigned char *sha1);
>   #endif /* ODB_HELPER_H */
>