All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <ravi@prevas.dk>
To: Jeff King <peff@peff.net>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>,
	 Masahiro Yamada <masahiroy@kernel.org>,
	 linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	 git@vger.kernel.org
Subject: Re: [PATCH] setlocalversion: Add workaround for "git describe" performance issue
Date: Fri, 01 Nov 2024 11:23:05 +0100	[thread overview]
Message-ID: <8734kbjlrq.fsf@prevas.dk> (raw)
In-Reply-To: <20241031122456.GB593548@coredump.intra.peff.net> (Jeff King's message of "Thu, 31 Oct 2024 08:24:56 -0400")

On Thu, Oct 31 2024, Jeff King <peff@peff.net> wrote:

> On Thu, Oct 31, 2024 at 07:42:10AM -0400, Jeff King wrote:
>
>> That works, but I have a feeling that figured out what the heck is going
>> on with gave_up_on might produce a more elegant solution.
>
> OK, I think I might have made some sense of this.
>
> In finish_depth_computation(), we traverse down "list" forever, passing
> flags up to our parents, until we find a commit that is marked with the
> same "within" flag as our candidate. And then if everything left has
> that same "within" flag set, we can bail.
>
> So I _think_ the point is to basically count up what we'd get from this
> traversal:
>
>   $tag..$commit
>
> where "$tag" is the candidate tag we found, and "$commit" is what we're
> trying to describe (so imagine "git describe --match=$tag $commit").

Yeah, so this is really just what the setlocalversion script wants to
know. For a few diffent possible values of $tag (in most cases just 1),
we ask: Is $tag an annotated tag? Is it an ancestor of HEAD? And if so,
how many commits are in $tag..HEAD.

Perhaps we could on the kernel side replace the "git describe --match"
calls with a helper, something like this (needs a lot of polishing):

===
# Produce output similar to what "git describe --match=$tag 2>
# /dev/null" would.  It doesn't have to match exactly as the caller is
# only interested in whether $tag == HEAD, and if not, the number
# between the tag and the short sha1.
describe()
{
    # Is $tag an annotated tag? Could/should probably be written using
    # some plumbing instead of git describe, but with --exact-match,
    # we avoid the walk-to-the-start-of-history behaviour, so fine for
    # this demo.
    git describe --exact-match --match=$tag $tag >/dev/null 2>/dev/null || return 1

    # Can it be used to describe HEAD, i.e. is it an ancestor of HEAD?
    git merge-base --is-ancestor $tag HEAD || return 1

    # Find the number that "git describe" would append.
    count=$(git rev-list --count $tag..HEAD)
    if [ $count -eq 0 ] ; then
        echo "$tag"
    else
        echo "$tag-$count-$head"
    fi
}
===

But if we go this route, we should probably rework the logic
somewhat. There's no point getting the count ourselves, stuffing that
into a string, and then splitting that string with awk to %05d format
the count.

I also don't know if either the --is-ancestor or the rev-list count
could end up doing the same walk-all-commits we're trying to avoid.

Rasmus

  parent reply	other threads:[~2024-11-01 10:23 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-31  1:20 [PATCH] setlocalversion: Add workaround for "git describe" performance issue Josh Poimboeuf
2024-10-31 10:37 ` Rasmus Villemoes
2024-10-31 11:42   ` Jeff King
2024-10-31 12:24     ` Jeff King
2024-10-31 14:43       ` Jeff King
2024-11-04 12:37         ` Benno Evers
2024-11-06 19:22           ` [PATCH 0/4] perf improvements for git-describe with few tags Jeff King
2024-11-06 19:26             ` Jeff King
2024-11-06 21:16             ` [PATCH 1/4] t6120: demonstrate weakness in disjoint-root handling Jeff King
2024-11-06 21:17             ` [PATCH 2/4] t/perf: add tests for git-describe Jeff King
2024-11-06 21:17             ` [PATCH 3/4] describe: stop digging for max_candidates+1 Jeff King
2024-11-06 21:17             ` [PATCH 4/4] describe: stop traversing when we run out of names Jeff King
2024-12-04 23:15               ` [PATCH] fixup! " Josh Steadmon
2024-12-04 23:27                 ` Jeff King
2024-12-04 23:54                   ` Jeff King
2024-12-05 20:14                   ` [PATCH] describe: drop early return for max_candidates == 0 Jeff King
2024-12-05 22:28                     ` Josh Steadmon
2024-12-05 23:21                       ` Jeff King
2024-12-06  3:01                     ` Junio C Hamano
2024-12-06  3:28                       ` Jeff King
2024-12-06  4:19                         ` Junio C Hamano
2024-12-06  5:42                           ` [PATCH] describe: split "found all tags" and max_candidates logic Jeff King
2024-11-26  5:05             ` [PATCH 0/4] perf improvements for git-describe with few tags Junio C Hamano
2024-12-04 23:04               ` Josh Steadmon
2024-12-04 23:20                 ` Jeff King
2024-11-01 10:23       ` Rasmus Villemoes [this message]
2024-11-01 11:39         ` [PATCH] setlocalversion: Add workaround for "git describe" performance issue Jeff King
2024-10-31 11:43   ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8734kbjlrq.fsf@prevas.dk \
    --to=ravi@prevas.dk \
    --cc=git@vger.kernel.org \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.