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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> +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.

> 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.