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
next prev parent reply other threads:[~2024-11-01 10:23 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <309549cafdcfe50c4fceac3263220cc3d8b109b2.1730337435.git.jpoimboe@kernel.org>
2024-10-31 10:37 ` [PATCH] setlocalversion: Add workaround for "git describe" performance issue 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).