Re: [PATCH on sb/more-repo-in-api] revision: use commit graph in get_reference()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 5, 2018, 23:15 UTC
- Message-ID
- <xmqqin07bm5w.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20181204224238.50966-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> Looking at the bigger picture, the speed of the connectivity check > during a fetch might be further improved by passing only the negotiation > tips (obtained through --negotiation-tip) instead of "--all". This patch > just handles the low-hanging fruit first.
That sounds like a good direction, when having to list excessive number of refs is the primary problem. When fetching their 'master' into our 'remotes/origin/master' and doing nothing else, we may end up showing only the latter, which will miss optimization opportunity a lot if the latest change made over there is to merge in the change we asked them to pull earlier (which would be greatly helped if we let them know about the tip of the topic they earlier pulled from us), but also avoids having to send irrelevant refs that point at tags addded to months old states. So there is a subtle trade-off between sending more refs to reduce the resulting packfile, and sending fewer refs to reduce the cost of the "have" exchange.
Changing the way to throw each object pointed at by a ref into the queue to be emitted in the "have" exchange from regular object parsing to peeking of precomputed data would reduce the local cost of "have" exchange, but it does not reduce the network cost at all, though.
As to the change being specific to get_reference() and not to parse_object(), I think what we see here is probably better, simply because parse_object() is not in the position to asssume that it is likely to be asked to parse commits, but I think get_reference() is, after looking at its callsites in revision.c.
I do share the meta-comment concern with Peff, though.
Show 30 quoted lines
> ---
> revision.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index b5108b75ab..e7da2c57ab 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -212,7 +212,20 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
> {
> struct object *object;
>
> - object = parse_object(revs->repo, oid);
> + /*
> + * If the repository has commit graphs, repo_parse_commit() avoids
> + * reading the object buffer, so use it whenever possible.
> + */
> + if (oid_object_info(revs->repo, oid, NULL) == OBJ_COMMIT) {
> + struct commit *c = lookup_commit(revs->repo, oid);
> + if (!repo_parse_commit(revs->repo, c))
> + object = (struct object *) c;
> + else
> + object = NULL;
> + } else {
> + object = parse_object(revs->repo, oid);
> + }
> +
> if (!object) {
> if (revs->ignore_missing)
> return object;