Re: [PATCH 07/12] fetch-pack: cache results of for_each_alternate_ref

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2017, 19:21 UTC
Message-ID: <xmqqinp3aqu0.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170124004559.vlsrwwphuzdsfqoq@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> +struct alternate_object_cache {
> +	struct object **items;
> +	size_t nr, alloc;
> +};
> +
> +static void cache_one_alternate(const char *refname,
> +				const struct object_id *oid,
> +				void *vcache)
> +{
> +	struct alternate_object_cache *cache = vcache;
> +	struct object *obj = parse_object(oid->hash);
> +
> +	if (!obj || (obj->flags & ALTERNATE))
> +		return;
> +
> +	obj->flags |= ALTERNATE;
> +	ALLOC_GROW(cache->items, cache->nr + 1, cache->alloc);
> +	cache->items[cache->nr++] = obj;
> +}

Nice.  I love simplicity.

Show 13 quoted lines

> diff --git a/object.h b/object.h
> index 614a00675..f52957dcb 100644
> --- a/object.h
> +++ b/object.h
> @@ -29,7 +29,7 @@ struct object_array {
>  /*
>   * object flag allocation:
>   * revision.h:      0---------10                                26
> - * fetch-pack.c:    0---4
> + * fetch-pack.c:    0---5
>   * walker.c:        0-2
>   * upload-pack.c:       4       11----------------19
>   * builtin/blame.c:               12-13

This is a tangent, but I am not sure how much it buys us to keep track of the information here in object.h, as all that picture says is "revision traversal machinery given by revision.[ch] can never be used inside fetch-pack and upload-pack", and that is already said by the current picture that says fetch-pack.c uses 0 thru 4, and updating it to say that we now use 0 thru 5 would not change the conclusion.

What I am trying to get at is that we may want to (re)consider how we manage these bits. But that is totally outside the scope of this series, and updating the above is the right thing to do here.

Thanks.